diff mbox series

[net,v2] selftests: net: cope with slow env in gro.sh test

Message ID c777f75ac70e70aabf1398cefa5c51c0f4ea00f2.1707330768.git.pabeni@redhat.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net,v2] selftests: net: cope with slow env in gro.sh test | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 8 this patch: 8
netdev/build_tools success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 8 of 8 maintainers
netdev/build_clang success Errors and warnings before: 8 this patch: 8
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success net selftest script(s) already in Makefile
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 8 this patch: 8
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 10 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-02-10--12-00 (tests: 1262)

Commit Message

Paolo Abeni Feb. 7, 2024, 6:36 p.m. UTC
The gro self-tests sends the packets to be aggregated with
multiple write operations.

When running is slow environment, it's hard to guarantee that
the GRO engine will wait for the last packet in an intended
train.

The above causes almost deterministic failures in our CI for
the 'large' test-case.

Address the issue explicitly ignoring failures for such case
in slow environments (KSFT_MACHINE_SLOW==true).

Fixes: 7d1575014a63 ("selftests/net: GRO coalesce test")
Reviewed-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
v1 -> v2:
  - replace the '-a' operator with '&&' - Mattbe

Note that the fixes tag is there mainly to justify targeting the net
tree, and this is aiming at net to hopefully make the test more stable
ASAP for both trees.

I experimented with a largish refactory replacing the multiple writes
with a single GSO packet, but exhausted by time budget before reaching
any good result.
---
 tools/testing/selftests/net/gro.sh | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Matthieu Baerts Feb. 7, 2024, 10:14 p.m. UTC | #1
Hi Paolo,

On 07/02/2024 19:36, Paolo Abeni wrote:
> The gro self-tests sends the packets to be aggregated with
> multiple write operations.
> 
> When running is slow environment, it's hard to guarantee that
> the GRO engine will wait for the last packet in an intended
> train.
> 
> The above causes almost deterministic failures in our CI for
> the 'large' test-case.
> 
> Address the issue explicitly ignoring failures for such case
> in slow environments (KSFT_MACHINE_SLOW==true).
> 
> Fixes: 7d1575014a63 ("selftests/net: GRO coalesce test")
> Reviewed-by: Willem de Bruijn <willemb@google.com>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> v1 -> v2:
>   - replace the '-a' operator with '&&' - Mattbe

Thank you for the v2!

Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>

Cheers,
Matt
Jakub Kicinski Feb. 9, 2024, 7:15 p.m. UTC | #2
On Wed,  7 Feb 2024 19:36:46 +0100 Paolo Abeni wrote:
> +    if [[ ${test} == "large" && -n "${KSFT_MACHINE_SLOW}" ]]; then
> +        echo "Ignoring errors due to slow environment" 1>&2
> +        exit_code=0
> +    fi

Would it make sense to also add "&& $exit_code -ne 0" ?
It may be useful to see in logs how many times we actually
ignored the error?
Paolo Abeni Feb. 12, 2024, 8:40 a.m. UTC | #3
On Fri, 2024-02-09 at 11:15 -0800, Jakub Kicinski wrote:
> On Wed,  7 Feb 2024 19:36:46 +0100 Paolo Abeni wrote:
> > +    if [[ ${test} == "large" && -n "${KSFT_MACHINE_SLOW}" ]]; then
> > +        echo "Ignoring errors due to slow environment" 1>&2
> > +        exit_code=0
> > +    fi
> 
> Would it make sense to also add "&& $exit_code -ne 0" ?
> It may be useful to see in logs how many times we actually
> ignored the error?

Yep, I'll send a v3.

Thanks!

Paolo
diff mbox series

Patch

diff --git a/tools/testing/selftests/net/gro.sh b/tools/testing/selftests/net/gro.sh
index 19352f106c1d..3190b41e8bfc 100755
--- a/tools/testing/selftests/net/gro.sh
+++ b/tools/testing/selftests/net/gro.sh
@@ -31,6 +31,10 @@  run_test() {
       1>>log.txt
     wait "${server_pid}"
     exit_code=$?
+    if [[ ${test} == "large" && -n "${KSFT_MACHINE_SLOW}" ]]; then
+        echo "Ignoring errors due to slow environment" 1>&2
+        exit_code=0
+    fi
     if [[ "${exit_code}" -eq 0 ]]; then
         break;
     fi