netdev - Re: [PATCH net-next v8 5/6] selftests: net: Add busy_poll

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: <672e4ea4e979a_2bc2f629490@willemb.c.googlers.com.notmuch>
Date: Fri, 08 Nov 2024 12:47:16 -0500
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Joe Damato <jdamato@...tly.com>, 
 Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc: netdev@...r.kernel.org, 
 corbet@....net, 
 hdanton@...a.com, 
 bagasdotme@...il.com, 
 pabeni@...hat.com, 
 namangulati@...gle.com, 
 edumazet@...gle.com, 
 amritha.nambiar@...el.com, 
 sridhar.samudrala@...el.com, 
 sdf@...ichev.me, 
 peter@...eblog.net, 
 m2shafiei@...terloo.ca, 
 bjorn@...osinc.com, 
 hch@...radead.org, 
 willy@...radead.org, 
 skhawaja@...gle.com, 
 kuba@...nel.org, 
 Martin Karsten <mkarsten@...terloo.ca>, 
 "David S. Miller" <davem@...emloft.net>, 
 Simon Horman <horms@...nel.org>, 
 Shuah Khan <shuah@...nel.org>, 
 open list <linux-kernel@...r.kernel.org>, 
 "open list:KERNEL SELFTEST FRAMEWORK" <linux-kselftest@...r.kernel.org>
Subject: Re: [PATCH net-next v8 5/6] selftests: net: Add busy_poll_test

Joe Damato wrote:
> On Fri, Nov 08, 2024 at 09:57:48AM -0500, Willem de Bruijn wrote:
> > Joe Damato wrote:
> > > Add an epoll busy poll test using netdevsim.
> > > 
> > > This test is comprised of:
> > >   - busy_poller (via busy_poller.c)
> > >   - busy_poll_test.sh which loads netdevsim, sets up network namespaces,
> > >     and runs busy_poller to receive data and socat to send data.
> > > 
> > > The selftest tests two different scenarios:
> > >   - busy poll (the pre-existing version in the kernel)
> > >   - busy poll with suspend enabled (what this series adds)
> > > 
> > > The data transmit is a 1MiB temporary file generated from /dev/urandom
> > > and the test is considered passing if the md5sum of the input file to
> > > socat matches the md5sum of the output file from busy_poller.
> > 
> > Nice test.
> > 
> > Busy polling does not affect data integrity. Is the goal of this test
> > mainly to get coverage, maybe observe if the process would stall
> > indefinitely?
> 
> Just to get coverage and make sure data makes it from point A to
> point B intact despite suspend being enabled.
> 
> The last paragraph of the commit message highlights that netdevsim
> functionality is limited, so the test uses what is available. It can
> be extended in the future, when netdevsim supports more
> functionality.
> 
> Paolo wanted a test and this is the best test we can provide given
> the limitations of the testing environment.
> 
> > > netdevsim was chosen instead of veth due to netdevsim's support for
> > > netdev-genl.
> > > 
> > > For now, this test uses the functionality that netdevsim provides. In the
> > > future, perhaps netdevsim can be extended to emulate device IRQs to more
> > > thoroughly test all pre-existing kernel options (like defer_hard_irqs)
> > > and suspend.
> 
> [...]
> 
> The rest of the feedback below seems pretty minor; I don't think
> it's worth spinning a v9 and re-sending just for this.
> 
> If anything this can be handled with a clean up commit in the
> future.

FWIW no objections from me.

> Jakub: please let me know if you prefer to see a v9 for this?