Subject: Re: [PATCH net-next v2 07/12] selftests: ncdevmem: Properly reset flow steering

From: Stanislav Fomichev <[email-masked]>
Date: Thu, 3 Oct 2024 09:42:21 -0700

On 10/03, Mina Almasry wrote:
> On Mon, Sep 30, 2024 at 10:18 AM Stanislav Fomichev <[email-masked]> wrote:
> >
> > ntuple off/on might be not enough to do it on all NICs.
> > Add a bunch of shell crap to explicitly remove the rules.
> >
> > Cc: Mina Almasry <[email-masked]>
> > Signed-off-by: Stanislav Fomichev <[email-masked]>
> > ---
> >  tools/testing/selftests/net/ncdevmem.c | 13 ++++++-------
> >  1 file changed, 6 insertions(+), 7 deletions(-)
> >
> > diff --git a/tools/testing/selftests/net/ncdevmem.c b/tools/testing/selftests/net/ncdevmem.c
> > index 47458a13eff5..48cbf057fde7 100644
> > --- a/tools/testing/selftests/net/ncdevmem.c
> > +++ b/tools/testing/selftests/net/ncdevmem.c
> > @@ -207,13 +207,12 @@ void validate_buffer(void *line, size_t size)
> >
> >  static int reset_flow_steering(void)
> >  {
> > -       int ret = 0;
> > -
> > -       ret = run_command("sudo ethtool -K %s ntuple off >&2", ifname);
> > -       if (ret)
> > -               return ret;
> > -
> > -       return run_command("sudo ethtool -K %s ntuple on >&2", ifname);
> > +       run_command("sudo ethtool -K %s ntuple off >&2", ifname);
> > +       run_command("sudo ethtool -K %s ntuple on >&2", ifname);
> > +       run_command(
> > +               "sudo ethtool -n %s | grep 'Filter:' | awk '{print $2}' | xargs -n1 ethtool -N %s delete >&2",
> > +               ifname, ifname);
> > +       return 0;
> 
> Any reason to remove the checking of the return codes? Silent failures
> can waste time if the test fails and someone has to spend time finding
> out its the flow steering reset that failed (it may not be very
> obvious without the checking of the return code.

IIRC, for me the 'ntuple off' part fails because the NIC doesn't let me
turn it of. And the new "ethtool .. | grep 'Filter: ' | ..." also fails
when there are no existing filters. 

I will add a comment to clarify..