netdev - Re: [PATCH net-next v4 2/2] selftests: fib_tests: Add a test case for IPv6 garbage collection

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f7ba3fc5327f88a0e9b20e177a0ce599b77833db.camel@redhat.com>
Date: Tue, 25 Jul 2023 12:27:52 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: kuifeng@...a.com, dsahern@...nel.org, davem@...emloft.net, 
 edumazet@...gle.com, kuba@...nel.org, netdev@...r.kernel.org,
 martin.lau@...ux.dev,  kernel-team@...a.com, yhs@...a.com
Cc: thinker.li@...il.com
Subject: Re: [PATCH net-next v4 2/2] selftests: fib_tests: Add a test case
 for IPv6 garbage collection

On Fri, 2023-07-21 at 17:38 -0700, kuifeng@...a.com wrote:
> From: Kui-Feng Lee <kuifeng@...a.com>
> 
> Add 10 IPv6 routes with expiration time.  Wait for a few seconds
> to make sure they are removed correctly.
> 
> Signed-off-by: Kui-Feng Lee <kuifeng@...a.com>
> ---
>  tools/testing/selftests/net/fib_tests.sh | 90 +++++++++++++++++++++++-
>  1 file changed, 87 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/fib_tests.sh b/tools/testing/selftests/net/fib_tests.sh
> index 35d89dfa6f11..4c92fb3c3844 100755
> --- a/tools/testing/selftests/net/fib_tests.sh
> +++ b/tools/testing/selftests/net/fib_tests.sh
> @@ -9,13 +9,16 @@ ret=0
>  ksft_skip=4
>  
>  # all tests in this script. Can be overridden with -t option
> -TESTS="unregister down carrier nexthop suppress ipv6_notify ipv4_notify ipv6_rt ipv4_rt ipv6_addr_metric ipv4_addr_metric ipv6_route_metrics ipv4_route_metrics ipv4_route_v6_gw rp_filter ipv4_del_addr ipv4_mangle ipv6_mangle ipv4_bcast_neigh"
> +TESTS="unregister down carrier nexthop suppress ipv6_notify ipv4_notify \
> +       ipv6_rt ipv4_rt ipv6_addr_metric ipv4_addr_metric ipv6_route_metrics \
> +       ipv4_route_metrics ipv4_route_v6_gw rp_filter ipv4_del_addr \
> +       ipv4_mangle ipv6_mangle ipv4_bcast_neigh fib6_gc_test"
>  
>  VERBOSE=0
>  PAUSE_ON_FAIL=no
>  PAUSE=no
> -IP="ip -netns ns1"
> -NS_EXEC="ip netns exec ns1"
> +IP="$(which ip) -netns ns1"
> +NS_EXEC="$(which ip) netns exec ns1"
>  
>  which ping6 > /dev/null 2>&1 && ping6=$(which ping6) || ping6=$(which ping)
>  
> @@ -747,6 +750,86 @@ fib_notify_test()
>  	cleanup &> /dev/null
>  }
>  
> +fib6_gc_test()
> +{
> +	setup
> +
> +	echo
> +	echo "Fib6 garbage collection test"
> +	set -e
> +
> +	# Check expiration of routes every 3 seconds (GC)
> +	$NS_EXEC sysctl -wq net.ipv6.route.gc_interval=300
> +
> +	$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
> +
> +	$NS_EXEC sysctl -wq net.ipv6.route.flush=1
> +
> +	# Temporary routes
> +	for i in $(seq 1 1000); do
> +	    # Expire route after 4 seconds
> +	    $IP -6 route add 2001:20::$i \
> +		via 2001:10::2 dev dummy_10 expires 4
> +	done
> +	N_EXP=$($IP -6 route list |grep expires|wc -l)
> +	if [ $N_EXP -ne 1000 ]; then
> +		echo "FAIL: expected 1000 routes with expires, got $N_EXP"
> +		ret=1
> +	else
> +	    sleep 5
> +	    REALTM_P=$($NS_EXEC strace -T sysctl \
> +		       -wq net.ipv6.route.flush=1 2>&1 | \
> +		       awk -- '/write\(.*"1\\n", 2\)/ { gsub("(.*<|>.*)", ""); print $0;}')

I guess the above works somehow ?!?

But I think something alike:

	# just after printing the banner
	TIME=$(which time)
	if [ -z "$TIME" ]; then
		echo "command 'time' is missing, skipping test"
		return
	fi
	# ...

		# replacing the strace command
		REALTM_P=$(time -f %e $NS_EXEC sysctl \
		       -wq net.ipv6.route.flush=1 2>&1)

would be better.

In any case you should check explicitly for the additionally needed
command ('strace' in your code, 'time' here).

And you could include the expected output in the commit message (just a
line, right?)

Cheers

Paolo


> +	    N_EXP_s5=$($IP -6 route list |grep expires|wc -l)
> +
> +	    if [ $N_EXP_s5 -ne 0 ]; then
> +		echo "FAIL: expected 0 routes with expires, got $N_EXP_s5"
> +		ret=1
> +	    else
> +		ret=0
> +	    fi
> +	fi
> +
> +	# Permanent routes
> +	for i in $(seq 1 5000); do
> +	    $IP -6 route add 2001:30::$i \
> +		via 2001:10::2 dev dummy_10
> +	done
> +	# Temporary routes
> +	for i in $(seq 1 1000); do
> +	    # Expire route after 4 seconds
> +	    $IP -6 route add 2001:20::$i \
> +		via 2001:10::2 dev dummy_10 expires 4
> +	done
> +	N_EXP=$($IP -6 route list |grep expires|wc -l)
> +	if [ $N_EXP -ne 1000 ]; then
> +	    echo
> +	    "FAIL: expected 1000 routes with expires, got $N_EXP (5000 permanent routes)"
> +		ret=1
> +	else
> +	    sleep 5
> +	    REALTM_T=$($NS_EXEC strace -T sysctl \
> +		       -wq net.ipv6.route.flush=1 2>&1 | \
> +		       awk -- '/write\(.*"1\\n", 2\)/ { gsub("(.*<|>.*)", ""); print $0;}')
> +	    N_EXP_s5=$($IP -6 route list |grep expires|wc -l)
> +
> +	    if [ $N_EXP_s5 -ne 0 ]; then
> +		echo "FAIL: expected 0 routes with expires, got $N_EXP_s5 (5000 permanent routes)"
> +		ret=1
> +	    else
> +		ret=0
> +	    fi
> +	fi
> +
> +	set +e
> +
> +	log_test $ret 0 "ipv6 route garbage collection (${REALTM_P}s, ${REALTM_T}s)"
> +
> +	cleanup &> /dev/null
> +}
> +
>  fib_suppress_test()
>  {
>  	echo
> @@ -2217,6 +2300,7 @@ do
>  	ipv4_mangle)			ipv4_mangle_test;;
>  	ipv6_mangle)			ipv6_mangle_test;;
>  	ipv4_bcast_neigh)		ipv4_bcast_neigh_test;;
> +	fib6_gc_test|ipv6_gc)		fib6_gc_test;;
>  
>  	help) echo "Test names: $TESTS"; exit 0;;
>  	esac