[PATCH net-next v3] selftests: net: move log_test to lib file and remove duplicate code

mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed

* [PATCH net-next v3] selftests: net: move log_test to lib file and remove duplicate code
@ 2026-09-15  3:14 Hangbin Liu
  2026-09-17  2:24 ` netdev-bot+sashiko
  0 siblings, 1 reply; 3+ messages in thread
From: Hangbin Liu @ 2026-09-15  3:14 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Shuah Khan, David Ahern, Ido Schimmel,
	Andrea Mayer
  Cc: Hangbin Liu, netdev, linux-kselftest, linux-kernel, Hangbin Liu

From: Hangbin Liu <liuhangbin@kylinos.cn>

Many test scripts under tools/testing/selftests/net/ each define their
own log_test() function with near-identical logic for comparing a return
code against an expected value and printing OK/FAIL. Add a shared
log_test_expected() to lib.sh so we can replace each local definitions.

The function is named log_test_expected() rather than log_test() because
lib.sh already exports log_test() with a different signature used by
the forwarding tests.

Most of the checks in log_test_expected() are the same as log_test()
in other tests. The differences are:

- On failure, ret is set via ksft_exit_status_merge() instead of ret=1,
  so we can propagate ksft_xfail status correctly.
- The function always returns 0 to avoid influencing later code.
- On failure with VERBOSE=1, the actual and expected return codes are
  printed (echo "    rc=$rc, expected $expected").
- A PAUSE_ON_FAIL check is added via pause_on_fail() for scripts that
  did not have one.
- A PAUSE=yes check is added for scripts that did not have one.
- A trailing [ "$VERBOSE" = "1" ] && echo is added for scripts that
  did not have one.

Several tests required special handling:

- fcnal-test.sh
  - Print format: it uses %-70s, while log_test_expected() uses %-60s.
  - The old code always printed "expected rc $expected; actual rc $rc"
    on failure. The new code only prints when VERBOSE=1.
  - The old [ "${VERBOSE}" = "1" ] && echo ran before the comparison;
    now it runs after the PAUSE check.
- fdb_flush.sh
  - It used local ret, nsuccess, and nfail, which are not used outside
    the function. The log_test_expected() uses global variables as all
    other tests do.
- fib-onlink-tests.sh
  - Print format: it uses %-50s, while log_test_expected() uses %-60s.
- fib_nexthops.sh
  - this test checked for ksft_skip (exit code 4) inside log_test(),
    but no test in the file produces that code. The check is removed,
    callers that need to report a skip should use log_test_skip() instead.
  - Two callers compared against exit code 2 are left as-is since code 2
    correctly counts as a failure in the previous log_test().
- srv6_end_dx*.sh and srv6_end_flavors_test.sh
  - These three tests previously defined ksft_skip locally instead of
    sourcing lib.sh. They now source lib.sh, which provides ksft_skip
    and other framework constants. Note that srv6_end_flavors_test.sh
    previously declared ksft_skip as readonly; the lib.sh definition
    does not use readonly.

In addition to the above, 13 tests that previously used "TEST:" now use
"    TEST:" (4-space prefix), and srv6 tests plus vrf_strict_mode_test.sh
that previously used "\n    TEST:" (newline + 4-space prefix) now with no
leading newline.

Signed-off-by: Hangbin Liu <liuhangbin@kylinos.cn>
---
Changes in v3:
- Always return 0 for log_test_expected (Jakub)
- Add detailed change log in commit description.
- Link to v2: https://lore.kernel.org/r/20260831-self_log_test-v2-1-eda2e8490cf9@kylinos.cn

Changes in v2:
- Remove skip/xfail checking in lib.sh, as any command can exits with 2 or 4 (sashiko)
- Remove nskip counter in fib_nexthops.sh
- Link to v1: https://lore.kernel.org/r/20260813-self_log_test-v1-1-f88b1107842e@kylinos.cn
---
 tools/testing/selftests/net/drop_monitor_tests.sh  | 13 +------
 tools/testing/selftests/net/fcnal-test.sh          | 29 +-------------
 tools/testing/selftests/net/fdb_flush.sh           | 35 +----------------
 tools/testing/selftests/net/fib-onlink-tests.sh    | 18 +--------
 .../selftests/net/fib_nexthop_multiprefix.sh       | 21 +----------
 tools/testing/selftests/net/fib_nexthop_nongw.sh   | 21 +----------
 tools/testing/selftests/net/fib_nexthops.sh        | 44 ++--------------------
 tools/testing/selftests/net/fib_rule_tests.sh      | 19 +---------
 tools/testing/selftests/net/fib_tests.sh           | 26 +------------
 tools/testing/selftests/net/gre_gso.sh             | 26 +------------
 tools/testing/selftests/net/icmp_redirect.sh       | 19 +---------
 tools/testing/selftests/net/l2tp.sh                | 19 +---------
 tools/testing/selftests/net/lib.sh                 | 33 ++++++++++++++++
 .../selftests/net/ndisc_unsolicited_na_test.sh     | 26 +------------
 .../selftests/net/srv6_encap_lookup_l3vpn_test.sh  | 19 +---------
 .../selftests/net/srv6_end_dt46_l3vpn_test.sh      | 19 +---------
 .../selftests/net/srv6_end_dt4_l3vpn_test.sh       | 19 +---------
 .../selftests/net/srv6_end_dt6_l3vpn_test.sh       | 19 +---------
 .../selftests/net/srv6_end_dx4_netfilter_test.sh   | 23 ++---------
 .../selftests/net/srv6_end_dx6_netfilter_test.sh   | 23 ++---------
 .../testing/selftests/net/srv6_end_flavors_test.sh | 23 ++---------
 .../selftests/net/srv6_end_next_csid_l3vpn_test.sh | 19 +---------
 .../net/srv6_end_x_next_csid_l3vpn_test.sh         | 19 +---------
 .../selftests/net/srv6_hencap_red_l3vpn_test.sh    | 19 +---------
 .../selftests/net/srv6_hl2encap_red_l2vpn_test.sh  | 19 +---------
 .../selftests/net/test_bridge_backup_port.sh       | 32 +---------------
 .../selftests/net/test_bridge_neigh_suppress.sh    | 34 +----------------
 tools/testing/selftests/net/test_vxlan_mdb.sh      | 32 +---------------
 .../selftests/net/test_vxlan_nolocalbypass.sh      | 32 +---------------
 .../selftests/net/test_vxlan_vnifiltering.sh       | 26 +------------
 tools/testing/selftests/net/vrf-xfrm-tests.sh      | 19 +---------
 tools/testing/selftests/net/vrf_route_leaking.sh   | 19 +---------
 .../testing/selftests/net/vrf_strict_mode_test.sh  | 19 +---------
 33 files changed, 73 insertions(+), 710 deletions(-)

diff --git a/tools/testing/selftests/net/drop_monitor_tests.sh b/tools/testing/selftests/net/drop_monitor_tests.sh
index 507d0a82f5f0..7da85608561b 100755
--- a/tools/testing/selftests/net/drop_monitor_tests.sh
+++ b/tools/testing/selftests/net/drop_monitor_tests.sh
@@ -18,18 +18,7 @@ DEVLINK_DEV=netdevsim/${DEV}
 
 log_test()
 {
-	local rc=$1
-	local expected=$2
-	local msg="$3"
-
-	if [ ${rc} -eq ${expected} ]; then
-		printf "    TEST: %-60s  [ OK ]\n" "${msg}"
-		nsuccess=$((nsuccess+1))
-	else
-		ret=1
-		nfail=$((nfail+1))
-		printf "    TEST: %-60s  [FAIL]\n" "${msg}"
-	fi
+	log_test_expected "$1" "$2" "$3"
 }
 
 setup()
diff --git a/tools/testing/selftests/net/fcnal-test.sh b/tools/testing/selftests/net/fcnal-test.sh
index 890c3f8e51bb..a50609535fed 100755
--- a/tools/testing/selftests/net/fcnal-test.sh
+++ b/tools/testing/selftests/net/fcnal-test.sh
@@ -97,34 +97,7 @@ fi
 
 log_test()
 {
-	local rc=$1
-	local expected=$2
-	local msg="$3"
-	local ans
-
-	[ "${VERBOSE}" = "1" ] && echo
-
-	if [ ${rc} -eq ${expected} ]; then
-		nsuccess=$((nsuccess+1))
-		printf "TEST: %-70s  [ OK ]\n" "${msg}"
-	else
-		nfail=$((nfail+1))
-		printf "TEST: %-70s  [FAIL]\n" "${msg}"
-		echo "    expected rc $expected; actual rc $rc"
-		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
-			echo
-			echo "hit enter to continue, 'q' to quit"
-			read ans
-			[ "$ans" = "q" ] && exit 1
-		fi
-	fi
-
-	if [ "${PAUSE}" = "yes" ]; then
-		echo
-		echo "hit enter to continue, 'q' to quit"
-		read ans
-		[ "$ans" = "q" ] && exit 1
-	fi
+	log_test_expected "$1" "$2" "$3"
 
 	kill_procs
 }
diff --git a/tools/testing/selftests/net/fdb_flush.sh b/tools/testing/selftests/net/fdb_flush.sh
index 9931a1e36e3d..4965e52d5ef5 100755
--- a/tools/testing/selftests/net/fdb_flush.sh
+++ b/tools/testing/selftests/net/fdb_flush.sh
@@ -67,40 +67,7 @@ run_cmd()
 
 log_test()
 {
-	local rc=$1
-	local expected=$2
-	local msg="$3"
-	local nsuccess
-	local nfail
-	local ret
-
-	if [ ${rc} -eq ${expected} ]; then
-		printf "TEST: %-60s  [ OK ]\n" "${msg}"
-		nsuccess=$((nsuccess+1))
-	else
-		ret=1
-		nfail=$((nfail+1))
-		printf "TEST: %-60s  [FAIL]\n" "${msg}"
-		if [ "$VERBOSE" = "1" ]; then
-			echo "    rc=$rc, expected $expected"
-		fi
-
-		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
-		echo
-			echo "hit enter to continue, 'q' to quit"
-			read a
-			[ "$a" = "q" ] && exit 1
-		fi
-	fi
-
-	if [ "${PAUSE}" = "yes" ]; then
-		echo
-		echo "hit enter to continue, 'q' to quit"
-		read a
-		[ "$a" = "q" ] && exit 1
-	fi
-
-	[ "$VERBOSE" = "1" ] && echo
+	log_test_expected "$1" "$2" "$3"
 }
 
 MAC_POOL_1="
diff --git a/tools/testing/selftests/net/fib-onlink-tests.sh b/tools/testing/selftests/net/fib-onlink-tests.sh
index e0d45292a298..a26075abf7d2 100755
--- a/tools/testing/selftests/net/fib-onlink-tests.sh
+++ b/tools/testing/selftests/net/fib-onlink-tests.sh
@@ -85,23 +85,7 @@ PBR_TABLE=101
 
 log_test()
 {
-	local rc=$1
-	local expected=$2
-	local msg="$3"
-
-	if [ ${rc} -eq ${expected} ]; then
-		nsuccess=$((nsuccess+1))
-		printf "    TEST: %-50s  [ OK ]\n" "${msg}"
-	else
-		nfail=$((nfail+1))
-		printf "    TEST: %-50s  [FAIL]\n" "${msg}"
-		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
-			echo
-			echo "hit enter to continue, 'q' to quit"
-			read a
-			[ "$a" = "q" ] && exit 1
-		fi
-	fi
+	log_test_expected "$1" "$2" "$3"
 }
 
 log_section()
diff --git a/tools/testing/selftests/net/fib_nexthop_multiprefix.sh b/tools/testing/selftests/net/fib_nexthop_multiprefix.sh
index e85248609af4..bd0b8a053ea1 100755
--- a/tools/testing/selftests/net/fib_nexthop_multiprefix.sh
+++ b/tools/testing/selftests/net/fib_nexthop_multiprefix.sh
@@ -23,26 +23,7 @@ which ping6 > /dev/null 2>&1 && ping6=$(which ping6) || ping6=$(which ping)
 
 log_test()
 {
-	local rc=$1
-	local expected=$2
-	local msg="$3"
-
-	if [ ${rc} -eq ${expected} ]; then
-		printf "TEST: %-60s  [ OK ]\n" "${msg}"
-		nsuccess=$((nsuccess+1))
-	else
-		ret=1
-		nfail=$((nfail+1))
-		printf "TEST: %-60s  [FAIL]\n" "${msg}"
-		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
-			echo
-			echo "hit enter to continue, 'q' to quit"
-			read a
-			[ "$a" = "q" ] && exit 1
-		fi
-	fi
-
-	[ "$VERBOSE" = "1" ] && echo
+	log_test_expected "$1" "$2" "$3"
 }
 
 run_cmd()
diff --git a/tools/testing/selftests/net/fib_nexthop_nongw.sh b/tools/testing/selftests/net/fib_nexthop_nongw.sh
index 1ccf56f10171..4d483cb83593 100755
--- a/tools/testing/selftests/net/fib_nexthop_nongw.sh
+++ b/tools/testing/selftests/net/fib_nexthop_nongw.sh
@@ -18,26 +18,7 @@ ret=0
 
 log_test()
 {
-	local rc=$1
-	local expected=$2
-	local msg="$3"
-
-	if [ ${rc} -eq ${expected} ]; then
-		printf "TEST: %-60s  [ OK ]\n" "${msg}"
-		nsuccess=$((nsuccess+1))
-	else
-		ret=1
-		nfail=$((nfail+1))
-		printf "TEST: %-60s  [FAIL]\n" "${msg}"
-		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
-			echo
-			echo "hit enter to continue, 'q' to quit"
-			read a
-			[ "$a" = "q" ] && exit 1
-		fi
-	fi
-
-	[ "$VERBOSE" = "1" ] && echo
+	log_test_expected "$1" "$2" "$3"
 }
 
 run_cmd()
diff --git a/tools/testing/selftests/net/fib_nexthops.sh b/tools/testing/selftests/net/fib_nexthops.sh
index 431d7bed7622..d0d979e8e438 100755
--- a/tools/testing/selftests/net/fib_nexthops.sh
+++ b/tools/testing/selftests/net/fib_nexthops.sh
@@ -70,44 +70,7 @@ nsid=100
 
 log_test()
 {
-	local rc=$1
-	local expected=$2
-	local msg="$3"
-
-	if [ ${rc} -eq ${expected} ]; then
-		printf "TEST: %-60s  [ OK ]\n" "${msg}"
-		nsuccess=$((nsuccess+1))
-	else
-		if [[ $rc -eq $ksft_skip ]]; then
-			[[ $ret -eq 0 ]] && ret=$ksft_skip
-			nskip=$((nskip+1))
-			printf "TEST: %-60s  [SKIP]\n" "${msg}"
-		else
-			ret=1
-			nfail=$((nfail+1))
-			printf "TEST: %-60s  [FAIL]\n" "${msg}"
-		fi
-
-		if [ "$VERBOSE" = "1" ]; then
-			echo "    rc=$rc, expected $expected"
-		fi
-
-		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
-		echo
-			echo "hit enter to continue, 'q' to quit"
-			read a
-			[ "$a" = "q" ] && exit 1
-		fi
-	fi
-
-	if [ "${PAUSE}" = "yes" ]; then
-		echo
-		echo "hit enter to continue, 'q' to quit"
-		read a
-		[ "$a" = "q" ] && exit 1
-	fi
-
-	[ "$VERBOSE" = "1" ] && echo
+	log_test_expected "$1" "$2" "$3"
 }
 
 run_cmd()
@@ -1258,7 +1221,7 @@ ipv6_fcnal_runtime()
 		run_cmd "ip netns exec $me ping -c1 -w$PING_TIMEOUT 2001:db8:101::1"
 		log_test $? 0 "Ping - group blackhole replaced with gateways"
 	else
-		log_test 2 0 "Ping - multipath failed"
+		log_test $rc 0 "Ping - multipath failed"
 	fi
 
 	#
@@ -1915,7 +1878,7 @@ ipv4_fcnal_runtime()
 		run_cmd "ip netns exec $me ping -c1 -w$PING_TIMEOUT 172.16.101.1"
 		log_test $? 0 "Ping - group blackhole replaced with gateways"
 	else
-		log_test 2 0 "Ping - multipath failed"
+		log_test $rc 0 "Ping - multipath failed"
 	fi
 
 	#
@@ -2729,7 +2692,6 @@ done
 if [ "$TESTS" != "none" ]; then
 	printf "\nTests passed: %3d\n" ${nsuccess}
 	printf "Tests failed: %3d\n"   ${nfail}
-	printf "Tests skipped: %2d\n"  ${nskip}
 fi
 
 exit $ret
diff --git a/tools/testing/selftests/net/fib_rule_tests.sh b/tools/testing/selftests/net/fib_rule_tests.sh
index 5fbdd2a0b537..5d66b00e61b2 100755
--- a/tools/testing/selftests/net/fib_rule_tests.sh
+++ b/tools/testing/selftests/net/fib_rule_tests.sh
@@ -31,24 +31,7 @@ SELFTEST_PATH=""
 
 log_test()
 {
-	local rc=$1
-	local expected=$2
-	local msg="$3"
-
-	if [ ${rc} -eq ${expected} ]; then
-		nsuccess=$((nsuccess+1))
-		printf "    TEST: %-60s  [ OK ]\n" "${msg}"
-	else
-		ret=1
-		nfail=$((nfail+1))
-		printf "    TEST: %-60s  [FAIL]\n" "${msg}"
-		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
-			echo
-			echo "hit enter to continue, 'q' to quit"
-			read a
-			[ "$a" = "q" ] && exit 1
-		fi
-	fi
+	log_test_expected "$1" "$2" "$3"
 }
 
 setup()
diff --git a/tools/testing/selftests/net/fib_tests.sh b/tools/testing/selftests/net/fib_tests.sh
index b338bfb196a2..7df967a2d669 100755
--- a/tools/testing/selftests/net/fib_tests.sh
+++ b/tools/testing/selftests/net/fib_tests.sh
@@ -24,31 +24,7 @@ which ping6 > /dev/null 2>&1 && ping6=$(which ping6) || ping6=$(which ping)
 
 log_test()
 {
-	local rc=$1
-	local expected=$2
-	local msg="$3"
-
-	if [ ${rc} -eq ${expected} ]; then
-		printf "    TEST: %-60s  [ OK ]\n" "${msg}"
-		nsuccess=$((nsuccess+1))
-	else
-		ret=1
-		nfail=$((nfail+1))
-		printf "    TEST: %-60s  [FAIL]\n" "${msg}"
-		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
-		echo
-			echo "hit enter to continue, 'q' to quit"
-			read a
-			[ "$a" = "q" ] && exit 1
-		fi
-	fi
-
-	if [ "${PAUSE}" = "yes" ]; then
-		echo
-		echo "hit enter to continue, 'q' to quit"
-		read a
-		[ "$a" = "q" ] && exit 1
-	fi
+	log_test_expected "$1" "$2" "$3"
 }
 
 setup()
diff --git a/tools/testing/selftests/net/gre_gso.sh b/tools/testing/selftests/net/gre_gso.sh
index 5100d90f92d2..4ebe1ed6e9c9 100755
--- a/tools/testing/selftests/net/gre_gso.sh
+++ b/tools/testing/selftests/net/gre_gso.sh
@@ -16,31 +16,7 @@ PID=
 
 log_test()
 {
-	local rc=$1
-	local expected=$2
-	local msg="$3"
-
-	if [ ${rc} -eq ${expected} ]; then
-		printf "    TEST: %-60s  [ OK ]\n" "${msg}"
-		nsuccess=$((nsuccess+1))
-	else
-		ret=1
-		nfail=$((nfail+1))
-		printf "    TEST: %-60s  [FAIL]\n" "${msg}"
-		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
-		echo
-			echo "hit enter to continue, 'q' to quit"
-			read a
-			[ "$a" = "q" ] && exit 1
-		fi
-	fi
-
-	if [ "${PAUSE}" = "yes" ]; then
-		echo
-		echo "hit enter to continue, 'q' to quit"
-		read a
-		[ "$a" = "q" ] && exit 1
-	fi
+	log_test_expected "$1" "$2" "$3"
 }
 
 setup()
diff --git a/tools/testing/selftests/net/icmp_redirect.sh b/tools/testing/selftests/net/icmp_redirect.sh
index 35357d02e823..e724f895ae2c 100755
--- a/tools/testing/selftests/net/icmp_redirect.sh
+++ b/tools/testing/selftests/net/icmp_redirect.sh
@@ -61,24 +61,7 @@ log_section()
 
 log_test()
 {
-	local rc=$1
-	local expected=$2
-	local msg="$3"
-
-	if [ ${rc} -eq ${expected} ]; then
-		printf "TEST: %-60s  [ OK ]\n" "${msg}"
-		nsuccess=$((nsuccess+1))
-	else
-		ret=1
-		nfail=$((nfail+1))
-		printf "TEST: %-60s  [FAIL]\n" "${msg}"
-		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
-			echo
-			echo "hit enter to continue, 'q' to quit"
-			read a
-			[ "$a" = "q" ] && exit 1
-		fi
-	fi
+	log_test_expected "$1" "$2" "$3"
 }
 
 log_debug()
diff --git a/tools/testing/selftests/net/l2tp.sh b/tools/testing/selftests/net/l2tp.sh
index 88de7166c8ae..41e8b19d1bd3 100755
--- a/tools/testing/selftests/net/l2tp.sh
+++ b/tools/testing/selftests/net/l2tp.sh
@@ -23,24 +23,7 @@ which ping6 > /dev/null 2>&1 && ping6=$(which ping6) || ping6=$(which ping)
 #
 log_test()
 {
-	local rc=$1
-	local expected=$2
-	local msg="$3"
-
-	if [ ${rc} -eq ${expected} ]; then
-		printf "TEST: %-60s  [ OK ]\n" "${msg}"
-		nsuccess=$((nsuccess+1))
-	else
-		ret=1
-		nfail=$((nfail+1))
-		printf "TEST: %-60s  [FAIL]\n" "${msg}"
-		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
-			echo
-			echo "hit enter to continue, 'q' to quit"
-			read a
-			[ "$a" = "q" ] && exit 1
-		fi
-	fi
+	log_test_expected "$1" "$2" "$3"
 }
 
 run_cmd()
diff --git a/tools/testing/selftests/net/lib.sh b/tools/testing/selftests/net/lib.sh
index d46d2cec89e4..f698fa3d2bd5 100644
--- a/tools/testing/selftests/net/lib.sh
+++ b/tools/testing/selftests/net/lib.sh
@@ -454,6 +454,39 @@ log_test_xfail()
 	RET=$ksft_xfail retmsg= log_test "$@"
 }
 
+# Log test result with expected return value
+log_test_expected()
+{
+	local rc=$1
+	local expected=$2
+	local msg="$3"
+
+	if [ "${rc}" -eq "${expected}" ]; then
+		nsuccess=$((nsuccess+1))
+		printf "    TEST: %-60s  [ OK ]\n" "${msg}"
+	else
+		ret=$(ksft_exit_status_merge "$ret" "$ksft_fail")
+		nfail=$((nfail+1))
+		printf "    TEST: %-60s  [FAIL]\n" "${msg}"
+		if [ "$VERBOSE" = "1" ]; then
+			echo "    rc=$rc, expected $expected"
+		fi
+
+		pause_on_fail
+	fi
+
+	if [ "${PAUSE}" = "yes" ]; then
+		echo
+		echo "hit enter to continue, 'q' to quit"
+		read -r a
+		[ "$a" = "q" ] && exit 1
+	fi
+
+	[ "$VERBOSE" = "1" ] && echo
+
+	return 0
+}
+
 log_info()
 {
 	local msg=$1
diff --git a/tools/testing/selftests/net/ndisc_unsolicited_na_test.sh b/tools/testing/selftests/net/ndisc_unsolicited_na_test.sh
index 5db69dad0cfc..89072d30fc59 100755
--- a/tools/testing/selftests/net/ndisc_unsolicited_na_test.sh
+++ b/tools/testing/selftests/net/ndisc_unsolicited_na_test.sh
@@ -30,31 +30,7 @@ tcpdump_stderr=
 
 log_test()
 {
-	local rc=$1
-	local expected=$2
-	local msg="$3"
-
-	if [ ${rc} -eq ${expected} ]; then
-		printf "    TEST: %-60s  [ OK ]\n" "${msg}"
-		nsuccess=$((nsuccess+1))
-	else
-		ret=1
-		nfail=$((nfail+1))
-		printf "    TEST: %-60s  [FAIL]\n" "${msg}"
-		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
-		echo
-			echo "hit enter to continue, 'q' to quit"
-			read a
-			[ "$a" = "q" ] && exit 1
-		fi
-	fi
-
-	if [ "${PAUSE}" = "yes" ]; then
-		echo
-		echo "hit enter to continue, 'q' to quit"
-		read a
-		[ "$a" = "q" ] && exit 1
-	fi
+	log_test_expected "$1" "$2" "$3"
 }
 
 setup()
diff --git a/tools/testing/selftests/net/srv6_encap_lookup_l3vpn_test.sh b/tools/testing/selftests/net/srv6_encap_lookup_l3vpn_test.sh
index d6249303b7ea..8241de6827de 100755
--- a/tools/testing/selftests/net/srv6_encap_lookup_l3vpn_test.sh
+++ b/tools/testing/selftests/net/srv6_encap_lookup_l3vpn_test.sh
@@ -202,24 +202,7 @@ PAUSE_ON_FAIL=${PAUSE_ON_FAIL:=no}
 
 log_test()
 {
-	local rc="$1"
-	local expected="$2"
-	local msg="$3"
-
-	if [ "${rc}" -eq "${expected}" ]; then
-		nsuccess=$((nsuccess+1))
-		printf "\n    TEST: %-60s  [ OK ]\n" "${msg}"
-	else
-		ret=1
-		nfail=$((nfail+1))
-		printf "\n    TEST: %-60s  [FAIL]\n" "${msg}"
-		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
-			echo
-			echo "hit enter to continue, 'q' to quit"
-			read -r a
-			[ "$a" = "q" ] && exit 1
-		fi
-	fi
+	log_test_expected "$1" "$2" "$3"
 }
 
 print_log_test_results()
diff --git a/tools/testing/selftests/net/srv6_end_dt46_l3vpn_test.sh b/tools/testing/selftests/net/srv6_end_dt46_l3vpn_test.sh
index 50e37d3217ea..900a2ae42335 100755
--- a/tools/testing/selftests/net/srv6_end_dt46_l3vpn_test.sh
+++ b/tools/testing/selftests/net/srv6_end_dt46_l3vpn_test.sh
@@ -208,24 +208,7 @@ PAUSE_ON_FAIL=${PAUSE_ON_FAIL:=no}
 
 log_test()
 {
-	local rc=$1
-	local expected=$2
-	local msg="$3"
-
-	if [ ${rc} -eq ${expected} ]; then
-		nsuccess=$((nsuccess+1))
-		printf "\n    TEST: %-60s  [ OK ]\n" "${msg}"
-	else
-		ret=1
-		nfail=$((nfail+1))
-		printf "\n    TEST: %-60s  [FAIL]\n" "${msg}"
-		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
-			echo
-			echo "hit enter to continue, 'q' to quit"
-			read a
-			[ "$a" = "q" ] && exit 1
-		fi
-	fi
+	log_test_expected "$1" "$2" "$3"
 }
 
 print_log_test_results()
diff --git a/tools/testing/selftests/net/srv6_end_dt4_l3vpn_test.sh b/tools/testing/selftests/net/srv6_end_dt4_l3vpn_test.sh
index 037e5fe1da2a..260170dc8443 100755
--- a/tools/testing/selftests/net/srv6_end_dt4_l3vpn_test.sh
+++ b/tools/testing/selftests/net/srv6_end_dt4_l3vpn_test.sh
@@ -177,24 +177,7 @@ PAUSE_ON_FAIL=${PAUSE_ON_FAIL:=no}
 
 log_test()
 {
-	local rc=$1
-	local expected=$2
-	local msg="$3"
-
-	if [ ${rc} -eq ${expected} ]; then
-		nsuccess=$((nsuccess+1))
-		printf "\n    TEST: %-60s  [ OK ]\n" "${msg}"
-	else
-		ret=1
-		nfail=$((nfail+1))
-		printf "\n    TEST: %-60s  [FAIL]\n" "${msg}"
-		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
-			echo
-			echo "hit enter to continue, 'q' to quit"
-			read a
-			[ "$a" = "q" ] && exit 1
-		fi
-	fi
+	log_test_expected "$1" "$2" "$3"
 }
 
 print_log_test_results()
diff --git a/tools/testing/selftests/net/srv6_end_dt6_l3vpn_test.sh b/tools/testing/selftests/net/srv6_end_dt6_l3vpn_test.sh
index 9a29e0d6c912..6d4d6a23ecc4 100755
--- a/tools/testing/selftests/net/srv6_end_dt6_l3vpn_test.sh
+++ b/tools/testing/selftests/net/srv6_end_dt6_l3vpn_test.sh
@@ -178,24 +178,7 @@ PAUSE_ON_FAIL=${PAUSE_ON_FAIL:=no}
 
 log_test()
 {
-	local rc=$1
-	local expected=$2
-	local msg="$3"
-
-	if [ ${rc} -eq ${expected} ]; then
-		nsuccess=$((nsuccess+1))
-		printf "\n    TEST: %-60s  [ OK ]\n" "${msg}"
-	else
-		ret=1
-		nfail=$((nfail+1))
-		printf "\n    TEST: %-60s  [FAIL]\n" "${msg}"
-		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
-			echo
-			echo "hit enter to continue, 'q' to quit"
-			read a
-			[ "$a" = "q" ] && exit 1
-		fi
-	fi
+	log_test_expected "$1" "$2" "$3"
 }
 
 print_log_test_results()
diff --git a/tools/testing/selftests/net/srv6_end_dx4_netfilter_test.sh b/tools/testing/selftests/net/srv6_end_dx4_netfilter_test.sh
index e23210aa547f..aeddbef536d6 100755
--- a/tools/testing/selftests/net/srv6_end_dx4_netfilter_test.sh
+++ b/tools/testing/selftests/net/srv6_end_dx4_netfilter_test.sh
@@ -111,8 +111,8 @@
 # +---------------------------------------------------+
 #
 
-# Kselftest framework requirement - SKIP code is 4.
-ksft_skip=4
+# shellcheck source=lib.sh
+source lib.sh
 
 readonly IPv6_RT_NETWORK=2001:11
 readonly IPv4_HS_NETWORK=10.0.0
@@ -126,24 +126,7 @@ PAUSE_ON_FAIL=${PAUSE_ON_FAIL:=no}
 
 log_test()
 {
-	local rc=$1
-	local expected=$2
-	local msg="$3"
-
-	if [ ${rc} -eq ${expected} ]; then
-		nsuccess=$((nsuccess+1))
-		printf "\n    TEST: %-60s  [ OK ]\n" "${msg}"
-	else
-		ret=1
-		nfail=$((nfail+1))
-		printf "\n    TEST: %-60s  [FAIL]\n" "${msg}"
-		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
-			echo
-			echo "hit enter to continue, 'q' to quit"
-			read a
-			[ "$a" = "q" ] && exit 1
-		fi
-	fi
+	log_test_expected "$1" "$2" "$3"
 }
 
 print_log_test_results()
diff --git a/tools/testing/selftests/net/srv6_end_dx6_netfilter_test.sh b/tools/testing/selftests/net/srv6_end_dx6_netfilter_test.sh
index 9e69a2ed5bc3..8fbeda1372e1 100755
--- a/tools/testing/selftests/net/srv6_end_dx6_netfilter_test.sh
+++ b/tools/testing/selftests/net/srv6_end_dx6_netfilter_test.sh
@@ -111,8 +111,8 @@
 # +---------------------------------------------------+
 #
 
-# Kselftest framework requirement - SKIP code is 4.
-ksft_skip=4
+# shellcheck source=lib.sh
+source lib.sh
 
 readonly IPv6_RT_NETWORK=2001:11
 readonly IPv6_HS_NETWORK=cafe
@@ -126,24 +126,7 @@ PAUSE_ON_FAIL=${PAUSE_ON_FAIL:=no}
 
 log_test()
 {
-	local rc=$1
-	local expected=$2
-	local msg="$3"
-
-	if [ ${rc} -eq ${expected} ]; then
-		nsuccess=$((nsuccess+1))
-		printf "\n    TEST: %-60s  [ OK ]\n" "${msg}"
-	else
-		ret=1
-		nfail=$((nfail+1))
-		printf "\n    TEST: %-60s  [FAIL]\n" "${msg}"
-		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
-			echo
-			echo "hit enter to continue, 'q' to quit"
-			read a
-			[ "$a" = "q" ] && exit 1
-		fi
-	fi
+	log_test_expected "$1" "$2" "$3"
 }
 
 print_log_test_results()
diff --git a/tools/testing/selftests/net/srv6_end_flavors_test.sh b/tools/testing/selftests/net/srv6_end_flavors_test.sh
index 318487eda671..30a939392eea 100755
--- a/tools/testing/selftests/net/srv6_end_flavors_test.sh
+++ b/tools/testing/selftests/net/srv6_end_flavors_test.sh
@@ -194,8 +194,8 @@
 # after the IPv6 header. At this point, the packet with IPv6 DA=cafe::1 is sent
 # to the destination, i.e. hs-1.
 
-# Kselftest framework requirement - SKIP code is 4.
-readonly ksft_skip=4
+# shellcheck source=lib.sh
+source lib.sh
 
 readonly RDMSUFF="$(mktemp -u XXXXXXXX)"
 readonly DUMMY_DEVNAME="dum0"
@@ -224,24 +224,7 @@ nfail=0
 
 log_test()
 {
-	local rc="$1"
-	local expected="$2"
-	local msg="$3"
-
-	if [ "${rc}" -eq "${expected}" ]; then
-		nsuccess=$((nsuccess+1))
-		printf "\n    TEST: %-60s  [ OK ]\n" "${msg}"
-	else
-		ret=1
-		nfail=$((nfail+1))
-		printf "\n    TEST: %-60s  [FAIL]\n" "${msg}"
-		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
-			echo
-			echo "hit enter to continue, 'q' to quit"
-			read a
-			[ "$a" = "q" ] && exit 1
-		fi
-	fi
+	log_test_expected "$1" "$2" "$3"
 }
 
 print_log_test_results()
diff --git a/tools/testing/selftests/net/srv6_end_next_csid_l3vpn_test.sh b/tools/testing/selftests/net/srv6_end_next_csid_l3vpn_test.sh
index 4bc135e5c22c..2e2ae21974ae 100755
--- a/tools/testing/selftests/net/srv6_end_next_csid_l3vpn_test.sh
+++ b/tools/testing/selftests/net/srv6_end_next_csid_l3vpn_test.sh
@@ -323,24 +323,7 @@ nfail=0
 
 log_test()
 {
-	local rc="$1"
-	local expected="$2"
-	local msg="$3"
-
-	if [ "${rc}" -eq "${expected}" ]; then
-		nsuccess=$((nsuccess+1))
-		printf "\n    TEST: %-60s  [ OK ]\n" "${msg}"
-	else
-		ret=1
-		nfail=$((nfail+1))
-		printf "\n    TEST: %-60s  [FAIL]\n" "${msg}"
-		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
-			echo
-			echo "hit enter to continue, 'q' to quit"
-			read a
-			[ "$a" = "q" ] && exit 1
-		fi
-	fi
+	log_test_expected "$1" "$2" "$3"
 }
 
 print_log_test_results()
diff --git a/tools/testing/selftests/net/srv6_end_x_next_csid_l3vpn_test.sh b/tools/testing/selftests/net/srv6_end_x_next_csid_l3vpn_test.sh
index 34b781a2ae74..b492a7f0297f 100755
--- a/tools/testing/selftests/net/srv6_end_x_next_csid_l3vpn_test.sh
+++ b/tools/testing/selftests/net/srv6_end_x_next_csid_l3vpn_test.sh
@@ -368,24 +368,7 @@ nfail=0
 
 log_test()
 {
-	local rc="$1"
-	local expected="$2"
-	local msg="$3"
-
-	if [ "${rc}" -eq "${expected}" ]; then
-		nsuccess=$((nsuccess+1))
-		printf "\n    TEST: %-60s  [ OK ]\n" "${msg}"
-	else
-		ret=1
-		nfail=$((nfail+1))
-		printf "\n    TEST: %-60s  [FAIL]\n" "${msg}"
-		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
-			echo
-			echo "hit enter to continue, 'q' to quit"
-			read a
-			[ "$a" = "q" ] && exit 1
-		fi
-	fi
+	log_test_expected "$1" "$2" "$3"
 }
 
 print_log_test_results()
diff --git a/tools/testing/selftests/net/srv6_hencap_red_l3vpn_test.sh b/tools/testing/selftests/net/srv6_hencap_red_l3vpn_test.sh
index cd7d061e21f8..64ea4e2308b6 100755
--- a/tools/testing/selftests/net/srv6_hencap_red_l3vpn_test.sh
+++ b/tools/testing/selftests/net/srv6_hencap_red_l3vpn_test.sh
@@ -197,24 +197,7 @@ HAS_TUNSRC=false
 
 log_test()
 {
-	local rc="$1"
-	local expected="$2"
-	local msg="$3"
-
-	if [ "${rc}" -eq "${expected}" ]; then
-		nsuccess=$((nsuccess+1))
-		printf "\n    TEST: %-60s  [ OK ]\n" "${msg}"
-	else
-		ret=1
-		nfail=$((nfail+1))
-		printf "\n    TEST: %-60s  [FAIL]\n" "${msg}"
-		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
-			echo
-			echo "hit enter to continue, 'q' to quit"
-			read a
-			[ "$a" = "q" ] && exit 1
-		fi
-	fi
+	log_test_expected "$1" "$2" "$3"
 }
 
 print_log_test_results()
diff --git a/tools/testing/selftests/net/srv6_hl2encap_red_l2vpn_test.sh b/tools/testing/selftests/net/srv6_hl2encap_red_l2vpn_test.sh
index 0979b5316fdf..31e41cc4922e 100755
--- a/tools/testing/selftests/net/srv6_hl2encap_red_l2vpn_test.sh
+++ b/tools/testing/selftests/net/srv6_hl2encap_red_l2vpn_test.sh
@@ -146,24 +146,7 @@ nfail=0
 
 log_test()
 {
-	local rc="$1"
-	local expected="$2"
-	local msg="$3"
-
-	if [ "${rc}" -eq "${expected}" ]; then
-		nsuccess=$((nsuccess+1))
-		printf "\n    TEST: %-60s  [ OK ]\n" "${msg}"
-	else
-		ret=1
-		nfail=$((nfail+1))
-		printf "\n    TEST: %-60s  [FAIL]\n" "${msg}"
-		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
-			echo
-			echo "hit enter to continue, 'q' to quit"
-			read a
-			[ "$a" = "q" ] && exit 1
-		fi
-	fi
+	log_test_expected "$1" "$2" "$3"
 }
 
 print_log_test_results()
diff --git a/tools/testing/selftests/net/test_bridge_backup_port.sh b/tools/testing/selftests/net/test_bridge_backup_port.sh
index 2a7224fe74f2..8bc76be2b2d3 100755
--- a/tools/testing/selftests/net/test_bridge_backup_port.sh
+++ b/tools/testing/selftests/net/test_bridge_backup_port.sh
@@ -56,37 +56,7 @@ PING_TIMEOUT=5
 
 log_test()
 {
-	local rc=$1
-	local expected=$2
-	local msg="$3"
-
-	if [ ${rc} -eq ${expected} ]; then
-		printf "TEST: %-60s  [ OK ]\n" "${msg}"
-		nsuccess=$((nsuccess+1))
-	else
-		ret=1
-		nfail=$((nfail+1))
-		printf "TEST: %-60s  [FAIL]\n" "${msg}"
-		if [ "$VERBOSE" = "1" ]; then
-			echo "    rc=$rc, expected $expected"
-		fi
-
-		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
-		echo
-			echo "hit enter to continue, 'q' to quit"
-			read a
-			[ "$a" = "q" ] && exit 1
-		fi
-	fi
-
-	if [ "${PAUSE}" = "yes" ]; then
-		echo
-		echo "hit enter to continue, 'q' to quit"
-		read a
-		[ "$a" = "q" ] && exit 1
-	fi
-
-	[ "$VERBOSE" = "1" ] && echo
+	log_test_expected "$1" "$2" "$3"
 }
 
 run_cmd()
diff --git a/tools/testing/selftests/net/test_bridge_neigh_suppress.sh b/tools/testing/selftests/net/test_bridge_neigh_suppress.sh
index e9ed0d750996..9d2dc0faf741 100755
--- a/tools/testing/selftests/net/test_bridge_neigh_suppress.sh
+++ b/tools/testing/selftests/net/test_bridge_neigh_suppress.sh
@@ -72,39 +72,7 @@ PAUSE=no
 
 log_test()
 {
-	local rc=$1
-	local expected=$2
-	local msg="$3"
-
-	if [ ${rc} -eq ${expected} ]; then
-		printf "TEST: %-60s  [ OK ]\n" "${msg}"
-		nsuccess=$((nsuccess+1))
-	else
-		# shellcheck disable=SC2154
-		ret=$(ksft_exit_status_merge "$ret" "$ksft_fail")
-		nfail=$((nfail+1))
-		printf "TEST: %-60s  [FAIL]\n" "${msg}"
-		if [ "$VERBOSE" = "1" ]; then
-			echo "    rc=$rc, expected $expected"
-		fi
-
-		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
-		echo
-			echo "hit enter to continue, 'q' to quit"
-			read a
-			[ "$a" = "q" ] && exit 1
-		fi
-	fi
-
-	if [ "${PAUSE}" = "yes" ]; then
-		echo
-		echo "hit enter to continue, 'q' to quit"
-		read a
-		[ "$a" = "q" ] && exit 1
-	fi
-
-	[ "$VERBOSE" = "1" ] && echo
-	return 0
+	log_test_expected "$1" "$2" "$3"
 }
 
 run_cmd()
diff --git a/tools/testing/selftests/net/test_vxlan_mdb.sh b/tools/testing/selftests/net/test_vxlan_mdb.sh
index f9600aabd4a2..2ebb4d8a3026 100755
--- a/tools/testing/selftests/net/test_vxlan_mdb.sh
+++ b/tools/testing/selftests/net/test_vxlan_mdb.sh
@@ -133,37 +133,7 @@ PAUSE=no
 
 log_test()
 {
-	local rc=$1
-	local expected=$2
-	local msg="$3"
-
-	if [ ${rc} -eq ${expected} ]; then
-		printf "TEST: %-60s  [ OK ]\n" "${msg}"
-		nsuccess=$((nsuccess+1))
-	else
-		ret=1
-		nfail=$((nfail+1))
-		printf "TEST: %-60s  [FAIL]\n" "${msg}"
-		if [ "$VERBOSE" = "1" ]; then
-			echo "    rc=$rc, expected $expected"
-		fi
-
-		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
-		echo
-			echo "hit enter to continue, 'q' to quit"
-			read a
-			[ "$a" = "q" ] && exit 1
-		fi
-	fi
-
-	if [ "${PAUSE}" = "yes" ]; then
-		echo
-		echo "hit enter to continue, 'q' to quit"
-		read a
-		[ "$a" = "q" ] && exit 1
-	fi
-
-	[ "$VERBOSE" = "1" ] && echo
+	log_test_expected "$1" "$2" "$3"
 }
 
 run_cmd()
diff --git a/tools/testing/selftests/net/test_vxlan_nolocalbypass.sh b/tools/testing/selftests/net/test_vxlan_nolocalbypass.sh
index b8805983b728..c51ca0b532dd 100755
--- a/tools/testing/selftests/net/test_vxlan_nolocalbypass.sh
+++ b/tools/testing/selftests/net/test_vxlan_nolocalbypass.sh
@@ -24,37 +24,7 @@ PAUSE=no
 
 log_test()
 {
-	local rc=$1
-	local expected=$2
-	local msg="$3"
-
-	if [ ${rc} -eq ${expected} ]; then
-		printf "TEST: %-60s  [ OK ]\n" "${msg}"
-		nsuccess=$((nsuccess+1))
-	else
-		ret=1
-		nfail=$((nfail+1))
-		printf "TEST: %-60s  [FAIL]\n" "${msg}"
-		if [ "$VERBOSE" = "1" ]; then
-			echo "    rc=$rc, expected $expected"
-		fi
-
-		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
-		echo
-			echo "hit enter to continue, 'q' to quit"
-			read a
-			[ "$a" = "q" ] && exit 1
-		fi
-	fi
-
-	if [ "${PAUSE}" = "yes" ]; then
-		echo
-		echo "hit enter to continue, 'q' to quit"
-		read a
-		[ "$a" = "q" ] && exit 1
-	fi
-
-	[ "$VERBOSE" = "1" ] && echo
+	log_test_expected "$1" "$2" "$3"
 }
 
 run_cmd()
diff --git a/tools/testing/selftests/net/test_vxlan_vnifiltering.sh b/tools/testing/selftests/net/test_vxlan_vnifiltering.sh
index 8deacc565afa..6fb5af013dac 100755
--- a/tools/testing/selftests/net/test_vxlan_vnifiltering.sh
+++ b/tools/testing/selftests/net/test_vxlan_vnifiltering.sh
@@ -98,31 +98,7 @@ which ping6 > /dev/null 2>&1 && ping6=$(which ping6) || ping6=$(which ping)
 
 log_test()
 {
-	local rc=$1
-	local expected=$2
-	local msg="$3"
-
-	if [ ${rc} -eq ${expected} ]; then
-		printf "    TEST: %-60s  [ OK ]\n" "${msg}"
-		nsuccess=$((nsuccess+1))
-	else
-		ret=1
-		nfail=$((nfail+1))
-		printf "    TEST: %-60s  [FAIL]\n" "${msg}"
-		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
-		echo
-			echo "hit enter to continue, 'q' to quit"
-			read a
-			[ "$a" = "q" ] && exit 1
-		fi
-	fi
-
-	if [ "${PAUSE}" = "yes" ]; then
-		echo
-		echo "hit enter to continue, 'q' to quit"
-		read a
-		[ "$a" = "q" ] && exit 1
-	fi
+	log_test_expected "$1" "$2" "$3"
 }
 
 run_cmd()
diff --git a/tools/testing/selftests/net/vrf-xfrm-tests.sh b/tools/testing/selftests/net/vrf-xfrm-tests.sh
index b64dd891699d..a4617e5dace7 100755
--- a/tools/testing/selftests/net/vrf-xfrm-tests.sh
+++ b/tools/testing/selftests/net/vrf-xfrm-tests.sh
@@ -35,24 +35,7 @@ which ping6 > /dev/null 2>&1 && ping6=$(which ping6) || ping6=$(which ping)
 #
 log_test()
 {
-	local rc=$1
-	local expected=$2
-	local msg="$3"
-
-	if [ ${rc} -eq ${expected} ]; then
-		printf "TEST: %-60s  [ OK ]\n" "${msg}"
-		nsuccess=$((nsuccess+1))
-	else
-		ret=1
-		nfail=$((nfail+1))
-		printf "TEST: %-60s  [FAIL]\n" "${msg}"
-		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
-			echo
-			echo "hit enter to continue, 'q' to quit"
-			read a
-			[ "$a" = "q" ] && exit 1
-		fi
-	fi
+	log_test_expected "$1" "$2" "$3"
 }
 
 run_cmd_host1()
diff --git a/tools/testing/selftests/net/vrf_route_leaking.sh b/tools/testing/selftests/net/vrf_route_leaking.sh
index ce34cb2e6e0b..abf106e0f0c0 100755
--- a/tools/testing/selftests/net/vrf_route_leaking.sh
+++ b/tools/testing/selftests/net/vrf_route_leaking.sh
@@ -99,24 +99,7 @@ log_section()
 
 log_test()
 {
-	local rc=$1
-	local expected=$2
-	local msg="$3"
-
-	if [ "${rc}" -eq "${expected}" ]; then
-		printf "TEST: %-60s  [ OK ]\n" "${msg}"
-		nsuccess=$((nsuccess+1))
-	else
-		ret=1
-		nfail=$((nfail+1))
-		printf "TEST: %-60s  [FAIL]\n" "${msg}"
-		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
-			echo
-			echo "hit enter to continue, 'q' to quit"
-			read -r a
-			[ "$a" = "q" ] && exit 1
-		fi
-	fi
+	log_test_expected "$1" "$2" "$3"
 }
 
 run_cmd()
diff --git a/tools/testing/selftests/net/vrf_strict_mode_test.sh b/tools/testing/selftests/net/vrf_strict_mode_test.sh
index 01552b542544..7de1873362ba 100755
--- a/tools/testing/selftests/net/vrf_strict_mode_test.sh
+++ b/tools/testing/selftests/net/vrf_strict_mode_test.sh
@@ -16,24 +16,7 @@ TESTS="init testns mix"
 
 log_test()
 {
-	local rc=$1
-	local expected=$2
-	local msg="$3"
-
-	if [ ${rc} -eq ${expected} ]; then
-		nsuccess=$((nsuccess+1))
-		printf "\n    TEST: %-60s  [ OK ]\n" "${msg}"
-	else
-		ret=1
-		nfail=$((nfail+1))
-		printf "\n    TEST: %-60s  [FAIL]\n" "${msg}"
-		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
-			echo
-			echo "hit enter to continue, 'q' to quit"
-			read a
-			[ "$a" = "q" ] && exit 1
-		fi
-	fi
+	log_test_expected "$1" "$2" "$3"
 }
 
 print_log_test_results()

---
base-commit: 272a65db243bfa34b9277632830e0e06d7e3518e
change-id: 20260812-self_log_test-b76eaf73e23f

Best regards,
-- 
Hangbin Liu <liuhangbin@kylinos.cn>


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net-next v3] selftests: net: move log_test to lib file and remove duplicate code
  2026-09-15  3:14 [PATCH net-next v3] selftests: net: move log_test to lib file and remove duplicate code Hangbin Liu
@ 2026-09-17  2:24 ` netdev-bot+sashiko
  2026-09-17  5:54   ` Hangbin Liu
  0 siblings, 1 reply; 3+ messages in thread
From: netdev-bot+sashiko @ 2026-09-17  2:24 UTC (permalink / raw)
  To: hangbin.liu
  Cc: davem, edumazet, kuba, pabeni, horms, shuah, dsahern, idosch,
	andrea.mayer, netdev, linux-kselftest, linux-kernel, liuhangbin

Thank you for your contribution! Sashiko AI review found 5 potential
issue(s) to consider:

Critical: 0 · High: 1 · Medium: 1 (2) · Low: 3

- [High] SKIP-to-FAIL regression in fib_nexthops.sh, based on a factually
  false claim in the commit message.
- [Medium] log_test_expected() in lib.sh unconditionally executes
  `ret=$(ksft_exit_status_merge "$ret" "$ksft_fail")` on the failure…
- [Low] The commit message states "Two callers compared against exit code
  2 are left as-is since code 2 correctly counts as a failure in the…
- [Low] The migration path advertised in the changelog does not work.
- [Low] The changelog justifies a change with a rationale that cannot
  hold: "On failure, ret is set via ksft_exit_status_merge() instead of…

Pre-existing issues:
- [Medium] fdb_flush.sh, fib-onlink-tests.sh and l2tp.sh can print [FAIL]
  subtests while exiting with status 0, because they never consume the…
- [Medium] fib_tests.sh uses the same global `ret` that the shared helper
  treats as cumulative exit status as per-test scratch storage, so a…

--

selftests: net: move log_test to lib file and remove duplicate code

This adds a shared log_test_expected() helper to
tools/testing/selftests/net/lib.sh and replaces the near-identical
per-script log_test() implementations in 32 net selftests with a thin
wrapper around it. A few scripts also start sourcing lib.sh instead of
defining ksft_skip locally.

Link: https://lore.kernel.org/r/20260831-self_log_test-v2-1-eda2e8490cf9@kylinos.cn
Link: https://lore.kernel.org/r/20260813-self_log_test-v1-1-f88b1107842e@kylinos.cn

A few questions about the conversion below.

> diff --git a/tools/testing/selftests/net/fdb_flush.sh b/tools/testing/selftests/net/fdb_flush.sh
> index 9931a1e36e3db..4965e52d5ef54 100755
> --- a/tools/testing/selftests/net/fdb_flush.sh
> +++ b/tools/testing/selftests/net/fdb_flush.sh
> @@ -67,40 +67,7 @@ run_cmd()
>  
>  log_test()
>  {
> -	local rc=$1
> -	local expected=$2
> -	local msg="$3"
> -	local nsuccess
> -	local nfail
> -	local ret
> -
> -	if [ ${rc} -eq ${expected} ]; then
> -		printf "TEST: %-60s  [ OK ]\n" "${msg}"
> -		nsuccess=$((nsuccess+1))
> -	else
> -		ret=1
> -		nfail=$((nfail+1))
> -		printf "TEST: %-60s  [FAIL]\n" "${msg}"
> -		if [ "$VERBOSE" = "1" ]; then
> -			echo "    rc=$rc, expected $expected"
> -		fi
> -
> -		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
> -		echo
> -			echo "hit enter to continue, 'q' to quit"
> -			read a
> -			[ "$a" = "q" ] && exit 1
> -		fi
> -	fi
> -
> -	if [ "${PAUSE}" = "yes" ]; then
> -		echo
> -		echo "hit enter to continue, 'q' to quit"
> -		read a
> -		[ "$a" = "q" ] && exit 1
> -	fi
> -
> -	[ "$VERBOSE" = "1" ] && echo
> +	log_test_expected "$1" "$2" "$3"
>  }

[Severity: Medium]
The changelog notes that fdb_flush.sh "used local ret, nsuccess, and nfail,
which are not used outside the function", but this script has no global ret
either.  A grep for '\bret\b' in fdb_flush.sh, fcnal-test.sh,
fib-onlink-tests.sh and l2tp.sh at this commit returns no matches.

The failure path of the new helper reads ret unconditionally:

lib.sh:log_test_expected() {
	...
	ret=$(ksft_exit_status_merge "$ret" "$ksft_fail")

With ret unset, the empty first operand ends up as an associative array
subscript in the merge helper:

lib.sh:__ksft_status_merge() {
	...
	if [[ ${weights[$a]} -ge ${weights[$b]} ]]; then

Does bash not print "bad array subscript" here for every failing subtest in
those four scripts?  The merged value still comes out as 1 because the empty
expansion evaluates as 0, so the visible effect is stderr noise from inside
lib.sh, one line per failure (fcnal-test.sh has a lot of subtests).

Would it make sense for the shared helper to default the variable, e.g.
: "${ret:=0}" before the merge, rather than relying on each caller having
defined it?  Also worth noting the copy of this line that previously lived in
test_bridge_neigh_suppress.sh carried a "# shellcheck disable=SC2154" marker
for exactly this referenced-but-not-assigned case, and that marker was
dropped when the code moved into lib.sh.

> diff --git a/tools/testing/selftests/net/fib-onlink-tests.sh b/tools/testing/selftests/net/fib-onlink-tests.sh
> index e0d45292a2983..a26075abf7d27 100755
> --- a/tools/testing/selftests/net/fib-onlink-tests.sh
> +++ b/tools/testing/selftests/net/fib-onlink-tests.sh
> @@ -85,23 +85,7 @@ PBR_TABLE=101
>  
>  log_test()
>  {
> -	local rc=$1
> -	local expected=$2
> -	local msg="$3"
> -
> -	if [ ${rc} -eq ${expected} ]; then
> -		nsuccess=$((nsuccess+1))
> -		printf "    TEST: %-50s  [ OK ]\n" "${msg}"
> -	else
> -		nfail=$((nfail+1))
> -		printf "    TEST: %-50s  [FAIL]\n" "${msg}"
> -		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
> -			echo
> -			echo "hit enter to continue, 'q' to quit"
> -			read a
> -			[ "$a" = "q" ] && exit 1
> -		fi
> -	fi
> +	log_test_expected "$1" "$2" "$3"
>  }

[Severity: Medium]
This isn't a bug introduced by this patch, but it is worth noting while the
failure bookkeeping is being centralised: fib-onlink-tests.sh, l2tp.sh and
fdb_flush.sh never consume the status the helper records.

fib-onlink-tests.sh ends with:

	if [ "$TESTS" != "none" ]; then
		printf "\nTests passed: %3d\n" ${nsuccess}
		printf "Tests failed: %3d\n"   ${nfail}
	fi

l2tp.sh ends with the same two printf lines, and fdb_flush.sh ends with:

	for t in $TESTS
	do
		setup; $t; cleanup;
	done

None of the three does exit $ret or derives an exit code from nfail, so can
these scripts print [FAIL] subtests and still report success to the kselftest
runner?  Since this patch is what makes ret the shared failure accumulator,
would adding exit $ret to those scripts fit here?

> diff --git a/tools/testing/selftests/net/fib_nexthops.sh b/tools/testing/selftests/net/fib_nexthops.sh
> index 431d7bed76224..d0d979e8e438b 100755
> --- a/tools/testing/selftests/net/fib_nexthops.sh
> +++ b/tools/testing/selftests/net/fib_nexthops.sh
> @@ -70,44 +70,7 @@ nsid=100
>  
>  log_test()
>  {
> -	local rc=$1
> -	local expected=$2
> -	local msg="$3"
> -
> -	if [ ${rc} -eq ${expected} ]; then
> -		printf "TEST: %-60s  [ OK ]\n" "${msg}"
> -		nsuccess=$((nsuccess+1))
> -	else
> -		if [[ $rc -eq $ksft_skip ]]; then
> -			[[ $ret -eq 0 ]] && ret=$ksft_skip
> -			nskip=$((nskip+1))
> -			printf "TEST: %-60s  [SKIP]\n" "${msg}"
> -		else
> -			ret=1
> -			nfail=$((nfail+1))
> -			printf "TEST: %-60s  [FAIL]\n" "${msg}"
> -		fi

[Severity: High]
The changelog states:

  "this test checked for ksft_skip (exit code 4) inside log_test(),
   but no test in the file produces that code."

Are these two call sites not producing exactly that code?

fib_nexthops.sh:ipv6_grp_fcnal() {
	...
	run_cmd "$IP nexthop add id 103 group 62,1000"
	if [[ $? == 0 ]]; then
		local GRP="id 103 group 62,254/63,255/64,256/65,257/66,65535"
		run_cmd "$IP nexthop replace $GRP"
		check_nexthop "id 103" "$GRP"
		rc=$?
	else
		rc=$ksft_skip
	fi

	$IP nexthop flush >/dev/null 2>&1

	log_test $rc 0 "16-bit weights"
}

ipv6_res_grp_fcnal() has the same shape with "group 62,1000 type resilient
buckets 32".

The else branch is taken whenever iproute2 is too old to accept 16-bit
nexthop group weights.  After this change, log_test_expected() gets rc=4
against expected 0, so both subtests take the failure branch: [FAIL] is
printed, nfail is incremented and ret is merged with $ksft_fail.

Combined with exit $ret at the end of the script, does a run on such a host
change from two [SKIP] lines and exit status 4 to two [FAIL] lines and exit
status 1?

The changelog suggests "callers that need to report a skip should use
log_test_skip() instead", but neither of these two callers was converted.

> -
> -		if [ "$VERBOSE" = "1" ]; then
> -			echo "    rc=$rc, expected $expected"
> -		fi
> -
> -		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
> -		echo
> -			echo "hit enter to continue, 'q' to quit"
> -			read a
> -			[ "$a" = "q" ] && exit 1
> -		fi
> -	fi
> -
> -	if [ "${PAUSE}" = "yes" ]; then
> -		echo
> -		echo "hit enter to continue, 'q' to quit"
> -		read a
> -		[ "$a" = "q" ] && exit 1
> -	fi
> -
> -	[ "$VERBOSE" = "1" ] && echo
> +	log_test_expected "$1" "$2" "$3"
>  }
>  
>  run_cmd()
> @@ -1258,7 +1221,7 @@ ipv6_fcnal_runtime()
>  		run_cmd "ip netns exec $me ping -c1 -w$PING_TIMEOUT 2001:db8:101::1"
>  		log_test $? 0 "Ping - group blackhole replaced with gateways"
>  	else
> -		log_test 2 0 "Ping - multipath failed"
> +		log_test $rc 0 "Ping - multipath failed"
>  	fi

[Severity: Low]
The changelog says:

  "Two callers compared against exit code 2 are left as-is since code 2
   correctly counts as a failure in the previous log_test()."

These are the two callers in question, in ipv6_fcnal_runtime() and
ipv4_fcnal_runtime(), and they are changed rather than left as-is.  Could the
changelog be updated to describe the substitution?

The change itself looks harmless: run_cmd() assigns rc=$? without declaring
it local, so under bash dynamic scoping it writes into the caller's local rc,
and the else branch is only reached when that value is non-zero, so the
subtest still reports [FAIL] with the same message.

>  
>  	#
> @@ -1915,7 +1878,7 @@ ipv4_fcnal_runtime()
>  		run_cmd "ip netns exec $me ping -c1 -w$PING_TIMEOUT 172.16.101.1"
>  		log_test $? 0 "Ping - group blackhole replaced with gateways"
>  	else
> -		log_test 2 0 "Ping - multipath failed"
> +		log_test $rc 0 "Ping - multipath failed"
>  	fi
>  
>  	#
> @@ -2729,7 +2692,6 @@ done
>  if [ "$TESTS" != "none" ]; then
>  	printf "\nTests passed: %3d\n" ${nsuccess}
>  	printf "Tests failed: %3d\n"   ${nfail}
> -	printf "Tests skipped: %2d\n"  ${nskip}
>  fi
>  
>  exit $ret

[Severity: High]
With the skip counter line removed and no skip branch left in the shared
helper, is there any remaining way for this script to report a skipped
subtest?  The two 16-bit weight probes above are the only producers of
ksft_skip and they are now counted in nfail.

> diff --git a/tools/testing/selftests/net/fib_tests.sh b/tools/testing/selftests/net/fib_tests.sh
> index b338bfb196a27..7df967a2d6697 100755
> --- a/tools/testing/selftests/net/fib_tests.sh
> +++ b/tools/testing/selftests/net/fib_tests.sh
> @@ -24,31 +24,7 @@ which ping6 > /dev/null 2>&1 && ping6=$(which ping6) || ping6=$(which ping)
>  
>  log_test()
>  {
> -	local rc=$1
> -	local expected=$2
> -	local msg="$3"
> -
> -	if [ ${rc} -eq ${expected} ]; then
> -		printf "    TEST: %-60s  [ OK ]\n" "${msg}"
> -		nsuccess=$((nsuccess+1))
> -	else
> -		ret=1
> -		nfail=$((nfail+1))
> -		printf "    TEST: %-60s  [FAIL]\n" "${msg}"

[Severity: Medium]
This is a pre-existing issue, but it interacts with making ret a shared
cumulative status in lib.sh: fib_tests.sh also uses the same global ret as
per-test scratch storage.

fib_carrier_unicast_test() starts with an unconditional:

	ret=0

and fib6_notify_test()/fib_notify_test() do:

	if [ -z "$err" ];then
		ret=0
	else
		ret=1
	fi

	log_test $ret 0 "ipv6 route add notify"

None of these declare ret local, and the script finishes with exit $ret.  If
an earlier subtest failed and a later function assigns ret=0 and then passes,
does the script exit 0 despite the printed [FAIL] lines?  Should these
functions use a local variable now that the helper owns ret?

> -		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
> -		echo
> -			echo "hit enter to continue, 'q' to quit"
> -			read a
> -			[ "$a" = "q" ] && exit 1
> -		fi
> -	fi
> -
> -	if [ "${PAUSE}" = "yes" ]; then
> -		echo
> -		echo "hit enter to continue, 'q' to quit"
> -		read a
> -		[ "$a" = "q" ] && exit 1
> -	fi
> +	log_test_expected "$1" "$2" "$3"
>  }
>  
>  setup()

[ ... ]

> diff --git a/tools/testing/selftests/net/lib.sh b/tools/testing/selftests/net/lib.sh
> index d46d2cec89e45..f698fa3d2bd5f 100644
> --- a/tools/testing/selftests/net/lib.sh
> +++ b/tools/testing/selftests/net/lib.sh
> @@ -454,6 +454,39 @@ log_test_xfail()
>  	RET=$ksft_xfail retmsg= log_test "$@"
>  }
>  
> +# Log test result with expected return value
> +log_test_expected()
> +{
> +	local rc=$1
> +	local expected=$2
> +	local msg="$3"
> +
> +	if [ "${rc}" -eq "${expected}" ]; then
> +		nsuccess=$((nsuccess+1))
> +		printf "    TEST: %-60s  [ OK ]\n" "${msg}"
> +	else

[Severity: Low]
The changelog points callers at log_test_skip() as the replacement for the
removed ksft_skip handling.  Can that helper work in the converted scripts?
It dispatches through log_test:

lib.sh:log_test_skip() {
	RET=$ksft_skip retmsg= log_test "$@"
}

and every script touched here defines its own log_test() after sourcing
lib.sh:

fib_nexthops.sh:log_test() {
	log_test_expected "$1" "$2" "$3"
}

so the local definition shadows lib.sh's log_test().  A call like
log_test_skip "16-bit weights" would then reach log_test_expected with
rc="16-bit weights", expected="" and msg="", i.e.:

	[ "16-bit weights" -eq "" ]

which is an "integer expression expected" error landing in the failure
branch and printing [FAIL] with an empty test name.  There are no callers
today, so nothing breaks right now, but should the wrapper names be
reworked so the suggested migration actually works?

> +		ret=$(ksft_exit_status_merge "$ret" "$ksft_fail")

[Severity: Low]
The changelog gives this as the reason for the change:

  "On failure, ret is set via ksft_exit_status_merge() instead of ret=1,
   so we can propagate ksft_xfail status correctly."

Is that achievable here?  ksft_exit_status_merge() orders the operands as:

lib.sh:ksft_exit_status_merge() {
	...
	__ksft_status_merge "$a" "$b" \
		$ksft_xfail $ksft_pass $ksft_skip $ksft_fail
}

so xfail=0, pass=1, skip=2, fail=3, and the operand with the higher weight
wins.  Since $ksft_fail is always passed as the second operand and carries
the maximum weight, doesn't the result come out as $ksft_fail regardless of
the previous value of ret, including $ksft_xfail?  That makes this line
equivalent to the old ret=1.

> +		nfail=$((nfail+1))
> +		printf "    TEST: %-60s  [FAIL]\n" "${msg}"
> +		if [ "$VERBOSE" = "1" ]; then
> +			echo "    rc=$rc, expected $expected"
> +		fi
> +
> +		pause_on_fail
> +	fi
> +
> +	if [ "${PAUSE}" = "yes" ]; then
> +		echo
> +		echo "hit enter to continue, 'q' to quit"
> +		read -r a
> +		[ "$a" = "q" ] && exit 1
> +	fi
> +
> +	[ "$VERBOSE" = "1" ] && echo
> +
> +	return 0
> +}
> +
>  log_info()
>  {
>  	local msg=$1

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260915-self_log_test-v3-1-a17728176334%40kylinos.cn

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net-next v3] selftests: net: move log_test to lib file and remove duplicate code
  2026-09-17  2:24 ` netdev-bot+sashiko
