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

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]

Date:   Thu, 12 May 2022 10:40:07 -0600
From:   Shuah Khan <skhan@...uxfoundation.org>
To:     Amit Cohen <amcohen@...dia.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Cc:     "davem@...emloft.net" <davem@...emloft.net>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "shuah@...nel.org" <shuah@...nel.org>, mlxsw <mlxsw@...dia.com>,
        "dsahern@...nel.org" <dsahern@...nel.org>,
        Shuah Khan <skhan@...uxfoundation.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@...uxfoundation.org>
>> Sent: Thursday, May 12, 2022 5:28 PM
>> To: Amit Cohen <amcohen@...dia.com>; netdev@...r.kernel.org
>> Cc: davem@...emloft.net; edumazet@...gle.com; kuba@...nel.org; pabeni@...hat.com; shuah@...nel.org; mlxsw
>> <mlxsw@...dia.com>; Shuah Khan <skhan@...uxfoundation.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@...dia.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