diff mbox series

[net-next,v2,2/2] selftests: fib_tests: Add tests for toggling between w/ and w/o expires.

Message ID 20231208194523.312416-3-thinker.li@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series Fix dangling pointer at f6i->gc_link. | 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/cc_maintainers warning 2 maintainers not CCed: linux-kselftest@vger.kernel.org shuah@kernel.org
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 success total: 0 errors, 0 warnings, 0 checks, 77 lines checked
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

Commit Message

Kui-Feng Lee Dec. 8, 2023, 7:45 p.m. UTC 
From: Kui-Feng Lee <thinker.li@gmail.com>

Make sure that toggling routes between w/ expires and w/o expires works
properly with GC list.

When a route with expires is replaced by a permanent route, the entry
should be removed from the gc list. When a permanent routes is replaced by
a temporary route, the new entry should be added to the gc list. The new
tests check if these basic operators work properly.

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

Comments

Hangbin Liu Dec. 12, 2023, 2:20 a.m. UTC | #1 
On Fri, Dec 08, 2023 at 11:45:23AM -0800, thinker.li@gmail.com wrote:
> From: Kui-Feng Lee <thinker.li@gmail.com>
> 
> Make sure that toggling routes between w/ expires and w/o expires works
> properly with GC list.
> 
> When a route with expires is replaced by a permanent route, the entry
> should be removed from the gc list. When a permanent routes is replaced by
> a temporary route, the new entry should be added to the gc list. The new
> tests check if these basic operators work properly.
> 
> Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
> ---
>  tools/testing/selftests/net/fib_tests.sh | 69 +++++++++++++++++++++++-
>  1 file changed, 67 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/fib_tests.sh b/tools/testing/selftests/net/fib_tests.sh
> index 66d0db7a2614..a8b4628fd7d2 100755
> --- a/tools/testing/selftests/net/fib_tests.sh
> +++ b/tools/testing/selftests/net/fib_tests.sh
> @@ -806,10 +806,75 @@ fib6_gc_test()
>  	    ret=0
>  	fi
>  
> -	set +e
> -
>  	log_test $ret 0 "ipv6 route garbage collection"
>  
> +	# Delete permanent routes
> +	for i in $(seq 1 5000); do
> +	    $IP -6 route del 2001:30::$i \
> +		via 2001:10::2 dev dummy_10
> +	done
> +
> +	# Permanent 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
> +	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_SLEEP=$($IP -6 route list |grep expires|wc -l)
> +	if [ $N_EXP_SLEEP -ne 100 ]; then
> +	    echo "FAIL: expected 100 routes with expires, got $N_EXP_SLEEP"

Hi,

Here the test failed, but ret is not updated.

> +	fi
> +	sleep $(($EXPIRE * 2 + 1))
> +	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"
> +	    ret=1

Here the ret is updated.

> +	else
> +	    ret=0
> +	fi
> +
> +	log_test $ret 0 "ipv6 route garbage collection (replace with expires)"
> +
> +	PERM_BASE=$($IP -6 route list |grep -v expires|wc -l)
> +	# 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
> +	    # Expire route after $EXPIRE seconds
> +	    $IP -6 route replace 2001:20::$i \
> +		via 2001:10::2 dev dummy_10
> +	done
> +	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"

Same here.

Thanks
Hangbin
> +	fi
> +	sleep $(($EXPIRE * 2 + 1))
> +	N_EXP_PERM=$($IP -6 route list |grep -v expires|wc -l)
> +	N_EXP_PERM=$(($N_EXP_PERM - $PERM_BASE))
> +	if [ $N_EXP_PERM -ne 100 ]; then
> +	    echo "FAIL: expected 100 permanent routes," \
> +		 "got $N_EXP_PERM"
> +	    ret=1
> +	else
> +	    ret=0
> +	fi
> +
> +	log_test $ret 0 "ipv6 route garbage collection (replace with permanent)"
> +
> +	set +e
> +
>  	cleanup &> /dev/null
>  }
>  
> -- 
> 2.34.1
>
Kui-Feng Lee Dec. 12, 2023, 2:40 a.m. UTC | #2 
On 12/11/23 18:20, Hangbin Liu wrote:
> On Fri, Dec 08, 2023 at 11:45:23AM -0800, thinker.li@gmail.com wrote:
>> From: Kui-Feng Lee <thinker.li@gmail.com>
>>
>> Make sure that toggling routes between w/ expires and w/o expires works
>> properly with GC list.
>>
>> When a route with expires is replaced by a permanent route, the entry
>> should be removed from the gc list. When a permanent routes is replaced by
>> a temporary route, the new entry should be added to the gc list. The new
>> tests check if these basic operators work properly.
>>
>> Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
>> ---
>>   tools/testing/selftests/net/fib_tests.sh | 69 +++++++++++++++++++++++-
>>   1 file changed, 67 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/testing/selftests/net/fib_tests.sh b/tools/testing/selftests/net/fib_tests.sh
>> index 66d0db7a2614..a8b4628fd7d2 100755
>> --- a/tools/testing/selftests/net/fib_tests.sh
>> +++ b/tools/testing/selftests/net/fib_tests.sh
>> @@ -806,10 +806,75 @@ fib6_gc_test()
>>   	    ret=0
>>   	fi
>>   
>> -	set +e
>> -
>>   	log_test $ret 0 "ipv6 route garbage collection"
>>   
>> +	# Delete permanent routes
>> +	for i in $(seq 1 5000); do
>> +	    $IP -6 route del 2001:30::$i \
>> +		via 2001:10::2 dev dummy_10
>> +	done
>> +
>> +	# Permanent 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
>> +	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_SLEEP=$($IP -6 route list |grep expires|wc -l)
>> +	if [ $N_EXP_SLEEP -ne 100 ]; then
>> +	    echo "FAIL: expected 100 routes with expires, got $N_EXP_SLEEP"
> 
> Hi,
> 
> Here the test failed, but ret is not updated.
> 
>> +	fi
>> +	sleep $(($EXPIRE * 2 + 1))
>> +	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"
>> +	    ret=1
> 
> Here the ret is updated.
> 
>> +	else
>> +	    ret=0
>> +	fi
>> +
>> +	log_test $ret 0 "ipv6 route garbage collection (replace with expires)"
>> +
>> +	PERM_BASE=$($IP -6 route list |grep -v expires|wc -l)
>> +	# 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
>> +	    # Expire route after $EXPIRE seconds
>> +	    $IP -6 route replace 2001:20::$i \
>> +		via 2001:10::2 dev dummy_10
>> +	done
>> +	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"
> 
> Same here.
> 
> Thanks
> Hangbin
>> +	fi
>> +	sleep $(($EXPIRE * 2 + 1))
>> +	N_EXP_PERM=$($IP -6 route list |grep -v expires|wc -l)
>> +	N_EXP_PERM=$(($N_EXP_PERM - $PERM_BASE))
>> +	if [ $N_EXP_PERM -ne 100 ]; then
>> +	    echo "FAIL: expected 100 permanent routes," \
>> +		 "got $N_EXP_PERM"
>> +	    ret=1
>> +	else
>> +	    ret=0
>> +	fi
>> +
>> +	log_test $ret 0 "ipv6 route garbage collection (replace with permanent)"
>> +
>> +	set +e
>> +
>>   	cleanup &> /dev/null
>>   }
>>   
>> -- 
>> 2.34.1
>>
Got it! Thanks!
Hangbin Liu Dec. 12, 2023, 5:35 a.m. UTC | #3 
On Mon, Dec 11, 2023 at 06:40:43PM -0800, Kui-Feng Lee wrote:
> > > +	N_EXP_SLEEP=$($IP -6 route list |grep expires|wc -l)
> > > +	if [ $N_EXP_SLEEP -ne 100 ]; then
> > > +	    echo "FAIL: expected 100 routes with expires, got $N_EXP_SLEEP"
> > 
> > Hi,
> > 
> > Here the test failed, but ret is not updated.
> > 
> > > +	fi
> > > +	sleep $(($EXPIRE * 2 + 1))
> > > +	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"
> > > +	    ret=1
> > 
> > Here the ret is updated.

BTW, the current fib6_gc_test() use $ret to store the result. But the latter
check would cover the previous one. e.g.

if [ $N_EXP_SLEEP -ne 0 ]; then
	ret=1
