netdev - Re: [PATCH net-next v3 6/7] 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: <ZyU24Kogy3HlzNqx@LQ3V64L9R2>
Date: Fri, 1 Nov 2024 13:15:28 -0700
From: Joe Damato <jdamato@...tly.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: netdev@...r.kernel.org, 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, willemdebruijn.kernel@...il.com,
	skhawaja@...gle.com, 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 v3 6/7] selftests: net: Add busy_poll_test

On Fri, Nov 01, 2024 at 06:34:26AM -0700, Jakub Kicinski wrote:
> On Fri,  1 Nov 2024 00:48:33 +0000 Joe Damato wrote:
> > +	ip netns exec nscl nc -N 192.168.1.1 48675 < $tmp_file
> 
> Thanks a lot for adding the test.

Thanks for the review.

> Could you replace nc with socat or
> a similar tool? There are multiple incompatible implementations of
> netcat packaged by various distros, we get:
> 
> # selftests: net: busy_poll_test.sh
> # nc: invalid option -- 'N'
> # Ncat: Try `--help' or man(1) ncat for more information, usage options and help. QUITTING.
> 
> nc is a known PITA.

OK, I've replaced with socat for the v4 I am working on. I've also
added some additional documentation to the FAQ in the cover letter
and the kernel documentation to help answer Sridhar's question
because it may be a question others have as well.

> > +	# set the suspend parameter for the server via its IFIDX
> > +
> > +	DUMP_CMD="${YNL_PATH} --spec ${SPEC_PATH} --dump napi-get --json=\"{\\\"ifindex\\\": ${NSIM_DEV_1_IFIDX}}\" --output-json"
> > +	NSIM_DEV_1_NAPIID=$(ip netns exec nssv bash -c "$DUMP_CMD")
> > +	NSIM_DEV_1_NAPIID=$(echo $NSIM_DEV_1_NAPIID | jq '.[] | .id')
> > +
> > +	SUSPEND_CMD="${YNL_PATH} --spec ${SPEC_PATH} --do napi-set --json=\"{\\\"id\\\": ${NSIM_DEV_1_NAPIID}, \\\"irq-suspend-timeout\\\": 20000000, \\\"gro-flush-timeout\\\": 50000, \\\"defer-hard-irqs\\\": 100}\""
> > +	NSIM_DEV_1_SETCONFIG=$(ip netns exec nssv bash -c "$SUSPEND_CMD")
> 
> Can you try to run this test in installed mode?
> 
> https://docs.kernel.org/dev-tools/kselftest.html#install-selftests
> 
> IIRC YNL moves around when we install, you'd either need to do
> autodetection of the path (see tools/testing/selftests/net/lib/py/ynl.py
> and if you go down this route please move the helper which exports the
> YNL variables to lib.sh so other tests can reuse); or teach the C code
> to do the setup, you can link against YNL fairly easily (look at where
> ncdevmem is added in the Makefile, it uses YNL)

I think I'm going to go the C route.

Just to make sure I'm following how to do this properly:
  - I've added busy_poller to YNL_GEN_FILES (and removed it from
    TEST_GEN_FILES)
  - It still needs to be run via the script (which sets up netdevsim
    etc), so I am leaving that script (busy_poll_test.sh) in
    TEST_PROGS. busy_poller is not intended to be run on its own for
    selftest purposes.

I am not sure if YNL_GEN_FILES are expected to run standalone when
invoked or if this will work as I expect it to (busy_poll_test.sh is
run):

  [...]
  TEST_PROGS += busy_poll_test.sh
  
  # YNL files, must be before "include ..lib.mk"
  YNL_GEN_FILES := ncdevmem busy_poller
  [...]

Doing the above: I am hoping that busy_poll_test.sh will be run
(which will use busy_poller) and that busy_poller won't be run on
its own.

Thanks for the guidance.