Merge branch 'selftests-mark-auto-deferring-functions-clearly'

Petr Machata says:

====================
selftests: Mark auto-deferring functions clearly

selftests/net/lib.sh contains a suite of iproute2 wrappers that
automatically schedule the corresponding cleanup through defer. The fact
they do so is however not immediately obvious, one needs to know which
functions are handling the deferral behind the scenes, and which expect the
caller to handle cleanups themselves.

A convention for these auto-deferring functions would help both writing and
patch review. This patchset does so by marking these functions with an adf_
prefix. We already have a few such functions: forwarding/lib.sh has
adf_mcd_start() and a few selftests add private helpers that conform to
this convention.

Patches kernel-patches#1 to kernel-patches#8 gradually convert individual functions, one per patch.

Patch kernel-patches#9 renames an auto-deferring private helpers named dfr_* to adf_*.
The plan is not to retro-rename all private helpers, but I happened to know
about this one.

Patches kernel-patches#10 to kernel-patches#12 introduce several autodefer helpers for commonly used
forwarding/lib.sh functions, and opportunistically convert straightforward
instances of 'action; defer counteraction' to the new helpers.

Patch kernel-patches#13 adds some README verbiage to pitch defer and the adf_*
convention.
====================

Link: https://patch.msgid.link/[email protected]
Signed-off-by: Jakub Kicinski <[email protected]>

Author: kuba-moo

tools/testing/selftests/drivers/net/mlxsw/devlink_trap_policer.sh

@@ -44,8 +44,7 @@ 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
+	adf_simple_if_init $h1 192.0.2.1/24
 
 	mtu_set $h1 10000
 	defer mtu_restore $h1
