[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZXp3PYOBji7AArUG@Laptop-X1>
Date: Thu, 14 Dec 2023 11:32:13 +0800
From: Hangbin Liu <liuhangbin@...il.com>
To: thinker.li@...il.com
Cc: netdev@...r.kernel.org, martin.lau@...ux.dev, kernel-team@...a.com,
davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com,
dsahern@...nel.org, edumazet@...gle.com, sinquersw@...il.com,
kuifeng@...a.com
Subject: Re: [PATCH net-next v3 2/2] selftests: fib_tests: Add tests for
toggling between w/ and w/o expires.
Hi Kui-Feng,
On Wed, Dec 13, 2023 at 01:37:35PM -0800, thinker.li@...il.com wrote:
> From: Kui-Feng Lee <thinker.li@...il.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@...il.com>
> Cc: Hangbin Liu <liuhangbin@...il.com>
> ---
> tools/testing/selftests/net/fib_tests.sh | 82 +++++++++++++++++++++++-
> 1 file changed, 80 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/net/fib_tests.sh b/tools/testing/selftests/net/fib_tests.sh
> index 66d0db7a2614..337d0febd796 100755
> --- a/tools/testing/selftests/net/fib_tests.sh
> +++ b/tools/testing/selftests/net/fib_tests.sh
> @@ -785,6 +785,8 @@ fib6_gc_test()
> ret=0
> fi
>
> + log_test $ret 0 "ipv6 route garbage collection"
If the ret doesn't affect the later tests. You can simple do like:
if [ $N_EXP_SLEEP -ne 0 ]; then
log_test 1 0 "fib6_gc: expected 0 routes with expires, got $N_EXP_SLEEP"
fi
> +
> # Permanent routes
> for i in $(seq 1 5000); do
> $IP -6 route add 2001:30::$i \
> @@ -806,9 +808,85 @@ fib6_gc_test()
> ret=0
> fi
>
> - set +e
> + log_test $ret 0 "ipv6 route garbage collection (with permanent routes)"
>
> - 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
^^ These are permanent routes, no expires
> + $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"
> + ret=1
> + else
> + ret=0
> + fi
> +
> + if [ $ret -eq 0 ]; then
All these if/else ret setting/checking looks redundant. Either we can just return
when one test failed (so there is no need to check if $ret -eq 0).
if [ $N_EXP_SLEEP -ne 100 ]; then
log_test 1 0 "fib6_gc: replace permanent to temporary: expected 100 routes with expires, got $N_EXP_SLEEP"
cleanup &> /dev/null
return
fi
Or, use different subnet for testing. So the next one doesn't affect the
previous test. Then there is no need to call "cleanup && return" for every
failed check. e.g.
do temporary route test with 2001:20:: subnet
if [ $N_EXP_SLEEP -ne 0 ]; then
log_test 1 0 "some log info"
fi
do permanent route + temp route with 2001:30:: subnet
if [ $N_EXP_SLEEP -ne 0 ]; then
log_test 1 0 "some log info"
fi
(Here we'd better remove the 5000 permanent route :), or just del and re-add
the interface directly.)
do permanent route with replace to temp route with 2001:40:: subnet
if [ $N_EXP_SLEEP -ne 100 ]; then
log_test 1 0 "some log info"
else
sleep and recheck the route number
fi
etc.
> + 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
> + fi
> +
> + if [ $ret -eq 0 ]; then
> + 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
^^ These are permanent routes.
Thanks
Hangbin
Powered by blists - more mailing lists