Linux Kernel Selftest development
 help / color / mirror / Atom feed
From: Petr Machata <petrm@nvidia.com>
To: "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	<netdev@vger.kernel.org>
Cc: <linux-kselftest@vger.kernel.org>, Shuah Khan <shuah@kernel.org>,
	"Benjamin Poirier" <bpoirier@nvidia.com>,
	Hangbin Liu <liuhangbin@gmail.com>,
	"Vladimir Oltean" <vladimir.oltean@nxp.com>,
	Ido Schimmel <idosch@nvidia.com>,
	"Przemek Kitszel" <przemyslaw.kitszel@intel.com>,
	Petr Machata <petrm@nvidia.com>,
	Willem de Bruijn <willemb@google.com>, <mlxsw@nvidia.com>
Subject: [PATCH net-next v2 01/10] selftests: net: lib: Introduce deferred commands
Date: Thu, 17 Oct 2024 11:45:43 +0200	[thread overview]
Message-ID: <daa8551601ac52140a0278d729d233e031b96853.1729157566.git.petrm@nvidia.com> (raw)
In-Reply-To: <cover.1729157566.git.petrm@nvidia.com>

In commit 8510801a9dbd ("selftests: drv-net: add ability to schedule
cleanup with defer()"), a defer helper was added to Python selftests.
The idea is to keep cleanup commands close to their dirtying counterparts,
thereby making it more transparent what is cleaning up what, making it
harder to miss a cleanup, and make the whole cleanup business exception
safe. All these benefits are applicable to bash as well, exception safety
can be interpreted in terms of safety vs. a SIGINT.

This patch therefore introduces a framework of several helpers that serve
to schedule cleanups in bash selftests:

- defer_scope_push(), defer_scope_pop(): Deferred statements can be batched
  together in scopes. When a scope is popped, the deferred commands
  scheduled in that scope are executed in the order opposite to order of
  their scheduling.

- defer(): Schedules a defer to the most recently pushed scope (or the
  default scope if none was pushed.)

- defer_prio(): Schedules a defer on the priority track. The priority defer
  queue is run before the default defer queue when scope is popped.

  The issue that this is addressing is specifically the one of restoring
  devlink shared buffer threshold type. When setting up static thresholds,
  one has to first change the threshold type to static, then override the
  individual thresholds. When cleaning up, it would be natural to reset the
  threshold values first, then change the threshold type. But the values
  that are valid for dynamic thresholds are generally invalid for static
  thresholds and vice versa. Attempts to restore the values first would be
  bounced. Thus one has to first reset the threshold type, then adjust the
  thresholds.

  (You could argue that the shared buffer threshold type API is broken and
  you would be right, but here we are.)

  This cannot be solved by pure defers easily. I considered making it
  possible to disable an existing defer, so that one could then schedule a
  new defer and disable the original. But this forward-shifting of the
  defer job would have to take place after every threshold-adjusting
  command, which would make it very awkward to schedule these jobs.

- defer_scopes_cleanup(): Pops any unpopped scopes, including the default
  one. The selftests that use defer should run this in their exit trap.
  This is important to get cleanups of interrupted scripts.

- in_defer_scope(): Sometimes a function would like to introduce a new
  defer scope, then run whatever it is that it wants to run, and then pop
  the scope to run the deferred cleanups. The helper in_defer_scope() can
  be used to run another command within such environment, such that any
  scheduled defers run after the command finishes.

The framework is added as a separate file lib/sh/defer.sh so that it can be
used by all bash selftests, including those that do not currently use
lib.sh. lib.sh however includes the file by default, because ideally all
tests would use these helpers instead of hand-rolling their cleanups.

Signed-off-by: Petr Machata <petrm@nvidia.com>
---

Notes:
    v2:
    - In __defer__schedule(), use ndefers in place of
      ${__DEFER__NJOBS[$ndefers_key]}

 tools/testing/selftests/net/forwarding/lib.sh |   3 +-
 tools/testing/selftests/net/lib.sh            |   3 +
 tools/testing/selftests/net/lib/Makefile      |   2 +-
 tools/testing/selftests/net/lib/sh/defer.sh   | 115 ++++++++++++++++++
 4 files changed, 121 insertions(+), 2 deletions(-)
 create mode 100644 tools/testing/selftests/net/lib/sh/defer.sh

diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index c992e385159c..d24b6af7ebfa 100644
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -1403,7 +1403,8 @@ tests_run()
 	local current_test
 
 	for current_test in ${TESTS:-$ALL_TESTS}; do
-		$current_test
+		in_defer_scope \
+			$current_test
 	done
 }
 
diff --git a/tools/testing/selftests/net/lib.sh b/tools/testing/selftests/net/lib.sh
index be8707bfb46e..c8991cc6bf28 100644
--- a/tools/testing/selftests/net/lib.sh
+++ b/tools/testing/selftests/net/lib.sh
@@ -1,6 +1,9 @@
 #!/bin/bash
 # SPDX-License-Identifier: GPL-2.0
 
+net_dir=$(dirname "$(readlink -e "${BASH_SOURCE[0]}")")
+source "$net_dir/lib/sh/defer.sh"
+
 ##############################################################################
 # Defines
 
diff --git a/tools/testing/selftests/net/lib/Makefile b/tools/testing/selftests/net/lib/Makefile
index 82c3264b115e..18b9443454a9 100644
--- a/tools/testing/selftests/net/lib/Makefile
+++ b/tools/testing/selftests/net/lib/Makefile
@@ -10,6 +10,6 @@ TEST_FILES += ../../../../net/ynl
 
 TEST_GEN_FILES += csum
 
