[PATCH net-next] selftests: fib_nexthops: Make the test more robust

[PATCH net-next] selftests: fib_nexthops: Make the test more robust

[Amit Cohen]

Rarely some of the test cases fail. Make the test more robust by increasing
the timeout of ping commands to 5 seconds.

Signed-off-by: Amit Cohen <amcohen@nvidia.com>
---
 tools/testing/selftests/net/fib_nexthops.sh | 48 ++++++++++-----------
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/tools/testing/selftests/net/fib_nexthops.sh b/tools/testing/selftests/net/fib_nexthops.sh
index b3bf5319bb0e..a99ee3fb2e13 100755
--- a/tools/testing/selftests/net/fib_nexthops.sh
+++ b/tools/testing/selftests/net/fib_nexthops.sh
@@ -882,13 +882,13 @@ ipv6_fcnal_runtime()
 	log_test $? 0 "Route delete"
 
 	run_cmd "$IP ro add 2001:db8:101::1/128 nhid 81"
-	run_cmd "ip netns exec me ping -c1 -w1 2001:db8:101::1"
+	run_cmd "ip netns exec me ping -c1 -w5 2001:db8:101::1"
 	log_test $? 0 "Ping with nexthop"
 
 	run_cmd "$IP nexthop add id 82 via 2001:db8:92::2 dev veth3"
 	run_cmd "$IP nexthop add id 122 group 81/82"
 	run_cmd "$IP ro replace 2001:db8:101::1/128 nhid 122"
-	run_cmd "ip netns exec me ping -c1 -w1 2001:db8:101::1"
+	run_cmd "ip netns exec me ping -c1 -w5 2001:db8:101::1"
 	log_test $? 0 "Ping - multipath"
 
 	#
@@ -896,26 +896,26 @@ ipv6_fcnal_runtime()
 	#
 	run_cmd "$IP -6 nexthop add id 83 blackhole"
 	run_cmd "$IP ro replace 2001:db8:101::1/128 nhid 83"
-	run_cmd "ip netns exec me ping -c1 -w1 2001:db8:101::1"
+	run_cmd "ip netns exec me ping -c1 -w5 2001:db8:101::1"
 	log_test $? 2 "Ping - blackhole"
 
 	run_cmd "$IP nexthop replace id 83 via 2001:db8:91::2 dev veth1"
-	run_cmd "ip netns exec me ping -c1 -w1 2001:db8:101::1"
+	run_cmd "ip netns exec me ping -c1 -w5 2001:db8:101::1"
 	log_test $? 0 "Ping - blackhole replaced with gateway"
 
 	run_cmd "$IP -6 nexthop replace id 83 blackhole"
-	run_cmd "ip netns exec me ping -c1 -w1 2001:db8:101::1"
+	run_cmd "ip netns exec me ping -c1 -w5 2001:db8:101::1"
 	log_test $? 2 "Ping - gateway replaced by blackhole"
 
 	run_cmd "$IP ro replace 2001:db8:101::1/128 nhid 122"
-	run_cmd "ip netns exec me ping -c1 -w1 2001:db8:101::1"
+	run_cmd "ip netns exec me ping -c1 -w5 2001:db8:101::1"
 	if [ $? -eq 0 ]; then
 		run_cmd "$IP nexthop replace id 122 group 83"
-		run_cmd "ip netns exec me ping -c1 -w1 2001:db8:101::1"
+		run_cmd "ip netns exec me ping -c1 -w5 2001:db8:101::1"
 		log_test $? 2 "Ping - group with blackhole"
 
 		run_cmd "$IP nexthop replace id 122 group 81/82"
-		run_cmd "ip netns exec me ping -c1 -w1 2001:db8:101::1"
+		run_cmd "ip netns exec me ping -c1 -w5 2001:db8:101::1"
 		log_test $? 0 "Ping - group blackhole replaced with gateways"
 	else
 		log_test 2 0 "Ping - multipath failed"
@@ -1003,10 +1003,10 @@ ipv6_fcnal_runtime()
 	run_cmd "$IP nexthop add id 92 via 2001:db8:92::2 dev veth3"
 	run_cmd "$IP nexthop add id 93 group 91/92"
 	run_cmd "$IP -6 ro add default nhid 91"
-	run_cmd "ip netns exec me ping -c1 -w1 2001:db8:101::1"
+	run_cmd "ip netns exec me ping -c1 -w5 2001:db8:101::1"
 	log_test $? 0 "Nexthop with default route and rpfilter"
 	run_cmd "$IP -6 ro replace default nhid 93"
-	run_cmd "ip netns exec me ping -c1 -w1 2001:db8:101::1"
+	run_cmd "ip netns exec me ping -c1 -w5 2001:db8:101::1"
 	log_test $? 0 "Nexthop with multipath default route and rpfilter"
 
 	# TO-DO:
