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

Joe Damato
2025-02-18T21:52:44Z

On Tue, Feb 18, 2025 at 11:50:46AM -0800, Jakub Kicinski wrote:
> We use wait_port_listen() extensively to wait for a process
> we spawned to be ready. Not all processes will open listening
> sockets. Add a method of explicitly waiting for a child to
> be ready. Pass a FD to the spawned process and wait for it
> to write a message to us. FD number is passed via KSFT_READY_FD
> env variable.
>
> Make use of this method in the queues test to make it less flaky.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> .../selftests/drivers/net/xdp_helper.c | 22 ++++++-
> tools/testing/selftests/drivers/net/queues.py | 46 ++++++---------
> tools/testing/selftests/net/lib/py/utils.py | 58 +++++++++++++++++--
> 3 files changed, 93 insertions(+), 33 deletions(-)
>
> diff --git a/tools/testing/selftests/drivers/net/xdp_helper.c b/tools/testing/selftests/drivers/net/xdp_helper.c
> index cf06a88b830b..8f77da4f798f 100644
> --- a/tools/testing/selftests/drivers/net/xdp_helper.c
> +++ b/tools/testing/selftests/drivers/net/xdp_helper.c
> @@ -14,6 +14,25 @@
> #define UMEM_SZ (1U << 16)
> #define NUM_DESC (UMEM_SZ / 2048)
>
> +/* Move this to a common header when reused! */
> +static void ksft_ready(void)
> +{
> + const char msg[7] = "ready\n";
> + char *env_str;
> + int fd;
> +
> + env_str = getenv("KSFT_READY_FD");
> + if (!env_str)
> + return;
> +
> + fd = atoi(env_str);
> + if (!fd)
> + return;
> +
> + write(fd, msg, sizeof(msg));
> + close(fd);
> +}
> +
> /* this is a simple helper program that creates an XDP socket and does the
> * minimum necessary to get bind() to succeed.
> *
> @@ -85,8 +104,7 @@ int main(int argc, char **argv)
> return 1;
> }
>
> - /* give the parent program some data when the socket is ready*/
> - fprintf(stdout, "%d\n", sock_fd);
> + ksft_ready();
>
> /* parent program will write a byte to stdin when its ready for this
> * helper to exit
> diff --git a/tools/testing/selftests/drivers/net/queues.py b/tools/testing/selftests/drivers/net/queues.py
> index b6896a57a5fd..91e344d108ee 100755
> --- a/tools/testing/selftests/drivers/net/queues.py
> +++ b/tools/testing/selftests/drivers/net/queues.py
> @@ -5,13 +5,12 @@ from lib.py import ksft_disruptive, ksft_exit, ksft_run
> from lib.py import ksft_eq, ksft_raises, KsftSkipEx, KsftFailEx
> from lib.py import EthtoolFamily, NetdevFamily, NlError
> from lib.py import NetDrvEnv
> -from lib.py import cmd, defer, ip
> +from lib.py import bkg, cmd, defer, ip
> import errno
> import glob
> import os
> import socket
> import struct
> -import subprocess
>
> def sys_get_queues(ifname, qtype='rx') -> int:
> folders = glob.glob(f'/sys/class/net/{ifname}/queues/{qtype}-*')
> @@ -25,37 +24,30 @@ import subprocess
> return None
>
> def check_xdp(cfg, nl, xdp_queue_id=0) -> None:
> - xdp = subprocess.Popen([cfg.rpath("xdp_helper"), f"{cfg.ifindex}", f"{xdp_queue_id}"],
> - stdin=subprocess.PIPE, stdout=subprocess.PIPE, bufsize=1,
> - text=True)
> - defer(xdp.kill)
> + with bkg(f'{cfg.rpath("xdp_helper")} {cfg.ifindex} {xdp_queue_id}',
> + wait_init=3):
>
> - stdout, stderr = xdp.communicate(timeout=10)
> - rx = tx = False
> + rx = tx = False
>
> - if xdp.returncode == 255:
> - raise KsftSkipEx('AF_XDP unsupported')
> - elif xdp.returncode > 0:

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.

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?