Re: [LTP] [PATCH 1/1] net: tst_netload_compare(): Ignore performance failures

Martin Doucha
2024-01-02T17:08:21Z

Hi,

On 19. 12. 23 13:37, Petr Vorel wrote:
> Performance failures in tests which use tst_netload_compare() (tests in
> runtest/net.features) can hide a real error (e.g. test fails due missing
> required kernel module). Best solution would be to have feature tests
> (likely written in C API) and performance tests (the current ones).
>
> But until it happens, just disable performance failure by default,
> print TINFO message instead unless TST_NET_FEATURES_TEST_PERFORMANCE=1
> environment variable is set.
>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> testcases/lib/tst_net.sh | 12 +++++++++---
> testcases/lib/tst_test.sh | 5 +++--
> testcases/network/README.md | 4 ++++
> 3 files changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/testcases/lib/tst_net.sh b/testcases/lib/tst_net.sh
> index e47ee9676..46d49c266 100644
> --- a/testcases/lib/tst_net.sh
> +++ b/testcases/lib/tst_net.sh
> @@ -869,16 +869,22 @@ tst_netload_compare()
> local new_time=$2
> local threshold_low=$3
> local threshold_hi=$4
> + local ttype='TFAIL'
> + local msg res
>
> if [ -z "$base_time" -o -z "$new_time" -o -z "$threshold_low" ]; then
> tst_brk_ TBROK "tst_netload_compare: invalid argument(s)"
> fi
>
> - local res=$(((base_time - new_time) * 100 / base_time))
> - local msg="performance result is ${res}%"
> + res=$(((base_time - new_time) * 100 / base_time))
> + msg="performance result is ${res}%"
>
> if [ "$res" -lt "$threshold_low" ]; then
> - tst_res_ TFAIL "$msg < threshold ${threshold_low}%"
> + if [ -z "$TST_NET_FEATURES_TEST_PERFORMANCE" ]; then
> + ttype='TINFO';
> + tst_res_ TINFO "WARNING: slow performance is not treated as error, change it with TST_NET_FEATURES_TEST_PERFORMANCE=1"

This TINFO message should probably be moved to tst_net_setup().
Otherwise some tests will print it multiple times.

I'd also slightly prefer to keep the default as is and use a variable to
disable perf checks instead.

> + fi
> + tst_res_ $ttype "$msg < threshold ${threshold_low}%"
> return
> fi
>
> diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
> index 5f178a1be..06ee6005a 100644
> --- a/testcases/lib/tst_test.sh
> +++ b/testcases/lib/tst_test.sh
> @@ -1,6 +1,6 @@
> #!/bin/sh
> # SPDX-License-Identifier: GPL-2.0-or-later
> -# Copyright (c) Linux Test Project, 2014-2022
> +# Copyright (c) Linux Test Project, 2014-2023
> # Author: Cyril Hrubis <chrubis@suse.cz>
> #
> # LTP test library for shell.
> @@ -681,7 +681,8 @@ tst_run()
> NEEDS_KCONFIGS|NEEDS_KCONFIGS_IFS);;
> IPV6|IPV6_FLAG|IPVER|TEST_DATA|TEST_DATA_IFS);;
> RETRY_FUNC|RETRY_FN_EXP_BACKOFF|TIMEOUT);;
> - NET_DATAROOT|NET_MAX_PKT|NET_RHOST_RUN_DEBUG|NETLOAD_CLN_NUMBER);;
> + NETLOAD_CLN_NUMBER|NET_DATAROOT|NET_FEATURES_TEST_PERFORMANCE);;
> + NET_MAX_PKT|NET_RHOST_RUN_DEBUG);;
> NET_SKIP_VARIABLE_INIT|NEEDS_CHECKPOINTS);;
> CHECKPOINT_WAIT|CHECKPOINT_WAKE);;
> CHECKPOINT_WAKE2|CHECKPOINT_WAKE_AND_WAIT);;
> diff --git a/testcases/network/README.md b/testcases/network/README.md
> index a0a1d3d95..0547c3f9f 100644
> --- a/testcases/network/README.md
> +++ b/testcases/network/README.md
> @@ -84,6 +84,10 @@ Where
> Default values for all LTP network parameters are set in `testcases/lib/tst_net.sh`.
> Network stress parameters are documented in `testcases/network/stress/README`.
>
> +Tests which use `tst_netload_compare()` test just basic functionality,
> +performance failure is just printed with 'TINFO'. To enable also performance
> +testing, set `TST_NET_FEATURES_TEST_PERFORMANCE=1` environment variable.
> +
> ## Debugging
> Both single and two host configurations support debugging via
> `TST_NET_RHOST_RUN_DEBUG=1` environment variable.

--
Martin Doucha mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic