On Thu, Aug 13, 2026 at 04:11:57PM +0800, Hangbin Liu wrote:
> From: Hangbin Liu <[email protected]>
>
> 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 <[email protected]>
> ---
[...]
> 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()
[...]
> 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"
> }
[...]
>
> 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
> 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
