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 :(