@@ -1460,13 +1460,13 @@ ipv4_fcnal_runtime()
 	#
 	run_cmd "$IP nexthop replace id 21 via 172.16.1.2 dev veth1"
 	run_cmd "$IP ro replace 172.16.101.1/32 nhid 21"
-	run_cmd "ip netns exec me ping -c1 -w1 172.16.101.1"
+	run_cmd "ip netns exec me ping -c1 -w5 172.16.101.1"
 	log_test $? 0 "Basic ping"
 
 	run_cmd "$IP nexthop replace id 22 via 172.16.2.2 dev veth3"
 	run_cmd "$IP nexthop add id 122 group 21/22"
 	run_cmd "$IP ro replace 172.16.101.1/32 nhid 122"
-	run_cmd "ip netns exec me ping -c1 -w1 172.16.101.1"
+	run_cmd "ip netns exec me ping -c1 -w5 172.16.101.1"
 	log_test $? 0 "Ping - multipath"
 
 	run_cmd "$IP ro delete 172.16.101.1/32 nhid 122"
@@ -1477,7 +1477,7 @@ ipv4_fcnal_runtime()
 	run_cmd "$IP nexthop add id 501 via 172.16.1.2 dev veth1"
 	run_cmd "$IP ro add default nhid 501"
 	run_cmd "$IP ro add default via 172.16.1.3 dev veth1 metric 20"
-	run_cmd "ip netns exec me ping -c1 -w1 172.16.101.1"
+	run_cmd "ip netns exec me ping -c1 -w5 172.16.101.1"
 	log_test $? 0 "Ping - multiple default routes, nh first"
 
 	# flip the order
@@ -1486,7 +1486,7 @@ ipv4_fcnal_runtime()
 	run_cmd "$IP ro add default via 172.16.1.2 dev veth1 metric 20"
 	run_cmd "$IP nexthop replace id 501 via 172.16.1.3 dev veth1"
 	run_cmd "$IP ro add default nhid 501 metric 20"
-	run_cmd "ip netns exec me ping -c1 -w1 172.16.101.1"
+	run_cmd "ip netns exec me ping -c1 -w5 172.16.101.1"
 	log_test $? 0 "Ping - multiple default routes, nh second"
 
 	run_cmd "$IP nexthop delete nhid 501"
@@ -1497,26 +1497,26 @@ ipv4_fcnal_runtime()
 	#
 	run_cmd "$IP nexthop add id 23 blackhole"
 	run_cmd "$IP ro replace 172.16.101.1/32 nhid 23"
-	run_cmd "ip netns exec me ping -c1 -w1 172.16.101.1"
+	run_cmd "ip netns exec me ping -c1 -w5 172.16.101.1"
 	log_test $? 2 "Ping - blackhole"
 
 	run_cmd "$IP nexthop replace id 23 via 172.16.1.2 dev veth1"
-	run_cmd "ip netns exec me ping -c1 -w1 172.16.101.1"
+	run_cmd "ip netns exec me ping -c1 -w5 172.16.101.1"
 	log_test $? 0 "Ping - blackhole replaced with gateway"
 
 	run_cmd "$IP nexthop replace id 23 blackhole"
-	run_cmd "ip netns exec me ping -c1 -w1 172.16.101.1"
+	run_cmd "ip netns exec me ping -c1 -w5 172.16.101.1"
 	log_test $? 2 "Ping - gateway replaced by blackhole"
 
 	run_cmd "$IP ro replace 172.16.101.1/32 nhid 122"
-	run_cmd "ip netns exec me ping -c1 -w1 172.16.101.1"
+	run_cmd "ip netns exec me ping -c1 -w5 172.16.101.1"
 	if [ $? -eq 0 ]; then
 		run_cmd "$IP nexthop replace id 122 group 23"
-		run_cmd "ip netns exec me ping -c1 -w1 172.16.101.1"
+		run_cmd "ip netns exec me ping -c1 -w5 172.16.101.1"
 		log_test $? 2 "Ping - group with blackhole"
 
 		run_cmd "$IP nexthop replace id 122 group 21/22"
-		run_cmd "ip netns exec me ping -c1 -w1 172.16.101.1"
+		run_cmd "ip netns exec me ping -c1 -w5 172.16.101.1"
 		log_test $? 0 "Ping - group blackhole replaced with gateways"
 	else
 		log_test 2 0 "Ping - multipath failed"
@@ -1543,7 +1543,7 @@ ipv4_fcnal_runtime()
 	run_cmd "$IP nexthop add id 24 via ${lladdr} dev veth1"
 	set +e
 	run_cmd "$IP ro replace 172.16.101.1/32 nhid 24"
-	run_cmd "ip netns exec me ping -c1 -w1 172.16.101.1"
+	run_cmd "ip netns exec me ping -c1 -w5 172.16.101.1"
 	log_test $? 0 "IPv6 nexthop with IPv4 route"
 
 	$IP neigh sh | grep -q "${lladdr} dev veth1"
