[PATCH net-next v2 0/7] selftests: drv-net: improve the queue test for XSK

[PATCH net-next v2 0/7] selftests: drv-net: improve the queue test for XSK

[Jakub Kicinski]

We see some flakes in the the XSK test:

   Exception| Traceback (most recent call last):
   Exception|   File "/home/virtme/testing-18/tools/testing/selftests/net/lib/py/ksft.py", line 218, in ksft_run
   Exception|     case(*args)
   Exception|   File "/home/virtme/testing-18/tools/testing/selftests/drivers/net/./queues.py", line 53, in check_xdp
   Exception|     ksft_eq(q['xsk'], {})
   Exception| KeyError: 'xsk'

I think it's because the method or running the helper in the background
is racy. Add more solid infra for waiting for a background helper to be
initialized.

v2:
 - add patch 1, 3 and 4
 - redo patch 5
v1: https://lore.kernel.org/20250218195048.74692-1-kuba@kernel.org

Jakub Kicinski (7):
  selftests: drv-net: add a warning for bkg + shell + terminate
  selftests: drv-net: use cfg.rpath() in netlink xsk attr test
  selftests: drv-net: add missing new line in xdp_helper
  selftests: drv-net: probe for AF_XDP sockets more explicitly
  selftests: drv-net: add a way to wait for a local process
  selftests: drv-net: improve the use of ksft helpers in XSK queue test
  selftests: drv-net: rename queues check_xdp to check_xsk

 .../selftests/drivers/net/xdp_helper.c        | 63 ++++++++++++++--
 tools/testing/selftests/drivers/net/queues.py | 61 ++++++++--------
 tools/testing/selftests/net/lib/py/ksft.py    |  5 ++
 tools/testing/selftests/net/lib/py/utils.py   | 72 +++++++++++++++++--
 4 files changed, 161 insertions(+), 40 deletions(-)

-- 
2.48.1

[PATCH net-next v2 1/7] selftests: drv-net: add a warning for bkg + shell + terminate

[Jakub Kicinski]

Joe Damato reports that some shells will fork before running
the command when python does "sh -c $cmd", while bash does
an exec of $cmd directly. This will have implications for our
ability to terminate the child process on bash vs other shells.
Warn about using

	bkg(... shell=True, termininate=True)

most background commands can hopefully exit cleanly (exit_wait).

Link: https://lore.kernel.org/Z7Yld21sv_Ip3gQx@LQ3V64L9R2
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
v2: new
---
 tools/testing/selftests/net/lib/py/utils.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/testing/selftests/net/lib/py/utils.py b/tools/testing/selftests/net/lib/py/utils.py
index 9e3bcddcf3e8..33b153767d89 100644
--- a/tools/testing/selftests/net/lib/py/utils.py
+++ b/tools/testing/selftests/net/lib/py/utils.py
@@ -61,6 +61,10 @@ import time
         self.terminate = not exit_wait
         self.check_fail = fail
 
+        if shell and self.terminate:
+            print("# Warning: combining shell and terminate is risky!")
+            print("#          SIGTERM may not reach the child on zsh/ksh!")
+
     def __enter__(self):
         return self
 
-- 
2.48.1

Re: [PATCH net-next v2 1/7] selftests: drv-net: add a warning for bkg + shell + terminate

[Stanislav Fomichev]

On 02/19, Jakub Kicinski wrote:
> Joe Damato reports that some shells will fork before running
> the command when python does "sh -c $cmd", while bash does
> an exec of $cmd directly. This will have implications for our
> ability to terminate the child process on bash vs other shells.
> Warn about using
> 
> 	bkg(... shell=True, termininate=True)
> 
> most background commands can hopefully exit cleanly (exit_wait).
> 
> Link: https://lore.kernel.org/Z7Yld21sv_Ip3gQx@LQ3V64L9R2
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Acked-by: Stanislav Fomichev <sdf@fomichev.me>

Re: [PATCH net-next v2 1/7] selftests: drv-net: add a warning for bkg + shell + terminate

[Joe Damato]

On Wed, Feb 19, 2025 at 03:49:50PM -0800, Jakub Kicinski wrote:
> Joe Damato reports that some shells will fork before running
> the command when python does "sh -c $cmd", while bash does
> an exec of $cmd directly.

I'm not sure what's going on, but as I mentioned in the other thread
and below, I am using bash as well.

