netdev - Re: [PATCH net-next 2/4] selftests: drv-net: add a way to wait for a local process

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: <20250219144844.1206b1fc@kernel.org>
Date: Wed, 19 Feb 2025 14:48:44 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: Joe Damato <jdamato@...tly.com>
Cc: davem@...emloft.net, netdev@...r.kernel.org, edumazet@...gle.com,
 pabeni@...hat.com, andrew+netdev@...n.ch, horms@...nel.org,
 shuah@...nel.org, hawk@...nel.org, petrm@...dia.com,
 willemdebruijn.kernel@...il.com
Subject: Re: [PATCH net-next 2/4] selftests: drv-net: add a way to wait for
 a local process

On Wed, 19 Feb 2025 13:39:51 -0500 Joe Damato wrote:
> On Tue, Feb 18, 2025 at 03:05:12PM -0800, Jakub Kicinski wrote:
> > On Tue, 18 Feb 2025 16:52:39 -0500 Joe Damato wrote:  
> > Then we can run the helper with no arguments, just to check if af_xdp
> > is supported. If that returns 0 we go on, otherwise we print your nice
> > error.
> > 
> > LMK if that sounds good, assuming a respin is needed I can add that :)  
> 
> That seems to fine; if you do decide to go this route in a re-spin,
> would you mind also adding a "\n" in the fprintf after "ifindex
> queue_id" ? Sorry I missed that on my initial implementation.
> 
> That missing \n was mentioned by Kurt in another thread:

Will do!

> > > Separately: I retested this on a machine with XDP enabled, both with
> > > and without NETIF set and the test seems to hang because the helper
> > > is blocked on:
> > > 
> > > read(STDIN_FILENO, &byte, 1);
> > > 
> > > according to strace:
> > > 
> > > strace: Process 14198 attached
> > > 21:50:02 read(0,
> > > 
> > > So, I think this patch needs to be tweaked to write a byte to the
> > > helper so it exits (I assume before the defer was killing it?) or
> > > the helper needs to be modified in way?  
> > 
> > What Python version do you have?   
> 
> Python 3.12.3 (via Ubuntu 24.04.1 LTS).
> 
> I can re-test with a different version using pyenv if you'd like?
> 
> Are there docs which mention which python version tests should be
> compatible with? If so, could you pass along a link? Sorry if I
> missed that.

You're good, sorry, tests are supposed to run on any non-EOL
version of python.

<snip>
> [pid 448278] 18:27:15 kill(448303, SIGTERM) = 0
> [...]
> [pid 448303] 18:27:15 +++ killed by SIGTERM +++
> 
> But pid 448304 is xdp_helper, which is still running and should be
> the one to get the TERM.

Very interesting. I dug deeper into this, and it turns out its shell
dependent. I'm guessing you're using one of the cool shells, I use
bash. bash does a direct exec for "sh -c X", other shells fork first.

I'll add a warning in bkg() for combining shell=True and terminate=True.

> I have no idea why this would be different on your system vs mine.
> Maybe something changed with Python between Python versions?

More digging still necessary here, as NIPA also runs on bash.
So your problem is different than NIPA's.
NIPA runs:

  make -C tools/testing/selftests TARGETS="drivers/net" \
	  TEST_PROGS=queues.py TEST_GEN_PROGS="" run_tests

which runs thru a layer of perl for output prefixing:

	tools/testing/selftests/kselftest/prefix.pl

which in turn send a SIGTTIN when we call read(), and hangs the helper.

> > We shall find out if NIPA agrees with my local system at 4p.  
> 
> Sorry for the noob question, but is there a NIPA url or something I
> can look at to see if this worked / if future tests I submit work?

https://netdev.bots.linux.dev/status.html

With the disclaimer that we discourage people from looking at it. 
It tests everything on  the list combined, we can't support the
corporate "try until CI is green" development model. Not that 
I'd accuse you of such practices :)