Re: [PATCH net-next v2] selftests: drv-net: replace the nsim ring test with a drv-net one

[Jakub Kicinski <kuba@kernel.org>]

On Mon, 27 Oct 2025 20:46:00 +0100 Andrew Lunn wrote:
> > +    def test_config(config):
> > +        try:
> > +            cfg.eth.channels_set(ehdr | config)
> > +            get = cfg.eth.channels_get(ehdr)
> > +            for k, v in config.items():
> > +                ksft_eq(get.get(k, 0), v)
> > +        except NlError as e:
> > +            failed.append(mix)
> > +            ksft_pr("Can't set", config, e)
> > +        else:
> > +            ksft_pr("Okay", config)  
> 
> We expect failure to leave the configuration unchanged. So i would
> actually do:
> 
> try:
> 	before = get()
> 	set()
> except:
> 	after = get()
> 	fail(after != before)

Please allow me to introduce you to the magic of defer() ;)
This registers a command to run after the test completely exits:

+    defer(cfg.eth.channels_set, ehdr | restore)

> Also, does nlError contain the error code?
> 
>         fail(e.errcode not in (EINVAL, EOPNOTSUPP))
> 
> It would be good to detect and fail ENOTSUPP, which does appear every
> so often, when it should not.

Dunno, checkpatch warns about ENOTSUPP. I don't that think checking 
for funny error codes in every test scales :(

> > +    # Try to reach min on all settings
> > +    for param in params:
> > +        val = rings[param]
> > +        while True:
> > +            try:
> > +                cfg.eth.rings_set({'header':{'dev-index': cfg.ifindex},
> > +                                   param: val // 2})
> > +                val //= 2
> > +                if val <= 1:
> > +                    break
> > +            except NlError:
> > +                break  
> 
> Is 0 ever valid? I would actually test 0 and make sure it fails with
> EINVAL, or EOPNOTSUPP. Getting range checks wrong is a typical bug, so
> it is good to test them. The happy days cases are boring because
> developers tend to test those, so they are hardly worth testings. Its
> the edge cases which should be tested.

I believe that 0 is a valid settings. I don't have much experience with
devices which support it. But presumably using 0 to disable mini/jumbo
rings would make sense for example? And max validation is done by the
core so nothing interesting to explore there at the driver level :(