> This will have implications for our
> ability to terminate the child process on bash vs other shells.
> Warn about using
> 
> 	bkg(... shell=True, termininate=True)
> 
> most background commands can hopefully exit cleanly (exit_wait).
> 
> Link: https://lore.kernel.org/Z7Yld21sv_Ip3gQx@LQ3V64L9R2
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> v2: new
> ---
>  tools/testing/selftests/net/lib/py/utils.py | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tools/testing/selftests/net/lib/py/utils.py b/tools/testing/selftests/net/lib/py/utils.py
> index 9e3bcddcf3e8..33b153767d89 100644
> --- a/tools/testing/selftests/net/lib/py/utils.py
> +++ b/tools/testing/selftests/net/lib/py/utils.py
> @@ -61,6 +61,10 @@ import time
>          self.terminate = not exit_wait
>          self.check_fail = fail
>  
> +        if shell and self.terminate:
> +            print("# Warning: combining shell and terminate is risky!")
> +            print("#          SIGTERM may not reach the child on zsh/ksh!")

I'm not opposed to putting a warning here, but just as a disclaimer
and for anyone else following along -- I am using bash:

$ echo $SHELL
/bin/bash

$ bash --version
GNU bash, version 5.2.21(1)-release (x86_64-pc-linux-gnu)

Re: [PATCH net-next v2 1/7] selftests: drv-net: add a warning for bkg + shell + terminate

[Joe Damato]

On Wed, Feb 19, 2025 at 03:49:50PM -0800, Jakub Kicinski wrote:
> Joe Damato reports that some shells will fork before running
> the command when python does "sh -c $cmd", while bash does
> an exec of $cmd directly. This will have implications for our
> ability to terminate the child process on bash vs other shells.
> Warn about using
> 
> 	bkg(... shell=True, termininate=True)
> 
> most background commands can hopefully exit cleanly (exit_wait).
> 
> Link: https://lore.kernel.org/Z7Yld21sv_Ip3gQx@LQ3V64L9R2
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> v2: new
> ---
>  tools/testing/selftests/net/lib/py/utils.py | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tools/testing/selftests/net/lib/py/utils.py b/tools/testing/selftests/net/lib/py/utils.py
> index 9e3bcddcf3e8..33b153767d89 100644
> --- a/tools/testing/selftests/net/lib/py/utils.py
> +++ b/tools/testing/selftests/net/lib/py/utils.py
> @@ -61,6 +61,10 @@ import time
>          self.terminate = not exit_wait
>          self.check_fail = fail
>  
> +        if shell and self.terminate:
> +            print("# Warning: combining shell and terminate is risky!")
> +            print("#          SIGTERM may not reach the child on zsh/ksh!")
> +

This warning did not print on my system, FWIW. Up to you if you want
to respin and drop it or just leave it be.

I am fine with this warning being added, although I disagree with
the commit message as mentioned in my previous email.

Since the code seems fine though:

Acked-by: Joe Damato <jdamato@fastly.com>

Re: [PATCH net-next v2 1/7] selftests: drv-net: add a warning for bkg + shell + terminate

[Jakub Kicinski]

On Thu, 20 Feb 2025 14:31:35 -0500 Joe Damato wrote:
> > +        if shell and self.terminate:
> > +            print("# Warning: combining shell and terminate is risky!")
> > +            print("#          SIGTERM may not reach the child on zsh/ksh!")
> > +  
> 
> This warning did not print on my system, FWIW. Up to you if you want
> to respin and drop it or just leave it be.

You mean when running this patchset? It shouldn't, the current 
test uses ksft_wait rather than terminate. The warning is for
test developers trying to use the risky config in new tests.

> I am fine with this warning being added, although I disagree with
> the commit message as mentioned in my previous email.

I'll edit the commit message when applying. Unless you want to dig
deeper and find out why your bash/env is different than mine :(

GNU bash, version 5.2.32(1)-release (x86_64-redhat-linux-gnu)

We know that "some shells" fork, that's the important point.

Re: [PATCH net-next v2 1/7] selftests: drv-net: add a warning for bkg + shell + terminate

[Joe Damato]

On Thu, Feb 20, 2025 at 01:10:59PM -0800, Jakub Kicinski wrote:
> On Thu, 20 Feb 2025 14:31:35 -0500 Joe Damato wrote:
> > > +        if shell and self.terminate:
> > > +            print("# Warning: combining shell and terminate is risky!")
> > > +            print("#          SIGTERM may not reach the child on zsh/ksh!")
> > > +  
> > 
> > This warning did not print on my system, FWIW. Up to you if you want
> > to respin and drop it or just leave it be.
> 
> You mean when running this patchset? It shouldn't, the current 
> test uses ksft_wait rather than terminate. The warning is for
> test developers trying to use the risky config in new tests.
> 
> > I am fine with this warning being added, although I disagree with
> > the commit message as mentioned in my previous email.
> 
> I'll edit the commit message when applying. Unless you want to dig
> deeper and find out why your bash/env is different than mine :(

No, no - no need for that much extra work. Just wanted to mention
for anyone following along or who stumbles on this in the future
that my shell is bash, is all :)

Thanks again for the work on this.

[PATCH net-next v2 2/7] selftests: drv-net: use cfg.rpath() in netlink xsk attr test

[Jakub Kicinski]

The cfg.rpath() helper was been recently added to make formatting
paths for helper binaries easier.

Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Reviewed-by: Joe Damato <jdamato@fastly.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 tools/testing/selftests/drivers/net/queues.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/queues.py b/tools/testing/selftests/drivers/net/queues.py
index 5fdfebc6415f..b6896a57a5fd 100755
--- a/tools/testing/selftests/drivers/net/queues.py
+++ b/tools/testing/selftests/drivers/net/queues.py
@@ -25,8 +25,7 @@ import subprocess
     return None
 
 def check_xdp(cfg, nl, xdp_queue_id=0) -> None:
-    test_dir = os.path.dirname(os.path.realpath(__file__))
-    xdp = subprocess.Popen([f"{test_dir}/xdp_helper", f"{cfg.ifindex}", f"{xdp_queue_id}"],
+    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)
-- 
2.48.1

[PATCH net-next v2 3/7] selftests: drv-net: add missing new line in xdp_helper

[Jakub Kicinski]

Kurt and Joe report missing new line at the end of Usage.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
v2: new
---
 tools/testing/selftests/drivers/net/xdp_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/drivers/net/xdp_helper.c b/tools/testing/selftests/drivers/net/xdp_helper.c
index cf06a88b830b..80f86c2fe1a5 100644
--- a/tools/testing/selftests/drivers/net/xdp_helper.c
+++ b/tools/testing/selftests/drivers/net/xdp_helper.c
@@ -35,7 +35,7 @@ int main(int argc, char **argv)
 	char byte;
 
 	if (argc != 3) {
-		fprintf(stderr, "Usage: %s ifindex queue_id", argv[0]);
+		fprintf(stderr, "Usage: %s ifindex queue_id\n", argv[0]);
 		return 1;
 	}
 
-- 
2.48.1

Re: [PATCH net-next v2 3/7] selftests: drv-net: add missing new line in xdp_helper

[Kurt Kanzenbach]

[-- Attachment #1: Type: text/plain, Size: 205 bytes --]

On Wed Feb 19 2025, Jakub Kicinski wrote:
> Kurt and Joe report missing new line at the end of Usage.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Reviewed-by: Kurt Kanzenbach <kurt@linutronix.de>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

Re: [PATCH net-next v2 3/7] selftests: drv-net: add missing new line in xdp_helper

[Joe Damato]

On Wed, Feb 19, 2025 at 03:49:52PM -0800, Jakub Kicinski wrote:
> Kurt and Joe report missing new line at the end of Usage.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> v2: new
> ---
>  tools/testing/selftests/drivers/net/xdp_helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/drivers/net/xdp_helper.c b/tools/testing/selftests/drivers/net/xdp_helper.c
> index cf06a88b830b..80f86c2fe1a5 100644
> --- a/tools/testing/selftests/drivers/net/xdp_helper.c
> +++ b/tools/testing/selftests/drivers/net/xdp_helper.c
> @@ -35,7 +35,7 @@ int main(int argc, char **argv)
>  	char byte;
>  
>  	if (argc != 3) {
> -		fprintf(stderr, "Usage: %s ifindex queue_id", argv[0]);
> +		fprintf(stderr, "Usage: %s ifindex queue_id\n", argv[0]);
>  		return 1;
>  	}

Reviewed-by: Joe Damato <jdamato@fastly.com>

[PATCH net-next v2 4/7] selftests: drv-net: probe for AF_XDP sockets more explicitly

[Jakub Kicinski]

Separate the support check from socket binding for easier refactoring.
Use: ./helper - - just to probe if we can open the socket.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
v2: new
---
 tools/testing/selftests/drivers/net/xdp_helper.c |  7 +++++++
 tools/testing/selftests/drivers/net/queues.py    | 12 +++++++-----
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/xdp_helper.c b/tools/testing/selftests/drivers/net/xdp_helper.c
index 80f86c2fe1a5..2bad3b4d616c 100644
--- a/tools/testing/selftests/drivers/net/xdp_helper.c
+++ b/tools/testing/selftests/drivers/net/xdp_helper.c
@@ -50,6 +50,13 @@ int main(int argc, char **argv)
 		return 1;
 	}
 
+	/* "Probing mode", just checking if AF_XDP sockets are supported */
+	if (!strcmp(argv[1], "-") && !strcmp(argv[2], "-")) {
+		printf("AF_XDP support detected\n");
+		close(sock_fd);
+		return 0;
+	}
+
 	ifindex = atoi(argv[1]);
 	queue = atoi(argv[2]);
 
diff --git a/tools/testing/selftests/drivers/net/queues.py b/tools/testing/selftests/drivers/net/queues.py
index b6896a57a5fd..0c959b2eb618 100755
--- a/tools/testing/selftests/drivers/net/queues.py
+++ b/tools/testing/selftests/drivers/net/queues.py
@@ -25,6 +25,13 @@ import subprocess
     return None
 
 def check_xdp(cfg, nl, xdp_queue_id=0) -> None:
+    # Probe for support
+    xdp = cmd(cfg.rpath("xdp_helper") + ' - -', fail=False)
+    if xdp.ret == 255:
+        raise KsftSkipEx('AF_XDP unsupported')
+    elif xdp.ret > 0:
+        raise KsftFailEx('unable to create AF_XDP socket')
+
     xdp = subprocess.Popen([cfg.rpath("xdp_helper"), f"{cfg.ifindex}", f"{xdp_queue_id}"],
                            stdin=subprocess.PIPE, stdout=subprocess.PIPE, bufsize=1,
                            text=True)
@@ -33,11 +40,6 @@ import subprocess
     stdout, stderr = xdp.communicate(timeout=10)
     rx = tx = False
 
-    if xdp.returncode == 255:
-        raise KsftSkipEx('AF_XDP unsupported')
-    elif xdp.returncode > 0:
-        raise KsftFailEx('unable to create AF_XDP socket')
-
     queues = nl.queue_get({'ifindex': cfg.ifindex}, dump=True)
     if not queues:
         raise KsftSkipEx("Netlink reports no queues")
-- 
2.48.1

Re: [PATCH net-next v2 4/7] selftests: drv-net: probe for AF_XDP sockets more explicitly

[Stanislav Fomichev]

On 02/19, Jakub Kicinski wrote:
> Separate the support check from socket binding for easier refactoring.
> Use: ./helper - - just to probe if we can open the socket.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Acked-by: Stanislav Fomichev <sdf@fomichev.me>

Re: [PATCH net-next v2 4/7] selftests: drv-net: probe for AF_XDP sockets more explicitly

[Joe Damato]

On Wed, Feb 19, 2025 at 03:49:53PM -0800, Jakub Kicinski wrote:
> Separate the support check from socket binding for easier refactoring.
> Use: ./helper - - just to probe if we can open the socket.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> v2: new
> ---
>  tools/testing/selftests/drivers/net/xdp_helper.c |  7 +++++++
>  tools/testing/selftests/drivers/net/queues.py    | 12 +++++++-----
>  2 files changed, 14 insertions(+), 5 deletions(-)
>

I've tested this on a kernel with XDP enabled and also a kernel with
XDP disabled and the change appears to work as intended.

Here's what it looks like on a kernel with XDP disabled:

KTAP version 1
1..4
ok 1 queues.get_queues
2ok 2 queues.addremove_queues
ok 3 queues.check_down
# 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 33, in check_xsk
# Exception|     raise KsftFailEx('unable to create AF_XDP socket')
# Exception| net.lib.py.ksft.KsftFailEx: unable to create AF_XDP socket
not ok 4 queues.check_xsk
# Totals: pass:3 fail:1 xfail:0 xpass:0 skip:0 error:0

Reviewed-by: Joe Damato <jdamato@fastly.com>

[PATCH net-next v2 5/7] selftests: drv-net: add a way to wait for a local process

[Jakub Kicinski]

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.

Similarly use KSFT_WAIT_FD to let the child process for a sign
that we are done and child should exit. Sending a signal to
a child with shell=True can get tricky.

Make use of this method in the queues test to make it less flaky.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
v2:
 - use fd for exit, too
 - warn if env variables are bad
v1: https://lore.kernel.org/20250218195048.74692-3-kuba@kernel.org
---
 .../selftests/drivers/net/xdp_helper.c        | 54 +++++++++++++--
 tools/testing/selftests/drivers/net/queues.py | 43 ++++++------
 tools/testing/selftests/net/lib/py/utils.py   | 68 +++++++++++++++++--
 3 files changed, 133 insertions(+), 32 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/xdp_helper.c b/tools/testing/selftests/drivers/net/xdp_helper.c
index 2bad3b4d616c..aeed25914104 100644
--- a/tools/testing/selftests/drivers/net/xdp_helper.c
+++ b/tools/testing/selftests/drivers/net/xdp_helper.c
@@ -14,6 +14,54 @@
 #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) {
+		fd = atoi(env_str);
+		if (!fd) {
+			fprintf(stderr, "invalid KSFT_READY_FD = '%s'\n",
+				env_str);
+			return;
+		}
+	} else {
+		fd = STDOUT_FILENO;
+	}
+
+	write(fd, msg, sizeof(msg));
+	if (fd != STDOUT_FILENO)
+		close(fd);
+}
+
+static void ksft_wait(void)
+{
+	char *env_str;
+	char byte;
+	int fd;
+
+	env_str = getenv("KSFT_WAIT_FD");
+	if (env_str) {
+		fd = atoi(env_str);
+		if (!fd) {
+			fprintf(stderr, "invalid KSFT_WAIT_FD = '%s'\n",
+				env_str);
+			return;
+		}
+	} else {
+		/* Not running in KSFT env, wait for input from STDIN instead */
+		fd = STDIN_FILENO;
+	}
+
+	read(fd, &byte, sizeof(byte));
+	if (fd != STDIN_FILENO)
+		close(fd);
+}
+
 /* this is a simple helper program that creates an XDP socket and does the
  * minimum necessary to get bind() to succeed.
  *
@@ -32,7 +80,6 @@ int main(int argc, char **argv)
 	int ifindex;
 	int sock_fd;
 	int queue;
-	char byte;
 
 	if (argc != 3) {
 		fprintf(stderr, "Usage: %s ifindex queue_id\n", argv[0]);
@@ -92,13 +139,12 @@ 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();
+	ksft_wait();
 
 	/* parent program will write a byte to stdin when its ready for this
 	 * helper to exit
 	 */
-	read(STDIN_FILENO, &byte, 1);
 
 	close(sock_fd);
 	return 0;
diff --git a/tools/testing/selftests/drivers/net/queues.py b/tools/testing/selftests/drivers/net/queues.py
index 0c959b2eb618..7af2adb61c25 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}-*')
@@ -32,32 +31,30 @@ import subprocess
     elif xdp.ret > 0:
         raise KsftFailEx('unable to create AF_XDP socket')
 
-    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}',
+             ksft_wait=3):
 
-    stdout, stderr = xdp.communicate(timeout=10)
-    rx = tx = False
+        rx = tx = False
 
-    queues = nl.queue_get({'ifindex': cfg.ifindex}, dump=True)
-    if not queues:
-        raise KsftSkipEx("Netlink reports no queues")
+        queues = nl.queue_get({'ifindex': cfg.ifindex}, dump=True)
+        if not queues:
+            raise KsftSkipEx("Netlink reports no queues")
 
-    for q in queues:
-        if q['id'] == 0:
-            if q['type'] == 'rx':
-                rx = True
-            if q['type'] == 'tx':
-                tx = True
+        for q in queues:
+            if q['id'] == 0:
+                if q['type'] == 'rx':
+                    rx = True
+                if q['type'] == 'tx':
+                    tx = True
 