@@ -1567,11 +1567,11 @@ ipv4_fcnal_runtime()
 
 	check_route "172.16.101.1" "172.16.101.1 nhid 101 nexthop via inet6 ${lladdr} dev veth1 weight 1 nexthop via 172.16.1.2 dev veth1 weight 1"
 
-	run_cmd "ip netns exec me ping -c1 -w1 172.16.101.1"
+	run_cmd "ip netns exec me ping -c1 -w5 172.16.101.1"
 	log_test $? 0 "IPv6 nexthop with IPv4 route"
 
 	run_cmd "$IP ro replace 172.16.101.1/32 via inet6 ${lladdr} dev veth1"
-	run_cmd "ip netns exec me ping -c1 -w1 172.16.101.1"
+	run_cmd "ip netns exec me ping -c1 -w5 172.16.101.1"
 	log_test $? 0 "IPv4 route with IPv6 gateway"
 
 	$IP neigh sh | grep -q "${lladdr} dev veth1"
@@ -1588,7 +1588,7 @@ ipv4_fcnal_runtime()
 
 	run_cmd "$IP ro del 172.16.101.1/32 via inet6 ${lladdr} dev veth1"
 	run_cmd "$IP -4 ro add default via inet6 ${lladdr} dev veth1"
-	run_cmd "ip netns exec me ping -c1 -w1 172.16.101.1"
+	run_cmd "ip netns exec me ping -c1 -w5 172.16.101.1"
 	log_test $? 0 "IPv4 default route with IPv6 gateway"
 
 	#
-- 
2.35.1

RE: [PATCH net-next] selftests: fib_nexthops: Make the test more robust

[Amit Cohen]

+ David Ahern