@@ -56,8 +55,7 @@ h1_create()
 
 h2_create()
 {
-	simple_if_init $h2 198.51.100.1/24
-	defer simple_if_fini $h2 198.51.100.1/24
+	adf_simple_if_init $h2 198.51.100.1/24
 
 	mtu_set $h2 10000
 	defer mtu_restore $h2
@@ -106,8 +104,7 @@ setup_prepare()
 	# Reload to ensure devlink-trap settings are back to default.
 	defer devlink_reload
 
-	vrf_prepare
-	defer vrf_cleanup
+	adf_vrf_prepare
 
 	h1_create
 	h2_create

tools/testing/selftests/drivers/net/mlxsw/qos_ets_strict.sh

@@ -57,8 +57,7 @@ source qos_lib.sh
 
 h1_create()
 {
-	simple_if_init $h1
-	defer simple_if_fini $h1
+	adf_simple_if_init $h1
 
 	mtu_set $h1 10000
 	defer mtu_restore $h1
@@ -70,8 +69,7 @@ h1_create()
 
 h2_create()
 {
-	simple_if_init $h2
-	defer simple_if_fini $h2
+	adf_simple_if_init $h2
 
 	mtu_set $h2 10000
 	defer mtu_restore $h2
@@ -83,8 +81,7 @@ h2_create()
 
 h3_create()
 {
-	simple_if_init $h3
-	defer simple_if_fini $h3
+	adf_simple_if_init $h3
 
 	mtu_set $h3 10000
 	defer mtu_restore $h3
@@ -225,8 +222,7 @@ setup_prepare()
 
 	h3mac=$(mac_get $h3)
 
-	vrf_prepare
-	defer vrf_cleanup
+	adf_vrf_prepare
 
 	h1_create
 	h2_create

tools/testing/selftests/drivers/net/mlxsw/qos_max_descriptors.sh

@@ -68,8 +68,7 @@ mlxsw_only_on_spectrum 2+ || exit
 
 h1_create()
 {
-	simple_if_init $h1
-	defer simple_if_fini $h1
+	adf_simple_if_init $h1
 
 	vlan_create $h1 111 v$h1 192.0.2.33/28
 	defer vlan_destroy $h1 111
@@ -78,8 +77,7 @@ h1_create()
 
 h2_create()
 {
-	simple_if_init $h2
-	defer simple_if_fini $h2
+	adf_simple_if_init $h2
 
 	vlan_create $h2 111 v$h2 192.0.2.34/28
 	defer vlan_destroy $h2 111
@@ -178,8 +176,7 @@ setup_prepare()
 
 	h2mac=$(mac_get $h2)
 
-	vrf_prepare
-	defer vrf_cleanup
+	adf_vrf_prepare
 
 	h1_create
 	h2_create

tools/testing/selftests/drivers/net/mlxsw/qos_mc_aware.sh

@@ -72,17 +72,15 @@ 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
+	adf_simple_if_init $h1 192.0.2.65/28
 
 	mtu_set $h1 10000
 	defer mtu_restore $h1
 }
 
 h2_create()
 {
-	simple_if_init $h2
-	defer simple_if_fini $h2
+	adf_simple_if_init $h2
 
 	mtu_set $h2 10000
 	defer mtu_restore $h2
@@ -94,8 +92,7 @@ h2_create()
 
 h3_create()
 {
-	simple_if_init $h3 192.0.2.66/28
-	defer simple_if_fini $h3 192.0.2.66/28
+	adf_simple_if_init $h3 192.0.2.66/28
 
 	mtu_set $h3 10000
 	defer mtu_restore $h3
@@ -196,8 +193,7 @@ setup_prepare()
 
 	h3mac=$(mac_get $h3)
 
-	vrf_prepare
-	defer vrf_cleanup
+	adf_vrf_prepare
 
 	h1_create
 	h2_create

tools/testing/selftests/drivers/net/mlxsw/sch_red_core.sh

@@ -100,8 +100,7 @@ host_create()
 	local dev=$1; shift
 	local host=$1; shift
 
-	simple_if_init $dev
-	defer simple_if_fini $dev
+	adf_simple_if_init $dev
 
 	mtu_set $dev 10000
 	defer mtu_restore $dev
@@ -250,8 +249,7 @@ setup_prepare()
 
 	h3_mac=$(mac_get $h3)
 
-	vrf_prepare
-	defer vrf_cleanup
+	adf_vrf_prepare
 
 	h1_create
 	h2_create

tools/testing/selftests/net/fdb_notify.sh

@@ -40,46 +40,46 @@ do_test_dup()
 
 test_dup_bridge()
 {
-	ip_link_add br up type bridge vlan_filtering 1
+	adf_ip_link_add br up type bridge vlan_filtering 1
 	do_test_dup add "bridge" dev br self
 	do_test_dup del "bridge" dev br self
 }
 
 test_dup_vxlan_self()
 {
-	ip_link_add br up type bridge vlan_filtering 1
-	ip_link_add vx up type vxlan id 2000 dstport 4789
-	ip_link_set_master vx br
+	adf_ip_link_add br up type bridge vlan_filtering 1
+	adf_ip_link_add vx up type vxlan id 2000 dstport 4789
+	adf_ip_link_set_master vx br
 
 	do_test_dup add "vxlan" dev vx self dst 192.0.2.1
 	do_test_dup del "vxlan" dev vx self dst 192.0.2.1
 }
 
 test_dup_vxlan_master()
 {
-	ip_link_add br up type bridge vlan_filtering 1
-	ip_link_add vx up type vxlan id 2000 dstport 4789
-	ip_link_set_master vx br
+	adf_ip_link_add br up type bridge vlan_filtering 1
+	adf_ip_link_add vx up type vxlan id 2000 dstport 4789
+	adf_ip_link_set_master vx br
 
 	do_test_dup add "vxlan master" dev vx master
 	do_test_dup del "vxlan master" dev vx master
 }
 
 test_dup_macvlan_self()
 {
-	ip_link_add dd up type dummy
-	ip_link_add mv up link dd type macvlan mode passthru
+	adf_ip_link_add dd up type dummy
+	adf_ip_link_add mv up link dd type macvlan mode passthru
 
 	do_test_dup add "macvlan self" dev mv self
 	do_test_dup del "macvlan self" dev mv self
 }
 
 test_dup_macvlan_master()
 {
-	ip_link_add br up type bridge vlan_filtering 1
-	ip_link_add dd up type dummy
-	ip_link_add mv up link dd type macvlan mode passthru
-	ip_link_set_master mv br
+	adf_ip_link_add br up type bridge vlan_filtering 1
+	adf_ip_link_add dd up type dummy
+	adf_ip_link_add mv up link dd type macvlan mode passthru
+	adf_ip_link_set_master mv br
 
 	do_test_dup add "macvlan master" dev mv self
 	do_test_dup del "macvlan master" dev mv self

tools/testing/selftests/net/forwarding/README

@@ -57,6 +57,21 @@ o Code shall be checked using ShellCheck [1] prior to submission.
 
 1. https://www.shellcheck.net/
 
+Cleanups
+--------
+
+o lib.sh brings in defer.sh (by way of ../lib.sh) by default. Consider
+  making use of the defer primitive to schedule automatic cleanups. This
+  makes it harder to forget to remove a temporary netdevice, kill a running
+  process or perform other cleanup when the test script is interrupted.
+
+o When adding a helper that dirties the environment, but schedules all
+  necessary cleanups through defer, consider prefixing it adf_ for
+  consistency with lib.sh and ../lib.sh helpers. This serves as an
+  immediately visible bit of documentation about the helper API.
+
+o Definitely do the above for any new code in lib.sh, if practical.
+
 Customization
 =============
 

tools/testing/selftests/net/forwarding/bridge_activity_notify.sh

@@ -26,27 +26,25 @@ source lib.sh
 
 h1_create()
 {
-	simple_if_init "$h1" 192.0.2.1/28
-	defer simple_if_fini "$h1" 192.0.2.1/28
+	adf_simple_if_init "$h1" 192.0.2.1/28
 }
 
 h2_create()
 {
-	simple_if_init "$h2" 192.0.2.2/28
-	defer simple_if_fini "$h2" 192.0.2.2/28
+	adf_simple_if_init "$h2" 192.0.2.2/28
 }
 
 switch_create()
 {
-	ip_link_add br1 type bridge vlan_filtering 0 mcast_snooping 0 \
+	adf_ip_link_add br1 type bridge vlan_filtering 0 mcast_snooping 0 \
 		ageing_time "$LOW_AGEING_TIME"
-	ip_link_set_up br1
+	adf_ip_link_set_up br1
 
-	ip_link_set_master "$swp1" br1
-	ip_link_set_up "$swp1"
+	adf_ip_link_set_master "$swp1" br1
+	adf_ip_link_set_up "$swp1"
 
-	ip_link_set_master "$swp2" br1
-	ip_link_set_up "$swp2"
+	adf_ip_link_set_master "$swp2" br1
+	adf_ip_link_set_up "$swp2"
 }
 
 setup_prepare()
@@ -57,8 +55,7 @@ setup_prepare()
 	swp2=${NETIFS[p3]}
 	h2=${NETIFS[p4]}
 
-	vrf_prepare
-	defer vrf_cleanup
+	adf_vrf_prepare
 
 	h1_create
 	h2_create