-TEST_INCLUDES := $(wildcard py/*.py)
+TEST_INCLUDES := $(wildcard py/*.py sh/*.sh)
 
 include ../../lib.mk
diff --git a/tools/testing/selftests/net/lib/sh/defer.sh b/tools/testing/selftests/net/lib/sh/defer.sh
new file mode 100644
index 000000000000..082f5d38321b
--- /dev/null
+++ b/tools/testing/selftests/net/lib/sh/defer.sh
@@ -0,0 +1,115 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+# map[(scope_id,track,cleanup_id) -> cleanup_command]
+# track={d=default | p=priority}
+declare -A __DEFER__JOBS
+
+# map[(scope_id,track) -> # cleanup_commands]
+declare -A __DEFER__NJOBS
+
+# scope_id of the topmost scope.
+__DEFER__SCOPE_ID=0
+
+__defer__ndefer_key()
+{
+	local track=$1; shift
+
+	echo $__DEFER__SCOPE_ID,$track
+}
+
+__defer__defer_key()
+{
+	local track=$1; shift
+	local defer_ix=$1; shift
+
+	echo $__DEFER__SCOPE_ID,$track,$defer_ix
+}
+
+__defer__ndefers()
+{
+	local track=$1; shift
+
+	echo ${__DEFER__NJOBS[$(__defer__ndefer_key $track)]}
+}
+
+__defer__run()
+{
+	local track=$1; shift
+	local defer_ix=$1; shift
+	local defer_key=$(__defer__defer_key $track $defer_ix)
+
+	${__DEFER__JOBS[$defer_key]}
+	unset __DEFER__JOBS[$defer_key]
+}
+
+__defer__schedule()
+{
+	local track=$1; shift
+	local ndefers=$(__defer__ndefers $track)
+	local ndefers_key=$(__defer__ndefer_key $track)
+	local defer_key=$(__defer__defer_key $track $ndefers)
+	local defer="$@"
+
+	__DEFER__JOBS[$defer_key]="$defer"
+	__DEFER__NJOBS[$ndefers_key]=$((ndefers + 1))
+}
+
+__defer__scope_wipe()
+{
+	__DEFER__NJOBS[$(__defer__ndefer_key d)]=0
+	__DEFER__NJOBS[$(__defer__ndefer_key p)]=0
+}
+
+defer_scope_push()
+{
+	((__DEFER__SCOPE_ID++))
+	__defer__scope_wipe
+}
+
+defer_scope_pop()
+{
+	local defer_ix
+
+	for ((defer_ix=$(__defer__ndefers p); defer_ix-->0; )); do
+		__defer__run p $defer_ix
+	done
+
+	for ((defer_ix=$(__defer__ndefers d); defer_ix-->0; )); do
+		__defer__run d $defer_ix
+	done
+
+	__defer__scope_wipe
+	((__DEFER__SCOPE_ID--))
+}
+
+defer()
+{
+	__defer__schedule d "$@"
+}
+
+defer_prio()
+{
+	__defer__schedule p "$@"
+}
+
+defer_scopes_cleanup()
+{
+	while ((__DEFER__SCOPE_ID >= 0)); do
+		defer_scope_pop
+	done
+}
+
+in_defer_scope()
+{
+	local ret
+
+	defer_scope_push
+	"$@"
+	ret=$?
+	defer_scope_pop
+
+	return $ret
+}
+
+__defer__scope_wipe
-- 
2.45.0


  reply	other threads:[~2024-10-17  9:47 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-17  9:45 [PATCH net-next v2 00/10] selftests: net: Introduce deferred commands Petr Machata
2024-10-17  9:45 ` Petr Machata [this message]
2024-10-17  9:45 ` [PATCH net-next v2 02/10] selftests: forwarding: Add a fallback cleanup() Petr Machata
2024-10-17  9:45 ` [PATCH net-next v2 03/10] selftests: forwarding: lib: Allow passing PID to stop_traffic() Petr Machata
2024-10-17  9:45 ` [PATCH net-next v2 04/10] selftests: RED: Use defer for test cleanup Petr Machata
2024-10-17  9:45 ` [PATCH net-next v2 05/10] selftests: TBF: " Petr Machata
2024-10-17  9:45 ` [PATCH net-next v2 06/10] selftests: ETS: " Petr Machata
2024-10-17  9:45 ` [PATCH net-next v2 07/10] selftests: mlxsw: qos_mc_aware: " Petr Machata
2024-10-17  9:45 ` [PATCH net-next v2 08/10] selftests: mlxsw: qos_ets_strict: " Petr Machata
2024-10-17  9:45 ` [PATCH net-next v2 09/10] selftests: mlxsw: qos_max_descriptors: " Petr Machata
2024-10-17  9:45 ` [PATCH net-next v2 10/10] selftests: mlxsw: devlink_trap_police: " Petr Machata
2024-10-22 11:50 ` [PATCH net-next v2 00/10] selftests: net: Introduce deferred commands patchwork-bot+netdevbpf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=daa8551601ac52140a0278d729d233e031b96853.1729157566.git.petrm@nvidia.com \
    --to=petrm@nvidia.com \
    --cc=bpoirier@nvidia.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=idosch@nvidia.com \
    --cc=kuba@kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=liuhangbin@gmail.com \
    --cc=mlxsw@nvidia.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=przemyslaw.kitszel@intel.com \
    --cc=shuah@kernel.org \
    --cc=vladimir.oltean@nxp.com \
    --cc=willemb@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox