* [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®