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

Joe Damato
2025-02-19T18:39:54Z

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:
> > Removing this check causes a stack trace on my XDP-disabled kernel,
> > whereas with the existing code it caused a skip.
> >
> > Maybe that's OK, though?
> >
> > The issue is that xdp_helper.c fails and exits with return -1 before
> > the call to ksft_ready() which results in the following:
> >
> > # Exception| Traceback (most recent call last):
> > # Exception| File "/home/jdamato/code/net-next/tools/testing/selftests/net/lib/py/ksft.py", line 223, in ksft_run
> > # Exception| case(*args)
> > # Exception| File "/home/jdamato/code/net-next/./tools/testing/selftests/drivers/net/queues.py", line 27, in check_xsk
> > # Exception| with bkg(f'{cfg.rpath("xdp_helper")} {cfg.ifindex} {xdp_queue_id}',
> > # Exception| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > # Exception| File "/home/jdamato/code/net-next/tools/testing/selftests/net/lib/py/utils.py", line 108, in __init__
> > # Exception| super().__init__(comm, background=True,
> > # Exception| File "/home/jdamato/code/net-next/tools/testing/selftests/net/lib/py/utils.py", line 63, in __init__
> > # Exception| raise Exception("Did not receive ready message")
> > # Exception| Exception: Did not receive ready message
> > not ok 4 queues.check_xsk
> > # Totals: pass:3 fail:1 xfail:0 xpass:0 skip:0 error:0
> >
> > I had originally modified the test so that if XDP is disabled in the
> > kernel it would skip, but I think you mentioned in a previous thread
> > that this was a "non-goal", IIRC ?
> >
> > No strong opinion on my side as to what the behavior should be when
> > XDP is disabled, but wanted to mention this so that the behavior
> > change was known.
>
> I thought of doing this:
>
> diff --git a/tools/testing/selftests/drivers/net/xdp_helper.c b/tools/testing/selftests/drivers/net/xdp_helper.c
> index 8f77da4f798f..8c34e8915fc4 100644
> --- a/tools/testing/selftests/drivers/net/xdp_helper.c
> +++ b/tools/testing/selftests/drivers/net/xdp_helper.c
> @@ -53,7 +53,7 @@ int main(int argc, char **argv)
> int queue;
> char byte;
>
> - if (argc != 3) {
> + if (argc > 1 && argc != 3) {
> fprintf(stderr, "Usage: %s ifindex queue_id", argv[0]);
> return 1;
> }
> @@ -69,6 +69,13 @@ int main(int argc, char **argv)
> return 1;
> }
>
> +
> + if (argc == 1) {
> + printf("AF_XDP support detected\n");
> + close(sock_fd);
> + return 0;
> + }
> +
> ifindex = atoi(argv[1]);
> queue = atoi(argv[2]);
>
>
> 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:

https://lore.kernel.org/netdev/878qq22xk3.fsf@kurt.kurt.home/T/#m4d0778b9e849bc72064074105182f4c84ba55eb2

> > 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.

>
> For me the xdp process doesn't wait at all. Running this under vng
> and Python 3.13 the read returns 0 immediately.

Interesting, so something changed between 3.12.3 and 3.13, I
suppose.

> Even if it doesn't we run bkg() with default params, so exit_wait=False
> init will set:
>
> self.terminate = not exit_wait
>
> and then __exit__ will do:
>
> return self.process(terminate=self.terminate, fail=self.check_fail)
>
> which does:
>
> if self.terminate:
> self.proc.terminate()
>
> so the helper should get a SIGINT, no?

(Minor nit, the python docs say terminate delivers a SIGTERM, but
please continue reading)

I'm not a python programmer but did try a bit to debug this. Looking
at strace on my system, it seems that the script forks twice and the
SIGTERM is sent to the wrong pid?

I suppose it forks twice because the helper is invoked via /bin/sh:

18:27:15 vfork(strace: Process 448303 attached
<unfinished ...>

So, pid 448303 is the first forked process, which does an execve:

[pid 448303] 18:27:15 execve("/bin/sh", ["/bin/sh", "-c", "/home/jdamato/code/net-next/tools/testing/selftests/drivers/net/xdp_helper 5 0"], 0x7f661511fad0 /* 26 vars */ <unfinished ...>

So /bin/sh runs and that then forks again to run the helper:

[pid 448278] 18:27:15 <... vfork resumed>) = 448303

[pid 448303] 18:27:15 vfork(strace: Process 448304 attached
<unfinished ...>

[pid 448304] 18:27:15 execve("/home/jdamato/code/net-next/tools/testing/selftests/drivers/net/xdp_helper", ["/home/jdamato/code/net-next/tools/testing/selftests/drivers/net/xdp_helper", "5", "0"], 0x55faf4f81918 /* 26 vars */ <unfinished ...>

So pid 448304 is the process doing the execve to run xdp_helper and
would need to be the one getting the SIGTERM/SIGINT

We can see it does the read:

[pid 448304] 18:27:15 read(0, <unfinished ...>

However, later the python script does the terminate flow you've
described in your message but sends the signal to the /bin/sh that
was forked off:

[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.

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

> 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?