* [PATCH net-next 00/10] selftests: net: Introduce deferred commands
@ 2024-10-09 12:06 Petr Machata
2024-10-09 12:06 ` [PATCH net-next 01/10] selftests: net: lib: " Petr Machata
` (9 more replies)
0 siblings, 10 replies; 15+ messages in thread
From: Petr Machata @ 2024-10-09 12:06 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
netdev
Cc: linux-kselftest, Shuah Khan, Benjamin Poirier, Hangbin Liu,
Vladimir Oltean, Ido Schimmel, Przemek Kitszel, Petr Machata,
mlxsw
Recently, 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 patchset therefore introduces a framework of several helpers that
serve to schedule cleanups in bash selftests.
- Patch #1 has more details about the primitives being introduced.
Patch #2 adds a fallback cleanup() function to lib.sh, because ideally
selftests wouldn't need to introduce a dedicated cleanup function at all.
- Patch #3 adds a parameter to stop_traffic(), which makes it possible to
start other background processes after the traffic is started without
confusing the cleanup.
- Patches #4 to #10 convert a number of selftests.
The goal was to convert all tests that use start_traffic / stop_traffic
to the defer framework. Leftover traffic generators are a particularly
painful sort of a missed cleanup. Normal unfinished cleanups can usually
be cleaned up simply by rerunning the test and interrupting it early to
let the cleanups run again / in full. This does not work with
stop_traffic, because it is only issued at the end of the test case that
starts the traffic. At the same time, leftover traffic generators
influence follow-up test runs, and are hard to notice.
The tests were however converted whole-sale, not just their traffic bits.
Thus they form a proof of concept of the defer framework.
v1 (from the RFC):
- Patch #1:
- Added the priority defer track
- Dropped defer_scoped_fn, added in_defer_scope
- Extracted to a separate independent module
- Patch #2:
- Moved this bit to a separate patch
- Patch #3:
- New patch
- Patch #4 (RED):
- Squashed the individual RED-related patches into one
- Converted the SW datapath RED selftest as well
- Patch #5 (TBF):
- Fully converted the selftest, not just stop_traffic
- Patches #6, #7, #8, #9, #10:
- New patch
Petr Machata (10):
selftests: net: lib: Introduce deferred commands
selftests: forwarding: Add a fallback cleanup()
selftests: forwarding: lib: Allow passing PID to stop_traffic()
selftests: RED: Use defer for test cleanup
selftests: TBF: Use defer for test cleanup
selftests: ETS: Use defer for test cleanup
selftests: mlxsw: qos_mc_aware: Use defer for test cleanup
selftests: mlxsw: qos_ets_strict: Use defer for test cleanup
selftests: mlxsw: qos_max_descriptors: Use defer for test cleanup
selftests: mlxsw: devlink_trap_police: Use defer for test cleanup
.../drivers/net/mlxsw/devlink_trap_policer.sh | 85 ++++-----
.../drivers/net/mlxsw/qos_ets_strict.sh | 167 ++++++++---------
.../drivers/net/mlxsw/qos_max_descriptors.sh | 118 +++++-------
.../drivers/net/mlxsw/qos_mc_aware.sh | 146 +++++++--------
.../selftests/drivers/net/mlxsw/sch_ets.sh | 26 ++-
.../drivers/net/mlxsw/sch_red_core.sh | 171 +++++++++---------
.../drivers/net/mlxsw/sch_red_ets.sh | 24 +--
.../drivers/net/mlxsw/sch_red_root.sh | 18 +-
tools/testing/selftests/net/forwarding/lib.sh | 13 +-
.../selftests/net/forwarding/sch_ets.sh | 7 +-
.../selftests/net/forwarding/sch_ets_core.sh | 81 +++------
.../selftests/net/forwarding/sch_ets_tests.sh | 14 +-
.../selftests/net/forwarding/sch_red.sh | 103 ++++-------
.../selftests/net/forwarding/sch_tbf_core.sh | 91 +++-------
.../net/forwarding/sch_tbf_etsprio.sh | 7 +-
.../selftests/net/forwarding/sch_tbf_root.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 ++++++++++++
19 files changed, 587 insertions(+), 607 deletions(-)
create mode 100644 tools/testing/selftests/net/lib/sh/defer.sh
--
2.45.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH net-next 01/10] selftests: net: lib: Introduce deferred commands
2024-10-09 12:06 [PATCH net-next 00/10] selftests: net: Introduce deferred commands Petr Machata
@ 2024-10-09 12:06 ` Petr Machata
2024-10-15 8:22 ` Paolo Abeni
2024-10-09 12:06 ` [PATCH net-next 02/10] selftests: forwarding: Add a fallback cleanup() Petr Machata
` (8 subsequent siblings)
9 siblings, 1 reply; 15+ messages in thread
From: Petr Machata @ 2024-10-09 12:06 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
netdev
Cc: linux-kselftest, Shuah Khan, Benjamin Poirier, Hangbin Liu,
Vladimir Oltean, Ido Schimmel, Przemek Kitszel, Petr Machata,
mlxsw
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>
---
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..8d205c3f0445
--- /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]=$((${__DEFER__NJOBS[$ndefers_key]} + 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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH net-next 02/10] selftests: forwarding: Add a fallback cleanup()
2024-10-09 12:06 [PATCH net-next 00/10] selftests: net: Introduce deferred commands Petr Machata
2024-10-09 12:06 ` [PATCH net-next 01/10] selftests: net: lib: " Petr Machata
@ 2024-10-09 12:06 ` Petr Machata
2024-10-09 12:06 ` [PATCH net-next 03/10] selftests: forwarding: lib: Allow passing PID to stop_traffic() Petr Machata
` (7 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Petr Machata @ 2024-10-09 12:06 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
netdev
Cc: linux-kselftest, Shuah Khan, Benjamin Poirier, Hangbin Liu,
Vladimir Oltean, Ido Schimmel, Przemek Kitszel, Petr Machata,
mlxsw
Consistent use of defers obviates the need for a separate test-specific
cleanup function -- everything is just taken care of in defers. So in this
patch, introduce a cleanup() helper in the forwarding lib.sh, which calls
just pre_cleanup() and defer_scopes_cleanup(). Selftests are obviously
still free to override the function.
Since pre_cleanup() is too entangled with forwarding-specific minutia, the
function cannot currently be in net/lib.sh.
Signed-off-by: Petr Machata <petrm@nvidia.com>
---
tools/testing/selftests/net/forwarding/lib.sh | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index d24b6af7ebfa..76e6d7698caf 100644
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -1408,6 +1408,12 @@ tests_run()
done
}
+cleanup()
+{
+ pre_cleanup
+ defer_scopes_cleanup
+}
+
multipath_eval()
{
local desc="$1"
--
2.45.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH net-next 03/10] selftests: forwarding: lib: Allow passing PID to stop_traffic()
2024-10-09 12:06 [PATCH net-next 00/10] selftests: net: Introduce deferred commands Petr Machata
2024-10-09 12:06 ` [PATCH net-next 01/10] selftests: net: lib: " Petr Machata
2024-10-09 12:06 ` [PATCH net-next 02/10] selftests: forwarding: Add a fallback cleanup() Petr Machata
@ 2024-10-09 12:06 ` Petr Machata
2024-10-09 12:06 ` [PATCH net-next 04/10] selftests: RED: Use defer for test cleanup Petr Machata
` (6 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Petr Machata @ 2024-10-09 12:06 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
netdev
Cc: linux-kselftest, Shuah Khan, Benjamin Poirier, Hangbin Liu,
Vladimir Oltean, Ido Schimmel, Przemek Kitszel, Petr Machata,
mlxsw
Now that it is possible to schedule a deferral of stop_traffic() right
after the traffic is started, we do not have to rely on the %% magic to
kill the background process that was started last. Instead we can just give
the PID explicitly. This makes it possible to start other background
processes after the traffic is started without confusing the cleanup.
Signed-off-by: Petr Machata <petrm@nvidia.com>
---
tools/testing/selftests/net/forwarding/lib.sh | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index 76e6d7698caf..89c25f72b10c 100644
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -1768,8 +1768,10 @@ start_tcp_traffic()
stop_traffic()
{
+ local pid=${1-%%}; shift
+
# Suppress noise from killing mausezahn.
- { kill %% && wait %%; } 2>/dev/null
+ { kill $pid && wait $pid; } 2>/dev/null
}
declare -A cappid
--
2.45.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH net-next 04/10] selftests: RED: Use defer for test cleanup
2024-10-09 12:06 [PATCH net-next 00/10] selftests: net: Introduce deferred commands Petr Machata
` (2 preceding siblings ...)
2024-10-09 12:06 ` [PATCH net-next 03/10] selftests: forwarding: lib: Allow passing PID to stop_traffic() Petr Machata
@ 2024-10-09 12:06 ` Petr Machata
2024-10-15 8:14 ` Paolo Abeni
2024-10-09 12:06 ` [PATCH net-next 05/10] selftests: TBF: " Petr Machata
` (5 subsequent siblings)
9 siblings, 1 reply; 15+ messages in thread
From: Petr Machata @ 2024-10-09 12:06 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
netdev
Cc: linux-kselftest, Shuah Khan, Benjamin Poirier, Hangbin Liu,
Vladimir Oltean, Ido Schimmel, Przemek Kitszel, Petr Machata,
mlxsw
Instead of having a suite of dedicated cleanup functions, use the defer
framework to schedule cleanups right as their setup functions are run.
Signed-off-by: Petr Machata <petrm@nvidia.com>
---
.../drivers/net/mlxsw/sch_red_core.sh | 171 +++++++++---------
.../drivers/net/mlxsw/sch_red_ets.sh | 24 +--
.../drivers/net/mlxsw/sch_red_root.sh | 18 +-
.../selftests/net/forwarding/sch_red.sh | 103 ++++-------
4 files changed, 141 insertions(+), 175 deletions(-)
diff --git a/tools/testing/selftests/drivers/net/mlxsw/sch_red_core.sh b/tools/testing/selftests/drivers/net/mlxsw/sch_red_core.sh
index f4c324957dcc..4282142c8eca 100644
--- a/tools/testing/selftests/drivers/net/mlxsw/sch_red_core.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/sch_red_core.sh
@@ -89,39 +89,31 @@ host_create()
local host=$1; shift
simple_if_init $dev
+ defer simple_if_fini $dev
+
mtu_set $dev 10000
+ defer mtu_restore $dev
vlan_create $dev 10 v$dev $(ipaddr $host 10)/28
+ defer vlan_destroy $dev 10
ip link set dev $dev.10 type vlan egress 0:0
vlan_create $dev 11 v$dev $(ipaddr $host 11)/28
+ defer vlan_destroy $dev 11
ip link set dev $dev.11 type vlan egress 0:1
}
-host_destroy()
-{
- local dev=$1; shift
-
- vlan_destroy $dev 11
- vlan_destroy $dev 10
- mtu_restore $dev
- simple_if_fini $dev
-}
-
h1_create()
{
host_create $h1 1
}
-h1_destroy()
-{
- host_destroy $h1
-}
-
h2_create()
{
host_create $h2 2
+
tc qdisc add dev $h2 clsact
+ defer tc qdisc del dev $h2 clsact
# Some of the tests in this suite use multicast traffic. As this traffic
# enters BR2_10 resp. BR2_11, it is flooded to all other ports. Thus
@@ -139,13 +131,7 @@ h2_create()
tc qdisc replace dev $h2 root handle 10: tbf rate 200mbit \
burst 128K limit 1G
-}
-
-h2_destroy()
-{
- tc qdisc del dev $h2 root handle 10:
- tc qdisc del dev $h2 clsact
- host_destroy $h2
+ defer tc qdisc del dev $h2 root handle 10:
}
h3_create()
@@ -153,40 +139,54 @@ h3_create()
host_create $h3 3
}
-h3_destroy()
-{
- host_destroy $h3
-}
-
switch_create()
{
local intf
local vlan
ip link add dev br1_10 type bridge
+ defer ip link del dev br1_10
+
ip link add dev br1_11 type bridge
+ defer ip link del dev br1_11
ip link add dev br2_10 type bridge
+ defer ip link del dev br2_10
+
ip link add dev br2_11 type bridge
+ defer ip link del dev br2_11
for intf in $swp1 $swp2 $swp3 $swp4 $swp5; do
ip link set dev $intf up
+ defer ip link set dev $intf down
+
mtu_set $intf 10000
+ defer mtu_restore $intf
done
for intf in $swp1 $swp4; do
for vlan in 10 11; do
vlan_create $intf $vlan
+ defer vlan_destroy $intf $vlan
+
ip link set dev $intf.$vlan master br1_$vlan
+ defer ip link set dev $intf.$vlan nomaster
+
ip link set dev $intf.$vlan up
+ defer ip link set dev $intf.$vlan up
done
done
for intf in $swp2 $swp3 $swp5; do
for vlan in 10 11; do
vlan_create $intf $vlan
+ defer vlan_destroy $intf $vlan
+
ip link set dev $intf.$vlan master br2_$vlan
+ defer ip link set dev $intf.$vlan nomaster
+
ip link set dev $intf.$vlan up
+ defer ip link set dev $intf.$vlan up
done
done
@@ -201,49 +201,25 @@ switch_create()
for intf in $swp3 $swp4; do
tc qdisc replace dev $intf root handle 1: tbf rate 200mbit \
burst 128K limit 1G
+ defer tc qdisc del dev $intf root handle 1:
done
ip link set dev br1_10 up
+ defer ip link set dev br1_10 down
+
ip link set dev br1_11 up
+ defer ip link set dev br1_11 down
+
ip link set dev br2_10 up
+ defer ip link set dev br2_10 down
+
ip link set dev br2_11 up
+ defer ip link set dev br2_11 down
local size=$(devlink_pool_size_thtype 0 | cut -d' ' -f 1)
devlink_port_pool_th_save $swp3 8
devlink_port_pool_th_set $swp3 8 $size
-}
-
-switch_destroy()
-{
- local intf
- local vlan
-
- devlink_port_pool_th_restore $swp3 8
-
- ip link set dev br2_11 down
- ip link set dev br2_10 down
- ip link set dev br1_11 down
- ip link set dev br1_10 down
-
- for intf in $swp4 $swp3; do
- tc qdisc del dev $intf root handle 1:
- done
-
- for intf in $swp5 $swp3 $swp2 $swp4 $swp1; do
- for vlan in 11 10; do
- ip link set dev $intf.$vlan down
- ip link set dev $intf.$vlan nomaster
- vlan_destroy $intf $vlan
- done
-
- mtu_restore $intf
- ip link set dev $intf down
- done
-
- ip link del dev br2_11
- ip link del dev br2_10
- ip link del dev br1_11
- ip link del dev br1_10
+ defer devlink_port_pool_th_restore $swp3 8
}
setup_prepare()
@@ -263,6 +239,7 @@ setup_prepare()
h3_mac=$(mac_get $h3)
vrf_prepare
+ defer vrf_cleanup
h1_create
h2_create
@@ -270,18 +247,6 @@ setup_prepare()
switch_create
}
-cleanup()
-{
- pre_cleanup
-
- switch_destroy
- h3_destroy
- h2_destroy
- h1_destroy
-
- vrf_cleanup
-}
-
ping_ipv4()
{
ping_test $h1.10 $(ipaddr 3 10) " from host 1, vlan 10"
@@ -450,6 +415,7 @@ __do_ecn_test()
start_tcp_traffic $h1.$vlan $(ipaddr 1 $vlan) $(ipaddr 3 $vlan) \
$h3_mac tos=0x01
+ defer stop_traffic $!
sleep 1
ecn_test_common "$name" "$get_nmarked" $vlan $limit
@@ -462,7 +428,6 @@ __do_ecn_test()
check_fail $? "UDP traffic went into backlog instead of being early-dropped"
log_test "TC $((vlan - 10)): $name backlog > limit: UDP early-dropped"
- stop_traffic
sleep 1
}
@@ -471,7 +436,8 @@ do_ecn_test()
local vlan=$1; shift
local limit=$1; shift
- __do_ecn_test get_nmarked "$vlan" "$limit"
+ in_defer_scope \
+ __do_ecn_test get_nmarked "$vlan" "$limit"
}
do_ecn_test_perband()
@@ -480,10 +446,11 @@ do_ecn_test_perband()
local limit=$1; shift
mlxsw_only_on_spectrum 3+ || return
- __do_ecn_test get_qdisc_nmarked "$vlan" "$limit" "per-band ECN"
+ in_defer_scope \
+ __do_ecn_test get_qdisc_nmarked "$vlan" "$limit" "per-band ECN"
}
-do_ecn_nodrop_test()
+__do_ecn_nodrop_test()
{
local vlan=$1; shift
local limit=$1; shift
@@ -491,6 +458,7 @@ do_ecn_nodrop_test()
start_tcp_traffic $h1.$vlan $(ipaddr 1 $vlan) $(ipaddr 3 $vlan) \
$h3_mac tos=0x01
+ defer stop_traffic $!
sleep 1
ecn_test_common "$name" get_nmarked $vlan $limit
@@ -503,11 +471,16 @@ do_ecn_nodrop_test()
check_err $? "UDP traffic was early-dropped instead of getting into backlog"
log_test "TC $((vlan - 10)): $name backlog > limit: UDP not dropped"
- stop_traffic
sleep 1
}
-do_red_test()
+do_ecn_nodrop_test()
+{
+ in_defer_scope \
+ __do_ecn_nodrop_test "$@"
+}
+
+__do_red_test()
{
local vlan=$1; shift
local limit=$1; shift
@@ -518,6 +491,7 @@ do_red_test()
# is above limit.
start_tcp_traffic $h1.$vlan $(ipaddr 1 $vlan) $(ipaddr 3 $vlan) \
$h3_mac tos=0x01
+ defer stop_traffic $!
# Pushing below the queue limit should work.
RET=0
@@ -540,11 +514,16 @@ do_red_test()
check_err $? "backlog $backlog / $limit expected <= 15% distance"
log_test "TC $((vlan - 10)): RED backlog > limit"
- stop_traffic
sleep 1
}
-do_mc_backlog_test()
+do_red_test()
+{
+ in_defer_scope \
+ __do_red_test "$@"
+}
+
+__do_mc_backlog_test()
{
local vlan=$1; shift
local limit=$1; shift
@@ -554,7 +533,10 @@ do_mc_backlog_test()
RET=0
start_tcp_traffic $h1.$vlan $(ipaddr 1 $vlan) $(ipaddr 3 $vlan) bc
+ defer stop_traffic $!
+
start_tcp_traffic $h2.$vlan $(ipaddr 2 $vlan) $(ipaddr 3 $vlan) bc
+ defer stop_traffic $!
qbl=$(busywait 5000 until_counter_is ">= 500000" \
get_qdisc_backlog $vlan)
@@ -567,13 +549,16 @@ do_mc_backlog_test()
get_mc_transmit_queue $vlan)
check_err $? "MC backlog reported by qdisc not visible in ethtool"
- stop_traffic
- stop_traffic
-
log_test "TC $((vlan - 10)): Qdisc reports MC backlog"
}
-do_mark_test()
+do_mc_backlog_test()
+{
+ in_defer_scope \
+ __do_mc_backlog_test "$@"
+}
+
+__do_mark_test()
{
local vlan=$1; shift
local limit=$1; shift
@@ -588,6 +573,7 @@ do_mark_test()
start_tcp_traffic $h1.$vlan $(ipaddr 1 $vlan) $(ipaddr 3 $vlan) \
$h3_mac tos=0x01
+ defer stop_traffic $!
# Create a bit of a backlog and observe no mirroring due to marks.
qevent_rule_install_$subtest
@@ -618,11 +604,16 @@ do_mark_test()
log_test "TC $((vlan - 10)): marked packets $subtest'd"
fi
- stop_traffic
sleep 1
}
-do_drop_test()
+do_mark_test()
+{
+ in_defer_scope \
+ __do_mark_test "$@"
+}
+
+__do_drop_test()
{
local vlan=$1; shift
local limit=$1; shift
@@ -637,6 +628,7 @@ do_drop_test()
RET=0
start_traffic $h1.$vlan $(ipaddr 1 $vlan) $(ipaddr 3 $vlan) $h3_mac
+ defer stop_traffic $!
# Create a bit of a backlog and observe no mirroring due to drops.
qevent_rule_install_$subtest
@@ -672,10 +664,15 @@ do_drop_test()
log_test "TC $((vlan - 10)): ${trigger}ped packets $subtest'd"
- stop_traffic
sleep 1
}
+do_drop_test()
+{
+ in_defer_scope \
+ __do_drop_test "$@"
+}
+
qevent_rule_install_mirror()
{
tc filter add block 10 pref 1234 handle 102 matchall skip_sw \
diff --git a/tools/testing/selftests/drivers/net/mlxsw/sch_red_ets.sh b/tools/testing/selftests/drivers/net/mlxsw/sch_red_ets.sh
index 576067b207a8..8902a115d9cd 100755
--- a/tools/testing/selftests/drivers/net/mlxsw/sch_red_ets.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/sch_red_ets.sh
@@ -80,36 +80,34 @@ uninstall_qdisc()
ecn_test()
{
install_qdisc ecn
+ defer uninstall_qdisc
do_ecn_test 10 $BACKLOG1
do_ecn_test 11 $BACKLOG2
-
- uninstall_qdisc
}
ecn_test_perband()
{
install_qdisc ecn
+ defer uninstall_qdisc
do_ecn_test_perband 10 $BACKLOG1
do_ecn_test_perband 11 $BACKLOG2
-
- uninstall_qdisc
}
ecn_nodrop_test()
{
install_qdisc ecn nodrop
+ defer uninstall_qdisc
do_ecn_nodrop_test 10 $BACKLOG1
do_ecn_nodrop_test 11 $BACKLOG2
-
- uninstall_qdisc
}
red_test()
{
install_qdisc
+ defer uninstall_qdisc
# Make sure that we get the non-zero value if there is any.
local cur=$(busywait 1100 until_counter_is "> 0" \
@@ -120,50 +118,44 @@ red_test()
do_red_test 10 $BACKLOG1
do_red_test 11 $BACKLOG2
-
- uninstall_qdisc
}
mc_backlog_test()
{
install_qdisc
+ defer uninstall_qdisc
# Note that the backlog numbers here do not correspond to RED
# configuration, but are arbitrary.
do_mc_backlog_test 10 $BACKLOG1
do_mc_backlog_test 11 $BACKLOG2
-
- uninstall_qdisc
}
red_mirror_test()
{
install_qdisc qevent early_drop block 10
+ defer uninstall_qdisc
do_drop_mirror_test 10 $BACKLOG1 early_drop
do_drop_mirror_test 11 $BACKLOG2 early_drop
-
- uninstall_qdisc
}
red_trap_test()
{
install_qdisc qevent early_drop block 10
+ defer uninstall_qdisc
do_drop_trap_test 10 $BACKLOG1 early_drop
do_drop_trap_test 11 $BACKLOG2 early_drop
-
- uninstall_qdisc
}
ecn_mirror_test()
{
install_qdisc ecn qevent mark block 10
+ defer uninstall_qdisc
do_mark_mirror_test 10 $BACKLOG1
do_mark_mirror_test 11 $BACKLOG2
-
- uninstall_qdisc
}
bail_on_lldpad "configure DCB" "configure Qdiscs"
diff --git a/tools/testing/selftests/drivers/net/mlxsw/sch_red_root.sh b/tools/testing/selftests/drivers/net/mlxsw/sch_red_root.sh
index 159108d02895..e9043771787b 100755
--- a/tools/testing/selftests/drivers/net/mlxsw/sch_red_root.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/sch_red_root.sh
@@ -32,45 +32,51 @@ uninstall_qdisc()
ecn_test()
{
install_qdisc ecn
+ defer uninstall_qdisc
+
do_ecn_test 10 $BACKLOG
- uninstall_qdisc
}
ecn_test_perband()
{
install_qdisc ecn
+ defer uninstall_qdisc
+
do_ecn_test_perband 10 $BACKLOG
- uninstall_qdisc
}
ecn_nodrop_test()
{
install_qdisc ecn nodrop
+ defer uninstall_qdisc
+
do_ecn_nodrop_test 10 $BACKLOG
- uninstall_qdisc
}
red_test()
{
install_qdisc
+ defer uninstall_qdisc
+
do_red_test 10 $BACKLOG
- uninstall_qdisc
}
mc_backlog_test()
{
install_qdisc
+ defer uninstall_qdisc
+
# Note that the backlog value here does not correspond to RED
# configuration, but is arbitrary.
do_mc_backlog_test 10 $BACKLOG
- uninstall_qdisc
}
red_mirror_test()
{
install_qdisc qevent early_drop block 10
+ defer uninstall_qdisc
+
do_drop_mirror_test 10 $BACKLOG
- uninstall_qdisc
}
bail_on_lldpad "configure DCB" "configure Qdiscs"
diff --git a/tools/testing/selftests/net/forwarding/sch_red.sh b/tools/testing/selftests/net/forwarding/sch_red.sh
index 17f28644568e..af166662b78a 100755
--- a/tools/testing/selftests/net/forwarding/sch_red.sh
+++ b/tools/testing/selftests/net/forwarding/sch_red.sh
@@ -53,71 +53,63 @@ PKTSZ=1400
h1_create()
{
simple_if_init $h1 192.0.2.1/28
+ defer simple_if_fini $h1 192.0.2.1/28
+
mtu_set $h1 10000
+ defer mtu_restore $h1
+
tc qdisc replace dev $h1 root handle 1: tbf \
rate 10Mbit burst 10K limit 1M
-}
-
-h1_destroy()
-{
- tc qdisc del dev $h1 root
- mtu_restore $h1
- simple_if_fini $h1 192.0.2.1/28
+ defer tc qdisc del dev $h1 root
}
h2_create()
{
simple_if_init $h2 192.0.2.2/28
+ defer simple_if_fini $h2 192.0.2.2/28
+
mtu_set $h2 10000
-}
-
-h2_destroy()
-{
- mtu_restore $h2
- simple_if_fini $h2 192.0.2.2/28
+ defer mtu_restore $h2
}
h3_create()
{
simple_if_init $h3 192.0.2.3/28
+ defer simple_if_fini $h3 192.0.2.3/28
+
mtu_set $h3 10000
-}
-
-h3_destroy()
-{
- mtu_restore $h3
- simple_if_fini $h3 192.0.2.3/28
+ defer mtu_restore $h3
}
switch_create()
{
ip link add dev br up type bridge
+ defer ip link del dev br
+
ip link set dev $swp1 up master br
+ defer ip link set dev $swp1 down nomaster
+
ip link set dev $swp2 up master br
+ defer ip link set dev $swp2 down nomaster
+
ip link set dev $swp3 up master br
+ defer ip link set dev $swp3 down nomaster
mtu_set $swp1 10000
+ defer mtu_restore $h1
+
mtu_set $swp2 10000
+ defer mtu_restore $h2
+
mtu_set $swp3 10000
+ defer mtu_restore $h3
tc qdisc replace dev $swp3 root handle 1: tbf \
rate 10Mbit burst 10K limit 1M
+ defer tc qdisc del dev $swp3 root
+
ip link add name _drop_test up type dummy
-}
-
-switch_destroy()
-{
- ip link del dev _drop_test
- tc qdisc del dev $swp3 root
-
- mtu_restore $h3
- mtu_restore $h2
- mtu_restore $h1
-
- ip link set dev $swp3 down nomaster
- ip link set dev $swp2 down nomaster
- ip link set dev $swp1 down nomaster
- ip link del dev br
+ defer ip link del dev _drop_test
}
setup_prepare()
@@ -134,6 +126,7 @@ setup_prepare()
h3_mac=$(mac_get $h3)
vrf_prepare
+ defer vrf_cleanup
h1_create
h2_create
@@ -141,18 +134,6 @@ setup_prepare()
switch_create
}
-cleanup()
-{
- pre_cleanup
-
- switch_destroy
- h3_destroy
- h2_destroy
- h1_destroy
-
- vrf_cleanup
-}
-
ping_ipv4()
{
ping_test $h1 192.0.2.3 " from host 1"
@@ -287,6 +268,7 @@ do_ecn_test()
$MZ $h1 -p $PKTSZ -A 192.0.2.1 -B 192.0.2.3 -c 0 \
-a own -b $h3_mac -t tcp -q tos=0x01 &
+ defer stop_traffic $!
sleep 1
ecn_test_common "$name" $limit
@@ -298,9 +280,6 @@ do_ecn_test()
build_backlog $((2 * limit)) udp >/dev/null
check_fail $? "UDP traffic went into backlog instead of being early-dropped"
log_test "$name backlog > limit: UDP early-dropped"
-
- stop_traffic
- sleep 1
}
do_ecn_nodrop_test()
@@ -310,6 +289,7 @@ do_ecn_nodrop_test()
$MZ $h1 -p $PKTSZ -A 192.0.2.1 -B 192.0.2.3 -c 0 \
-a own -b $h3_mac -t tcp -q tos=0x01 &
+ defer stop_traffic $!
sleep 1
ecn_test_common "$name" $limit
@@ -321,9 +301,6 @@ do_ecn_nodrop_test()
build_backlog $((2 * limit)) udp >/dev/null
check_err $? "UDP traffic was early-dropped instead of getting into backlog"
log_test "$name backlog > limit: UDP not dropped"
-
- stop_traffic
- sleep 1
}
do_red_test()
@@ -336,6 +313,7 @@ do_red_test()
# is above limit.
$MZ $h1 -p $PKTSZ -A 192.0.2.1 -B 192.0.2.3 -c 0 \
-a own -b $h3_mac -t tcp -q tos=0x01 &
+ defer stop_traffic $!
# Pushing below the queue limit should work.
RET=0
@@ -352,9 +330,6 @@ do_red_test()
pct=$(check_marking "== 0")
check_err $? "backlog $backlog / $limit Got $pct% marked packets, expected == 0."
log_test "RED backlog > limit"
-
- stop_traffic
- sleep 1
}
do_red_qevent_test()
@@ -369,6 +344,7 @@ do_red_qevent_test()
$MZ $h1 -p $PKTSZ -A 192.0.2.1 -B 192.0.2.3 -c 0 \
-a own -b $h3_mac -t udp -q &
+ defer stop_traffic $!
sleep 1
tc filter add block 10 pref 1234 handle 102 matchall skip_hw \
@@ -396,9 +372,6 @@ do_red_qevent_test()
check_err $? "Dropped packets still observed: 0 expected, $((now - base)) seen"
log_test "RED early_dropped packets mirrored"
-
- stop_traffic
- sleep 1
}
do_ecn_qevent_test()
@@ -410,6 +383,7 @@ do_ecn_qevent_test()
$MZ $h1 -p $PKTSZ -A 192.0.2.1 -B 192.0.2.3 -c 0 \
-a own -b $h3_mac -t tcp -q tos=0x01 &
+ defer stop_traffic $!
sleep 1
tc filter add block 10 pref 1234 handle 102 matchall skip_hw \
@@ -428,9 +402,6 @@ do_ecn_qevent_test()
tc filter del block 10 pref 1234 handle 102 matchall
log_test "ECN marked packets mirrored"
-
- stop_traffic
- sleep 1
}
install_qdisc()
@@ -451,36 +422,36 @@ uninstall_qdisc()
ecn_test()
{
install_qdisc ecn
+ defer uninstall_qdisc
xfail_on_slow do_ecn_test $BACKLOG
- uninstall_qdisc
}
ecn_nodrop_test()
{
install_qdisc ecn nodrop
+ defer uninstall_qdisc
xfail_on_slow do_ecn_nodrop_test $BACKLOG
- uninstall_qdisc
}
red_test()
{
install_qdisc
+ defer uninstall_qdisc
xfail_on_slow do_red_test $BACKLOG
- uninstall_qdisc
}
red_qevent_test()
{
install_qdisc qevent early_drop block 10
+ defer uninstall_qdisc
xfail_on_slow do_red_qevent_test $BACKLOG
- uninstall_qdisc
}
ecn_qevent_test()
{
install_qdisc ecn qevent mark block 10
+ defer uninstall_qdisc
xfail_on_slow do_ecn_qevent_test $BACKLOG
- uninstall_qdisc
}
trap cleanup EXIT
--
2.45.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH net-next 05/10] selftests: TBF: Use defer for test cleanup
2024-10-09 12:06 [PATCH net-next 00/10] selftests: net: Introduce deferred commands Petr Machata
` (3 preceding siblings ...)
2024-10-09 12:06 ` [PATCH net-next 04/10] selftests: RED: Use defer for test cleanup Petr Machata
@ 2024-10-09 12:06 ` Petr Machata
2024-10-09 12:06 ` [PATCH net-next 06/10] selftests: ETS: " Petr Machata
` (4 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Petr Machata @ 2024-10-09 12:06 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
netdev
Cc: linux-kselftest, Shuah Khan, Benjamin Poirier, Hangbin Liu,
Vladimir Oltean, Ido Schimmel, Przemek Kitszel, Petr Machata,
mlxsw
Use the defer framework to schedule cleanups as soon as the command is
executed.
Signed-off-by: Petr Machata <petrm@nvidia.com>
---
.../selftests/net/forwarding/sch_tbf_core.sh | 91 ++++++-------------
.../net/forwarding/sch_tbf_etsprio.sh | 7 +-
.../selftests/net/forwarding/sch_tbf_root.sh | 3 +-
3 files changed, 36 insertions(+), 65 deletions(-)
diff --git a/tools/testing/selftests/net/forwarding/sch_tbf_core.sh b/tools/testing/selftests/net/forwarding/sch_tbf_core.sh
index 9cd884d4a5de..ec309a5086bc 100644
--- a/tools/testing/selftests/net/forwarding/sch_tbf_core.sh
+++ b/tools/testing/selftests/net/forwarding/sch_tbf_core.sh
@@ -60,68 +60,65 @@ host_create()
local host=$1; shift
simple_if_init $dev
+ defer simple_if_fini $dev
+
mtu_set $dev 10000
+ defer mtu_restore $dev
vlan_create $dev 10 v$dev $(ipaddr $host 10)/28
+ defer vlan_destroy $dev 10
ip link set dev $dev.10 type vlan egress 0:0
vlan_create $dev 11 v$dev $(ipaddr $host 11)/28
+ defer vlan_destroy $dev 11
ip link set dev $dev.11 type vlan egress 0:1
}
-host_destroy()
-{
- local dev=$1; shift
-
- vlan_destroy $dev 11
- vlan_destroy $dev 10
- mtu_restore $dev
- simple_if_fini $dev
-}
-
h1_create()
{
host_create $h1 1
}
-h1_destroy()
-{
- host_destroy $h1
-}
-
h2_create()
{
host_create $h2 2
tc qdisc add dev $h2 clsact
+ defer tc qdisc del dev $h2 clsact
+
tc filter add dev $h2 ingress pref 1010 prot 802.1q \
flower $TCFLAGS vlan_id 10 action pass
tc filter add dev $h2 ingress pref 1011 prot 802.1q \
flower $TCFLAGS vlan_id 11 action pass
}
-h2_destroy()
-{
- tc qdisc del dev $h2 clsact
- host_destroy $h2
-}
-
switch_create()
{
local intf
local vlan
ip link add dev br10 type bridge
+ defer ip link del dev br10
+
ip link add dev br11 type bridge
+ defer ip link del dev br11
for intf in $swp1 $swp2; do
ip link set dev $intf up
+ defer ip link set dev $intf down
+
mtu_set $intf 10000
+ defer mtu_restore $intf
for vlan in 10 11; do
vlan_create $intf $vlan
+ defer vlan_destroy $intf $vlan
+
ip link set dev $intf.$vlan master br$vlan
+ defer ip link set dev $intf.$vlan nomaster
+
ip link set dev $intf.$vlan up
+ defer ip link set dev $intf.$vlan down
done
done
@@ -130,34 +127,10 @@ switch_create()
done
ip link set dev br10 up
+ defer ip link set dev br10 down
+
ip link set dev br11 up
-}
-
-switch_destroy()
-{
- local intf
- local vlan
-
- # A test may have been interrupted mid-run, with Qdisc installed. Delete
- # it here.
- tc qdisc del dev $swp2 root 2>/dev/null
-
- ip link set dev br11 down
- ip link set dev br10 down
-
- for intf in $swp2 $swp1; do
- for vlan in 11 10; do
- ip link set dev $intf.$vlan down
- ip link set dev $intf.$vlan nomaster
- vlan_destroy $intf $vlan
- done
-
- mtu_restore $intf
- ip link set dev $intf down
- done
-
- ip link del dev br11
- ip link del dev br10
+ defer ip link set dev br11 down
}
setup_prepare()
@@ -177,23 +150,13 @@ setup_prepare()
h2_mac=$(mac_get $h2)
vrf_prepare
+ defer vrf_cleanup
h1_create
h2_create
switch_create
}
-cleanup()
-{
- pre_cleanup
-
- switch_destroy
- h2_destroy
- h1_destroy
-
- vrf_cleanup
-}
-
ping_ipv4()
{
ping_test $h1.10 $(ipaddr 2 10) " vlan 10"
@@ -207,18 +170,18 @@ tbf_get_counter()
tc_rule_stats_get $h2 10$vlan ingress .bytes
}
-do_tbf_test()
+__tbf_test()
{
local vlan=$1; shift
local mbit=$1; shift
start_traffic $h1.$vlan $(ipaddr 1 $vlan) $(ipaddr 2 $vlan) $h2_mac
+ defer stop_traffic $!
sleep 5 # Wait for the burst to dwindle
local t2=$(busywait_for_counter 1000 +1 tbf_get_counter $vlan)
sleep 10
local t3=$(tbf_get_counter $vlan)
- stop_traffic
RET=0
@@ -231,3 +194,9 @@ do_tbf_test()
log_test "TC $((vlan - 10)): TBF rate ${mbit}Mbit"
}
+
+do_tbf_test()
+{
+ in_defer_scope \
+ __tbf_test "$@"
+}
diff --git a/tools/testing/selftests/net/forwarding/sch_tbf_etsprio.sh b/tools/testing/selftests/net/forwarding/sch_tbf_etsprio.sh
index df9bcd6a811a..c182a04282bc 100644
--- a/tools/testing/selftests/net/forwarding/sch_tbf_etsprio.sh
+++ b/tools/testing/selftests/net/forwarding/sch_tbf_etsprio.sh
@@ -30,8 +30,9 @@ tbf_test()
# This test is used for both ETS and PRIO. Even though we only need two
# bands, PRIO demands a minimum of three.
tc qdisc add dev $swp2 root handle 10: $QDISC 3 priomap 2 1 0
+ defer tc qdisc del dev $swp2 root
+
tbf_test_one 128K
- tc qdisc del dev $swp2 root
}
tbf_root_test()
@@ -42,6 +43,8 @@ tbf_root_test()
tc qdisc replace dev $swp2 root handle 1: \
tbf rate 400Mbit burst $bs limit 1M
+ defer tc qdisc del dev $swp2 root
+
tc qdisc replace dev $swp2 parent 1:1 handle 10: \
$QDISC 3 priomap 2 1 0
tc qdisc replace dev $swp2 parent 10:3 handle 103: \
@@ -53,8 +56,6 @@ tbf_root_test()
do_tbf_test 10 400 $bs
do_tbf_test 11 400 $bs
-
- tc qdisc del dev $swp2 root
}
if type -t sch_tbf_pre_hook >/dev/null; then
diff --git a/tools/testing/selftests/net/forwarding/sch_tbf_root.sh b/tools/testing/selftests/net/forwarding/sch_tbf_root.sh
index 96c997be0d03..9f20320f8d84 100755
--- a/tools/testing/selftests/net/forwarding/sch_tbf_root.sh
+++ b/tools/testing/selftests/net/forwarding/sch_tbf_root.sh
@@ -14,13 +14,14 @@ tbf_test_one()
tc qdisc replace dev $swp2 root handle 108: tbf \
rate 400Mbit burst $bs limit 1M
+ defer tc qdisc del dev $swp2 root
+
do_tbf_test 10 400 $bs
}
tbf_test()
{
tbf_test_one 128K
- tc qdisc del dev $swp2 root
}
if type -t sch_tbf_pre_hook >/dev/null; then
--
2.45.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH net-next 06/10] selftests: ETS: Use defer for test cleanup
2024-10-09 12:06 [PATCH net-next 00/10] selftests: net: Introduce deferred commands Petr Machata
` (4 preceding siblings ...)
2024-10-09 12:06 ` [PATCH net-next 05/10] selftests: TBF: " Petr Machata
@ 2024-10-09 12:06 ` Petr Machata
2024-10-09 12:06 ` [PATCH net-next 07/10] selftests: mlxsw: qos_mc_aware: " Petr Machata
` (3 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Petr Machata @ 2024-10-09 12:06 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
netdev
Cc: linux-kselftest, Shuah Khan, Benjamin Poirier, Hangbin Liu,
Vladimir Oltean, Ido Schimmel, Przemek Kitszel, Petr Machata,
mlxsw
Use the defer framework to schedule cleanups as soon as the command is
executed.
Signed-off-by: Petr Machata <petrm@nvidia.com>
---
.../selftests/drivers/net/mlxsw/sch_ets.sh | 26 +++---
.../selftests/net/forwarding/sch_ets.sh | 7 +-
.../selftests/net/forwarding/sch_ets_core.sh | 81 +++++++------------
.../selftests/net/forwarding/sch_ets_tests.sh | 14 ++--
4 files changed, 50 insertions(+), 78 deletions(-)
diff --git a/tools/testing/selftests/drivers/net/mlxsw/sch_ets.sh b/tools/testing/selftests/drivers/net/mlxsw/sch_ets.sh
index 139175fd03e7..4aaceb6b2b60 100755
--- a/tools/testing/selftests/drivers/net/mlxsw/sch_ets.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/sch_ets.sh
@@ -21,6 +21,7 @@ switch_create()
# Create a bottleneck so that the DWRR process can kick in.
tc qdisc replace dev $swp2 root handle 3: tbf rate 1gbit \
burst 128K limit 1G
+ defer tc qdisc del dev $swp2 root handle 3:
ets_switch_create
@@ -30,16 +31,27 @@ switch_create()
# for the DWRR process.
devlink_port_pool_th_save $swp1 0
devlink_port_pool_th_set $swp1 0 12
+ defer devlink_port_pool_th_restore $swp1 0
+
devlink_tc_bind_pool_th_save $swp1 0 ingress
devlink_tc_bind_pool_th_set $swp1 0 ingress 0 12
+ defer devlink_tc_bind_pool_th_restore $swp1 0 ingress
+
devlink_port_pool_th_save $swp2 4
devlink_port_pool_th_set $swp2 4 12
+ defer devlink_port_pool_th_restore $swp2 4
+
devlink_tc_bind_pool_th_save $swp2 7 egress
devlink_tc_bind_pool_th_set $swp2 7 egress 4 5
+ defer devlink_tc_bind_pool_th_restore $swp2 7 egress
+
devlink_tc_bind_pool_th_save $swp2 6 egress
devlink_tc_bind_pool_th_set $swp2 6 egress 4 5
+ defer devlink_tc_bind_pool_th_restore $swp2 6 egress
+
devlink_tc_bind_pool_th_save $swp2 5 egress
devlink_tc_bind_pool_th_set $swp2 5 egress 4 5
+ defer devlink_tc_bind_pool_th_restore $swp2 5 egress
# Note: sch_ets_core.sh uses VLAN ingress-qos-map to assign packet
# priorities at $swp1 based on their 802.1p headers. ingress-qos-map is
@@ -47,20 +59,6 @@ switch_create()
# 1:1, which is the mapping currently hard-coded by the driver.
}
-switch_destroy()
-{
- devlink_tc_bind_pool_th_restore $swp2 5 egress
- devlink_tc_bind_pool_th_restore $swp2 6 egress
- devlink_tc_bind_pool_th_restore $swp2 7 egress
- devlink_port_pool_th_restore $swp2 4
- devlink_tc_bind_pool_th_restore $swp1 0 ingress
- devlink_port_pool_th_restore $swp1 0
-
- ets_switch_destroy
-
- tc qdisc del dev $swp2 root handle 3:
-}
-
# Callback from sch_ets_tests.sh
collect_stats()
{
diff --git a/tools/testing/selftests/net/forwarding/sch_ets.sh b/tools/testing/selftests/net/forwarding/sch_ets.sh
index e60c8b4818cc..1f6f53e284b5 100755
--- a/tools/testing/selftests/net/forwarding/sch_ets.sh
+++ b/tools/testing/selftests/net/forwarding/sch_ets.sh
@@ -24,15 +24,10 @@ switch_create()
# Create a bottleneck so that the DWRR process can kick in.
tc qdisc add dev $swp2 root handle 1: tbf \
rate 1Gbit burst 1Mbit latency 100ms
+ defer tc qdisc del dev $swp2 root
PARENT="parent 1:"
}
-switch_destroy()
-{
- ets_switch_destroy
- tc qdisc del dev $swp2 root
-}
-
# Callback from sch_ets_tests.sh
collect_stats()
{
diff --git a/tools/testing/selftests/net/forwarding/sch_ets_core.sh b/tools/testing/selftests/net/forwarding/sch_ets_core.sh
index f906fcc66572..8f9922c695b0 100644
--- a/tools/testing/selftests/net/forwarding/sch_ets_core.sh
+++ b/tools/testing/selftests/net/forwarding/sch_ets_core.sh
@@ -166,89 +166,78 @@ h1_create()
local i;
simple_if_init $h1
+ defer simple_if_fini $h1
+
mtu_set $h1 9900
+ defer mtu_restore $h1
+
for i in {0..2}; do
vlan_create $h1 1$i v$h1 $(sip $i)/28
+ defer vlan_destroy $h1 1$i
ip link set dev $h1.1$i type vlan egress 0:$i
done
}
-h1_destroy()
-{
- local i
-
- for i in {0..2}; do
- vlan_destroy $h1 1$i
- done
- mtu_restore $h1
- simple_if_fini $h1
-}
-
h2_create()
{
local i
simple_if_init $h2
+ defer simple_if_fini $h2
+
mtu_set $h2 9900
+ defer mtu_restore $h2
+
for i in {0..2}; do
vlan_create $h2 1$i v$h2 $(dip $i)/28
+ defer vlan_destroy $h2 1$i
done
}
-h2_destroy()
-{
- local i
-
- for i in {0..2}; do
- vlan_destroy $h2 1$i
- done
- mtu_restore $h2
- simple_if_fini $h2
-}
-
ets_switch_create()
{
local i
ip link set dev $swp1 up
+ defer ip link set dev $swp1 down
+
mtu_set $swp1 9900
+ defer mtu_restore $swp1
ip link set dev $swp2 up
+ defer ip link set dev $swp2 down
+
mtu_set $swp2 9900
+ defer mtu_restore $swp2
for i in {0..2}; do
vlan_create $swp1 1$i
+ defer vlan_destroy $swp1 1$i
ip link set dev $swp1.1$i type vlan ingress 0:0 1:1 2:2
vlan_create $swp2 1$i
+ defer vlan_destroy $swp2 1$i
ip link add dev br1$i type bridge
+ defer ip link del dev br1$i
+
ip link set dev $swp1.1$i master br1$i
+ defer ip link set dev $swp1.1$i nomaster
+
ip link set dev $swp2.1$i master br1$i
+ defer ip link set dev $swp2.1$i nomaster
ip link set dev br1$i up
+ defer ip link set dev br1$i down
+
ip link set dev $swp1.1$i up
+ defer ip link set dev $swp1.1$i down
+
ip link set dev $swp2.1$i up
+ defer ip link set dev $swp2.1$i down
done
-}
-ets_switch_destroy()
-{
- local i
-
- ets_delete_qdisc
-
- for i in {0..2}; do
- ip link del dev br1$i
- vlan_destroy $swp2 1$i
- vlan_destroy $swp1 1$i
- done
-
- mtu_restore $swp2
- ip link set dev $swp2 down
-
- mtu_restore $swp1
- ip link set dev $swp1 down
+ defer ets_delete_qdisc
}
setup_prepare()
@@ -263,23 +252,13 @@ setup_prepare()
hut=$h2
vrf_prepare
+ defer vrf_cleanup
h1_create
h2_create
switch_create
}
-cleanup()
-{
- pre_cleanup
-
- switch_destroy
- h2_destroy
- h1_destroy
-
- vrf_cleanup
-}
-
ping_ipv4()
{
ping_test $h1.10 $(dip 0) " vlan 10"
diff --git a/tools/testing/selftests/net/forwarding/sch_ets_tests.sh b/tools/testing/selftests/net/forwarding/sch_ets_tests.sh
index f9d26a7911bb..08240d3e3c87 100644
--- a/tools/testing/selftests/net/forwarding/sch_ets_tests.sh
+++ b/tools/testing/selftests/net/forwarding/sch_ets_tests.sh
@@ -90,6 +90,7 @@ __ets_dwrr_test()
for stream in ${streams[@]}; do
ets_start_traffic $stream
+ defer stop_traffic $!
done
sleep 10
@@ -120,25 +121,24 @@ __ets_dwrr_test()
${d[0]} ${d[$i]}
fi
done
-
- for stream in ${streams[@]}; do
- stop_traffic
- done
}
ets_dwrr_test_012()
{
- __ets_dwrr_test 0 1 2
+ in_defer_scope \
+ __ets_dwrr_test 0 1 2
}
ets_dwrr_test_01()
{
- __ets_dwrr_test 0 1
+ in_defer_scope \
+ __ets_dwrr_test 0 1
}
ets_dwrr_test_12()
{
- __ets_dwrr_test 1 2
+ in_defer_scope \
+ __ets_dwrr_test 1 2
}
ets_qdisc_setup()
--
2.45.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH net-next 07/10] selftests: mlxsw: qos_mc_aware: Use defer for test cleanup
2024-10-09 12:06 [PATCH net-next 00/10] selftests: net: Introduce deferred commands Petr Machata
` (5 preceding siblings ...)
2024-10-09 12:06 ` [PATCH net-next 06/10] selftests: ETS: " Petr Machata
@ 2024-10-09 12:06 ` Petr Machata
2024-10-09 12:06 ` [PATCH net-next 08/10] selftests: mlxsw: qos_ets_strict: " Petr Machata
` (2 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Petr Machata @ 2024-10-09 12:06 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
netdev
Cc: linux-kselftest, Shuah Khan, Benjamin Poirier, Hangbin Liu,
Vladimir Oltean, Ido Schimmel, Przemek Kitszel, Petr Machata,
mlxsw
Use the defer framework to schedule cleanups as soon as the command is
executed.
Signed-off-by: Petr Machata <petrm@nvidia.com>
---
.../drivers/net/mlxsw/qos_mc_aware.sh | 146 ++++++++----------
1 file changed, 68 insertions(+), 78 deletions(-)
diff --git a/tools/testing/selftests/drivers/net/mlxsw/qos_mc_aware.sh b/tools/testing/selftests/drivers/net/mlxsw/qos_mc_aware.sh
index 6d892de43fa8..cd4a5c21360c 100755
--- a/tools/testing/selftests/drivers/net/mlxsw/qos_mc_aware.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/qos_mc_aware.sh
@@ -73,122 +73,114 @@ source qos_lib.sh
h1_create()
{
simple_if_init $h1 192.0.2.65/28
+ defer simple_if_fini $h1 192.0.2.65/28
+
mtu_set $h1 10000
-}
-
-h1_destroy()
-{
- mtu_restore $h1
- simple_if_fini $h1 192.0.2.65/28
+ defer mtu_restore $h1
}
h2_create()
{
simple_if_init $h2
+ defer simple_if_fini $h2
+
mtu_set $h2 10000
+ defer mtu_restore $h2
vlan_create $h2 111 v$h2 192.0.2.129/28
+ defer vlan_destroy $h2 111
ip link set dev $h2.111 type vlan egress-qos-map 0:1
}
-h2_destroy()
-{
- vlan_destroy $h2 111
-
- mtu_restore $h2
- simple_if_fini $h2
-}
-
h3_create()
{
simple_if_init $h3 192.0.2.66/28
+ defer simple_if_fini $h3 192.0.2.66/28
+
mtu_set $h3 10000
+ defer mtu_restore $h3
vlan_create $h3 111 v$h3 192.0.2.130/28
-}
-
-h3_destroy()
-{
- vlan_destroy $h3 111
-
- mtu_restore $h3
- simple_if_fini $h3 192.0.2.66/28
+ defer vlan_destroy $h3 111
}
switch_create()
{
ip link set dev $swp1 up
+ defer ip link set dev $swp1 down
+
mtu_set $swp1 10000
+ defer mtu_restore $swp1
ip link set dev $swp2 up
+ defer ip link set dev $swp2 down
+
mtu_set $swp2 10000
+ defer mtu_restore $swp2
ip link set dev $swp3 up
+ defer ip link set dev $swp3 down
+
mtu_set $swp3 10000
+ defer mtu_restore $swp3
vlan_create $swp2 111
+ defer vlan_destroy $swp2 111
+
vlan_create $swp3 111
+ defer vlan_destroy $swp3 111
tc qdisc replace dev $swp3 root handle 3: tbf rate 1gbit \
burst 128K limit 1G
+ defer tc qdisc del dev $swp3 root handle 3:
+
tc qdisc replace dev $swp3 parent 3:3 handle 33: \
prio bands 8 priomap 7 7 7 7 7 7 7 7
+ defer tc qdisc del dev $swp3 parent 3:3 handle 33:
ip link add name br1 type bridge vlan_filtering 0
+ defer ip link del dev br1
ip link set dev br1 addrgenmode none
ip link set dev br1 up
+
ip link set dev $swp1 master br1
+ defer ip link set dev $swp1 nomaster
+
ip link set dev $swp3 master br1
+ defer ip link set dev $swp3 nomaster
ip link add name br111 type bridge vlan_filtering 0
+ defer ip link del dev br111
ip link set dev br111 addrgenmode none
ip link set dev br111 up
+
ip link set dev $swp2.111 master br111
+ defer ip link set dev $swp2.111 nomaster
+
ip link set dev $swp3.111 master br111
+ defer ip link set dev $swp3.111 nomaster
# Make sure that ingress quotas are smaller than egress so that there is
# room for both streams of traffic to be admitted to shared buffer.
devlink_port_pool_th_save $swp1 0
devlink_port_pool_th_set $swp1 0 5
+ defer devlink_port_pool_th_restore $swp1 0
+
devlink_tc_bind_pool_th_save $swp1 0 ingress
devlink_tc_bind_pool_th_set $swp1 0 ingress 0 5
+ defer devlink_tc_bind_pool_th_restore $swp1 0 ingress
devlink_port_pool_th_save $swp2 0
devlink_port_pool_th_set $swp2 0 5
+ defer devlink_port_pool_th_restore $swp2 0
+
devlink_tc_bind_pool_th_save $swp2 1 ingress
devlink_tc_bind_pool_th_set $swp2 1 ingress 0 5
+ defer devlink_tc_bind_pool_th_restore $swp2 1 ingress
devlink_port_pool_th_save $swp3 4
devlink_port_pool_th_set $swp3 4 12
-}
-
-switch_destroy()
-{
- devlink_port_pool_th_restore $swp3 4
-
- devlink_tc_bind_pool_th_restore $swp2 1 ingress
- devlink_port_pool_th_restore $swp2 0
-
- devlink_tc_bind_pool_th_restore $swp1 0 ingress
- devlink_port_pool_th_restore $swp1 0
-
- ip link del dev br111
- ip link del dev br1
-
- tc qdisc del dev $swp3 parent 3:3 handle 33:
- tc qdisc del dev $swp3 root handle 3:
-
- vlan_destroy $swp3 111
- vlan_destroy $swp2 111
-
- mtu_restore $swp3
- ip link set dev $swp3 down
-
- mtu_restore $swp2
- ip link set dev $swp2 down
-
- mtu_restore $swp1
- ip link set dev $swp1 down
+ defer devlink_port_pool_th_restore $swp3 4
}
setup_prepare()
@@ -205,6 +197,7 @@ setup_prepare()
h3mac=$(mac_get $h3)
vrf_prepare
+ defer vrf_cleanup
h1_create
h2_create
@@ -212,45 +205,45 @@ setup_prepare()
switch_create
}
-cleanup()
-{
- pre_cleanup
-
- switch_destroy
- h3_destroy
- h2_destroy
- h1_destroy
-
- vrf_cleanup
-}
-
ping_ipv4()
{
ping_test $h2 192.0.2.130
}
-test_mc_aware()
+__run_uc_measure_rate()
{
- RET=0
-
+ local what=$1; shift
local -a uc_rate
+
start_traffic $h2.111 192.0.2.129 192.0.2.130 $h3mac
- uc_rate=($(measure_rate $swp2 $h3 rx_octets_prio_1 "UC-only"))
- check_err $? "Could not get high enough UC-only ingress rate"
- stop_traffic
+ defer stop_traffic $!
+
+ uc_rate=($(measure_rate $swp2 $h3 rx_octets_prio_1 "$what"))
+ check_err $? "Could not get high enough $what ingress rate"
+
+ echo ${uc_rate[@]}
+}
+
+run_uc_measure_rate()
+{
+ in_defer_scope __run_uc_measure_rate "$@"
+}
+
+test_mc_aware()
+{
+ RET=0
+
+ local -a uc_rate=($(run_uc_measure_rate "UC-only"))
local ucth1=${uc_rate[1]}
start_traffic $h1 192.0.2.65 bc bc
+ defer stop_traffic $!
local d0=$(date +%s)
local t0=$(ethtool_stats_get $h3 rx_octets_prio_0)
local u0=$(ethtool_stats_get $swp1 rx_octets_prio_0)
- local -a uc_rate_2
- start_traffic $h2.111 192.0.2.129 192.0.2.130 $h3mac
- uc_rate_2=($(measure_rate $swp2 $h3 rx_octets_prio_1 "UC+MC"))
- check_err $? "Could not get high enough UC+MC ingress rate"
- stop_traffic
+ local -a uc_rate_2=($(run_uc_measure_rate "UC+MC"))
local ucth2=${uc_rate_2[1]}
local d1=$(date +%s)
@@ -272,8 +265,6 @@ test_mc_aware()
local mc_ir=$(rate $u0 $u1 $interval)
local mc_er=$(rate $t0 $t1 $interval)
- stop_traffic
-
log_test "UC performance under MC overload"
echo "UC-only throughput $(humanize $ucth1)"
@@ -297,6 +288,7 @@ test_uc_aware()
RET=0
start_traffic $h2.111 192.0.2.129 192.0.2.130 $h3mac
+ defer stop_traffic $!
local d0=$(date +%s)
local t0=$(ethtool_stats_get $h3 rx_octets_prio_1)
@@ -326,8 +318,6 @@ test_uc_aware()
((attempts == passes))
check_err $?
- stop_traffic
-
log_test "MC performance under UC overload"
echo " ingress UC throughput $(humanize ${uc_ir})"
echo " egress UC throughput $(humanize ${uc_er})"
--
2.45.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH net-next 08/10] selftests: mlxsw: qos_ets_strict: Use defer for test cleanup
2024-10-09 12:06 [PATCH net-next 00/10] selftests: net: Introduce deferred commands Petr Machata
` (6 preceding siblings ...)
2024-10-09 12:06 ` [PATCH net-next 07/10] selftests: mlxsw: qos_mc_aware: " Petr Machata
@ 2024-10-09 12:06 ` Petr Machata
2024-10-09 12:06 ` [PATCH net-next 09/10] selftests: mlxsw: qos_max_descriptors: " Petr Machata
2024-10-09 12:06 ` [PATCH net-next 10/10] selftests: mlxsw: devlink_trap_police: " Petr Machata
9 siblings, 0 replies; 15+ messages in thread
From: Petr Machata @ 2024-10-09 12:06 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
netdev
Cc: linux-kselftest, Shuah Khan, Benjamin Poirier, Hangbin Liu,
Vladimir Oltean, Ido Schimmel, Przemek Kitszel, Petr Machata,
mlxsw
Use the defer framework to schedule cleanups as soon as the command is
executed.
Signed-off-by: Petr Machata <petrm@nvidia.com>
---
.../drivers/net/mlxsw/qos_ets_strict.sh | 167 +++++++++---------
1 file changed, 85 insertions(+), 82 deletions(-)
diff --git a/tools/testing/selftests/drivers/net/mlxsw/qos_ets_strict.sh b/tools/testing/selftests/drivers/net/mlxsw/qos_ets_strict.sh
index fee74f215cec..d5b6f2cc9a29 100755
--- a/tools/testing/selftests/drivers/net/mlxsw/qos_ets_strict.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/qos_ets_strict.sh
@@ -58,65 +58,62 @@ source qos_lib.sh
h1_create()
{
simple_if_init $h1
+ defer simple_if_fini $h1
+
mtu_set $h1 10000
+ defer mtu_restore $h1
vlan_create $h1 111 v$h1 192.0.2.33/28
+ defer vlan_destroy $h1 111
ip link set dev $h1.111 type vlan egress-qos-map 0:1
}
-h1_destroy()
-{
- vlan_destroy $h1 111
-
- mtu_restore $h1
- simple_if_fini $h1
-}
-
h2_create()
{
simple_if_init $h2
+ defer simple_if_fini $h2
+
mtu_set $h2 10000
+ defer mtu_restore $h2
vlan_create $h2 222 v$h2 192.0.2.65/28
+ defer vlan_destroy $h2 222
ip link set dev $h2.222 type vlan egress-qos-map 0:2
}
-h2_destroy()
-{
- vlan_destroy $h2 222
-
- mtu_restore $h2
- simple_if_fini $h2
-}
-
h3_create()
{
simple_if_init $h3
+ defer simple_if_fini $h3
+
mtu_set $h3 10000
+ defer mtu_restore $h3
vlan_create $h3 111 v$h3 192.0.2.34/28
+ defer vlan_destroy $h3 111
+
vlan_create $h3 222 v$h3 192.0.2.66/28
-}
-
-h3_destroy()
-{
- vlan_destroy $h3 222
- vlan_destroy $h3 111
-
- mtu_restore $h3
- simple_if_fini $h3
+ defer vlan_destroy $h3 222
}
switch_create()
{
ip link set dev $swp1 up
+ defer ip link set dev $swp1 down
+
mtu_set $swp1 10000
+ defer mtu_restore $swp1
ip link set dev $swp2 up
+ defer ip link set dev $swp2 down
+
mtu_set $swp2 10000
+ defer mtu_restore $swp2
# prio n -> TC n, strict scheduling
lldptool -T -i $swp3 -V ETS-CFG up2tc=0:0,1:1,2:2,3:3,4:4,5:5,6:6,7:7
+ defer lldptool -T -i $swp3 -V ETS-CFG up2tc=0:0,1:0,2:0,3:0,4:0,5:0,6:0,7:0
+
lldptool -T -i $swp3 -V ETS-CFG tsa=$(
)"0:strict,"$(
)"1:strict,"$(
@@ -129,85 +126,90 @@ switch_create()
sleep 1
ip link set dev $swp3 up
+ defer ip link set dev $swp3 down
+
mtu_set $swp3 10000
+ defer mtu_restore $swp3
+
tc qdisc replace dev $swp3 root handle 101: tbf rate 1gbit \
burst 128K limit 1G
+ defer tc qdisc del dev $swp3 root handle 101:
vlan_create $swp1 111
+ defer vlan_destroy $swp1 111
+
vlan_create $swp2 222
+ defer vlan_destroy $swp2 222
+
vlan_create $swp3 111
+ defer vlan_destroy $swp3 111
+
vlan_create $swp3 222
+ defer vlan_destroy $swp3 222
ip link add name br111 type bridge vlan_filtering 0
+ defer ip link del dev br111
ip link set dev br111 addrgenmode none
+
ip link set dev br111 up
+ defer ip link set dev br111 down
+
ip link set dev $swp1.111 master br111
+ defer ip link set dev $swp1.111 nomaster
+
ip link set dev $swp3.111 master br111
+ defer ip link set dev $swp3.111 nomaster
ip link add name br222 type bridge vlan_filtering 0
+ defer ip link del dev br222
ip link set dev br222 addrgenmode none
+
ip link set dev br222 up
+ defer ip link set dev br222 down
+
ip link set dev $swp2.222 master br222
+ defer ip link set dev $swp2.222 nomaster
+
ip link set dev $swp3.222 master br222
+ defer ip link set dev $swp3.222 nomaster
# Make sure that ingress quotas are smaller than egress so that there is
# room for both streams of traffic to be admitted to shared buffer.
devlink_pool_size_thtype_save 0
devlink_pool_size_thtype_set 0 dynamic 10000000
+ defer devlink_pool_size_thtype_restore 0
+
devlink_pool_size_thtype_save 4
devlink_pool_size_thtype_set 4 dynamic 10000000
+ defer devlink_pool_size_thtype_restore 4
devlink_port_pool_th_save $swp1 0
devlink_port_pool_th_set $swp1 0 6
+ defer devlink_port_pool_th_restore $swp1 0
+
devlink_tc_bind_pool_th_save $swp1 1 ingress
devlink_tc_bind_pool_th_set $swp1 1 ingress 0 6
+ defer devlink_tc_bind_pool_th_restore $swp1 1 ingress
devlink_port_pool_th_save $swp2 0
devlink_port_pool_th_set $swp2 0 6
+ defer devlink_port_pool_th_restore $swp2 0
+
devlink_tc_bind_pool_th_save $swp2 2 ingress
devlink_tc_bind_pool_th_set $swp2 2 ingress 0 6
+ defer devlink_tc_bind_pool_th_restore $swp2 2 ingress
devlink_tc_bind_pool_th_save $swp3 1 egress
devlink_tc_bind_pool_th_set $swp3 1 egress 4 7
+ defer devlink_tc_bind_pool_th_restore $swp3 1 egress
+
devlink_tc_bind_pool_th_save $swp3 2 egress
devlink_tc_bind_pool_th_set $swp3 2 egress 4 7
+ defer devlink_tc_bind_pool_th_restore $swp3 2 egress
+
devlink_port_pool_th_save $swp3 4
devlink_port_pool_th_set $swp3 4 7
-}
-
-switch_destroy()
-{
- devlink_port_pool_th_restore $swp3 4
- devlink_tc_bind_pool_th_restore $swp3 2 egress
- devlink_tc_bind_pool_th_restore $swp3 1 egress
-
- devlink_tc_bind_pool_th_restore $swp2 2 ingress
- devlink_port_pool_th_restore $swp2 0
-
- devlink_tc_bind_pool_th_restore $swp1 1 ingress
- devlink_port_pool_th_restore $swp1 0
-
- devlink_pool_size_thtype_restore 4
- devlink_pool_size_thtype_restore 0
-
- ip link del dev br222
- ip link del dev br111
-
- vlan_destroy $swp3 222
- vlan_destroy $swp3 111
- vlan_destroy $swp2 222
- vlan_destroy $swp1 111
-
- tc qdisc del dev $swp3 root handle 101:
- mtu_restore $swp3
- ip link set dev $swp3 down
- lldptool -T -i $swp3 -V ETS-CFG up2tc=0:0,1:0,2:0,3:0,4:0,5:0,6:0,7:0
-
- mtu_restore $swp2
- ip link set dev $swp2 down
-
- mtu_restore $swp1
- ip link set dev $swp1 down
+ defer devlink_port_pool_th_restore $swp3 4
}
setup_prepare()
@@ -224,6 +226,7 @@ setup_prepare()
h3mac=$(mac_get $h3)
vrf_prepare
+ defer vrf_cleanup
h1_create
h2_create
@@ -231,18 +234,6 @@ setup_prepare()
switch_create
}
-cleanup()
-{
- pre_cleanup
-
- switch_destroy
- h3_destroy
- h2_destroy
- h1_destroy
-
- vrf_cleanup
-}
-
ping_ipv4()
{
ping_test $h1 192.0.2.34 " from H1"
@@ -261,21 +252,38 @@ rel()
"
}
+__run_hi_measure_rate()
+{
+ local what=$1; shift
+ local -a uc_rate
+
+ start_traffic $h2.222 192.0.2.65 192.0.2.66 $h3mac
+ defer stop_traffic $!
+
+ uc_rate=($(measure_rate $swp2 $h3 rx_octets_prio_2 "$what"))
+ check_err $? "Could not get high enough $what ingress rate"
+
+ echo ${uc_rate[@]}
+}
+
+run_hi_measure_rate()
+{
+ in_defer_scope __run_hi_measure_rate "$@"
+}
+
test_ets_strict()
{
RET=0
# Run high-prio traffic on its own.
- start_traffic $h2.222 192.0.2.65 192.0.2.66 $h3mac
local -a rate_2
- rate_2=($(measure_rate $swp2 $h3 rx_octets_prio_2 "prio 2"))
- check_err $? "Could not get high enough prio-2 ingress rate"
+ rate_2=($(run_hi_measure_rate "prio 2"))
local rate_2_in=${rate_2[0]}
local rate_2_eg=${rate_2[1]}
- stop_traffic # $h2.222
# Start low-prio stream.
start_traffic $h1.111 192.0.2.33 192.0.2.34 $h3mac
+ defer stop_traffic $!
local -a rate_1
rate_1=($(measure_rate $swp1 $h3 rx_octets_prio_1 "prio 1"))
@@ -290,14 +298,9 @@ test_ets_strict()
check_err $(bc <<< "$rel21 > 105")
# Start the high-prio stream--now both streams run.
- start_traffic $h2.222 192.0.2.65 192.0.2.66 $h3mac
- rate_3=($(measure_rate $swp2 $h3 rx_octets_prio_2 "prio 2 w/ 1"))
- check_err $? "Could not get high enough prio-2 ingress rate with prio-1"
+ rate_3=($(run_hi_measure_rate "prio 2+1"))
local rate_3_in=${rate_3[0]}
local rate_3_eg=${rate_3[1]}
- stop_traffic # $h2.222
-
- stop_traffic # $h1.111
# High-prio should have about the same throughput whether or not
# low-prio is in the system.
--
2.45.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH net-next 09/10] selftests: mlxsw: qos_max_descriptors: Use defer for test cleanup
2024-10-09 12:06 [PATCH net-next 00/10] selftests: net: Introduce deferred commands Petr Machata
` (7 preceding siblings ...)
2024-10-09 12:06 ` [PATCH net-next 08/10] selftests: mlxsw: qos_ets_strict: " Petr Machata
@ 2024-10-09 12:06 ` Petr Machata
2024-10-09 12:06 ` [PATCH net-next 10/10] selftests: mlxsw: devlink_trap_police: " Petr Machata
9 siblings, 0 replies; 15+ messages in thread
From: Petr Machata @ 2024-10-09 12:06 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
netdev
Cc: linux-kselftest, Shuah Khan, Benjamin Poirier, Hangbin Liu,
Vladimir Oltean, Ido Schimmel, Przemek Kitszel, Petr Machata,
mlxsw
Use the defer framework to schedule cleanups as soon as the command is
executed.
Signed-off-by: Petr Machata <petrm@nvidia.com>
---
.../drivers/net/mlxsw/qos_max_descriptors.sh | 118 ++++++------------
1 file changed, 41 insertions(+), 77 deletions(-)
diff --git a/tools/testing/selftests/drivers/net/mlxsw/qos_max_descriptors.sh b/tools/testing/selftests/drivers/net/mlxsw/qos_max_descriptors.sh
index 5ac4f795e333..2b5d2c2751d5 100755
--- a/tools/testing/selftests/drivers/net/mlxsw/qos_max_descriptors.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/qos_max_descriptors.sh
@@ -69,127 +69,103 @@ mlxsw_only_on_spectrum 2+ || exit
h1_create()
{
simple_if_init $h1
+ defer simple_if_fini $h1
vlan_create $h1 111 v$h1 192.0.2.33/28
+ defer vlan_destroy $h1 111
ip link set dev $h1.111 type vlan egress-qos-map 0:1
}
-h1_destroy()
-{
- vlan_destroy $h1 111
-
- simple_if_fini $h1
-}
-
h2_create()
{
simple_if_init $h2
+ defer simple_if_fini $h2
vlan_create $h2 111 v$h2 192.0.2.34/28
-}
-
-h2_destroy()
-{
- vlan_destroy $h2 111
-
- simple_if_fini $h2
+ defer vlan_destroy $h2 111
}
switch_create()
{
# pools
# -----
+ # devlink_pool_size_thtype_restore needs to be done first so that we can
+ # reset the various limits to values that are only valid for the
+ # original static / dynamic setting.
devlink_pool_size_thtype_save 1
- devlink_pool_size_thtype_save 6
-
- devlink_port_pool_th_save $swp1 1
- devlink_port_pool_th_save $swp2 6
-
- devlink_tc_bind_pool_th_save $swp1 1 ingress
- devlink_tc_bind_pool_th_save $swp2 1 egress
-
devlink_pool_size_thtype_set 1 dynamic $MAX_POOL_SIZE
+ defer_prio devlink_pool_size_thtype_restore 1
+
+ devlink_pool_size_thtype_save 6
devlink_pool_size_thtype_set 6 static $MAX_POOL_SIZE
+ defer_prio devlink_pool_size_thtype_restore 6
# $swp1
# -----
ip link set dev $swp1 up
+ defer ip link set dev $swp1 down
+
vlan_create $swp1 111
+ defer vlan_destroy $swp1 111
ip link set dev $swp1.111 type vlan ingress-qos-map 0:0 1:1
+ devlink_port_pool_th_save $swp1 1
devlink_port_pool_th_set $swp1 1 16
+ defer devlink_tc_bind_pool_th_restore $swp1 1 ingress
+
+ devlink_tc_bind_pool_th_save $swp1 1 ingress
devlink_tc_bind_pool_th_set $swp1 1 ingress 1 16
+ defer devlink_port_pool_th_restore $swp1 1
tc qdisc replace dev $swp1 root handle 1: \
ets bands 8 strict 8 priomap 7 6
+ defer tc qdisc del dev $swp1 root
+
dcb buffer set dev $swp1 prio-buffer all:0 1:1
+ defer dcb buffer set dev $swp1 prio-buffer all:0
# $swp2
# -----
ip link set dev $swp2 up
+ defer ip link set dev $swp2 down
+
vlan_create $swp2 111
+ defer vlan_destroy $swp2 111
ip link set dev $swp2.111 type vlan egress-qos-map 0:0 1:1
+ devlink_port_pool_th_save $swp2 6
devlink_port_pool_th_set $swp2 6 $MAX_POOL_SIZE
+ defer devlink_tc_bind_pool_th_restore $swp2 1 egress
+
+ devlink_tc_bind_pool_th_save $swp2 1 egress
devlink_tc_bind_pool_th_set $swp2 1 egress 6 $MAX_POOL_SIZE
+ defer devlink_port_pool_th_restore $swp2 6
tc qdisc replace dev $swp2 root handle 1: tbf rate $SHAPER_RATE \
burst 128K limit 500M
+ defer tc qdisc del dev $swp2 root
+
tc qdisc replace dev $swp2 parent 1:1 handle 11: \
ets bands 8 strict 8 priomap 7 6
+ defer tc qdisc del dev $swp2 parent 1:1 handle 11:
# bridge
# ------
ip link add name br1 type bridge vlan_filtering 0
+ defer ip link del dev br1
+
ip link set dev $swp1.111 master br1
+ defer ip link set dev $swp1.111 nomaster
+
ip link set dev br1 up
+ defer ip link set dev br1 down
ip link set dev $swp2.111 master br1
-}
-
-switch_destroy()
-{
- # Do this first so that we can reset the limits to values that are only
- # valid for the original static / dynamic setting.
- devlink_pool_size_thtype_restore 6
- devlink_pool_size_thtype_restore 1
-
- # bridge
- # ------
-
- ip link set dev $swp2.111 nomaster
-
- ip link set dev br1 down
- ip link set dev $swp1.111 nomaster
- ip link del dev br1
-
- # $swp2
- # -----
-
- tc qdisc del dev $swp2 parent 1:1 handle 11:
- tc qdisc del dev $swp2 root
-
- devlink_tc_bind_pool_th_restore $swp2 1 egress
- devlink_port_pool_th_restore $swp2 6
-
- vlan_destroy $swp2 111
- ip link set dev $swp2 down
-
- # $swp1
- # -----
-
- dcb buffer set dev $swp1 prio-buffer all:0
- tc qdisc del dev $swp1 root
-
- devlink_tc_bind_pool_th_restore $swp1 1 ingress
- devlink_port_pool_th_restore $swp1 1
-
- vlan_destroy $swp1 111
- ip link set dev $swp1 down
+ defer ip link set dev $swp2.111 nomaster
}
setup_prepare()
@@ -203,23 +179,13 @@ setup_prepare()
h2mac=$(mac_get $h2)
vrf_prepare
+ defer vrf_cleanup
h1_create
h2_create
switch_create
}
-cleanup()
-{
- pre_cleanup
-
- switch_destroy
- h2_destroy
- h1_destroy
-
- vrf_cleanup
-}
-
ping_ipv4()
{
ping_test $h1 192.0.2.34 " h1->h2"
@@ -251,6 +217,7 @@ max_descriptors()
log_info "Send many small packets, packet size = $pktsize bytes"
start_traffic_pktsize $pktsize $h1.111 192.0.2.33 192.0.2.34 $h2mac
+ defer stop_traffic $!
# Sleep to wait for congestion.
sleep 5
@@ -268,9 +235,6 @@ max_descriptors()
check_err $(bc <<< "$perc_used < $exp_perc_used") \
"Expected > $exp_perc_used% of descriptors, handle $perc_used%"
- stop_traffic
- sleep 1
-
log_test "Maximum descriptors usage. The percentage used is $perc_used%"
}
--
2.45.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH net-next 10/10] selftests: mlxsw: devlink_trap_police: Use defer for test cleanup
2024-10-09 12:06 [PATCH net-next 00/10] selftests: net: Introduce deferred commands Petr Machata
` (8 preceding siblings ...)
2024-10-09 12:06 ` [PATCH net-next 09/10] selftests: mlxsw: qos_max_descriptors: " Petr Machata
@ 2024-10-09 12:06 ` Petr Machata
9 siblings, 0 replies; 15+ messages in thread
From: Petr Machata @ 2024-10-09 12:06 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
netdev
Cc: linux-kselftest, Shuah Khan, Benjamin Poirier, Hangbin Liu,
Vladimir Oltean, Ido Schimmel, Przemek Kitszel, Petr Machata,
mlxsw
Use the defer framework to schedule cleanups as soon as the command is
executed.
Note that the start_traffic commands in __burst_test() are each sending a
fixed number of packets (note the -c flag) and then ending. They therefore
do not need a matching stop_traffic.
Signed-off-by: Petr Machata <petrm@nvidia.com>
---
.../drivers/net/mlxsw/devlink_trap_policer.sh | 85 ++++++++-----------
1 file changed, 36 insertions(+), 49 deletions(-)
diff --git a/tools/testing/selftests/drivers/net/mlxsw/devlink_trap_policer.sh b/tools/testing/selftests/drivers/net/mlxsw/devlink_trap_policer.sh
index 0bd5ffc218ac..29a672c2270f 100755
--- a/tools/testing/selftests/drivers/net/mlxsw/devlink_trap_policer.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/devlink_trap_policer.sh
@@ -45,63 +45,52 @@ source $lib_dir/devlink_lib.sh
h1_create()
{
simple_if_init $h1 192.0.2.1/24
+ defer simple_if_fini $h1 192.0.2.1/24
+
mtu_set $h1 10000
+ defer mtu_restore $h1
ip -4 route add default vrf v$h1 nexthop via 192.0.2.2
-}
-
-h1_destroy()
-{
- ip -4 route del default vrf v$h1 nexthop via 192.0.2.2
-
- mtu_restore $h1
- simple_if_fini $h1 192.0.2.1/24
+ defer ip -4 route del default vrf v$h1 nexthop via 192.0.2.2
}
h2_create()
{
simple_if_init $h2 198.51.100.1/24
+ defer simple_if_fini $h2 198.51.100.1/24
+
mtu_set $h2 10000
+ defer mtu_restore $h2
ip -4 route add default vrf v$h2 nexthop via 198.51.100.2
-}
-
-h2_destroy()
-{
- ip -4 route del default vrf v$h2 nexthop via 198.51.100.2
-
- mtu_restore $h2
- simple_if_fini $h2 198.51.100.1/24
+ defer ip -4 route del default vrf v$h2 nexthop via 198.51.100.2
}
router_create()
{
ip link set dev $rp1 up
+ defer ip link set dev $rp1 down
+
ip link set dev $rp2 up
+ defer ip link set dev $rp2 down
__addr_add_del $rp1 add 192.0.2.2/24
+ defer __addr_add_del $rp1 del 192.0.2.2/24
+
__addr_add_del $rp2 add 198.51.100.2/24
+ defer __addr_add_del $rp2 del 198.51.100.2/24
+
mtu_set $rp1 10000
+ defer mtu_restore $rp1
+
mtu_set $rp2 10000
+ defer mtu_restore $rp2
ip -4 route add blackhole 198.51.100.100
+ defer ip -4 route del blackhole 198.51.100.100
devlink trap set $DEVLINK_DEV trap blackhole_route action trap
-}
-
-router_destroy()
-{
- devlink trap set $DEVLINK_DEV trap blackhole_route action drop
-
- ip -4 route del blackhole 198.51.100.100
-
- mtu_restore $rp2
- mtu_restore $rp1
- __addr_add_del $rp2 del 198.51.100.2/24
- __addr_add_del $rp1 del 192.0.2.2/24
-
- ip link set dev $rp2 down
- ip link set dev $rp1 down
+ defer devlink trap set $DEVLINK_DEV trap blackhole_route action drop
}
setup_prepare()
@@ -114,7 +103,11 @@ setup_prepare()
rp1_mac=$(mac_get $rp1)
+ # Reload to ensure devlink-trap settings are back to default.
+ defer devlink_reload
+
vrf_prepare
+ defer vrf_cleanup
h1_create
h2_create
@@ -122,21 +115,6 @@ setup_prepare()
router_create
}
-cleanup()
-{
- pre_cleanup
-
- router_destroy
-
- h2_destroy
- h1_destroy
-
- vrf_cleanup
-
- # Reload to ensure devlink-trap settings are back to default.
- devlink_reload
-}
-
rate_limits_test()
{
RET=0
@@ -214,7 +192,10 @@ __rate_test()
# by the policer. Make sure measured received rate is about 1000 pps
log_info "=== Tx rate: Highest, Policer rate: 1000 pps ==="
+ defer_scope_push
+
start_traffic $h1 192.0.2.1 198.51.100.100 $rp1_mac
+ defer stop_traffic $!
sleep 5 # Take measurements when rate is stable
@@ -229,13 +210,16 @@ __rate_test()
check_err $? "Expected non-zero policer drop rate, got 0"
log_info "Measured policer drop rate of $drop_rate pps"
- stop_traffic
+ defer_scope_pop
# Send packets at a rate of 1000 pps and make sure they are not dropped
# by the policer
log_info "=== Tx rate: 1000 pps, Policer rate: 1000 pps ==="
+ defer_scope_push
+
start_traffic $h1 192.0.2.1 198.51.100.100 $rp1_mac -d 1msec
+ defer stop_traffic $!
sleep 5 # Take measurements when rate is stable
@@ -244,7 +228,7 @@ __rate_test()
check_err $? "Expected zero policer drop rate, got a drop rate of $drop_rate pps"
log_info "Measured policer drop rate of $drop_rate pps"
- stop_traffic
+ defer_scope_pop
# Unbind the policer and send packets at highest possible rate. Make
# sure they are not dropped by the policer and that the measured
@@ -253,7 +237,10 @@ __rate_test()
devlink trap group set $DEVLINK_DEV group l3_drops nopolicer
+ defer_scope_push
+
start_traffic $h1 192.0.2.1 198.51.100.100 $rp1_mac
+ defer stop_traffic $!
rate=$(trap_rate_get)
(( rate > 1000 ))
@@ -265,7 +252,7 @@ __rate_test()
check_err $? "Expected zero policer drop rate, got a drop rate of $drop_rate pps"
log_info "Measured policer drop rate of $drop_rate pps"
- stop_traffic
+ defer_scope_pop
log_test "Trap policer rate"
}
--
2.45.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH net-next 04/10] selftests: RED: Use defer for test cleanup
2024-10-09 12:06 ` [PATCH net-next 04/10] selftests: RED: Use defer for test cleanup Petr Machata
@ 2024-10-15 8:14 ` Paolo Abeni
2024-10-15 9:03 ` Petr Machata
0 siblings, 1 reply; 15+ messages in thread
From: Paolo Abeni @ 2024-10-15 8:14 UTC (permalink / raw)
To: Petr Machata, David S. Miller, Eric Dumazet, Jakub Kicinski,
netdev
Cc: linux-kselftest, Shuah Khan, Benjamin Poirier, Hangbin Liu,
Vladimir Oltean, Ido Schimmel, Przemek Kitszel, mlxsw
On 10/9/24 14:06, Petr Machata wrote:
> @@ -450,6 +415,7 @@ __do_ecn_test()
>
> start_tcp_traffic $h1.$vlan $(ipaddr 1 $vlan) $(ipaddr 3 $vlan) \
> $h3_mac tos=0x01
> + defer stop_traffic $!
> sleep 1
>
> ecn_test_common "$name" "$get_nmarked" $vlan $limit
> @@ -462,7 +428,6 @@ __do_ecn_test()
> check_fail $? "UDP traffic went into backlog instead of being early-dropped"
> log_test "TC $((vlan - 10)): $name backlog > limit: UDP early-dropped"
>
> - stop_traffic
> sleep 1
I'm wodering what role/goal has the above 'sleep 1'?!? It looks like it
could/should be removed after moving the stop_traffic call to the
deferred cleanup.
Other similar instances below.
Cheers,
Paolo
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next 01/10] selftests: net: lib: Introduce deferred commands
2024-10-09 12:06 ` [PATCH net-next 01/10] selftests: net: lib: " Petr Machata
@ 2024-10-15 8:22 ` Paolo Abeni
2024-10-15 9:06 ` Petr Machata
0 siblings, 1 reply; 15+ messages in thread
From: Paolo Abeni @ 2024-10-15 8:22 UTC (permalink / raw)
To: Petr Machata, David S. Miller, Eric Dumazet, Jakub Kicinski,
netdev
Cc: linux-kselftest, Shuah Khan, Benjamin Poirier, Hangbin Liu,
Vladimir Oltean, Ido Schimmel, Przemek Kitszel, mlxsw
Hi,
On 10/9/24 14:06, Petr Machata wrote:
> 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..8d205c3f0445
> --- /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
Minor nit: IMHO the trailing shift is here a bit confusing: it let me
think about other arguments, which are not really expected.
[...]
> +__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]=$((${__DEFER__NJOBS[$ndefers_key]} + 1))
'${__DEFER__NJOBS[$ndefers_key]}' is actually '$ndefers', right? If so
it would be better to reuse the avail variable.
Thanks,
Paolo
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next 04/10] selftests: RED: Use defer for test cleanup
2024-10-15 8:14 ` Paolo Abeni
@ 2024-10-15 9:03 ` Petr Machata
0 siblings, 0 replies; 15+ messages in thread
From: Petr Machata @ 2024-10-15 9:03 UTC (permalink / raw)
To: Paolo Abeni
Cc: Petr Machata, David S. Miller, Eric Dumazet, Jakub Kicinski,
netdev, linux-kselftest, Shuah Khan, Benjamin Poirier,
Hangbin Liu, Vladimir Oltean, Ido Schimmel, Przemek Kitszel,
mlxsw
Paolo Abeni <pabeni@redhat.com> writes:
> On 10/9/24 14:06, Petr Machata wrote:
>> @@ -450,6 +415,7 @@ __do_ecn_test()
>> start_tcp_traffic $h1.$vlan $(ipaddr 1 $vlan) $(ipaddr 3 $vlan) \
>> $h3_mac tos=0x01
>> + defer stop_traffic $!
>> sleep 1
>> ecn_test_common "$name" "$get_nmarked" $vlan $limit
>> @@ -462,7 +428,6 @@ __do_ecn_test()
>> check_fail $? "UDP traffic went into backlog instead of being early-dropped"
>> log_test "TC $((vlan - 10)): $name backlog > limit: UDP early-dropped"
>> - stop_traffic
>> sleep 1
>
> I'm wodering what role/goal has the above 'sleep 1'?!? It looks like it could/should be removed
> after moving the stop_traffic call to the deferred cleanup.
Yeah, I'll drop these for v2.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next 01/10] selftests: net: lib: Introduce deferred commands
2024-10-15 8:22 ` Paolo Abeni
@ 2024-10-15 9:06 ` Petr Machata
0 siblings, 0 replies; 15+ messages in thread
From: Petr Machata @ 2024-10-15 9:06 UTC (permalink / raw)
To: Paolo Abeni
Cc: Petr Machata, David S. Miller, Eric Dumazet, Jakub Kicinski,
netdev, linux-kselftest, Shuah Khan, Benjamin Poirier,
Hangbin Liu, Vladimir Oltean, Ido Schimmel, Przemek Kitszel,
mlxsw
Paolo Abeni <pabeni@redhat.com> writes:
> Hi,
>
> On 10/9/24 14:06, Petr Machata wrote:
>> 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..8d205c3f0445
>> --- /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
>
> Minor nit: IMHO the trailing shift is here a bit confusing: it let me
> think about other arguments, which are not really expected.
This is IMHO how a function header should look like:
function()
{
local foo=$1; shift
local bar=$1; shift
local baz=$1; shift
...
}
Because it lets you reorder the arguments freely just by reordering the
lines, copy argument subsets to other functions without risking
forgetting / screwing up renumbering, etc. It's easy to parse visually
as well. If the function uses "$@" as rest argument, it will contain the
rest by default. It's just a very convenient notation overall. Vast
majority of net/lib.sh and net/forwarding/lib.sh use this.
>> +__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]=$((${__DEFER__NJOBS[$ndefers_key]} + 1))
>
> '${__DEFER__NJOBS[$ndefers_key]}' is actually '$ndefers', right? If so
> it would be better to reuse the avail variable.
I figured I would leave it all spelled out, because the left hand side
needs to be, and having the same expression on both sides makes it clear
that this is just an X++ sort of a deal.
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2024-10-15 9:28 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-09 12:06 [PATCH net-next 00/10] selftests: net: Introduce deferred commands Petr Machata
2024-10-09 12:06 ` [PATCH net-next 01/10] selftests: net: lib: " Petr Machata
2024-10-15 8:22 ` Paolo Abeni
2024-10-15 9:06 ` Petr Machata
2024-10-09 12:06 ` [PATCH net-next 02/10] selftests: forwarding: Add a fallback cleanup() Petr Machata
2024-10-09 12:06 ` [PATCH net-next 03/10] selftests: forwarding: lib: Allow passing PID to stop_traffic() Petr Machata
2024-10-09 12:06 ` [PATCH net-next 04/10] selftests: RED: Use defer for test cleanup Petr Machata
2024-10-15 8:14 ` Paolo Abeni
2024-10-15 9:03 ` Petr Machata
2024-10-09 12:06 ` [PATCH net-next 05/10] selftests: TBF: " Petr Machata
2024-10-09 12:06 ` [PATCH net-next 06/10] selftests: ETS: " Petr Machata
2024-10-09 12:06 ` [PATCH net-next 07/10] selftests: mlxsw: qos_mc_aware: " Petr Machata
2024-10-09 12:06 ` [PATCH net-next 08/10] selftests: mlxsw: qos_ets_strict: " Petr Machata
2024-10-09 12:06 ` [PATCH net-next 09/10] selftests: mlxsw: qos_max_descriptors: " Petr Machata
2024-10-09 12:06 ` [PATCH net-next 10/10] selftests: mlxsw: devlink_trap_police: " Petr Machata
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).