On Wed, 2024-02-07 at 12:16 +0100, Matthieu Baerts wrote:
> Hi Paolo,
>
> On 06/02/2024 16:27, 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).
>
> To what value is KSFT_MACHINE_SLOW set in the CI?
AFAIK, the CI initialize KSFT_MACHINE_SLOW (to true) only on slow env.
> Is it set to a different value if the machine is not slow? e.g.
>
> KSFT_MACHINE_SLOW == false
>
> (please see below)
>
> > Fixes: 7d1575014a63 ("selftests/net: GRO coalesce test")
> > Signed-off-by: Paolo Abeni <pabeni@xxxxxxxxxx>
> > ---
> > 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(+)
> >
> > diff --git a/tools/testing/selftests/net/gro.sh b/tools/testing/selftests/net/gro.sh
> > index 19352f106c1d..114b5281a3f5 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" -a -n "${KSFT_MACHINE_SLOW}" ]; then
>
> Maybe best to avoid using:
>
> -n "${KSFT_MACHINE_SLOW}"
>
> Otherwise, we have the same behaviour if KSFT_MACHINE_SLOW is set to
> 1/yes/true or 0/no/false.
For consistency, I followed the logic already in place in commit
c41dfb0dfbec ("selftests/net: ignore timing errors in so_txtime if
KSFT_MACHINE_SLOW").
> But maybe it is fine like that, and what is just missing is adding
> somewhere how KSFT_MACHINE_SLOW is supposed to be set/used? :)
>
>
> Not linked to that, but a small detail about the new line, just in case
> you need to send a v2: it looks like it is better to avoid using '-a':
>
> https://www.shellcheck.net/wiki/SC2166
Thank for the pointer, I was not aware of that.
I guess a v2 dropping '-a' would be better.
Thanks,
Paolo
