diff mbox series

[net-next,5/5] selftests/net: Adding test cases of replacing routes and route advertisements.

Message ID 20240131064041.3445212-6-thinker.li@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series Remove expired routes with a separated list of routes. | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 8 this patch: 8
netdev/build_tools success Errors and warnings before: 1 this patch: 0
netdev/cc_maintainers success CCed 0 of 0 maintainers
netdev/build_clang success Errors and warnings before: 8 this patch: 8
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success net selftest script(s) already in Makefile
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 8 this patch: 8
netdev/checkpatch warning WARNING: line length of 82 exceeds 80 columns WARNING: line length of 90 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest fail net-next-2024-01-31--12-00 (tests: 718)

Commit Message

Kui-Feng Lee Jan. 31, 2024, 6:40 a.m. UTC 
From: Kui-Feng Lee <thinker.li@gmail.com>

Add tests of changing permanent routes to temporary routes and the reversed
case to make sure GC working correctly in these cases.  Add tests for the
temporary routes from RA.

The existing device will be deleted between tests to remove all routes
associated with it, so that the earlier tests don't mess up the later ones.

Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
---
 tools/testing/selftests/net/fib_tests.sh | 159 +++++++++++++++++++++--
 1 file changed, 148 insertions(+), 11 deletions(-)

Comments

Jakub Kicinski Jan. 31, 2024, 7:20 p.m. UTC | #1 
On Tue, 30 Jan 2024 22:40:41 -0800 thinker.li@gmail.com wrote:
> +	N_PERM=$($IP -6 route list |grep -v expires|grep 2001:20::|wc -l)
> +	if [ $N_PERM -ne 100 ]; then
> +	    echo "FAIL: expected 100 permanent routes, got $N_PERM"
> +	    ret=1
> +	else
> +	    ret=0
> +	fi

This fails on our slow VM:

#     TEST: expected 100 routes with expires, got 98                      [FAIL]

on a VM with kernel debug enable the test times out (it doesn't print
anything for 10min):

https://netdev-3.bots.linux.dev/vmksft-net-dbg/results/445222/6-fib-tests-sh/stdout
Kui-Feng Lee Jan. 31, 2024, 9:14 p.m. UTC | #2 
On 1/31/24 11:20, Jakub Kicinski wrote:
> On Tue, 30 Jan 2024 22:40:41 -0800 thinker.li@gmail.com wrote:
>> +	N_PERM=$($IP -6 route list |grep -v expires|grep 2001:20::|wc -l)
>> +	if [ $N_PERM -ne 100 ]; then
>> +	    echo "FAIL: expected 100 permanent routes, got $N_PERM"
>> +	    ret=1
>> +	else
>> +	    ret=0
>> +	fi
> 
> This fails on our slow VM:
> 
> #     TEST: expected 100 routes with expires, got 98                      [FAIL]
> 
> on a VM with kernel debug enable the test times out (it doesn't print
> anything for 10min):
> 
> https://netdev-3.bots.linux.dev/vmksft-net-dbg/results/445222/6-fib-tests-sh/stdout

I will reduce the number of routes to 100 instead of 5000 to see if it
helps.
Hangbin Liu Feb. 1, 2024, 8:46 a.m. UTC | #3 
Hi,