> -----Original Message-----
> From: Amit Cohen <amcohen@nvidia.com>
> Sent: Thursday, May 12, 2022 4:12 PM
> To: netdev@vger.kernel.org
> Cc: davem@davemloft.net; edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; shuah@kernel.org; mlxsw
> <mlxsw@nvidia.com>; Amit Cohen <amcohen@nvidia.com>
> Subject: [PATCH net-next] selftests: fib_nexthops: Make the test more robust
> 
> Rarely some of the test cases fail. Make the test more robust by increasing
> the timeout of ping commands to 5 seconds.
> 
> Signed-off-by: Amit Cohen <amcohen@nvidia.com>
> ---
>  tools/testing/selftests/net/fib_nexthops.sh | 48 ++++++++++-----------
>  1 file changed, 24 insertions(+), 24 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/fib_nexthops.sh b/tools/testing/selftests/net/fib_nexthops.sh
> index b3bf5319bb0e..a99ee3fb2e13 100755
> --- a/tools/testing/selftests/net/fib_nexthops.sh
> +++ b/tools/testing/selftests/net/fib_nexthops.sh
> @@ -882,13 +882,13 @@ ipv6_fcnal_runtime()
>  	log_test $? 0 "Route delete"
> 
>  	run_cmd "$IP ro add 2001:db8:101::1/128 nhid 81"
> -	run_cmd "ip netns exec me ping -c1 -w1 2001:db8:101::1"
> +	run_cmd "ip netns exec me ping -c1 -w5 2001:db8:101::1"
>  	log_test $? 0 "Ping with nexthop"
> 
>  	run_cmd "$IP nexthop add id 82 via 2001:db8:92::2 dev veth3"
>  	run_cmd "$IP nexthop add id 122 group 81/82"
>  	run_cmd "$IP ro replace 2001:db8:101::1/128 nhid 122"
> -	run_cmd "ip netns exec me ping -c1 -w1 2001:db8:101::1"
> +	run_cmd "ip netns exec me ping -c1 -w5 2001:db8:101::1"
>  	log_test $? 0 "Ping - multipath"
> 
>  	#
> @@ -896,26 +896,26 @@ ipv6_fcnal_runtime()
>  	#
>  	run_cmd "$IP -6 nexthop add id 83 blackhole"
>  	run_cmd "$IP ro replace 2001:db8:101::1/128 nhid 83"
> -	run_cmd "ip netns exec me ping -c1 -w1 2001:db8:101::1"
> +	run_cmd "ip netns exec me ping -c1 -w5 2001:db8:101::1"
>  	log_test $? 2 "Ping - blackhole"
> 
>  	run_cmd "$IP nexthop replace id 83 via 2001:db8:91::2 dev veth1"
> -	run_cmd "ip netns exec me ping -c1 -w1 2001:db8:101::1"
> +	run_cmd "ip netns exec me ping -c1 -w5 2001:db8:101::1"
>  	log_test $? 0 "Ping - blackhole replaced with gateway"
> 
>  	run_cmd "$IP -6 nexthop replace id 83 blackhole"
> -	run_cmd "ip netns exec me ping -c1 -w1 2001:db8:101::1"
> +	run_cmd "ip netns exec me ping -c1 -w5 2001:db8:101::1"
>  	log_test $? 2 "Ping - gateway replaced by blackhole"
> 
>  	run_cmd "$IP ro replace 2001:db8:101::1/128 nhid 122"
> -	run_cmd "ip netns exec me ping -c1 -w1 2001:db8:101::1"
> +	run_cmd "ip netns exec me ping -c1 -w5 2001:db8:101::1"
>  	if [ $? -eq 0 ]; then
>  		run_cmd "$IP nexthop replace id 122 group 83"
> -		run_cmd "ip netns exec me ping -c1 -w1 2001:db8:101::1"
> +		run_cmd "ip netns exec me ping -c1 -w5 2001:db8:101::1"
>  		log_test $? 2 "Ping - group with blackhole"
> 
>  		run_cmd "$IP nexthop replace id 122 group 81/82"
> -		run_cmd "ip netns exec me ping -c1 -w1 2001:db8:101::1"
> +		run_cmd "ip netns exec me ping -c1 -w5 2001:db8:101::1"
>  		log_test $? 0 "Ping - group blackhole replaced with gateways"
>  	else
>  		log_test 2 0 "Ping - multipath failed"
> @@ -1003,10 +1003,10 @@ ipv6_fcnal_runtime()
>  	run_cmd "$IP nexthop add id 92 via 2001:db8:92::2 dev veth3"
>  	run_cmd "$IP nexthop add id 93 group 91/92"
>  	run_cmd "$IP -6 ro add default nhid 91"
> -	run_cmd "ip netns exec me ping -c1 -w1 2001:db8:101::1"
> +	run_cmd "ip netns exec me ping -c1 -w5 2001:db8:101::1"
>  	log_test $? 0 "Nexthop with default route and rpfilter"
>  	run_cmd "$IP -6 ro replace default nhid 93"
> -	run_cmd "ip netns exec me ping -c1 -w1 2001:db8:101::1"
> +	run_cmd "ip netns exec me ping -c1 -w5 2001:db8:101::1"
>  	log_test $? 0 "Nexthop with multipath default route and rpfilter"
> 
>  	# TO-DO:
> @@ -1460,13 +1460,13 @@ ipv4_fcnal_runtime()
>  	#
>  	run_cmd "$IP nexthop replace id 21 via 172.16.1.2 dev veth1"
>  	run_cmd "$IP ro replace 172.16.101.1/32 nhid 21"
> -	run_cmd "ip netns exec me ping -c1 -w1 172.16.101.1"
> +	run_cmd "ip netns exec me ping -c1 -w5 172.16.101.1"
>  	log_test $? 0 "Basic ping"
> 
>  	run_cmd "$IP nexthop replace id 22 via 172.16.2.2 dev veth3"
>  	run_cmd "$IP nexthop add id 122 group 21/22"
>  	run_cmd "$IP ro replace 172.16.101.1/32 nhid 122"
> -	run_cmd "ip netns exec me ping -c1 -w1 172.16.101.1"
> +	run_cmd "ip netns exec me ping -c1 -w5 172.16.101.1"
>  	log_test $? 0 "Ping - multipath"
> 
>  	run_cmd "$IP ro delete 172.16.101.1/32 nhid 122"
> @@ -1477,7 +1477,7 @@ ipv4_fcnal_runtime()
>  	run_cmd "$IP nexthop add id 501 via 172.16.1.2 dev veth1"
>  	run_cmd "$IP ro add default nhid 501"
>  	run_cmd "$IP ro add default via 172.16.1.3 dev veth1 metric 20"
> -	run_cmd "ip netns exec me ping -c1 -w1 172.16.101.1"
> +	run_cmd "ip netns exec me ping -c1 -w5 172.16.101.1"
>  	log_test $? 0 "Ping - multiple default routes, nh first"
> 
>  	# flip the order
> @@ -1486,7 +1486,7 @@ ipv4_fcnal_runtime()
>  	run_cmd "$IP ro add default via 172.16.1.2 dev veth1 metric 20"
>  	run_cmd "$IP nexthop replace id 501 via 172.16.1.3 dev veth1"
>  	run_cmd "$IP ro add default nhid 501 metric 20"
> -	run_cmd "ip netns exec me ping -c1 -w1 172.16.101.1"
> +	run_cmd "ip netns exec me ping -c1 -w5 172.16.101.1"
>  	log_test $? 0 "Ping - multiple default routes, nh second"
> 
>  	run_cmd "$IP nexthop delete nhid 501"
> @@ -1497,26 +1497,26 @@ ipv4_fcnal_runtime()
>  	#
>  	run_cmd "$IP nexthop add id 23 blackhole"
>  	run_cmd "$IP ro replace 172.16.101.1/32 nhid 23"
> -	run_cmd "ip netns exec me ping -c1 -w1 172.16.101.1"
> +	run_cmd "ip netns exec me ping -c1 -w5 172.16.101.1"
>  	log_test $? 2 "Ping - blackhole"
> 
>  	run_cmd "$IP nexthop replace id 23 via 172.16.1.2 dev veth1"
> -	run_cmd "ip netns exec me ping -c1 -w1 172.16.101.1"
> +	run_cmd "ip netns exec me ping -c1 -w5 172.16.101.1"
>  	log_test $? 0 "Ping - blackhole replaced with gateway"
> 
>  	run_cmd "$IP nexthop replace id 23 blackhole"
> -	run_cmd "ip netns exec me ping -c1 -w1 172.16.101.1"
> +	run_cmd "ip netns exec me ping -c1 -w5 172.16.101.1"
>  	log_test $? 2 "Ping - gateway replaced by blackhole"
> 
>  	run_cmd "$IP ro replace 172.16.101.1/32 nhid 122"
> -	run_cmd "ip netns exec me ping -c1 -w1 172.16.101.1"
> +	run_cmd "ip netns exec me ping -c1 -w5 172.16.101.1"
>  	if [ $? -eq 0 ]; then
>  		run_cmd "$IP nexthop replace id 122 group 23"
> -		run_cmd "ip netns exec me ping -c1 -w1 172.16.101.1"
> +		run_cmd "ip netns exec me ping -c1 -w5 172.16.101.1"
>  		log_test $? 2 "Ping - group with blackhole"
> 
>  		run_cmd "$IP nexthop replace id 122 group 21/22"
> -		run_cmd "ip netns exec me ping -c1 -w1 172.16.101.1"
> +		run_cmd "ip netns exec me ping -c1 -w5 172.16.101.1"
>  		log_test $? 0 "Ping - group blackhole replaced with gateways"
>  	else
>  		log_test 2 0 "Ping - multipath failed"
> @@ -1543,7 +1543,7 @@ ipv4_fcnal_runtime()
>  	run_cmd "$IP nexthop add id 24 via ${lladdr} dev veth1"
>  	set +e
>  	run_cmd "$IP ro replace 172.16.101.1/32 nhid 24"
> -	run_cmd "ip netns exec me ping -c1 -w1 172.16.101.1"
> +	run_cmd "ip netns exec me ping -c1 -w5 172.16.101.1"
>  	log_test $? 0 "IPv6 nexthop with IPv4 route"
> 
>  	$IP neigh sh | grep -q "${lladdr} dev veth1"
> @@ -1567,11 +1567,11 @@ ipv4_fcnal_runtime()
> 
>  	check_route "172.16.101.1" "172.16.101.1 nhid 101 nexthop via inet6 ${lladdr} dev veth1 weight 1 nexthop via 172.16.1.2 dev
> veth1 weight 1"
> 
> -	run_cmd "ip netns exec me ping -c1 -w1 172.16.101.1"
> +	run_cmd "ip netns exec me ping -c1 -w5 172.16.101.1"
>  	log_test $? 0 "IPv6 nexthop with IPv4 route"
> 
>  	run_cmd "$IP ro replace 172.16.101.1/32 via inet6 ${lladdr} dev veth1"
> -	run_cmd "ip netns exec me ping -c1 -w1 172.16.101.1"
> +	run_cmd "ip netns exec me ping -c1 -w5 172.16.101.1"
>  	log_test $? 0 "IPv4 route with IPv6 gateway"
> 
>  	$IP neigh sh | grep -q "${lladdr} dev veth1"
> @@ -1588,7 +1588,7 @@ ipv4_fcnal_runtime()
> 
>  	run_cmd "$IP ro del 172.16.101.1/32 via inet6 ${lladdr} dev veth1"
>  	run_cmd "$IP -4 ro add default via inet6 ${lladdr} dev veth1"
> -	run_cmd "ip netns exec me ping -c1 -w1 172.16.101.1"
> +	run_cmd "ip netns exec me ping -c1 -w5 172.16.101.1"
>  	log_test $? 0 "IPv4 default route with IPv6 gateway"
> 
>  	#
> --
> 2.35.1

