[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