| Message ID | 20230802004303.567266-3-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 |
On 8/1/23 6:43 PM, thinker.li@gmail.com wrote: \> @@ -747,6 +750,97 @@ fib_notify_test() > cleanup &> /dev/null > } > > +fib6_gc_test() > +{ > + echo > + echo "Fib6 garbage collection test" > + > + STRACE=$(which strace) > + if [ -z "$STRACE" ]; then > + echo " SKIP: strace not found" > + ret=$ksft_skip > + return > + fi > + > + EXPIRE=10 > + > + setup > + > + 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 $EXPIRE seconds > + $IP -6 route add 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 1000 ]; then race condition here ... that you can install all 1000 routes and then run this command before any expire. 10 seconds is normally more than enough time, but on a loaded server it might not be. And really it does not matter. What matters is that you install routes with an expires and they disappear when expected - and I believe the flush below should not be needed to validate they have been removed.
On 8/2/23 19:06, David Ahern wrote: > On 8/1/23 6:43 PM, thinker.li@gmail.com wrote: > \> @@ -747,6 +750,97 @@ fib_notify_test() >> cleanup &> /dev/null >> } >> >> +fib6_gc_test() >> +{ >> + echo >> + echo "Fib6 garbage collection test" >> + >> + STRACE=$(which strace) >> + if [ -z "$STRACE" ]; then >> + echo " SKIP: strace not found" >> + ret=$ksft_skip >> + return >> + fi >> + >> + EXPIRE=10 >> + >> + setup >> + >> + 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 $EXPIRE seconds >> + $IP -6 route add 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 1000 ]; then > > race condition here ... that you can install all 1000 routes and then > run this command before any expire. 10 seconds is normally more than > enough time, but on a loaded server it might not be. And really it does > not matter. What matters is that you install routes with an expires and > they disappear when expected - and I believe the flush below should not > be needed to validate they have been removed. Without the flush below, the result will be very unpredictable or need to wait longer, at least two gc_interval seconds. We can shorten gc_interval to 10s, but we need to wait for 20s to make it certain. It is more predictable with the flush. About race condition, I will remove the check. Just like what you said, it is not necessary.
On 8/2/23 10:09 PM, Kui-Feng Lee wrote: > > > On 8/2/23 19:06, David Ahern wrote: >> On 8/1/23 6:43 PM, thinker.li@gmail.com wrote: >> \> @@ -747,6 +750,97 @@ fib_notify_test() >>> cleanup &> /dev/null >>> } >>> +fib6_gc_test() >>> +{ >>> + echo >>> + echo "Fib6 garbage collection test" >>> + >>> + STRACE=$(which strace) >>> + if [ -z "$STRACE" ]; then >>> + echo " SKIP: strace not found" >>> + ret=$ksft_skip >>> + return >>> + fi >>> + >>> + EXPIRE=10 >>> + >>> + setup >>> + >>> + 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 $EXPIRE seconds >>> + $IP -6 route add 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 1000 ]; then >> >> race condition here ... that you can install all 1000 routes and then >> run this command before any expire. 10 seconds is normally more than >> enough time, but on a loaded server it might not be. And really it does >> not matter. What matters is that you install routes with an expires and >> they disappear when expected - and I believe the flush below should not >> be needed to validate they have been removed. > > Without the flush below, the result will be very unpredictable or need > to wait longer, at least two gc_interval seconds. We can > shorten gc_interval to 10s, but we need to wait for 20s to make it > certain. It is more predictable with the flush. > > About race condition, I will remove the check. Just like what you said, > it is not necessary. you do not need to measure how long it takes to remove expired routes in a selftest; you are only testing that in fact the expired routes are removed. EXPIRES=1 install routes # wait 2x expires time for removal sleep 2 verify routes are removed.
On 8/3/23 12:49, David Ahern wrote: > On 8/2/23 10:09 PM, Kui-Feng Lee wrote: >> >> >> On 8/2/23 19:06, David Ahern wrote: >>> On 8/1/23 6:43 PM, thinker.li@gmail.com wrote: >>> \> @@ -747,6 +750,97 @@ fib_notify_test() >>>> cleanup &> /dev/null >>>> } >>>> +fib6_gc_test() >>>> +{ >>>> + echo >>>> + echo "Fib6 garbage collection test" >>>> + >>>> + STRACE=$(which strace) >>>> + if [ -z "$STRACE" ]; then >>>> + echo " SKIP: strace not found" >>>> + ret=$ksft_skip >>>> + return >>>> + fi >>>> + >>>> + EXPIRE=10 >>>> + >>>> + setup >>>> + >>>> + 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 $EXPIRE seconds >>>> + $IP -6 route add 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 1000 ]; then >>> >>> race condition here ... that you can install all 1000 routes and then >>> run this command before any expire. 10 seconds is normally more than >>> enough time, but on a loaded server it might not be. And really it does >>> not matter. What matters is that you install routes with an expires and >>> they disappear when expected - and I believe the flush below should not >>> be needed to validate they have been removed. >> >> Without the flush below, the result will be very unpredictable or need >> to wait longer, at least two gc_interval seconds. We can >> shorten gc_interval to 10s, but we need to wait for 20s to make it >> certain. It is more predictable with the flush. >> >> About race condition, I will remove the check. Just like what you said, >> it is not necessary. > > you do not need to measure how long it takes to remove expired routes in > a selftest; you are only testing that in fact the expired routes are > removed. > > EXPIRES=1 > install routes > # wait 2x expires time for removal > sleep 2 > verify routes are removed. Got it!
diff --git a/tools/testing/selftests/net/fib_tests.sh b/tools/testing/selftests/net/fib_tests.sh index 35d89dfa6f11..ffd7a044f950 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,97 @@ fib_notify_test() cleanup &> /dev/null } +fib6_gc_test() +{ + echo + echo "Fib6 garbage collection test" + + STRACE=$(which strace) + if [ -z "$STRACE" ]; then + echo " SKIP: strace not found" + ret=$ksft_skip + return + fi + + EXPIRE=10 + + setup + + 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 $EXPIRE seconds + $IP -6 route add 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 1000 ]; then + echo "FAIL: expected 1000 routes with expires, got $N_EXP" + ret=1 + else + sleep $EXPIRE + REALTM_NP=$($NS_EXEC $STRACE -T sysctl \ + -wq net.ipv6.route.flush=1 2>&1 | \ + awk -- '/write\(.*"1\\n", 2\)/ { gsub("(.*<|>.*)", ""); print $0;}') + 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 + + # 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 $EXPIRE seconds + $IP -6 route add 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 1000 ]; then + echo + "FAIL: expected 1000 routes with expires, got $N_EXP (5000 permanent routes)" + ret=1 + else + sleep $EXPIRE + REALTM_P=$($NS_EXEC $STRACE -T sysctl \ + -wq net.ipv6.route.flush=1 2>&1 | \ + awk -- '/write\(.*"1\\n", 2\)/ { gsub("(.*<|>.*)", ""); print $0;}') + 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)" + ret=1 + else + ret=0 + fi + fi + + set +e + + log_test $ret 0 "ipv6 route garbage collection (${REALTM_NP}s, ${REALTM_P}s)" + + cleanup &> /dev/null +} + fib_suppress_test() { echo @@ -2217,6 +2311,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