Re: [PATCH net-next] selftests: fib_nexthops: Make the test more robust

[David Ahern]

On 5/12/22 7:31 AM, Amit Cohen wrote:
> + David Ahern
> 
>> -----Original Message-----
>> From: Amit Cohen <amcohen@nvidia.com>
>> Sent: Thursday, May 12, 2022 4:12 PM
>> To: netdev@vger.kernel.org
>> Cc: davem@davemloft.net; edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; shuah@kernel.org; mlxsw
>> <mlxsw@nvidia.com>; Amit Cohen <amcohen@nvidia.com>
>> Subject: [PATCH net-next] selftests: fib_nexthops: Make the test more robust
>>
>> Rarely some of the test cases fail. Make the test more robust by increasing
>> the timeout of ping commands to 5 seconds.
>>
>> Signed-off-by: Amit Cohen <amcohen@nvidia.com>
>> ---
>>  tools/testing/selftests/net/fib_nexthops.sh | 48 ++++++++++-----------
>>  1 file changed, 24 insertions(+), 24 deletions(-)
>>

Reviewed-by: David Ahern <dsahern@kernel.org>

Re: [PATCH net-next] selftests: fib_nexthops: Make the test more robust

[Shuah Khan]

On 5/12/22 7:12 AM, Amit Cohen wrote:
> Rarely some of the test cases fail. Make the test more robust by increasing
> the timeout of ping commands to 5 seconds.
> 