else
	ret=0
fi

do_some_other_tests
if [ $N_EXP_SLEEP -ne 0 ]; then
	ret=1
else
	ret=0
fi

If the previous one failed, but later one pass, the ret would be re-write to 0.
So I think we can use log_test for each checking.

Thanks
Hangbin
Kui-Feng Lee Dec. 12, 2023, 4:24 p.m. UTC | #4 
On 12/11/23 21:35, Hangbin Liu wrote:
> On Mon, Dec 11, 2023 at 06:40:43PM -0800, Kui-Feng Lee wrote:
>>>> +	N_EXP_SLEEP=$($IP -6 route list |grep expires|wc -l)
>>>> +	if [ $N_EXP_SLEEP -ne 100 ]; then
>>>> +	    echo "FAIL: expected 100 routes with expires, got $N_EXP_SLEEP"
>>>
>>> Hi,
>>>
>>> Here the test failed, but ret is not updated.
>>>
>>>> +	fi
>>>> +	sleep $(($EXPIRE * 2 + 1))
>>>> +	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"
>>>> +	    ret=1
>>>
>>> Here the ret is updated.
> 
> BTW, the current fib6_gc_test() use $ret to store the result. But the latter
> check would cover the previous one. e.g.
> 
> if [ $N_EXP_SLEEP -ne 0 ]; then
> 	ret=1
> else
> 	ret=0
> fi
> 
> do_some_other_tests
> if [ $N_EXP_SLEEP -ne 0 ]; then
> 	ret=1
> else
> 	ret=0
> fi
> 
> If the previous one failed, but later one pass, the ret would be re-write to 0.
> So I think we can use log_test for each checking.
> 
> Thanks
> Hangbin

Some of these tests has dependencies.  So, what I did is to perform the
following commands only if ret = 0.

if [ $N_EXP_SLEEP -ne 0 ]; then
     ret=1
else
     ret=0
fi

if [ $ret -eq 0 ]; then
   .....
   if [ ...]; then
       ret=1
   else
       ret=0
   fi
fi

if [ $ret -eq 0]; then
....
fi
diff mbox series

Patch

diff --git a/tools/testing/selftests/net/fib_tests.sh b/tools/testing/selftests/net/fib_tests.sh
index 66d0db7a2614..a8b4628fd7d2 100755
--- a/tools/testing/selftests/net/fib_tests.sh
+++ b/tools/testing/selftests/net/fib_tests.sh
@@ -806,10 +806,75 @@  fib6_gc_test()
 	    ret=0
 	fi
 
-	set +e
-
 	log_test $ret 0 "ipv6 route garbage collection"
 
+	# Delete permanent routes
+	for i in $(seq 1 5000); do
+	    $IP -6 route del 2001:30::$i \
+		via 2001:10::2 dev dummy_10
+	done
+
+	# Permanent 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
+	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_SLEEP=$($IP -6 route list |grep expires|wc -l)
+	if [ $N_EXP_SLEEP -ne 100 ]; then
+	    echo "FAIL: expected 100 routes with expires, got $N_EXP_SLEEP"
+	fi
+	sleep $(($EXPIRE * 2 + 1))
+	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"
+	    ret=1
+	else
+	    ret=0
+	fi
+
+	log_test $ret 0 "ipv6 route garbage collection (replace with expires)"
+
+	PERM_BASE=$($IP -6 route list |grep -v expires|wc -l)
+	# 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
+	    # Expire route after $EXPIRE seconds
+	    $IP -6 route replace 2001:20::$i \
+		via 2001:10::2 dev dummy_10
+	done
+	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"
+	fi
+	sleep $(($EXPIRE * 2 + 1))
+	N_EXP_PERM=$($IP -6 route list |grep -v expires|wc -l)
+	N_EXP_PERM=$(($N_EXP_PERM - $PERM_BASE))
+	if [ $N_EXP_PERM -ne 100 ]; then
+	    echo "FAIL: expected 100 permanent routes," \
+		 "got $N_EXP_PERM"
+	    ret=1
+	else
+	    ret=0
+	fi
+
+	log_test $ret 0 "ipv6 route garbage collection (replace with permanent)"
+
+	set +e
+
 	cleanup &> /dev/null
 }