[11/11] selftests: net/fcnal: Reduce client timeout (2023)

Reduce default client timeout from 5 seconds to 500 miliseconds.Can be overridden from environment by exporting NETTEST_CLIENT_TIMEOUT=5Some tests need ICMP timeouts so pass an explicit -t5 for those.Signed-off-by: Leonard Crestez <cdleonard@gmail.com>--- tools/testing/selftests/net/fcnal-test.sh | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)

On 10/6/21 5:47 AM, Leonard Crestez wrote:> Reduce default client timeout from 5 seconds to 500 miliseconds.> Can be overridden from environment by exporting NETTEST_CLIENT_TIMEOUT=5> > Some tests need ICMP timeouts so pass an explicit -t5 for those.> > Signed-off-by: Leonard Crestez <cdleonard@gmail.com>> ---> tools/testing/selftests/net/fcnal-test.sh | 17 +++++++++++------> 1 file changed, 11 insertions(+), 6 deletions(-)> The problem with blindly reducing the timeouts is running the script ona loaded server. Some tests are expected to timeout while for tests atimeout is a failure.

On 06.10.2021 18:01, David Ahern wrote:> On 10/6/21 5:47 AM, Leonard Crestez wrote:>> Reduce default client timeout from 5 seconds to 500 miliseconds.>> Can be overridden from environment by exporting NETTEST_CLIENT_TIMEOUT=5>>>> Some tests need ICMP timeouts so pass an explicit -t5 for those.>>>> Signed-off-by: Leonard Crestez <cdleonard@gmail.com>>> --->> tools/testing/selftests/net/fcnal-test.sh | 17 +++++++++++------>> 1 file changed, 11 insertions(+), 6 deletions(-)>>> > The problem with blindly reducing the timeouts is running the script on> a loaded server. Some tests are expected to timeout while for tests a> timeout is a failure.Keeping the default value "5" would be fine as long as it is possible to override externally and get fast results on a mostly-idle machine.Placing a default value in the environment which is overriden by certain tests achieves that.In theory it would also be possible for fcnal-test.sh to parse as "--timeout" option and pass it into every single test but that solution would cause much more code churn.Having default values in environment variables that can still be overridden by command-line arguments is a common pattern in many tools. It also avoids having to pass-through every flag through every intermediate wrapper.--Regards,Leonard

On 10/6/21 3:26 PM, Leonard Crestez wrote:> > > On 06.10.2021 18:01, David Ahern wrote:>> On 10/6/21 5:47 AM, Leonard Crestez wrote:>>> Reduce default client timeout from 5 seconds to 500 miliseconds.>>> Can be overridden from environment by exporting NETTEST_CLIENT_TIMEOUT=5>>>>>> Some tests need ICMP timeouts so pass an explicit -t5 for those.>>>>>> Signed-off-by: Leonard Crestez <cdleonard@gmail.com>>>> --->>> tools/testing/selftests/net/fcnal-test.sh | 17 +++++++++++------>>> 1 file changed, 11 insertions(+), 6 deletions(-)>>>>>>> The problem with blindly reducing the timeouts is running the script on>> a loaded server. Some tests are expected to timeout while for tests a>> timeout is a failure.> > Keeping the default value "5" would be fine as long as it is possible to> override externally and get fast results on a mostly-idle machine.5 is the default for nettest.c; the test script passes in -t1 for all tests.> > Placing a default value in the environment which is overriden by certain> tests achieves that.> > In theory it would also be possible for fcnal-test.sh to parse as> "--timeout" option and pass it into every single test but that solution> would cause much more code churn.> > Having default values in environment variables that can still be> overridden by command-line arguments is a common pattern in many tools.> It also avoids having to pass-through every flag through every> intermediate wrapper.I do not agree with env variables here.

