Re: [PATCH net-next] selftests: net: move log_test to lib file and remove duplicate code
From: Hangbin Liu <hidden>
Date: 2026-08-14 12:07:30
Also in:
linux-kselftest, lkml
On Thu, Aug 13, 2026 at 04:11:57PM +0800, Hangbin Liu wrote:
From: Hangbin Liu <redacted> When reviewing the test code, I saw many tests using the same or similar log_test functions. We can move them to lib.sh to save effort. However, due to historical reasons, we moved the log_test from the forwarding lib first, which has different usage. So, rename the log_test in the net folder to log_test_expected. Since renaming all the log_test functions in the test cases would change too many lines, I just use a wrapper in the old code. The fourth argument in icmp_redirect.sh is not needed, as the xfail issue has already been fixed and it should always pass. But I still keep the xfail logic in the log_test in lib.sh in case other tests need it. Signed-off-by: Hangbin Liu <redacted> ---
[...]
quoted hunk ↗ jump to hunk
diff --git a/tools/testing/selftests/net/fib_nexthops.sh b/tools/testing/selftests/net/fib_nexthops.sh index 3d347126730a..16fe92775ae0 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()
[...]
quoted hunk ↗ jump to hunk
diff --git a/tools/testing/selftests/net/icmp_redirect.sh b/tools/testing/selftests/net/icmp_redirect.sh index b13c89a99ecb..0107af73aef4 100755 --- a/tools/testing/selftests/net/icmp_redirect.sh +++ b/tools/testing/selftests/net/icmp_redirect.sh@@ -61,28 +61,7 @@ log_section() log_test() { - local rc=$1 - local expected=$2 - local msg="$3" - local xfail=$4 - - if [ ${rc} -eq ${expected} ]; then - printf "TEST: %-60s [ OK ]\n" "${msg}" - nsuccess=$((nsuccess+1)) - elif [ ${rc} -eq ${xfail} ]; then - printf "TEST: %-60s [XFAIL]\n" "${msg}" - nxfail=$((nxfail+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" }
[...]
quoted hunk ↗ jump to hunk
diff --git a/tools/testing/selftests/net/lib.sh b/tools/testing/selftests/net/lib.sh index d46d2cec89e4..e02a6a91ff91 100644 --- a/tools/testing/selftests/net/lib.sh +++ b/tools/testing/selftests/net/lib.sh@@ -454,6 +454,44 @@ 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}" + elif [ "${rc}" -eq "${ksft_skip}" ]; then + [[ "$ret" -eq 0 ]] && ret="$ksft_skip" + nskip=$((nskip+1)) + printf "TEST: %-60s [SKIP]\n" "${msg}" + elif [ "${rc}" -eq "${ksft_xfail}" ]; then + nxfail=$((nxfail+1)) + printf "TEST: %-60s [XFAIL]\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 +} +
Reply to sashiko's review.
"""
fib_nexthops.sh has two unconditional hard-failure calls:
tools/testing/selftests/net/fib_nexthops.sh:ipv6_fcnal_runtime() {
...
else
log_test 2 0 "Ping - multipath failed"
fi
...
}
"""
Correct, this need to be updated, otherwise the report will be xfail.
"""
The same concern applies to ordinary positive checks written as
"log_test $? 0 ...". iproute2 returns 2 for kernel-reported errors and ping
returns 2 for local errors such as "Network is unreachable", for example in
fcnal-test.sh:
run_cmd ping -c1 -w1 -I br0 ${NSB_IP}
log_test $? 0 "Bridge into VRF - IPv4 ping out"
and fcnal-test.sh decides its verdict from nfail alone:
if [ $nfail -ne 0 ]; then
exit 1 # KSFT_FAIL
Would it be safer to only take the SKIP/XFAIL arms when "expected" itself is
ksft_skip/ksft_xfail?
"""
If "expected" itself is ksft_skip/ksft_xfail, and we got the expected number,
then the result should be PASS.
"""
In the other direction, icmp_redirect.sh's checks are grep pipelines that
only produce 0 or 1, so after dropping the fourth argument its nxfail
counter and the "Tests xfailed" line can no longer be reached.
"""
We can remove the xfail count in icmp_redirect.sh testing
quoted hunk ↗ jump to hunk
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() {
[...]
- - [ "$VERBOSE" = "1" ] && echo - return 0 + log_test_expected "$1" "$2" "$3" }
""" Is dropping this "return 0" safe here? """ Hmm, yes, we'd better add return 0 in log_test as the read may return 1 if it's not "q". I will update the patch. Thanks Hangbin