Can you explain why test cases fail?

> Signed-off-by: Amit Cohen <amcohen@nvidia.com>
> ---
>   tools/testing/selftests/net/fib_nexthops.sh | 48 ++++++++++-----------
>   1 file changed, 24 insertions(+), 24 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/fib_nexthops.sh b/tools/testing/selftests/net/fib_nexthops.sh
> index b3bf5319bb0e..a99ee3fb2e13 100755
> --- a/tools/testing/selftests/net/fib_nexthops.sh
> +++ b/tools/testing/selftests/net/fib_nexthops.sh
> @@ -882,13 +882,13 @@ ipv6_fcnal_runtime()
>   	log_test $? 0 "Route delete"
>   
>   	run_cmd "$IP ro add 2001:db8:101::1/128 nhid 81"
> -	run_cmd "ip netns exec me ping -c1 -w1 2001:db8:101::1"
> +	run_cmd "ip netns exec me ping -c1 -w5 2001:db8:101::1"
>   	log_test $? 0 "Ping with nexthop"
>   
Looks like the change uses "-w deadline" - "-W timeout" might
be a better choice if ping fails with no response?

thanks,
-- Shuah

RE: [PATCH net-next] selftests: fib_nexthops: Make the test more robust

[Amit Cohen]

> -----Original Message-----
> From: Shuah Khan <skhan@linuxfoundation.org>
> Sent: Thursday, May 12, 2022 5:28 PM
> To: Amit Cohen <amcohen@nvidia.com>; netdev@vger.kernel.org
> Cc: davem@davemloft.net; edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; shuah@kernel.org; mlxsw
> <mlxsw@nvidia.com>; Shuah Khan <skhan@linuxfoundation.org>
> Subject: Re: [PATCH net-next] selftests: fib_nexthops: Make the test more robust
> 
> On 5/12/22 7:12 AM, Amit Cohen wrote:
> > Rarely some of the test cases fail. Make the test more robust by increasing
> > the timeout of ping commands to 5 seconds.
> >
> 
> Can you explain why test cases fail?

The failures are probably caused due to slow forwarding performance.
You can see similar commit - b6a4fd680042 ("selftests: forwarding: Make ping timeout configurable").

> 
> > Signed-off-by: Amit Cohen <amcohen@nvidia.com>
> > ---
> >   tools/testing/selftests/net/fib_nexthops.sh | 48 ++++++++++-----------
> >   1 file changed, 24 insertions(+), 24 deletions(-)
> >
> > diff --git a/tools/testing/selftests/net/fib_nexthops.sh b/tools/testing/selftests/net/fib_nexthops.sh
> > index b3bf5319bb0e..a99ee3fb2e13 100755
> > --- a/tools/testing/selftests/net/fib_nexthops.sh
> > +++ b/tools/testing/selftests/net/fib_nexthops.sh
> > @@ -882,13 +882,13 @@ ipv6_fcnal_runtime()
> >   	log_test $? 0 "Route delete"
> >
> >   	run_cmd "$IP ro add 2001:db8:101::1/128 nhid 81"
> > -	run_cmd "ip netns exec me ping -c1 -w1 2001:db8:101::1"
> > +	run_cmd "ip netns exec me ping -c1 -w5 2001:db8:101::1"
> >   	log_test $? 0 "Ping with nexthop"
> >
> Looks like the change uses "-w deadline" - "-W timeout" might
> be a better choice if ping fails with no response?

We usually use "-w" in ping commands in selftests, but I can change it if you prefer "-W".

> 
> thanks,
> -- Shuah

Re: [PATCH net-next] selftests: fib_nexthops: Make the test more robust

[Shuah Khan]

On 5/12/22 9:17 AM, Amit Cohen wrote:
> 
> 
>> -----Original Message-----
>> From: Shuah Khan <skhan@linuxfoundation.org>
>> Sent: Thursday, May 12, 2022 5:28 PM
>> To: Amit Cohen <amcohen@nvidia.com>; netdev@vger.kernel.org
>> Cc: davem@davemloft.net; edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; shuah@kernel.org; mlxsw
>> <mlxsw@nvidia.com>; Shuah Khan <skhan@linuxfoundation.org>
>> Subject: Re: [PATCH net-next] selftests: fib_nexthops: Make the test more robust
>>
>> On 5/12/22 7:12 AM, Amit Cohen wrote:
>>> Rarely some of the test cases fail. Make the test more robust by increasing
>>> the timeout of ping commands to 5 seconds.
>>>
>>
>> Can you explain why test cases fail?
> 
> The failures are probably caused due to slow forwarding performance.
> You can see similar commit - b6a4fd680042 ("selftests: forwarding: Make ping timeout configurable").
> 