On 07.10.2021 04:17, David Ahern wrote:> On 10/6/21 3:26 PM, Leonard Crestez wrote:>> On 06.10.2021 18:01, David Ahern wrote:>>> On 10/6/21 5:47 AM, Leonard Crestez wrote:>>>> Reduce default client timeout from 5 seconds to 500 miliseconds.>>>> Can be overridden from environment by exporting NETTEST_CLIENT_TIMEOUT=5>>>>>>>> Some tests need ICMP timeouts so pass an explicit -t5 for those.>>>>>>>> Signed-off-by: Leonard Crestez <cdleonard@gmail.com>>>>> --->>>> tools/testing/selftests/net/fcnal-test.sh | 17 +++++++++++------>>>> 1 file changed, 11 insertions(+), 6 deletions(-)>>>>>>>>>> The problem with blindly reducing the timeouts is running the script on>>> a loaded server. Some tests are expected to timeout while for tests a>>> timeout is a failure.>>>> Keeping the default value "5" would be fine as long as it is possible to>> override externally and get fast results on a mostly-idle machine.> > 5 is the default for nettest.c; the test script passes in -t1 for all tests.An explicit -t is only passed for some of the tests$ grep -c nettest.*-r tools/testing/selftests/net/fcnal-test.sh243$ grep -c nettest.*-t tools/testing/selftests/net/fcnal-test.sh15>> Placing a default value in the environment which is overriden by certain>> tests achieves that.>>>> In theory it would also be possible for fcnal-test.sh to parse as>> "--timeout" option and pass it into every single test but that solution>> would cause much more code churn.>>>> Having default values in environment variables that can still be>> overridden by command-line arguments is a common pattern in many tools.>> It also avoids having to pass-through every flag through every>> intermediate wrapper.> > I do not agree with env variables here.Would you agree with adding an option to fcnal-test.sh which decreases timeouts passed to nettest client calls?

On 10/7/21 2:52 PM, Leonard Crestez wrote:> Would you agree with adding an option to fcnal-test.sh which decreases> timeouts passed to nettest client calls?sure

Patch

diff --git a/tools/testing/selftests/net/fcnal-test.sh b/tools/testing/selftests/net/fcnal-test.shindex e73aeb3884c5..cf5dd96bb9db 100755--- a/tools/testing/selftests/net/fcnal-test.sh+++ b/tools/testing/selftests/net/fcnal-test.sh@@ -40,10 +40,15 @@  # Kselftest framework requirement - SKIP code is 4. ksft_skip=4 VERBOSE=0 +# Use a reduced client timeout by default+# Can be overridden by user+NETTEST_CLIENT_TIMEOUT=${NETTEST_CLIENT_TIMEOUT:-0.5}+export NETTEST_CLIENT_TIMEOUT+ NSA_DEV=eth1 NSA_DEV2=eth2 NSB_DEV=eth1 NSC_DEV=eth2 VRF=red@@ -1076,11 +1081,11 @@  ipv4_tcp_novrf() for a in ${NSA_LO_IP} 127.0.0.1 do log_start show_hint "Should fail 'No route to host' since addresses on loopback are out of device scope" run_cmd nettest -s -k-run_cmd nettest -r ${a} -d ${NSA_DEV}+run_cmd nettest -r ${a} -d ${NSA_DEV} -t5 log_test_addr ${a} $? 1 "Global server, device client, local connection" done a=${NSA_IP} log_start@@ -1379,23 +1384,23 @@  ipv4_udp_novrf() for a in ${NSA_LO_IP} 127.0.0.1 do log_start show_hint "Should fail since addresses on loopback are out of device scope" run_cmd nettest -D -s -k-run_cmd nettest -D -r ${a} -d ${NSA_DEV}+run_cmd nettest -D -r ${a} -d ${NSA_DEV} -t5 log_test_addr ${a} $? 2 "Global server, device client, local connection" log_start show_hint "Should fail since addresses on loopback are out of device scope" run_cmd nettest -D -s -k-run_cmd nettest -D -r ${a} -d ${NSA_DEV} -C+run_cmd nettest -D -r ${a} -d ${NSA_DEV} -C -t5 log_test_addr ${a} $? 1 "Global server, device send via cmsg, local connection" log_start show_hint "Should fail since addresses on loopback are out of device scope" run_cmd nettest -D -s -k-run_cmd nettest -D -r ${a} -d ${NSA_DEV} -S+run_cmd nettest -D -r ${a} -d ${NSA_DEV} -S -t5 log_test_addr ${a} $? 1 "Global server, device client via IP_UNICAST_IF, local connection" done a=${NSA_IP} log_start@@ -3443,11 +3448,11 @@  netfilter_icmp() for a in ${NSA_IP} ${VRF_IP} do log_start run_cmd nettest ${arg} -s -k-run_cmd_nsb nettest ${arg} -r ${a}+run_cmd_nsb nettest ${arg} -r ${a} -t5 log_test_addr ${a} $? 1 "Global ${stype} server, Rx reject icmp-port-unreach" done } ipv4_netfilter()@@ -3498,11 +3503,11 @@  netfilter_icmp6() for a in ${NSA_IP6} ${VRF_IP6} do log_start run_cmd nettest -6 -s ${arg} -k-run_cmd_nsb nettest -6 ${arg} -r ${a}+run_cmd_nsb nettest -6 ${arg} -r ${a} -t5 log_test_addr ${a} $? 1 "Global ${stype} server, Rx reject icmp-port-unreach" done } ipv6_netfilter()