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: <20240620192605.3ddba920@kernel.org>
Date: Thu, 20 Jun 2024 19:26:05 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: David Wei <dw@...idwei.uk>
Cc: davem@...emloft.net, netdev@...r.kernel.org, edumazet@...gle.com,
 pabeni@...hat.com, willemdebruijn.kernel@...il.com, ecree.xilinx@...il.com
Subject: Re: [PATCH net-next 4/4] selftests: drv-net: rss_ctx: add tests for
 RSS configuration and contexts

On Thu, 20 Jun 2024 18:52:21 -0700 David Wei wrote:
> On 2024-06-20 16:29, Jakub Kicinski wrote:
> > +def get_rss(cfg):
> > +    return ethtool(f"-x {cfg.ifname}", json=True)[0]  
> 
> At this point I think json=True can be the default.

for ethtool specifically I'm constantly annoyed by how many sub-commands
don't support JSON. I don't think we'll win much, and the grand plan
is to have a netlink API for all of this, and switch to YNL calls.

> > +    cnts = _get_rx_cnts(cfg)
> > +    GenerateTraffic(cfg).wait_pkts_and_stop(20000)
> > +    cnts = _get_rx_cnts(cfg, prev=cnts)
> > +    # First two queues get less traffic than all the rest
> > +    ksft_ge(sum(cnts[2:]), sum(cnts[:2]), "traffic distributed: " + str(cnts))  
> 
> Do you need to check the number of queues? If it's 3 then would this
> check potentially fail?

Yeah, I lazy'd out, because ethtool -l doesn't support JSON! Ugh.
At least on my machine. I'll add the check base on qstat, like I did
in one of the later tests.

> > +        for i in range(ctx_cnt):
> > +            cnts = _get_rx_cnts(cfg)
> > +            GenerateTraffic(cfg, port=ports[i]).wait_pkts_and_stop(20000)
> > +            cnts = _get_rx_cnts(cfg, prev=cnts)
> > +
> > +            ksft_lt(sum(cnts[ :2]), 10000, "traffic on main context:" + str(cnts))  
> 
> What if the host is getting significant traffic during the test?

Then IDK how to write a reasonable RSS test :) Hopefully 10k compared
to the 20k of iperf is pretty safe, but we'll find out...

> > +            ksft_ge(sum(cnts[2+i*2:4+i*2]), 20000, f"traffic on context {i}: " + str(cnts))  
> 
> Is this exactly 20000?

Not sure I follow but ge means >=, iperf will probably do some more
before we manage to stop it.

> > +
> > +def test_rss_context_overlap2(cfg):
> > +    test_rss_context_overlap(cfg, True)  
> 
> Add a test case for other_ctx=0?

context 0 (i.e. the main one) will get tested when called directly.
Annoyingly ethtool userspace is a bit picky about using context 0
as an id on the command line, hence the special handling in the test.