@ 2026-09-17  5:54   ` Hangbin Liu
  0 siblings, 0 replies; 3+ messages in thread
From: Hangbin Liu @ 2026-09-17  5:54 UTC (permalink / raw)
  To: netdev-bot+sashiko
  Cc: davem, edumazet, kuba, pabeni, horms, shuah, dsahern, idosch,
	andrea.mayer, netdev, linux-kselftest, linux-kernel, liuhangbin

Hi,

Thanks for the review. Since this patch is too long. I will reply the comments
in the summary part.

On Thu, Sep 17, 2026 at 02:24:01AM +0000, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 5 potential
> issue(s) to consider:
> 
> Critical: 0 · High: 1 · Medium: 1 (2) · Low: 3
> 
> - [High] SKIP-to-FAIL regression in fib_nexthops.sh, based on a factually
>   false claim in the commit message.

In patch v1, sashiko suggested to take the SKIP/XFAIL arms when "expected"
itself is ksft_skip/ksft_xfail? Which would looks like

  log_test $rc ksft_xfail.

But ksft_xfail is 2. I'm afraid some test may expect rc=2 and should report
PASS. So I dropped both skip/xfail arms in the function.

Adding a specific handler for fib_nexthops.sh specific seem a little over.
I'd prefer to drop the fib_nexthops.sh change since this is the only
test that covers ksft_xfail in log_test.

> - [Medium] log_test_expected() in lib.sh unconditionally executes
>   `ret=$(ksft_exit_status_merge "$ret" "$ksft_fail")` on the failure…

Ack, I will init ret in case some test not using it.

> - [Low] The commit message states "Two callers compared against exit code
>   2 are left as-is since code 2 correctly counts as a failure in the…

sashiko suggested to add a changelog. I will drop the fib_nexthops.sh
changes, so no need to bother.

> - [Low] The migration path advertised in the changelog does not work.

Ack, the fib_nexthops.sh change will be dropped to avoid this.

> - [Low] The changelog justifies a change with a rationale that cannot
>   hold: "On failure, ret is set via ksft_exit_status_merge() instead of…

Ack, I will use ret=1 directly.
> 
> Pre-existing issues:
> - [Medium] fdb_flush.sh, fib-onlink-tests.sh and l2tp.sh can print [FAIL]
>   subtests while exiting with status 0, because they never consume the…

Not update in this patch.

> - [Medium] fib_tests.sh uses the same global `ret` that the shared helper
>   treats as cumulative exit status as per-test scratch storage, so a…

Ack, I will add this on my todo list and fix it separately.

Thanks
Hangbin
> 
> --
> 
> selftests: net: move log_test to lib file and remove duplicate code
> 
> This adds a shared log_test_expected() helper to
> tools/testing/selftests/net/lib.sh and replaces the near-identical
> per-script log_test() implementations in 32 net selftests with a thin
> wrapper around it. A few scripts also start sourcing lib.sh instead of
> defining ksft_skip locally.
> 
> Link: https://lore.kernel.org/r/20260831-self_log_test-v2-1-eda2e8490cf9@kylinos.cn
> Link: https://lore.kernel.org/r/20260813-self_log_test-v1-1-f88b1107842e@kylinos.cn
> 
> A few questions about the conversion below.
> 
> > diff --git a/tools/testing/selftests/net/fdb_flush.sh b/tools/testing/selftests/net/fdb_flush.sh
> > index 9931a1e36e3db..4965e52d5ef54 100755
> > --- a/tools/testing/selftests/net/fdb_flush.sh
> > +++ b/tools/testing/selftests/net/fdb_flush.sh
> > @@ -67,40 +67,7 @@ run_cmd()
> >  
> >  log_test()
> >  {
> > -	local rc=$1
> > -	local expected=$2
> > -	local msg="$3"
> > -	local nsuccess
> > -	local nfail
> > -	local ret
> > -
> > -	if [ ${rc} -eq ${expected} ]; then
> > -		printf "TEST: %-60s  [ OK ]\n" "${msg}"
> > -		nsuccess=$((nsuccess+1))
> > -	else
> > -		ret=1
> > -		nfail=$((nfail+1))
> > -		printf "TEST: %-60s  [FAIL]\n" "${msg}"
> > -		if [ "$VERBOSE" = "1" ]; then
> > -			echo "    rc=$rc, expected $expected"
> > -		fi
> > -
> > -		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
> > -		echo
> > -			echo "hit enter to continue, 'q' to quit"
> > -			read a
> > -			[ "$a" = "q" ] && exit 1
> > -		fi
> > -	fi
> > -
> > -	if [ "${PAUSE}" = "yes" ]; then
> > -		echo
> > -		echo "hit enter to continue, 'q' to quit"
> > -		read a
> > -		[ "$a" = "q" ] && exit 1
> > -	fi
> > -
> > -	[ "$VERBOSE" = "1" ] && echo
> > +	log_test_expected "$1" "$2" "$3"
> >  }
> 
> [Severity: Medium]
> The changelog notes that fdb_flush.sh "used local ret, nsuccess, and nfail,
> which are not used outside the function", but this script has no global ret
> either.  A grep for '\bret\b' in fdb_flush.sh, fcnal-test.sh,
> fib-onlink-tests.sh and l2tp.sh at this commit returns no matches.
> 
> The failure path of the new helper reads ret unconditionally:
> 
> lib.sh:log_test_expected() {
> 	...
> 	ret=$(ksft_exit_status_merge "$ret" "$ksft_fail")
> 
> With ret unset, the empty first operand ends up as an associative array
> subscript in the merge helper:
> 
> lib.sh:__ksft_status_merge() {
> 	...
> 	if [[ ${weights[$a]} -ge ${weights[$b]} ]]; then
> 
> Does bash not print "bad array subscript" here for every failing subtest in
> those four scripts?  The merged value still comes out as 1 because the empty
> expansion evaluates as 0, so the visible effect is stderr noise from inside
> lib.sh, one line per failure (fcnal-test.sh has a lot of subtests).
> 
> Would it make sense for the shared helper to default the variable, e.g.
> : "${ret:=0}" before the merge, rather than relying on each caller having
> defined it?  Also worth noting the copy of this line that previously lived in
> test_bridge_neigh_suppress.sh carried a "# shellcheck disable=SC2154" marker
> for exactly this referenced-but-not-assigned case, and that marker was
> dropped when the code moved into lib.sh.
> 
> > diff --git a/tools/testing/selftests/net/fib-onlink-tests.sh b/tools/testing/selftests/net/fib-onlink-tests.sh
> > index e0d45292a2983..a26075abf7d27 100755
> > --- a/tools/testing/selftests/net/fib-onlink-tests.sh
> > +++ b/tools/testing/selftests/net/fib-onlink-tests.sh
> > @@ -85,23 +85,7 @@ PBR_TABLE=101
> >  
> >  log_test()
> >  {
> > -	local rc=$1
> > -	local expected=$2
> > -	local msg="$3"
> > -
> > -	if [ ${rc} -eq ${expected} ]; then
> > -		nsuccess=$((nsuccess+1))
> > -		printf "    TEST: %-50s  [ OK ]\n" "${msg}"
> > -	else
> > -		nfail=$((nfail+1))
> > -		printf "    TEST: %-50s  [FAIL]\n" "${msg}"
> > -		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
> > -			echo
> > -			echo "hit enter to continue, 'q' to quit"
> > -			read a
> > -			[ "$a" = "q" ] && exit 1
> > -		fi
> > -	fi
> > +	log_test_expected "$1" "$2" "$3"
> >  }
> 
> [Severity: Medium]
> This isn't a bug introduced by this patch, but it is worth noting while the
> failure bookkeeping is being centralised: fib-onlink-tests.sh, l2tp.sh and
> fdb_flush.sh never consume the status the helper records.
> 
> fib-onlink-tests.sh ends with:
> 
> 	if [ "$TESTS" != "none" ]; then
> 		printf "\nTests passed: %3d\n" ${nsuccess}
> 		printf "Tests failed: %3d\n"   ${nfail}
> 	fi
> 
> l2tp.sh ends with the same two printf lines, and fdb_flush.sh ends with:
> 
> 	for t in $TESTS
> 	do
> 		setup; $t; cleanup;
> 	done
> 
> None of the three does exit $ret or derives an exit code from nfail, so can
> these scripts print [FAIL] subtests and still report success to the kselftest
> runner?  Since this patch is what makes ret the shared failure accumulator,
> would adding exit $ret to those scripts fit here?
> 
> > diff --git a/tools/testing/selftests/net/fib_nexthops.sh b/tools/testing/selftests/net/fib_nexthops.sh
> > index 431d7bed76224..d0d979e8e438b 100755
> > --- a/tools/testing/selftests/net/fib_nexthops.sh
> > +++ b/tools/testing/selftests/net/fib_nexthops.sh
> > @@ -70,44 +70,7 @@ nsid=100
> >  
> >  log_test()
> >  {
> > -	local rc=$1
> > -	local expected=$2
> > -	local msg="$3"
> > -
> > -	if [ ${rc} -eq ${expected} ]; then
> > -		printf "TEST: %-60s  [ OK ]\n" "${msg}"
> > -		nsuccess=$((nsuccess+1))
> > -	else
> > -		if [[ $rc -eq $ksft_skip ]]; then
> > -			[[ $ret -eq 0 ]] && ret=$ksft_skip
> > -			nskip=$((nskip+1))
> > -			printf "TEST: %-60s  [SKIP]\n" "${msg}"
> > -		else
> > -			ret=1
> > -			nfail=$((nfail+1))
> > -			printf "TEST: %-60s  [FAIL]\n" "${msg}"
> > -		fi
> 
> [Severity: High]
> The changelog states:
> 
>   "this test checked for ksft_skip (exit code 4) inside log_test(),
>    but no test in the file produces that code."
> 
> Are these two call sites not producing exactly that code?
> 
> fib_nexthops.sh:ipv6_grp_fcnal() {
> 	...
> 	run_cmd "$IP nexthop add id 103 group 62,1000"
> 	if [[ $? == 0 ]]; then
> 		local GRP="id 103 group 62,254/63,255/64,256/65,257/66,65535"
> 		run_cmd "$IP nexthop replace $GRP"
> 		check_nexthop "id 103" "$GRP"
> 		rc=$?
> 	else
> 		rc=$ksft_skip
> 	fi
> 
> 	$IP nexthop flush >/dev/null 2>&1
> 
> 	log_test $rc 0 "16-bit weights"
> }
> 
> ipv6_res_grp_fcnal() has the same shape with "group 62,1000 type resilient
> buckets 32".
> 
> The else branch is taken whenever iproute2 is too old to accept 16-bit
> nexthop group weights.  After this change, log_test_expected() gets rc=4
> against expected 0, so both subtests take the failure branch: [FAIL] is
> printed, nfail is incremented and ret is merged with $ksft_fail.
> 
> Combined with exit $ret at the end of the script, does a run on such a host
> change from two [SKIP] lines and exit status 4 to two [FAIL] lines and exit
> status 1?
> 
> The changelog suggests "callers that need to report a skip should use
> log_test_skip() instead", but neither of these two callers was converted.
> 
> > -
> > -		if [ "$VERBOSE" = "1" ]; then
> > -			echo "    rc=$rc, expected $expected"
> > -		fi
> > -
> > -		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
> > -		echo
> > -			echo "hit enter to continue, 'q' to quit"
> > -			read a
> > -			[ "$a" = "q" ] && exit 1
> > -		fi
> > -	fi
> > -
> > -	if [ "${PAUSE}" = "yes" ]; then
> > -		echo
> > -		echo "hit enter to continue, 'q' to quit"
> > -		read a
> > -		[ "$a" = "q" ] && exit 1
> > -	fi
> > -
> > -	[ "$VERBOSE" = "1" ] && echo
> > +	log_test_expected "$1" "$2" "$3"
> >  }
> >  
> >  run_cmd()
> > @@ -1258,7 +1221,7 @@ ipv6_fcnal_runtime()
> >  		run_cmd "ip netns exec $me ping -c1 -w$PING_TIMEOUT 2001:db8:101::1"
> >  		log_test $? 0 "Ping - group blackhole replaced with gateways"
> >  	else
> > -		log_test 2 0 "Ping - multipath failed"
> > +		log_test $rc 0 "Ping - multipath failed"
> >  	fi
> 
> [Severity: Low]
> The changelog says:
> 
>   "Two callers compared against exit code 2 are left as-is since code 2
>    correctly counts as a failure in the previous log_test()."
> 
> These are the two callers in question, in ipv6_fcnal_runtime() and
> ipv4_fcnal_runtime(), and they are changed rather than left as-is.  Could the
> changelog be updated to describe the substitution?
> 
> The change itself looks harmless: run_cmd() assigns rc=$? without declaring
> it local, so under bash dynamic scoping it writes into the caller's local rc,
> and the else branch is only reached when that value is non-zero, so the
> subtest still reports [FAIL] with the same message.
> 
> >  
> >  	#
> > @@ -1915,7 +1878,7 @@ ipv4_fcnal_runtime()
> >  		run_cmd "ip netns exec $me ping -c1 -w$PING_TIMEOUT 172.16.101.1"
> >  		log_test $? 0 "Ping - group blackhole replaced with gateways"
> >  	else
> > -		log_test 2 0 "Ping - multipath failed"
> > +		log_test $rc 0 "Ping - multipath failed"
> >  	fi
> >  
> >  	#
> > @@ -2729,7 +2692,6 @@ done
> >  if [ "$TESTS" != "none" ]; then
> >  	printf "\nTests passed: %3d\n" ${nsuccess}
> >  	printf "Tests failed: %3d\n"   ${nfail}
> > -	printf "Tests skipped: %2d\n"  ${nskip}
> >  fi
> >  
> >  exit $ret
> 
> [Severity: High]
> With the skip counter line removed and no skip branch left in the shared
> helper, is there any remaining way for this script to report a skipped
> subtest?  The two 16-bit weight probes above are the only producers of
> ksft_skip and they are now counted in nfail.
> 
> > diff --git a/tools/testing/selftests/net/fib_tests.sh b/tools/testing/selftests/net/fib_tests.sh
> > index b338bfb196a27..7df967a2d6697 100755
> > --- a/tools/testing/selftests/net/fib_tests.sh
> > +++ b/tools/testing/selftests/net/fib_tests.sh
> > @@ -24,31 +24,7 @@ which ping6 > /dev/null 2>&1 && ping6=$(which ping6) || ping6=$(which ping)
> >  
> >  log_test()
> >  {
> > -	local rc=$1
> > -	local expected=$2
> > -	local msg="$3"
> > -
> > -	if [ ${rc} -eq ${expected} ]; then
> > -		printf "    TEST: %-60s  [ OK ]\n" "${msg}"
> > -		nsuccess=$((nsuccess+1))
> > -	else
> > -		ret=1
> > -		nfail=$((nfail+1))
> > -		printf "    TEST: %-60s  [FAIL]\n" "${msg}"
> 
> [Severity: Medium]
> This is a pre-existing issue, but it interacts with making ret a shared
> cumulative status in lib.sh: fib_tests.sh also uses the same global ret as
> per-test scratch storage.
> 
> fib_carrier_unicast_test() starts with an unconditional:
> 
> 	ret=0
> 
> and fib6_notify_test()/fib_notify_test() do:
> 
> 	if [ -z "$err" ];then
> 		ret=0
> 	else
> 		ret=1
> 	fi
> 
> 	log_test $ret 0 "ipv6 route add notify"
> 
> None of these declare ret local, and the script finishes with exit $ret.  If
> an earlier subtest failed and a later function assigns ret=0 and then passes,
> does the script exit 0 despite the printed [FAIL] lines?  Should these
> functions use a local variable now that the helper owns ret?
> 
> > -		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
> > -		echo
> > -			echo "hit enter to continue, 'q' to quit"
> > -			read a
> > -			[ "$a" = "q" ] && exit 1
> > -		fi
> > -	fi
> > -
> > -	if [ "${PAUSE}" = "yes" ]; then
> > -		echo
> > -		echo "hit enter to continue, 'q' to quit"
> > -		read a
> > -		[ "$a" = "q" ] && exit 1
> > -	fi
> > +	log_test_expected "$1" "$2" "$3"
> >  }
> >  
> >  setup()
> 
> [ ... ]
> 
> > diff --git a/tools/testing/selftests/net/lib.sh b/tools/testing/selftests/net/lib.sh
> > index d46d2cec89e45..f698fa3d2bd5f 100644
> > --- a/tools/testing/selftests/net/lib.sh
> > +++ b/tools/testing/selftests/net/lib.sh
> > @@ -454,6 +454,39 @@ log_test_xfail()
> >  	RET=$ksft_xfail retmsg= log_test "$@"
> >  }
> >  
> > +# Log test result with expected return value
> > +log_test_expected()
> > +{
> > +	local rc=$1
> > +	local expected=$2
> > +	local msg="$3"
> > +
> > +	if [ "${rc}" -eq "${expected}" ]; then
> > +		nsuccess=$((nsuccess+1))
> > +		printf "    TEST: %-60s  [ OK ]\n" "${msg}"
> > +	else
> 
> [Severity: Low]
> The changelog points callers at log_test_skip() as the replacement for the
> removed ksft_skip handling.  Can that helper work in the converted scripts?
> It dispatches through log_test:
> 
> lib.sh:log_test_skip() {
> 	RET=$ksft_skip retmsg= log_test "$@"
> }
> 
> and every script touched here defines its own log_test() after sourcing
> lib.sh:
> 
> fib_nexthops.sh:log_test() {
> 	log_test_expected "$1" "$2" "$3"
> }
> 
> so the local definition shadows lib.sh's log_test().  A call like
> log_test_skip "16-bit weights" would then reach log_test_expected with
> rc="16-bit weights", expected="" and msg="", i.e.:
> 
> 	[ "16-bit weights" -eq "" ]
> 
> which is an "integer expression expected" error landing in the failure
> branch and printing [FAIL] with an empty test name.  There are no callers
> today, so nothing breaks right now, but should the wrapper names be
> reworked so the suggested migration actually works?
> 
> > +		ret=$(ksft_exit_status_merge "$ret" "$ksft_fail")
> 
> [Severity: Low]
> The changelog gives this as the reason for the change:
> 
>   "On failure, ret is set via ksft_exit_status_merge() instead of ret=1,
>    so we can propagate ksft_xfail status correctly."
> 
> Is that achievable here?  ksft_exit_status_merge() orders the operands as:
> 
> lib.sh:ksft_exit_status_merge() {
> 	...
> 	__ksft_status_merge "$a" "$b" \
> 		$ksft_xfail $ksft_pass $ksft_skip $ksft_fail
> }
> 
> so xfail=0, pass=1, skip=2, fail=3, and the operand with the higher weight
> wins.  Since $ksft_fail is always passed as the second operand and carries
> the maximum weight, doesn't the result come out as $ksft_fail regardless of
> the previous value of ret, including $ksft_xfail?  That makes this line
> equivalent to the old ret=1.
> 
> > +		nfail=$((nfail+1))
> > +		printf "    TEST: %-60s  [FAIL]\n" "${msg}"
> > +		if [ "$VERBOSE" = "1" ]; then
> > +			echo "    rc=$rc, expected $expected"
> > +		fi
> > +
> > +		pause_on_fail
> > +	fi
> > +
> > +	if [ "${PAUSE}" = "yes" ]; then
> > +		echo
> > +		echo "hit enter to continue, 'q' to quit"
> > +		read -r a
> > +		[ "$a" = "q" ] && exit 1
> > +	fi
> > +
> > +	[ "$VERBOSE" = "1" ] && echo
> > +
> > +	return 0
> > +}
> > +
> >  log_info()
> >  {
> >  	local msg=$1
> 
> [ ... ]
> 
> -- 
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260915-self_log_test-v3-1-a17728176334%40kylinos.cn

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-09-17  5:55 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-15  3:14 [PATCH net-next v3] selftests: net: move log_test to lib file and remove duplicate code Hangbin Liu
2026-09-17  2:24 ` netdev-bot+sashiko
2026-09-17  5:54   ` Hangbin Liu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®