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

[Jakub Kicinski <kuba@kernel.org>]

On Sun, 23 Jun 2024 04:03:49 -0400 Willem de Bruijn wrote:
> > +def test_rss_key_indir(cfg):
> > +    """
> > +    Test basics like updating the main RSS key and indirection table.
> > +    """
> > +    data = get_rss(cfg)
> > +    want_keys = ['rss-hash-key', 'rss-hash-function', 'rss-indirection-table']
> > +    for k in want_keys:
> > +        if k not in data:
> > +            raise KsftFailEx("ethtool results missing key: " + k)
> > +        if not data[k]:
> > +            raise KsftFailEx(f"ethtool results empty for '{k}': {data[k]}")  
> 
> No point in printing data[k]?

It can be one of many things which evaluate to False: None, False, [],
""..

> > +
> > +    key_len = len(data['rss-hash-key'])
> > +
> > +    # Set the key
> > +    key = _rss_key_rand(key_len)
> > +    ethtool(f"-X {cfg.ifname} hkey " + _rss_key_str(key))  
> 
> Probably too paranoid, but in case failure is only for some randomized
> input, is the key logged on error?

Will add!

> > +    data = get_rss(cfg)
> > +    ksft_eq(key, data['rss-hash-key'])
> > +
> > +    # Set the indirection table
> > +    ethtool(f"-X {cfg.ifname} equal 2")
> > +    data = get_rss(cfg)
> > +    ksft_eq(0, min(data['rss-indirection-table']))
> > +    ksft_eq(1, max(data['rss-indirection-table']))
> > +
> > +    # Check we only get traffic on the first 2 queues
> > +    cnts = _get_rx_cnts(cfg)
> > +    GenerateTraffic(cfg).wait_pkts_and_stop(20000)
> > +    cnts = _get_rx_cnts(cfg, prev=cnts)
> > +    # 2 queues, 20k packets, must be at least 5k per queue
> > +    ksft_ge(cnts[0], 5000, "traffic on main context (1/2): " + str(cnts))
> > +    ksft_ge(cnts[1], 5000, "traffic on main context (2/2): " + str(cnts))
> > +    # The other queues should be unused
> > +    ksft_eq(sum(cnts[2:]), 0, "traffic on unused queues: " + str(cnts))
> > +
> > +    # Restore, and check traffic gets spread again
> > +    ethtool(f"-X {cfg.ifname} default")  
> 
> Consider save and restore state at the start of the test, in case
> default is overridden at boot.
> 
> Not important, but this also repeats some of toeplitz.sh. That has not
> been integrated into net-drv, and the .c and .sh code is more verbose
> than this python code. Perhaps can be replaced entirely eventually.

Agreed, I went with "default" because there seems to be no ethtool
support for setting exact indir table :(  I deferred exact restore
until we have YNL support for setting, which should be soon.

> > +            ethtool(f"-X {cfg.ifname} context {ctx_id[i]} start {2 + i * 2} equal 2")
> > +
> > +            ports.append(rand_port())
> > +            flow = f"flow-type tcp{cfg.addr_ipver} dst-port {ports[i]} context {ctx_id[i]}"
> > +            ntuple.append(ethtool_create(cfg, "-N", flow))  
> 
> Need to test feature ('-k') ntuple and skip test otherwise or set?

Will do!