My primary concern is that this patch simply changes the value
and doesn't make it configurable. Sounds like the above commit
does that. Why not use the same approach to keep it consistent.

>>
>>> Signed-off-by: Amit Cohen <amcohen@nvidia.com>
>>> ---
>>>    tools/testing/selftests/net/fib_nexthops.sh | 48 ++++++++++-----------
>>>    1 file changed, 24 insertions(+), 24 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/net/fib_nexthops.sh b/tools/testing/selftests/net/fib_nexthops.sh
>>> index b3bf5319bb0e..a99ee3fb2e13 100755
>>> --- a/tools/testing/selftests/net/fib_nexthops.sh
>>> +++ b/tools/testing/selftests/net/fib_nexthops.sh
>>> @@ -882,13 +882,13 @@ ipv6_fcnal_runtime()
>>>    	log_test $? 0 "Route delete"
>>>
>>>    	run_cmd "$IP ro add 2001:db8:101::1/128 nhid 81"
>>> -	run_cmd "ip netns exec me ping -c1 -w1 2001:db8:101::1"
>>> +	run_cmd "ip netns exec me ping -c1 -w5 2001:db8:101::1"
>>>    	log_test $? 0 "Ping with nexthop"
>>>
>> Looks like the change uses "-w deadline" - "-W timeout" might
>> be a better choice if ping fails with no response?
> 
> We usually use "-w" in ping commands in selftests, but I can change it if you prefer "-W".
> 

I will defer to networking experts/maintainers on the choice of
-w vs -W

I would like to see the timeout configurable.

thanks,
-- Shuah

Re: [PATCH net-next] selftests: fib_nexthops: Make the test more robust

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (master)
by David S. Miller <davem@davemloft.net>:

On Thu, 12 May 2022 16:12:07 +0300 you wrote:
> Rarely some of the test cases fail. Make the test more robust by increasing
> the timeout of ping commands to 5 seconds.
> 
> Signed-off-by: Amit Cohen <amcohen@nvidia.com>
> ---
>  tools/testing/selftests/net/fib_nexthops.sh | 48 ++++++++++-----------
>  1 file changed, 24 insertions(+), 24 deletions(-)

Here is the summary with links:
  - [net-next] selftests: fib_nexthops: Make the test more robust
    https://git.kernel.org/netdev/net-next/c/49bb39bddad2

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

RE: [PATCH net-next] selftests: fib_nexthops: Make the test more robust

[Amit Cohen]

> -----Original Message-----
> From: Shuah Khan <skhan@linuxfoundation.org>
> Sent: Thursday, May 12, 2022 7:40 PM
> To: Amit Cohen <amcohen@nvidia.com>; netdev@vger.kernel.org
> Cc: davem@davemloft.net; edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; shuah@kernel.org; mlxsw
> <mlxsw@nvidia.com>; dsahern@kernel.org; Shuah Khan <skhan@linuxfoundation.org>
> Subject: Re: [PATCH net-next] selftests: fib_nexthops: Make the test more robust
> 
> On 5/12/22 9:17 AM, Amit Cohen wrote:
> >
> >
> >> -----Original Message-----
> >> From: Shuah Khan <skhan@linuxfoundation.org>
> >> Sent: Thursday, May 12, 2022 5:28 PM
> >> To: Amit Cohen <amcohen@nvidia.com>; netdev@vger.kernel.org
> >> Cc: davem@davemloft.net; edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; shuah@kernel.org; mlxsw
> >> <mlxsw@nvidia.com>; Shuah Khan <skhan@linuxfoundation.org>
> >> Subject: Re: [PATCH net-next] selftests: fib_nexthops: Make the test more robust
> >>
> >> On 5/12/22 7:12 AM, Amit Cohen wrote:
> >>> Rarely some of the test cases fail. Make the test more robust by increasing
> >>> the timeout of ping commands to 5 seconds.
> >>>
> >>
> >> Can you explain why test cases fail?
> >
> > The failures are probably caused due to slow forwarding performance.
> > You can see similar commit - b6a4fd680042 ("selftests: forwarding: Make ping timeout configurable").
> >
> 
> My primary concern is that this patch simply changes the value
> and doesn't make it configurable. Sounds like the above commit
> does that. Why not use the same approach to keep it consistent.
> 
> >>
> >>> Signed-off-by: Amit Cohen <amcohen@nvidia.com>
> >>> ---
> >>>    tools/testing/selftests/net/fib_nexthops.sh | 48 ++++++++++-----------
> >>>    1 file changed, 24 insertions(+), 24 deletions(-)
> >>>
> >>> diff --git a/tools/testing/selftests/net/fib_nexthops.sh b/tools/testing/selftests/net/fib_nexthops.sh
> >>> index b3bf5319bb0e..a99ee3fb2e13 100755
> >>> --- a/tools/testing/selftests/net/fib_nexthops.sh
> >>> +++ b/tools/testing/selftests/net/fib_nexthops.sh
> >>> @@ -882,13 +882,13 @@ ipv6_fcnal_runtime()
> >>>    	log_test $? 0 "Route delete"
> >>>
> >>>    	run_cmd "$IP ro add 2001:db8:101::1/128 nhid 81"
> >>> -	run_cmd "ip netns exec me ping -c1 -w1 2001:db8:101::1"
> >>> +	run_cmd "ip netns exec me ping -c1 -w5 2001:db8:101::1"
> >>>    	log_test $? 0 "Ping with nexthop"
> >>>
> >> Looks like the change uses "-w deadline" - "-W timeout" might
> >> be a better choice if ping fails with no response?
> >
> > We usually use "-w" in ping commands in selftests, but I can change it if you prefer "-W".
> >
> 
> I will defer to networking experts/maintainers on the choice of
> -w vs -W
> 
> I would like to see the timeout configurable.

