[PATCH net] selftests: fib_tests: use per-test return value in subtests

[PATCH net] selftests: fib_tests: use per-test return value in subtests

[Hangbin Liu]

From: Hangbin Liu <liuhangbin@kylinos.cn>

fib_tests mixes use of $ret for both per-test return value and global exit
code. If an earlier subtest fails and a later function sets ret=0, the
script will exit with status 0 even though [FAIL] lines were printed.

Use RET as the per-test return value, as already defined in lib.sh. The
exit code ret will only be set in log_test().

Fixes: 607bd2e502f5 ("selftests: fib_tests: Add test cases for IPv4/IPv6 FIB")
Reported-by: Sashiko <netdev-bot+sashiko@kernel.org>
Closes: https://lore.kernel.org/all/178961184152.22033.9267194009793243294@kernel.org
Signed-off-by: Hangbin Liu <liuhangbin@kylinos.cn>
---
 tools/testing/selftests/net/fib_tests.sh | 40 ++++++++++++++++----------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/tools/testing/selftests/net/fib_tests.sh b/tools/testing/selftests/net/fib_tests.sh
index b338bfb196a2..9ce5623b049c 100755
--- a/tools/testing/selftests/net/fib_tests.sh
+++ b/tools/testing/selftests/net/fib_tests.sh
@@ -369,7 +369,7 @@ fib_carrier_local_test()
 
 fib_carrier_unicast_test()
 {
-	ret=0
+	RET=0
 
 	echo
 	echo "Single path route carrier test"
@@ -685,12 +685,12 @@ fib6_notify_test()
 
 	err=`cat errors.txt |grep "Message too long"`
 	if [ -z "$err" ];then
-		ret=0
+		RET=0
 	else
-		ret=1
+		RET=1
 	fi
 
-	log_test $ret 0 "ipv6 route add notify"
+	log_test "$RET" 0 "ipv6 route add notify"
 
 	kill_process %%
 
@@ -732,12 +732,12 @@ fib_notify_test()
 
 	err=`cat errors.txt |grep "Message too long"`
 	if [ -z "$err" ];then
-		ret=0
+		RET=0
 	else
-		ret=1
+		RET=1
 	fi
 
-	log_test $ret 0 "ipv4 route add notify"
+	log_test "$RET" 0 "ipv4 route add notify"
 
 	kill_process %%
 
@@ -763,9 +763,9 @@ check_rt_num()
 
     if [ $num -ne $expected ]; then
 	echo "FAIL: Expected $expected routes, got $num"
-	ret=1
+	RET=1
     else
-	ret=0
+	RET=0
     fi
 }
 
@@ -812,7 +812,7 @@ fib6_gc_test()
 	sleep $GC_WAIT_TIME
 	$NS_EXEC sysctl -wq net.ipv6.route.flush=1
 	check_rt_num 0 $($IP -6 route list |grep expires|wc -l)
-	log_test $ret 0 "ipv6 route garbage collection"
+	log_test "$RET" 0 "ipv6 route garbage collection"
 
 	reset_dummy_10
 
@@ -830,7 +830,7 @@ fib6_gc_test()
 	# Wait for GC
 	sleep $GC_WAIT_TIME
 	check_rt_num 0 $($IP -6 route list |grep expires|wc -l)
-	log_test $ret 0 "ipv6 route garbage collection (with permanent routes)"
+	log_test "$RET" 0 "ipv6 route garbage collection (with permanent routes)"
 
 	reset_dummy_10
 
@@ -848,7 +848,7 @@ fib6_gc_test()
 	# Wait for GC
 	sleep $GC_WAIT_TIME
 	check_rt_num 0 $($IP -6 route list |grep expires|wc -l)
-	log_test $ret 0 "ipv6 route garbage collection (replace with expires)"
+	log_test "$RET" 0 "ipv6 route garbage collection (replace with expires)"
 
 	reset_dummy_10
 
@@ -868,7 +868,7 @@ fib6_gc_test()
 	# Wait for GC
 	sleep $GC_WAIT_TIME
 	check_rt_num 5 $($IP -6 route list |grep -v expires|grep 2001:20::|wc -l)
-	log_test $ret 0 "ipv6 route garbage collection (replace with permanent)"
+	log_test "$RET" 0 "ipv6 route garbage collection (replace with permanent)"
 
 	# Delete dummy_10 and remove all routes
 	$IP link del dev dummy_10
@@ -923,7 +923,7 @@ fib6_gc_test()
 	# rt6_nh_dump_exceptions() just skips expired exceptions.
 	$NS_EXEC sysctl -wq net.ipv6.route.flush=1
 	check_rt_num 0 $($IP -6 route list cache | grep 2001:10:: | wc -l)
-	log_test $ret 0 "ipv6 route garbage collection (promote to permanent routes)"
+	log_test "$RET" 0 "ipv6 route garbage collection (promote to permanent routes)"
 
 	$IP neigh del fe80:dead::3 lladdr 00:11:22:33:44:55 dev veth1 router
 	$IP link del veth1