-            ksft_eq(q['xsk'], {})
-        else:
-            if 'xsk' in q:
-                _fail("Check failed: xsk attribute set.")
+                ksft_eq(q['xsk'], {})
+            else:
+                if 'xsk' in q:
+                    _fail("Check failed: xsk attribute set.")
+
+        ksft_eq(rx, True)
+        ksft_eq(tx, True)
 
-    ksft_eq(rx, True)
-    ksft_eq(tx, True)
 
 def get_queues(cfg, nl) -> None:
     snl = NetdevFamily(recv_size=4096)
diff --git a/tools/testing/selftests/net/lib/py/utils.py b/tools/testing/selftests/net/lib/py/utils.py
index 33b153767d89..d879700ef2b9 100644
--- a/tools/testing/selftests/net/lib/py/utils.py
+++ b/tools/testing/selftests/net/lib/py/utils.py
@@ -2,8 +2,10 @@
 
 import errno
 import json as _json
+import os
 import random
 import re
+import select
 import socket
 import subprocess
 import time
@@ -15,21 +17,56 @@ import time
         self.cmd = cmd_obj
 
 
+def fd_read_timeout(fd, timeout):
+    rlist, _, _ = select.select([fd], [], [], timeout)
+    if rlist:
+        return os.read(fd, 1024)
+    else:
+        raise TimeoutError("Timeout waiting for fd read")
+
+
 class cmd:
-    def __init__(self, comm, shell=True, fail=True, ns=None, background=False, host=None, timeout=5):
+    """
+    Execute a command on local or remote host.
+
+    Use bkg() instead to run a command in the background.
+    """
+    def __init__(self, comm, shell=True, fail=True, ns=None, background=False,
+                 host=None, timeout=5, ksft_wait=None):
         if ns:
             comm = f'ip netns exec {ns} ' + comm
 
         self.stdout = None
         self.stderr = None
         self.ret = None
+        self.ksft_term_fd = None
 
         self.comm = comm
         if host:
             self.proc = host.cmd(comm)
         else:
+            # ksft_wait lets us wait for the background process to fully start,
+            # we pass an FD to the child process, and wait for it to write back.
+            # Similarly term_fd tells child it's time to exit.
+            pass_fds = ()
+            env = os.environ.copy()
+            if ksft_wait is not None:
+                rfd, ready_fd = os.pipe()
+                wait_fd, self.ksft_term_fd = os.pipe()
+                pass_fds = (ready_fd, wait_fd, )
+                env["KSFT_READY_FD"] = str(ready_fd)
+                env["KSFT_WAIT_FD"]  = str(wait_fd)
+
             self.proc = subprocess.Popen(comm, shell=shell, stdout=subprocess.PIPE,
-                                         stderr=subprocess.PIPE)
+                                         stderr=subprocess.PIPE, pass_fds=pass_fds,
+                                         env=env)
+            if ksft_wait is not None:
+                os.close(ready_fd)
+                os.close(wait_fd)
+                msg = fd_read_timeout(rfd, ksft_wait)
+                os.close(rfd)
+                if not msg:
+                    raise Exception("Did not receive ready message")
         if not background:
             self.process(terminate=False, fail=fail, timeout=timeout)
 
@@ -37,6 +74,8 @@ import time
         if fail is None:
             fail = not terminate
 
+        if self.ksft_term_fd:
+            os.write(self.ksft_term_fd, b"1")
         if terminate:
             self.proc.terminate()
         stdout, stderr = self.proc.communicate(timeout)
@@ -54,11 +93,30 @@ import time
 
 
 class bkg(cmd):
+    """
+    Run a command in the background.
+
+    Examples usage:
+
+    Run a command on remote host, and wait for it to finish.
+    This is usually paired with wait_port_listen() to make sure
+    the command has initialized:
+
+        with bkg("socat ...", exit_wait=True, host=cfg.remote) as nc:
+            ...
+
+    Run a command and expect it to let us know that it's ready
+    by writing to a special file descriptor passed via KSFT_READY_FD.
+    Command will be terminated when we exit the context manager:
+
+        with bkg("my_binary", ksft_wait=5):
+    """
     def __init__(self, comm, shell=True, fail=None, ns=None, host=None,
-                 exit_wait=False):
+                 exit_wait=False, ksft_wait=None):
         super().__init__(comm, background=True,
-                         shell=shell, fail=fail, ns=ns, host=host)
-        self.terminate = not exit_wait
+                         shell=shell, fail=fail, ns=ns, host=host,
+                         ksft_wait=ksft_wait)
+        self.terminate = not exit_wait and not ksft_wait
         self.check_fail = fail
 
         if shell and self.terminate:
-- 
2.48.1

Re: [PATCH net-next v2 5/7] selftests: drv-net: add a way to wait for a local process

[Stanislav Fomichev]

On 02/19, 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.
> 
> Similarly use KSFT_WAIT_FD to let the child process for a sign
> that we are done and child should exit. Sending a signal to
> a child with shell=True can get tricky.
> 
> Make use of this method in the queues test to make it less flaky.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Acked-by: Stanislav Fomichev <sdf@fomichev.me>

Re: [PATCH net-next v2 5/7] selftests: drv-net: add a way to wait for a local process

[Joe Damato]

On Wed, Feb 19, 2025 at 03:49:54PM -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.
> 
> Similarly use KSFT_WAIT_FD to let the child process for a sign
> that we are done and child should exit. Sending a signal to
> a child with shell=True can get tricky.
> 
> Make use of this method in the queues test to make it less flaky.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> v2:
>  - use fd for exit, too
>  - warn if env variables are bad
> v1: https://lore.kernel.org/20250218195048.74692-3-kuba@kernel.org
> ---
>  .../selftests/drivers/net/xdp_helper.c        | 54 +++++++++++++--
>  tools/testing/selftests/drivers/net/queues.py | 43 ++++++------
>  tools/testing/selftests/net/lib/py/utils.py   | 68 +++++++++++++++++--
>  3 files changed, 133 insertions(+), 32 deletions(-)

This appears to have fixed the test on my system and the hang is
gone. Thanks.

Acked-by: Joe Damato <jdamato@fastly.com>

[PATCH net-next v2 6/7] selftests: drv-net: improve the use of ksft helpers in XSK queue test

