netdev - Re: [PATCH net-next 4/4] selftests: drv-net: rss_ctx: add tests for RSS configuration and contexts

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240624075442.58b50bc7@kernel.org>
Date: Mon, 24 Jun 2024 07:54:42 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Edward Cree <ecree.xilinx@...il.com>
Cc: davem@...emloft.net, netdev@...r.kernel.org, edumazet@...gle.com,
 pabeni@...hat.com, willemdebruijn.kernel@...il.com
Subject: Re: [PATCH net-next 4/4] selftests: drv-net: rss_ctx: add tests for
 RSS configuration and contexts

On Mon, 24 Jun 2024 13:55:34 +0100 Edward Cree wrote:
> > +    try:
> > +        # Use queues 0 and 1 for normal traffic
> > +        ethtool(f"-X {cfg.ifname} equal 2")
> > +
> > +        for i in range(ctx_cnt):
> > +            try:
> > +                ctx_id.append(ethtool_create(cfg, "-X", "context new"))
> > +            except CmdExitFailure:
> > +                # try to carry on and skip at the end
> > +                if i == 0:
> > +                    raise
> > +                ksft_pr(f"Failed to create context {i + 1}, trying to test what we got")
> > +                ctx_cnt = i
> > +                break
> > +
> > +            ethtool(f"-X {cfg.ifname} context {ctx_id[i]} start {2 + i * 2} equal 2")  
> 
> Is it worth also testing the single command
>     f"ethtool -X {cfg.ifname} context new start {2 + i * 2} equal 2"
>  as that will exercise the kernel & driver slightly differently to
>  first creating a context and then configuring it?

Will add!

> > diff --git a/tools/testing/selftests/net/lib/py/ksft.py b/tools/testing/selftests/net/lib/py/ksft.py
> > index 4769b4eb1ea1..91648c5baf40 100644
> > --- a/tools/testing/selftests/net/lib/py/ksft.py
> > +++ b/tools/testing/selftests/net/lib/py/ksft.py
> > @@ -57,6 +57,11 @@ KSFT_RESULT_ALL = True
> >          _fail("Check failed", a, "<", b, comment)
> >  
> >  
> > +def ksft_lt(a, b, comment=""):
> > +    if a > b:
> > +        _fail("Check failed", a, ">", b, comment)  
> 
> AFAICT this implements 'le' (less-or-equal), not 'lt' (less than) as
>  the name implies.

Good catch!

> Apart from that these tests LGTM as far as they go.  One thing that I
>  notice *isn't* tested here, that I generally make a point of testing,
>  is: add a bunch of contexts (and ntuple filters), remove some of
>  them, then run your traffic and make sure that the ones you left
>  intact still work (and that the deleted ones are actually gone).

Good idea, will add.

> Also wonder if it's worth adding tests for 'ethtool -x ... context N'?
>  You have it for context 0 in test_rss_key_indir(), but on custom
>  contexts it can exercise different code in the kernel.

Good point, in my head I deferred all "context N read" tests until we
have dump, but the simple read test should be added here.