If you feel strongly about that, I can send another patch to make it configurable.
Personally, I don't think that someone will use it, and from our experience, 5 seconds seems to be enough.


> 
> thanks,
> -- Shuah

Re: [PATCH net-next] selftests: fib_nexthops: Make the test more robust

[Shuah Khan]

On 5/15/22 8:26 AM, Amit Cohen wrote:
> 
> 
>> -----Original Message-----
>> From: Shuah Khan <skhan@linuxfoundation.org>
>> Sent: Thursday, May 12, 2022 7:40 PM
>> To: Amit Cohen <amcohen@nvidia.com>; netdev@vger.kernel.org
>> Cc: davem@davemloft.net; edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; shuah@kernel.org; mlxsw
>> <mlxsw@nvidia.com>; dsahern@kernel.org; Shuah Khan <skhan@linuxfoundation.org>
>> Subject: Re: [PATCH net-next] selftests: fib_nexthops: Make the test more robust
>>
>> On 5/12/22 9:17 AM, Amit Cohen wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Shuah Khan <skhan@linuxfoundation.org>
>>>> Sent: Thursday, May 12, 2022 5:28 PM
>>>> To: Amit Cohen <amcohen@nvidia.com>; netdev@vger.kernel.org
>>>> Cc: davem@davemloft.net; edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; shuah@kernel.org; mlxsw
>>>> <mlxsw@nvidia.com>; Shuah Khan <skhan@linuxfoundation.org>
>>>> Subject: Re: [PATCH net-next] selftests: fib_nexthops: Make the test more robust
>>>>
>>>> On 5/12/22 7:12 AM, Amit Cohen wrote:
>>>>> Rarely some of the test cases fail. Make the test more robust by increasing
>>>>> the timeout of ping commands to 5 seconds.
>>>>>
>>>>
>>>> Can you explain why test cases fail?
>>>
>>> The failures are probably caused due to slow forwarding performance.
>>> You can see similar commit - b6a4fd680042 ("selftests: forwarding: Make ping timeout configurable").
>>>
>>
>> My primary concern is that this patch simply changes the value
>> and doesn't make it configurable. Sounds like the above commit
>> does that. Why not use the same approach to keep it consistent.
>>
>>>>
>>>>> Signed-off-by: Amit Cohen <amcohen@nvidia.com>
>>>>> ---
>>>>>     tools/testing/selftests/net/fib_nexthops.sh | 48 ++++++++++-----------
>>>>>     1 file changed, 24 insertions(+), 24 deletions(-)
>>>>>
>>>>> diff --git a/tools/testing/selftests/net/fib_nexthops.sh b/tools/testing/selftests/net/fib_nexthops.sh
>>>>> index b3bf5319bb0e..a99ee3fb2e13 100755
>>>>> --- a/tools/testing/selftests/net/fib_nexthops.sh
>>>>> +++ b/tools/testing/selftests/net/fib_nexthops.sh
>>>>> @@ -882,13 +882,13 @@ ipv6_fcnal_runtime()
>>>>>     	log_test $? 0 "Route delete"
>>>>>
>>>>>     	run_cmd "$IP ro add 2001:db8:101::1/128 nhid 81"
>>>>> -	run_cmd "ip netns exec me ping -c1 -w1 2001:db8:101::1"
>>>>> +	run_cmd "ip netns exec me ping -c1 -w5 2001:db8:101::1"
>>>>>     	log_test $? 0 "Ping with nexthop"
>>>>>
>>>> Looks like the change uses "-w deadline" - "-W timeout" might
>>>> be a better choice if ping fails with no response?
>>>
>>> We usually use "-w" in ping commands in selftests, but I can change it if you prefer "-W".
>>>
>>
>> I will defer to networking experts/maintainers on the choice of
>> -w vs -W
>>
>> I would like to see the timeout configurable.
> 
> If you feel strongly about that, I can send another patch to make it configurable.
> Personally, I don't think that someone will use it, and from our experience, 5 seconds seems to be enough.
> 

Please make this configurable. It will be consistent with other tests.

thanks,
-- Shuah