Re: [PATCH net-next 7/7] testing: net-drv: add a driver test for stats reporting

Jakub Kicinski
2024-04-02T23:36:50Z

On Wed, 3 Apr 2024 00:04:14 +0200 Petr Machata wrote:
> > Yes, I was wondering about that. It must be doable, IIRC
> > the multi-threading API "injects" args from a tuple.
> > I was thinking something along the lines of:
> >
> > with NetDrvEnv(__file__) as cfg:
> > ksft_run([check_pause, check_fec, pkt_byte_sum],
> > args=(cfg, ))
> >
> > I got lazy, let me take a closer look. Another benefit
> > will be that once we pass in "env" / cfg - we can "register"
> > objects in there for auto-cleanup (in the future, current
> > tests don't need cleanup)
>
> Yeah, though some of those should probably just be their own context
> managers IMHO, not necessarily hooked to cfg. I'm thinking something
> fairly general, so that the support boilerplate doesn't end up costing
> an arm and leg:
>
> with build("ip route add 192.0.2.1/28 nexthop via 192.0.2.17",
> "ip route del 192.0.2.1/28"),
> build("ip link set dev %s master %s" % (swp1, h1),
> "ip link set dev %s nomaster" % swp1):
> le_test()
>
> Dunno. I guess it makes sense to have some of the common stuff
> predefined, e.g. "with vrf() as h1". And then the stuff that's typically
> in lib.sh's setup() and cleanup(), can be losslessly hooked up to cfg.

I was thinking of something along the lines of:

def test_abc(cfg):
cfg.build("ip route add 192.0.2.1/28 nexthop via 192.0.2.17",
"ip route del 192.0.2.1/28")
cfg.build("ip link set dev %s master %s" % (swp1, h1),
"ip link set dev %s nomaster" % swp1)

optionally we could then also:

thing = cfg.build("ip link set dev %s master %s" % (swp1, h1),
"ip link set dev %s nomaster" % swp1)

# ... some code which may raise ...

# unlink to do something else with the device
del thing
# ... more code ...

cfg may not be best here, could be cleaner to create a "test" object,
always pass it in as the first param, and destroy it after each test.

> This is what I ended up gravitating towards after writing a handful of
> LNST tests anyway. The scoping makes it clear where the object exists,
> lifetime is taken care of, it's all ponies rainbows basically. At least
> as long as your object lifetimes can be cleanly nested, which admittedly
> is not always.

Should be fairly easy to support all cases - "with", "recording on
cfg/test" and del. Unfortunately in the two tests I came up with
quickly for this series cleanup is only needed for the env itself.
It's a bit awkward to add the lifetime helpers without any users.