netdev - Re: [PATCH net-next 1/5] selftests: drv-net: rss

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: <668c1983f09b5_1960bd294c3@willemb.c.googlers.com.notmuch>
Date: Mon, 08 Jul 2024 12:53:23 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Jakub Kicinski <kuba@...nel.org>, 
 Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc: davem@...emloft.net, 
 netdev@...r.kernel.org, 
 edumazet@...gle.com, 
 pabeni@...hat.com, 
 petrm@...dia.com, 
 przemyslaw.kitszel@...el.com, 
 ecree.xilinx@...il.com
Subject: Re: [PATCH net-next 1/5] selftests: drv-net: rss_ctx: fix cleanup in
 the basic test

Jakub Kicinski wrote:
> On Sat, 06 Jul 2024 09:53:41 -0400 Willem de Bruijn wrote:
> > > @@ -89,6 +88,7 @@ from lib.py import ethtool, ip, defer, GenerateTraffic, CmdExitFailure
> > >  
> > >      # Set the indirection table
> > >      ethtool(f"-X {cfg.ifname} equal 2")
> > > +    reset_indir = defer(ethtool, f"-X {cfg.ifname} default")
> > >      data = get_rss(cfg)
> > >      ksft_eq(0, min(data['rss-indirection-table']))
> > >      ksft_eq(1, max(data['rss-indirection-table']))
> > > @@ -104,7 +104,7 @@ from lib.py import ethtool, ip, defer, GenerateTraffic, CmdExitFailure
> > >      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")
> > > +    reset_indir.exec()  
> > 
> > When is this explicit exec needed?
> 
> When you want to run the cleanup _now_.
> 
> We construct the cleanup as soon as we allocate the resource,
> it stays on the deferred list in case some exception makes us abort,
> but the test may want to free the resource or reconfigure it further
> as part of the test, in which case it can run .exec() (or cancel() to
> discard the clean up without running it).
> 
> Here we do:
>  1. constrain RSS
>  2. run traffic (to check we're hitting expected queues)
>  3. reset RSS
>  4. run traffic (to check we're hitting all queues)
> 
> so step 3 runs the cleanup of step 1 explicitly.

Thanks. I was wondering why this test calls it explicitly, while
others do not. Had overlooked step 4, which requires the reset.