Date: Mon,  4 Mar 2024 17:37:34 -0800
From: Kui-Feng Lee <thinker.li@...il.com>
To: netdev@...r.kernel.org,
	ast@...nel.org,
	martin.lau@...ux.dev,
	kernel-team@...a.com,
	kuba@...nel.org,
	davem@...emloft.net,
	dsahern@...nel.org,
	pabeni@...hat.com
Cc: sinquersw@...il.com,
	kuifeng@...a.com,
	Kui-Feng Lee <thinker.li@...il.com>
Subject: [PATCH net-next v2] selftests/net: fix waiting time for ipv6_gc test in fib_tests.sh.

ipv6_gc fails occasionally. According to the study, fib6_run_gc() using
jiffies_round() to round the GC interval could increase the waiting time up
to 750ms (3/4 seconds). The timer has a granularity of 512ms at the range
4s to 32s. That means a route with an expiration time E seconds can wait
for more than E * 2 + 1 seconds if the GC interval is also E seconds.

E * 2 + 2 seconds should be enough for waiting for removing routes.

Also remove a check immediately after replacing 5 routes since it is very
likely to remove some of routes before completing the last route with a
slow environment.

Signed-off-by: Kui-Feng Lee <thinker.li@...il.com>

---

I made the follow change to simulate the case that a route misses the first
GC, and the waiting time of the next GC is rounded up by round_jiffies())
adding 750ms extra waiting time.

    diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
    index 6540d877d369..88479daa6ff7 100644
    --- a/net/ipv6/ip6_fib.c
    +++ b/net/ipv6/ip6_fib.c
    @@ -1487,8 +1487,10 @@ int fib6_add(struct fib6_node *root, struct fib6_info *rt,
     			list_add(&rt->nh_list, &rt->nh->f6i_list);
     		__fib6_update_sernum_upto_root(rt, fib6_new_sernum(info->nl_net));

    -		if (rt->fib6_flags & RTF_EXPIRES)
    +		if (rt->fib6_flags & RTF_EXPIRES) {
    +			printk(KERN_CRIT "fib6_add expires\n");
     			fib6_add_gc_list(rt);
    +		}

     		fib6_start_gc(info->nl_net, rt);
     	}
    @@ -2296,6 +2298,7 @@ static void fib6_flush_trees(struct net *net)
     static int fib6_age(struct fib6_info *rt, struct fib6_gc_args *gc_args)
     {
     	unsigned long now = jiffies;
    +	static int expire_cnt = 0;

     	/*
     	 *	check addrconf expiration here.
    @@ -2303,8 +2306,9 @@ static int fib6_age(struct fib6_info *rt, struct fib6_gc_args *gc_args)
     	 */

     	if (rt->fib6_flags & RTF_EXPIRES && rt->expires) {
    -		if (time_after(now, rt->expires)) {
    +		if (time_after(now, rt->expires) && expire_cnt++) {
     			pr_debug("expiring %p\n", rt);
    +			printk(KERN_CRIT "fib6_age expiring\n");
     			return -1;
     		}
     		gc_args->more++;
    @@ -2376,8 +2380,7 @@ void fib6_run_gc(unsigned long expires, struct net *net, bool force)

     	if (gc_args.more)
     		mod_timer(&net->ipv6.ip6_fib_timer,
    -			  round_jiffies(now
    -					+ net->ipv6.sysctl.ip6_rt_gc_interval));
    +			  now + net->ipv6.sysctl.ip6_rt_gc_interval + HZ * 3 / 4);
     	else
     		del_timer(&net->ipv6.ip6_fib_timer);
     	spin_unlock_bh(&net->ipv6.fib6_gc_lock);

The following is the test case.

    fib6_gc_test()
    {
    	setup

    	echo
    	echo "Fib6 garbage collection test"
    	set -e

    	EXPIRE=5

    	# Check expiration of routes every $EXPIRE seconds (GC)
    	$NS_EXEC sysctl -wq net.ipv6.route.gc_interval=$EXPIRE

    	$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
    	$IP -6 route add 2001:20::1 \
    		via 2001:10::2 dev dummy_10 expires $EXPIRE

    	sleep $(($EXPIRE * 4 + 1))
    	set +e

    	cleanup &> /dev/null
    }

According to what I found from the kernel messages, the route waited
11.258s before being removed. It is longer than $EXPIRE * 2 + 1 seconds
where EXPIRE=5.

    [    8.674004] fib6_add expires
    [   19.932557] fib6_age expiring

Waiting for EXPIRE * 2 + 2 seconds should be enough for non-debug build.
---
 tools/testing/selftests/net/fib_tests.sh | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/tools/testing/selftests/net/fib_tests.sh b/tools/testing/selftests/net/fib_tests.sh
index 3ec1050e47a2..52c5c8730879 100755
--- a/tools/testing/selftests/net/fib_tests.sh
+++ b/tools/testing/selftests/net/fib_tests.sh
@@ -805,7 +805,7 @@ fib6_gc_test()
 	    $IP -6 route add 2001:20::$i \
 		via 2001:10::2 dev dummy_10 expires $EXPIRE
 	done
-	sleep $(($EXPIRE * 2 + 1))
+	sleep $(($EXPIRE * 2 + 2))
 	$NS_EXEC sysctl -wq net.ipv6.route.flush=1
 	check_rt_num 0 $($IP -6 route list |grep expires|wc -l)
 	log_test $ret 0 "ipv6 route garbage collection"
@@ -823,7 +823,8 @@ fib6_gc_test()
 	    $IP -6 route add 2001:20::$i \
 		via 2001:10::2 dev dummy_10 expires $EXPIRE
 	done
-	sleep $(($EXPIRE * 2 + 1))
+	# Wait for GC
+	sleep $(($EXPIRE * 2 + 2))
 	check_rt_num 0 $($IP -6 route list |grep expires|wc -l)
 	log_test $ret 0 "ipv6 route garbage collection (with permanent routes)"
 
@@ -840,10 +841,8 @@ fib6_gc_test()
 	    $IP -6 route replace 2001:20::$i \
 		via 2001:10::2 dev dummy_10 expires $EXPIRE
 	done
-	check_rt_num_clean 5 $($IP -6 route list |grep expires|wc -l) || return
 	# Wait for GC
-	sleep $(($EXPIRE * 2 + 1))
-	$NS_EXEC sysctl -wq net.ipv6.route.flush=1
+	sleep $(($EXPIRE * 2 + 2))
 	check_rt_num 0 $($IP -6 route list |grep expires|wc -l)
 	log_test $ret 0 "ipv6 route garbage collection (replace with expires)"
 
@@ -863,8 +862,7 @@ fib6_gc_test()
 	check_rt_num_clean 0 $($IP -6 route list |grep expires|wc -l) || return
 
 	# Wait for GC
-	sleep $(($EXPIRE * 2 + 1))
-
+	sleep $(($EXPIRE * 2 + 2))
 	check_rt_num 5 $($IP -6 route list |grep -v expires|grep 2001:20::|wc -l)
 	log_test $ret 0 "ipv6 route garbage collection (replace with permanent)"
 
@@ -901,9 +899,7 @@ fib6_gc_test()
 	check_rt_num_clean 1 $($IP -6 route list|grep expires|wc -l) || return
 
 	# Wait for GC
-	sleep $(($EXPIRE * 2 + 1))
-
-	$NS_EXEC sysctl -wq net.ipv6.route.flush=1
+	sleep $(($EXPIRE * 2 + 2))
 	check_rt_num 0 $($IP -6 route list |grep expires|wc -l)
 	log_test $ret 0 "ipv6 route garbage collection (RA message)"
 
-- 
2.34.1

Powered by blists - more mailing lists