@@ -960,7 +960,7 @@ fib6_gc_test()
 	# Wait for GC
 	sleep $GC_WAIT_TIME
 	check_rt_num 0 $($IP -6 route list |grep expires|wc -l)
-	log_test $ret 0 "ipv6 route garbage collection (RA message)"
+	log_test "$RET" 0 "ipv6 route garbage collection (RA message)"
 
 	set +e
 
@@ -1589,7 +1589,7 @@ fib6_ra_to_static()
 	# Expire is back, on-link route is now owned by RA again
 	check_rt_num 2 $($IP -6 route list |grep expires|wc -l)
 
-	log_test $ret 0 "ipv6 promote RA route to static"
+	log_test "$RET" 0 "ipv6 promote RA route to static"
 
 	# Prepare for RA route with gateway
 	$NS_EXEC sysctl -wq net.ipv6.conf.veth1.accept_ra_rt_info_max_plen=64
@@ -1606,7 +1606,7 @@ fib6_ra_to_static()
 
 	check_rt_num 2 "$($IP -6 route list | grep -c "nexthop via")"
 
-	log_test "$ret" 0 "ipv6 RA route with nexthop do not merge into ECMP with static"
+	log_test "$RET" 0 "ipv6 RA route with nexthop do not merge into ECMP with static"
 
 	set +e
 
