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

[Jakub Kicinski <kuba@kernel.org>]

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.