On Tue, Jan 30, 2024 at 10:40:41PM -0800, thinker.li@gmail.com wrote:
> +# Create a new dummy_10 to remove all associated routes.
> +reset_dummy_10()
> +{
> +	$IP link del dev dummy_10
> +
> +	$IP link add dummy_10 type dummy
> +	$IP link set dev dummy_10 up
> +	$IP -6 address add 2001:10::1/64 dev dummy_10
> +}
> +
>  fib6_gc_test()
>  {
>  	setup
> @@ -768,15 +778,19 @@ fib6_gc_test()
>  	    $IP -6 route add 2001:20::$i \
>  		via 2001:10::2 dev dummy_10 expires $EXPIRE
>  	done
> -	sleep $(($EXPIRE * 2))
> -	N_EXP_SLEEP=$($IP -6 route list |grep expires|wc -l)
> -	if [ $N_EXP_SLEEP -ne 0 ]; then
> -	    echo "FAIL: expected 0 routes with expires, got $N_EXP_SLEEP"
> +	sleep $(($EXPIRE * 2 + 1))
> +	N_EXP=$($IP -6 route list |grep expires|wc -l)
> +	if [ $N_EXP -ne 0 ]; then
> +	    echo "FAIL: expected 0 routes with expires, got $N_EXP"
>  	    ret=1
>  	else
>  	    ret=0
>  	fi
>  
> +	log_test $ret 0 "ipv6 route garbage collection"
> +
> +	reset_dummy_10

Since you reset the dummy device and will not affect the later tests. Maybe
you can log the test directly, e.g.

	if [ "$($IP -6 route list |grep expires|wc -l)" -ne 0 ]; then
		log_test $ret 0 "ipv6 route garbage collection"
	fi

Or, if you want to keep ret and also report passed log, you can wrapper the
number checking like

check_exp_number()
{
	local exp=$1
	local n_exp=$($IP -6 route list |grep expires|wc -l)
	if [ "$n_exp" -ne "$exp" ]; then
		echo "FAIL: expected $exp routes with expires, got $n_exp"
		ret=1
	else
		ret=0
	fi
}

Then we can call it without repeating the if/else lines

	check_exp_number 0
	log_test $ret 0 "ipv6 route garbage collection"

Thanks
Hangbin
Kui-Feng Lee Feb. 1, 2024, 5:14 p.m. UTC | #4 
On 2/1/24 00:46, Hangbin Liu wrote:
> Hi,
> 
> On Tue, Jan 30, 2024 at 10:40:41PM -0800, thinker.li@gmail.com wrote:
>> +# Create a new dummy_10 to remove all associated routes.
>> +reset_dummy_10()
>> +{
>> +	$IP link del dev dummy_10
>> +
>> +	$IP link add dummy_10 type dummy
>> +	$IP link set dev dummy_10 up
>> +	$IP -6 address add 2001:10::1/64 dev dummy_10
>> +}
>> +
>>   fib6_gc_test()
>>   {
>>   	setup
>> @@ -768,15 +778,19 @@ fib6_gc_test()
>>   	    $IP -6 route add 2001:20::$i \
>>   		via 2001:10::2 dev dummy_10 expires $EXPIRE
>>   	done
>> -	sleep $(($EXPIRE * 2))
>> -	N_EXP_SLEEP=$($IP -6 route list |grep expires|wc -l)
>> -	if [ $N_EXP_SLEEP -ne 0 ]; then
>> -	    echo "FAIL: expected 0 routes with expires, got $N_EXP_SLEEP"
>> +	sleep $(($EXPIRE * 2 + 1))
>> +	N_EXP=$($IP -6 route list |grep expires|wc -l)
>> +	if [ $N_EXP -ne 0 ]; then
>> +	    echo "FAIL: expected 0 routes with expires, got $N_EXP"
>>   	    ret=1
>>   	else
>>   	    ret=0
>>   	fi
>>   
>> +	log_test $ret 0 "ipv6 route garbage collection"
>> +
>> +	reset_dummy_10
> 
> Since you reset the dummy device and will not affect the later tests. Maybe
> you can log the test directly, e.g.
> 
> 	if [ "$($IP -6 route list |grep expires|wc -l)" -ne 0 ]; then
> 		log_test $ret 0 "ipv6 route garbage collection"
> 	fi
> 
> Or, if you want to keep ret and also report passed log, you can wrapper the
> number checking like
> 
> check_exp_number()
> {
> 	local exp=$1
> 	local n_exp=$($IP -6 route list |grep expires|wc -l)
> 	if [ "$n_exp" -ne "$exp" ]; then
> 		echo "FAIL: expected $exp routes with expires, got $n_exp"
> 		ret=1
> 	else
> 		ret=0
> 	fi
> }
> 
> Then we can call it without repeating the if/else lines
> 
> 	check_exp_number 0
> 	log_test $ret 0 "ipv6 route garbage collection"

If I read it correctly, the point here is too many boilerplate checks,
and you prefer to reduce them. Right?
No problem! I will do it.


> 
> Thanks
> Hangbin
Hangbin Liu Feb. 2, 2024, 2:25 a.m. UTC | #5 
On Thu, Feb 01, 2024 at 09:14:17AM -0800, Kui-Feng Lee wrote:
> > > +	N_EXP=$($IP -6 route list |grep expires|wc -l)
> > > +	if [ $N_EXP -ne 0 ]; then
> > > +	    echo "FAIL: expected 0 routes with expires, got $N_EXP"
> > >   	    ret=1
> > >   	else
> > >   	    ret=0
> > >   	fi
> > > +	log_test $ret 0 "ipv6 route garbage collection"
> > > +
> > > +	reset_dummy_10
> > 
> > Since you reset the dummy device and will not affect the later tests. Maybe
> > you can log the test directly, e.g.
> > 
> > 	if [ "$($IP -6 route list |grep expires|wc -l)" -ne 0 ]; then
> > 		log_test $ret 0 "ipv6 route garbage collection"
> > 	fi
> > 
> > Or, if you want to keep ret and also report passed log, you can wrapper the
> > number checking like
> > 
> > check_exp_number()
> > {
> > 	local exp=$1
> > 	local n_exp=$($IP -6 route list |grep expires|wc -l)
> > 	if [ "$n_exp" -ne "$exp" ]; then
> > 		echo "FAIL: expected $exp routes with expires, got $n_exp"
> > 		ret=1
> > 	else
> > 		ret=0
> > 	fi
> > }
> > 
> > Then we can call it without repeating the if/else lines
> > 
> > 	check_exp_number 0
> > 	log_test $ret 0 "ipv6 route garbage collection"
> 
> If I read it correctly, the point here is too many boilerplate checks,
> and you prefer to reduce them. Right?
> No problem! I will do it.

Yes, thanks!

Hangbin
diff mbox series

Patch

diff --git a/tools/testing/selftests/net/fib_tests.sh b/tools/testing/selftests/net/fib_tests.sh
index b3ecccbbfcd2..f69b55304ebb 100755
--- a/tools/testing/selftests/net/fib_tests.sh
+++ b/tools/testing/selftests/net/fib_tests.sh
@@ -743,6 +743,16 @@  fib_notify_test()
 	cleanup &> /dev/null
 }
 
+# Create a new dummy_10 to remove all associated routes.
+reset_dummy_10()
+{
+	$IP link del dev dummy_10
+
+	$IP link add dummy_10 type dummy
+	$IP link set dev dummy_10 up
+	$IP -6 address add 2001:10::1/64 dev dummy_10
+}
+
 fib6_gc_test()
 {
 	setup
@@ -768,15 +778,19 @@  fib6_gc_test()
 	    $IP -6 route add 2001:20::$i \
 		via 2001:10::2 dev dummy_10 expires $EXPIRE
 	done
-	sleep $(($EXPIRE * 2))
-	N_EXP_SLEEP=$($IP -6 route list |grep expires|wc -l)
-	if [ $N_EXP_SLEEP -ne 0 ]; then
-	    echo "FAIL: expected 0 routes with expires, got $N_EXP_SLEEP"
+	sleep $(($EXPIRE * 2 + 1))
+	N_EXP=$($IP -6 route list |grep expires|wc -l)
+	if [ $N_EXP -ne 0 ]; then
+	    echo "FAIL: expected 0 routes with expires, got $N_EXP"
 	    ret=1
 	else
 	    ret=0
 	fi
 
+	log_test $ret 0 "ipv6 route garbage collection"
+
+	reset_dummy_10
+
 	# Permanent routes
 	for i in $(seq 1 5000); do
 	    $IP -6 route add 2001:30::$i \
@@ -788,19 +802,142 @@  fib6_gc_test()
 	    $IP -6 route add 2001:20::$i \
 		via 2001:10::2 dev dummy_10 expires $EXPIRE
 	done
-	sleep $(($EXPIRE * 2))
-	N_EXP_SLEEP=$($IP -6 route list |grep expires|wc -l)
-	if [ $N_EXP_SLEEP -ne 0 ]; then
-	    echo "FAIL: expected 0 routes with expires," \
-		 "got $N_EXP_SLEEP (5000 permanent routes)"
+	sleep $(($EXPIRE * 2 + 1))
+	N_EXP=$($IP -6 route list |grep expires|wc -l)
+	if [ $N_EXP -ne 0 ]; then
+	    echo "FAIL: expected 0 routes with expires, got $N_EXP"
 	    ret=1
 	else
 	    ret=0
 	fi
 
-	set +e
+	log_test $ret 0 "ipv6 route garbage collection (with permanent routes)"
 
-	log_test $ret 0 "ipv6 route garbage collection"
+	reset_dummy_10
+
+	# Permanent routes
+	for i in $(seq 1 100); do
+	    $IP -6 route add 2001:20::$i \
+		via 2001:10::2 dev dummy_10
+	done
+	# Replace with temporary routes
+	for i in $(seq 1 100); do
+	    # Expire route after $EXPIRE seconds
+	    $IP -6 route replace 2001:20::$i \
+		via 2001:10::2 dev dummy_10 expires $EXPIRE
+	done
+	N_EXP=$($IP -6 route list |grep expires|wc -l)
+	if [ $N_EXP -ne 100 ]; then
+	    log_test 1 0 "expected 100 routes with expires, got $N_EXP"
+	    set +e
+	    cleanup &> /dev/null
+	    return
+	fi
+	# Wait for GC
+	sleep $(($EXPIRE * 2 + 1))
+	N_EXP=$($IP -6 route list |grep expires|wc -l)
+	if [ $N_EXP -ne 0 ]; then
+	    echo "FAIL: expected 0 routes with expires, got $N_EXP"
+	    ret=1
+	else
+	    ret=0
+	fi
+
+	log_test $ret 0 "ipv6 route garbage collection (replace with expires)"
+
+	reset_dummy_10
+
+	# Temporary routes
+	for i in $(seq 1 100); do
+	    # Expire route after $EXPIRE seconds
+	    $IP -6 route add 2001:20::$i \
+		via 2001:10::2 dev dummy_10 expires $EXPIRE
+	done
+	# Replace with permanent routes
+	for i in $(seq 1 100); do
+	    $IP -6 route replace 2001:20::$i \
+		via 2001:10::2 dev dummy_10
+	done
+	N_EXP=$($IP -6 route list |grep expires|wc -l)
+	if [ $N_EXP -ne 0 ]; then
+	    log_test 1 0 "expected 0 routes with expires, got $N_EXP"
+	    set +e
+	    cleanup &> /dev/null
+	    return
+	fi
+
+	# Wait for GC
+	sleep $(($EXPIRE * 2 + 1))
+
+	N_PERM=$($IP -6 route list |grep -v expires|grep 2001:20::|wc -l)
+	if [ $N_PERM -ne 100 ]; then
+	    echo "FAIL: expected 100 permanent routes, got $N_PERM"
+	    ret=1
+	else
+	    ret=0
+	fi
+
+	log_test $ret 0 "ipv6 route garbage collection (replace with permanent)"
+
+	# ra6 is required for the next test. (ipv6toolkit)
+	if [ ! -x "$(command -v ra6)" ]; then
+	    echo "SKIP: ra6 not found."
+	    set +e
+	    cleanup &> /dev/null
+	    return
+	fi
+
+	# Delete dummy_10 and remove all routes
+	$IP link del dev dummy_10
+
+	# Create a pair of veth devices to send a RA message from one
+	# device to another.
+	$IP link add veth1 type veth peer name veth2
+	$IP link set dev veth1 up
+	$IP link set dev veth2 up
+	$IP -6 address add 2001:10::1/64 dev veth1 nodad
+	$IP -6 address add 2001:10::2/64 dev veth2 nodad
+
+	# Without stopping these two services, systemd may mess up the test
+	# by intercepting the RA message and adding routes.
+	if [ -x "$(command -v systemctl)" ]; then
+	    systemctl stop systemd-networkd.socket
+	    systemctl stop systemd-networkd.service
+	fi
+	# Make veth1 ready to receive RA messages.
+	$NS_EXEC sysctl -w net.ipv6.conf.veth1.accept_ra=2 &> /dev/null
+	$NS_EXEC sysctl -w net.ipv6.conf.veth1.accept_ra_rt_info_max_plen=127 &> /dev/null
+
+	# Send a RA message with a route from veth2 to veth1.
+	$NS_EXEC ra6 -i veth2 -d 2001:10::1 -R "2003:10::/64#1#$EXPIRE" -t $EXPIRE
+
+	# Wait for the RA message.
+	sleep 1
+
+	# There are 2 routes with expires. One is a default route and the
+	# other is the route to 2003:10::/64.
+	N_EXP=$($IP -6 route list |grep expires|wc -l)
+	if [ $N_EXP -ne 2 ]; then
+	    log_test 1 0 "expected 2 routes with expires, got $N_EXP"
+	    set +e
+	    cleanup &> /dev/null
+	    return
+	fi
+
+	# Wait for GC
+	sleep $(($EXPIRE * 2 + 1))
+
+	N_EXP=$($IP -6 route list |grep expires|wc -l)
+	if [ $N_EXP -ne 0 ]; then
+	    echo "FAIL: expected 0 routes with expires, got $N_EXP"
+	    ret=1
+	else
+	    ret=0
+	fi
+
+	log_test $ret 0 "ipv6 route garbage collection (RA message)"
+
+	set +e
 
 	cleanup &> /dev/null
 }