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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240402163649.4fdc2d3b@kernel.org>
Date: Tue, 2 Apr 2024 16:36:49 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Petr Machata <petrm@...dia.com>
Cc: <davem@...emloft.net>, <netdev@...r.kernel.org>, <edumazet@...gle.com>,
 <pabeni@...hat.com>, <shuah@...nel.org>, <sdf@...gle.com>,
 <donald.hunter@...il.com>, <linux-kselftest@...r.kernel.org>
Subject: Re: [PATCH net-next 7/7] testing: net-drv: add a driver test for
 stats reporting

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.