@@ -1651,18 +1651,18 @@ fib6_temp_addr_renewal() {
 	# Restore it
 	$NS_EXEC ra6 -i veth2 -s fe80::1 -d ff02::1 -P 2001:12::/64\#LA\#3600\#3600 -e
 
-	ret=1
+	RET=1
 	for i in $(seq 1 25); do
 		sleep 1
 		num_dep="$($IP -6 addr | grep -c "temporary deprecated" || true)"
 		num_tot="$($IP -6 addr | grep -c "temporary" || true)"
 
 		if [ "$num_dep" -eq 1 ] && [ "$num_tot" -ge 2 ]; then
-			ret=0
+			RET=0
 			break
 		fi
 	done
-	log_test "$ret" 0 "IPv6 temporary address cleanly deprecated and regenerated"
+	log_test "$RET" 0 "IPv6 temporary address cleanly deprecated and regenerated"
 
 	set +e
 

---
base-commit: a7bfaba4823e3c165bb2004c74eff7c096672bc7
change-id: 20260928-self_fib_tests-0d05f96eafbb

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

Re: [PATCH net] selftests: fib_tests: use per-test return value in subtests

[Ido Schimmel]

On Tue, Sep 29, 2026 at 09:57:01AM +0800, Hangbin Liu wrote:
> From: Hangbin Liu <liuhangbin@kylinos.cn>
> 
> fib_tests mixes use of $ret for both per-test return value and global exit
> code. If an earlier subtest fails and a later function sets ret=0, the
> script will exit with status 0 even though [FAIL] lines were printed.
> 
> Use RET as the per-test return value, as already defined in lib.sh. The
> exit code ret will only be set in log_test().
> 
> Fixes: 607bd2e502f5 ("selftests: fib_tests: Add test cases for IPv4/IPv6 FIB")
> Reported-by: Sashiko <netdev-bot+sashiko@kernel.org>
> Closes: https://lore.kernel.org/all/178961184152.22033.9267194009793243294@kernel.org
> Signed-off-by: Hangbin Liu <liuhangbin@kylinos.cn>

Please target the patch at net-next and drop the Fixes tag. The patch
doesn't fix a regression, nothing is failing (or passing when it
shouldn't) because of it and it cannot be backported cleanly to old
kernels anyway. I targeted similar patches at net-next in the past. See
[1], for example.

[1] https://lore.kernel.org/all/20250908073238.119240-5-idosch@nvidia.com/

> ---
>  tools/testing/selftests/net/fib_tests.sh | 40 ++++++++++++++++----------------
>  1 file changed, 20 insertions(+), 20 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/fib_tests.sh b/tools/testing/selftests/net/fib_tests.sh
> index b338bfb196a2..9ce5623b049c 100755
> --- a/tools/testing/selftests/net/fib_tests.sh
> +++ b/tools/testing/selftests/net/fib_tests.sh
> @@ -369,7 +369,7 @@ fib_carrier_local_test()
>  
>  fib_carrier_unicast_test()
>  {
> -	ret=0
> +	RET=0

Looks like this line can be removed (similar to fib_carrier_local_test()
above it) given that RET is never used in this function

>  
>  	echo
>  	echo "Single path route carrier test"

Re: [PATCH net] selftests: fib_tests: use per-test return value in subtests

[Hangbin Liu]

On Tue, Sep 29, 2026 at 04:45:17PM +0300, Ido Schimmel wrote:
> On Tue, Sep 29, 2026 at 09:57:01AM +0800, Hangbin Liu wrote:
> > From: Hangbin Liu <liuhangbin@kylinos.cn>
> > 
> > fib_tests mixes use of $ret for both per-test return value and global exit
> > code. If an earlier subtest fails and a later function sets ret=0, the
> > script will exit with status 0 even though [FAIL] lines were printed.
> > 
> > Use RET as the per-test return value, as already defined in lib.sh. The
> > exit code ret will only be set in log_test().
> > 
> > Fixes: 607bd2e502f5 ("selftests: fib_tests: Add test cases for IPv4/IPv6 FIB")
> > Reported-by: Sashiko <netdev-bot+sashiko@kernel.org>
> > Closes: https://lore.kernel.org/all/178961184152.22033.9267194009793243294@kernel.org
> > Signed-off-by: Hangbin Liu <liuhangbin@kylinos.cn>
> 
> Please target the patch at net-next and drop the Fixes tag. The patch
> doesn't fix a regression, nothing is failing (or passing when it
> shouldn't) because of it and it cannot be backported cleanly to old
> kernels anyway. I targeted similar patches at net-next in the past. See
> [1], for example.
> 
> [1] https://lore.kernel.org/all/20250908073238.119240-5-idosch@nvidia.com/

OK, got it.

> 
> > ---
> >  tools/testing/selftests/net/fib_tests.sh | 40 ++++++++++++++++----------------
> >  1 file changed, 20 insertions(+), 20 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/net/fib_tests.sh b/tools/testing/selftests/net/fib_tests.sh
> > index b338bfb196a2..9ce5623b049c 100755
> > --- a/tools/testing/selftests/net/fib_tests.sh
> > +++ b/tools/testing/selftests/net/fib_tests.sh
> > @@ -369,7 +369,7 @@ fib_carrier_local_test()
> >  
> >  fib_carrier_unicast_test()
> >  {
> > -	ret=0
> > +	RET=0
> 
> Looks like this line can be removed (similar to fib_carrier_local_test()
> above it) given that RET is never used in this function

Right, I will remove it.

Thanks
Hangbin

Re: [PATCH net] selftests: fib_tests: use per-test return value in subtests

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] In tools/testing/selftests/net/fib_tests.sh, the patch
  converts check_rt_num() from `ret` to `RET`, but the helper still sets…

--

selftests: fib_tests: use per-test return value in subtests

This commit changes fib_tests.sh to use RET, as defined in lib.sh, for the
per-test return value. The global exit code ret is now only set in
log_test(), so a later subtest can no longer clear an earlier failure.

Closes: https://lore.kernel.org/all/178961184152.22033.9267194009793243294@kernel.org

> diff --git a/tools/testing/selftests/net/fib_tests.sh b/tools/testing/selftests/net/fib_tests.sh
> index b338bfb196a27..9ce5623b049c1 100755
> --- a/tools/testing/selftests/net/fib_tests.sh
> +++ b/tools/testing/selftests/net/fib_tests.sh

[ ... ]

> @@ -763,9 +763,9 @@ check_rt_num()
>  
>      if [ $num -ne $expected ]; then
>  	echo "FAIL: Expected $expected routes, got $num"
> -	ret=1
> +	RET=1
>      else
> -	ret=0
> +	RET=0
>      fi
>  }

[Severity: Medium]
check_rt_num() still sets RET=0 when a check passes. Can a later passing
check_rt_num() in the same subtest hide an earlier failure?

lib.sh describes RET as "Per-test return value. Clear at the beginning of
each test." Its helpers only ever raise it. For example,
ret_set_ksft_status() does:

	RET=$(ksft_status_merge $RET $ksft_status)

Several subtests call check_rt_num() more than once before a single
log_test. For example, fib6_ra_to_static() does:

	check_rt_num 1 $($IP -6 route list |grep expires|wc -l)
	...
	check_rt_num 1 $($IP -6 route list |grep expires|wc -l)
	...
	check_rt_num 2 $($IP -6 route list |grep expires|wc -l)

	log_test "$RET" 0 "ipv6 promote RA route to static"

Say the on-link route still has an expiry after the static address is
added. The first check prints "FAIL: Expected 1 routes, got 2". If the
last check then passes, it resets RET to 0. log_test() prints [ OK ],
nfail is not incremented, the global ret stays 0, and the script exits
with 0.

The same pattern shows up in two more places:

- In fib6_gc_test(), for "promote to permanent routes": three check_rt_num
  calls run before the final "check_rt_num 0" on route list cache.

- In fib6_ra_to_static(), for "RA route with nexthop do not merge into
  ECMP with static": the "2001:12::/64 via" check is followed by the
  "nexthop via" check.

This looks like the same symptom the commit message describes, only
inside a single subtest. Could check_rt_num() set RET=1 only on failure,
with RET=0 cleared at the start of each logged subtest?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260929-self_fib_tests-v1-1-0eab6a3c3a3c%40kylinos.cn