[Jakub Kicinski]

Avoid exceptions when xsk attr is not present, and add a proper ksft
helper for "not in" condition.

Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Reviewed-by: Joe Damato <jdamato@fastly.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 tools/testing/selftests/drivers/net/queues.py | 9 +++++----
 tools/testing/selftests/net/lib/py/ksft.py    | 5 +++++
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/queues.py b/tools/testing/selftests/drivers/net/queues.py
index 7af2adb61c25..a49f1a146e28 100755
--- a/tools/testing/selftests/drivers/net/queues.py
+++ b/tools/testing/selftests/drivers/net/queues.py
@@ -2,7 +2,7 @@
 # SPDX-License-Identifier: GPL-2.0
 
 from lib.py import ksft_disruptive, ksft_exit, ksft_run
-from lib.py import ksft_eq, ksft_raises, KsftSkipEx, KsftFailEx
+from lib.py import ksft_eq, ksft_not_in, ksft_raises, KsftSkipEx, KsftFailEx
 from lib.py import EthtoolFamily, NetdevFamily, NlError
 from lib.py import NetDrvEnv
 from lib.py import bkg, cmd, defer, ip
@@ -47,10 +47,11 @@ import struct
                 if q['type'] == 'tx':
                     tx = True
 
-                ksft_eq(q['xsk'], {})
+                ksft_eq(q.get('xsk', None), {},
+                        comment="xsk attr on queue we configured")
             else:
-                if 'xsk' in q:
-                    _fail("Check failed: xsk attribute set.")
+                ksft_not_in('xsk', q,
+                            comment="xsk attr on queue we didn't configure")
 
         ksft_eq(rx, True)
         ksft_eq(tx, True)
diff --git a/tools/testing/selftests/net/lib/py/ksft.py b/tools/testing/selftests/net/lib/py/ksft.py
index 3efe005436cd..fd23349fa8ca 100644
--- a/tools/testing/selftests/net/lib/py/ksft.py
+++ b/tools/testing/selftests/net/lib/py/ksft.py
@@ -71,6 +71,11 @@ KSFT_DISRUPTIVE = True
         _fail("Check failed", a, "not in", b, comment)
 
 
+def ksft_not_in(a, b, comment=""):
+    if a in b:
+        _fail("Check failed", a, "in", b, comment)
+
+
 def ksft_is(a, b, comment=""):
     if a is not b:
         _fail("Check failed", a, "is not", b, comment)
-- 
2.48.1

Re: [PATCH net-next v2 6/7] selftests: drv-net: improve the use of ksft helpers in XSK queue test

[Kurt Kanzenbach]

[-- Attachment #1: Type: text/plain, Size: 1598 bytes --]

On Wed Feb 19 2025, Jakub Kicinski wrote:
> Avoid exceptions when xsk attr is not present, and add a proper ksft
> helper for "not in" condition.
>
> Acked-by: Stanislav Fomichev <sdf@fomichev.me>
> Reviewed-by: Joe Damato <jdamato@fastly.com>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  tools/testing/selftests/drivers/net/queues.py | 9 +++++----
>  tools/testing/selftests/net/lib/py/ksft.py    | 5 +++++
>  2 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/drivers/net/queues.py b/tools/testing/selftests/drivers/net/queues.py
> index 7af2adb61c25..a49f1a146e28 100755
> --- a/tools/testing/selftests/drivers/net/queues.py
> +++ b/tools/testing/selftests/drivers/net/queues.py
> @@ -2,7 +2,7 @@
>  # SPDX-License-Identifier: GPL-2.0
>  
>  from lib.py import ksft_disruptive, ksft_exit, ksft_run
> -from lib.py import ksft_eq, ksft_raises, KsftSkipEx, KsftFailEx
> +from lib.py import ksft_eq, ksft_not_in, ksft_raises, KsftSkipEx, KsftFailEx
>  from lib.py import EthtoolFamily, NetdevFamily, NlError
>  from lib.py import NetDrvEnv
>  from lib.py import bkg, cmd, defer, ip
> @@ -47,10 +47,11 @@ import struct
>                  if q['type'] == 'tx':
>                      tx = True
>  
> -                ksft_eq(q['xsk'], {})
> +                ksft_eq(q.get('xsk', None), {},
> +                        comment="xsk attr on queue we configured")

Thanks. That's much better than getting exceptions in the case where no
xsk attribute is available:

Tested-by: Kurt Kanzenbach <kurt@linutronix.de>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

[PATCH net-next v2 7/7] selftests: drv-net: rename queues check_xdp to check_xsk

[Jakub Kicinski]

The test is for AF_XDP, we refer to AF_XDP as XSK.

Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Reviewed-by: Joe Damato <jdamato@fastly.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 tools/testing/selftests/drivers/net/queues.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/queues.py b/tools/testing/selftests/drivers/net/queues.py
index a49f1a146e28..9040baf7b726 100755
--- a/tools/testing/selftests/drivers/net/queues.py
+++ b/tools/testing/selftests/drivers/net/queues.py
@@ -23,7 +23,8 @@ import struct
         return len([q for q in queues if q['type'] == qtype])
     return None
 
-def check_xdp(cfg, nl, xdp_queue_id=0) -> None:
+
+def check_xsk(cfg, nl, xdp_queue_id=0) -> None:
     # Probe for support
     xdp = cmd(cfg.rpath("xdp_helper") + ' - -', fail=False)
     if xdp.ret == 255:
@@ -116,7 +117,8 @@ import struct
 
 def main() -> None:
     with NetDrvEnv(__file__, queue_count=100) as cfg:
-        ksft_run([get_queues, addremove_queues, check_down, check_xdp], args=(cfg, NetdevFamily()))
+        ksft_run([get_queues, addremove_queues, check_down, check_xsk],
+                 args=(cfg, NetdevFamily()))
     ksft_exit()
 
 
-- 
2.48.1

Re: [PATCH net-next v2 0/7] selftests: drv-net: improve the queue test for XSK

[Joe Damato]

On Wed, Feb 19, 2025 at 03:49:49PM -0800, Jakub Kicinski wrote:
> We see some flakes in the the XSK test:
> 
>    Exception| Traceback (most recent call last):
>    Exception|   File "/home/virtme/testing-18/tools/testing/selftests/net/lib/py/ksft.py", line 218, in ksft_run
>    Exception|     case(*args)
>    Exception|   File "/home/virtme/testing-18/tools/testing/selftests/drivers/net/./queues.py", line 53, in check_xdp
>    Exception|     ksft_eq(q['xsk'], {})
>    Exception| KeyError: 'xsk'
> 
> I think it's because the method or running the helper in the background
> is racy. Add more solid infra for waiting for a background helper to be
> initialized.
> 
> v2:
>  - add patch 1, 3 and 4
>  - redo patch 5
> v1: https://lore.kernel.org/20250218195048.74692-1-kuba@kernel.org

Thanks for doing this work.

I tested this on my system in four cases:
  - XDP disabled:
    - NETIF=mlx5
    - NETIF unset
  - XDP enabled
    - NETIF=mlx5
    - NETIF unset

The warning in patch 1 does not print on my system, but the code
changes introduced in patch 5/7 fix the hang.

I've already given my Reviewed-by / Acked-by on all patches, so not
sure if it matters but since I did test this as described above:

Tested-by: Joe Damato <jdamato@fastly.com>

Re: [PATCH net-next v2 0/7] selftests: drv-net: improve the queue test for XSK

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 19 Feb 2025 15:49:49 -0800 you wrote:
> We see some flakes in the the XSK test:
> 
>    Exception| Traceback (most recent call last):
>    Exception|   File "/home/virtme/testing-18/tools/testing/selftests/net/lib/py/ksft.py", line 218, in ksft_run
>    Exception|     case(*args)
>    Exception|   File "/home/virtme/testing-18/tools/testing/selftests/drivers/net/./queues.py", line 53, in check_xdp
>    Exception|     ksft_eq(q['xsk'], {})
>    Exception| KeyError: 'xsk'
> 
> [...]

Here is the summary with links:
  - [net-next,v2,1/7] selftests: drv-net: add a warning for bkg + shell + terminate
    https://git.kernel.org/netdev/net-next/c/846742f7e32f
  - [net-next,v2,2/7] selftests: drv-net: use cfg.rpath() in netlink xsk attr test
    https://git.kernel.org/netdev/net-next/c/dabd31baa3b5
  - [net-next,v2,3/7] selftests: drv-net: add missing new line in xdp_helper
    https://git.kernel.org/netdev/net-next/c/bab59dcf71fb
  - [net-next,v2,4/7] selftests: drv-net: probe for AF_XDP sockets more explicitly
    (no matching commit)
  - [net-next,v2,5/7] selftests: drv-net: add a way to wait for a local process
    (no matching commit)
  - [net-next,v2,6/7] selftests: drv-net: improve the use of ksft helpers in XSK queue test
    https://git.kernel.org/netdev/net-next/c/4fde8398462f
  - [net-next,v2,7/7] selftests: drv-net: rename queues check_xdp to check_xsk
    https://git.kernel.org/netdev/net-next/c/932a9249f71f

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html