netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v2 0/4] Fix netdevim to correctly mark NAPI IDs
@ 2025-04-17  1:32 Joe Damato
  2025-04-17  1:32 ` [PATCH net-next v2 1/4] netdevsim: Mark NAPI ID on skb in nsim_rcv Joe Damato
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Joe Damato @ 2025-04-17  1:32 UTC (permalink / raw)
  To: netdev
  Cc: kuba, Joe Damato, Alexei Starovoitov, Andrew Lunn,
	open list:XDP (eXpress Data Path):Keyword:(?:b|_)xdp(?:b|_),
	Daniel Borkmann, David S. Miller, Eric Dumazet,
	Jesper Dangaard Brouer, John Fastabend, open list,
	open list:KERNEL SELFTEST FRAMEWORK, Paolo Abeni, Shuah Khan,
	Simon Horman, Willem de Bruijn, Xiao Liang

Greetings:

Welcome to v2.

This series fixes netdevsim to correctly set the NAPI ID on the skb.
This is helpful for writing tests around features that use
SO_INCOMING_NAPI_ID.

In addition to the netdevsim fix in patch 1, patches 2-4 do some self
test refactoring and add a test for NAPI IDs. The test itself (patch 4)
introduces a C helper because apparently python doesn't have
socket.SO_INCOMING_NAPI_ID.

Thanks,
Joe

v2:
  - No longer an RFC
  - Minor whitespace change in patch 1 (no functional change).
  - Patches 2-4 new in v2

rfcv1: https://lore.kernel.org/netdev/20250329000030.39543-1-jdamato@fastly.com/

Joe Damato (4):
  netdevsim: Mark NAPI ID on skb in nsim_rcv
  selftests: drv-net: Factor out ksft C helpers
  selftests: net: Allow custom net ns paths
  selftests: drv-net: Test that NAPI ID is non-zero

 drivers/net/netdevsim/netdev.c                |  2 +
 .../testing/selftests/drivers/net/.gitignore  |  1 +
 tools/testing/selftests/drivers/net/Makefile  |  6 +-
 tools/testing/selftests/drivers/net/ksft.h    | 56 +++++++++++++
 .../testing/selftests/drivers/net/napi_id.py  | 24 ++++++
 .../selftests/drivers/net/napi_id_helper.c    | 83 +++++++++++++++++++
 .../selftests/drivers/net/xdp_helper.c        | 49 +----------
 tools/testing/selftests/net/lib/py/netns.py   |  4 +-
 8 files changed, 175 insertions(+), 50 deletions(-)
 create mode 100644 tools/testing/selftests/drivers/net/ksft.h
 create mode 100755 tools/testing/selftests/drivers/net/napi_id.py
 create mode 100644 tools/testing/selftests/drivers/net/napi_id_helper.c


base-commit: bbfc077d457272bcea4f14b3a28247ade99b196d
-- 
2.43.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH net-next v2 1/4] netdevsim: Mark NAPI ID on skb in nsim_rcv
  2025-04-17  1:32 [PATCH net-next v2 0/4] Fix netdevim to correctly mark NAPI IDs Joe Damato
@ 2025-04-17  1:32 ` Joe Damato
  2025-04-17  1:32 ` [PATCH net-next v2 2/4] selftests: drv-net: Factor out ksft C helpers Joe Damato
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 12+ messages in thread
From: Joe Damato @ 2025-04-17  1:32 UTC (permalink / raw)
  To: netdev
  Cc: kuba, Joe Damato, Andrew Lunn, David S. Miller, Eric Dumazet,
	Paolo Abeni, open list

Previously, nsim_rcv was not marking the NAPI ID on the skb, leading to
applications seeing a napi ID of 0 when using SO_INCOMING_NAPI_ID.

To add to the userland confusion, netlink appears to correctly report
the NAPI IDs for netdevsim queues but the resulting file descriptor from
a call to accept() was reporting a NAPI ID of 0.

Signed-off-by: Joe Damato <jdamato@fastly.com>
---
 drivers/net/netdevsim/netdev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index 0e0321a7ddd7..2aa999345fe1 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -29,6 +29,7 @@
 #include <net/pkt_cls.h>
 #include <net/rtnetlink.h>
 #include <net/udp_tunnel.h>
+#include <net/busy_poll.h>
 
 #include "netdevsim.h"
 
@@ -357,6 +358,7 @@ static int nsim_rcv(struct nsim_rq *rq, int budget)
 			break;
 
 		skb = skb_dequeue(&rq->skb_queue);
+		skb_mark_napi_id(skb, &rq->napi);
 		netif_receive_skb(skb);
 	}
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next v2 2/4] selftests: drv-net: Factor out ksft C helpers
  2025-04-17  1:32 [PATCH net-next v2 0/4] Fix netdevim to correctly mark NAPI IDs Joe Damato
  2025-04-17  1:32 ` [PATCH net-next v2 1/4] netdevsim: Mark NAPI ID on skb in nsim_rcv Joe Damato
@ 2025-04-17  1:32 ` Joe Damato
  2025-04-17  1:32 ` [PATCH net-next v2 3/4] selftests: net: Allow custom net ns paths Joe Damato
  2025-04-17  1:32 ` [PATCH net-next v2 4/4] selftests: drv-net: Test that NAPI ID is non-zero Joe Damato
  3 siblings, 0 replies; 12+ messages in thread
From: Joe Damato @ 2025-04-17  1:32 UTC (permalink / raw)
  To: netdev
  Cc: kuba, Joe Damato, Andrew Lunn, David S. Miller, Eric Dumazet,
	Paolo Abeni, Shuah Khan, Alexei Starovoitov, Daniel Borkmann,
	Jesper Dangaard Brouer, John Fastabend, open list,
	open list:KERNEL SELFTEST FRAMEWORK,
	open list:XDP (eXpress Data Path):Keyword:(?:b|_)xdp(?:b|_)

Factor ksft C helpers to a header so they can be used by other C-based
tests.

Signed-off-by: Joe Damato <jdamato@fastly.com>
---
 tools/testing/selftests/drivers/net/ksft.h    | 56 +++++++++++++++++++
 .../selftests/drivers/net/xdp_helper.c        | 49 +---------------
 2 files changed, 58 insertions(+), 47 deletions(-)
 create mode 100644 tools/testing/selftests/drivers/net/ksft.h

diff --git a/tools/testing/selftests/drivers/net/ksft.h b/tools/testing/selftests/drivers/net/ksft.h
new file mode 100644
index 000000000000..3fd084006a16
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/ksft.h
@@ -0,0 +1,56 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#if !defined(__KSFT_H__)
+#define __KSFT_H__
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+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);
+}
+
+#endif
diff --git a/tools/testing/selftests/drivers/net/xdp_helper.c b/tools/testing/selftests/drivers/net/xdp_helper.c
index aeed25914104..d5bb8ac33efa 100644
--- a/tools/testing/selftests/drivers/net/xdp_helper.c
+++ b/tools/testing/selftests/drivers/net/xdp_helper.c
@@ -11,56 +11,11 @@
 #include <net/if.h>
 #include <inttypes.h>
 
+#include "ksft.h"
+
 #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.
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next v2 3/4] selftests: net: Allow custom net ns paths
  2025-04-17  1:32 [PATCH net-next v2 0/4] Fix netdevim to correctly mark NAPI IDs Joe Damato
  2025-04-17  1:32 ` [PATCH net-next v2 1/4] netdevsim: Mark NAPI ID on skb in nsim_rcv Joe Damato
  2025-04-17  1:32 ` [PATCH net-next v2 2/4] selftests: drv-net: Factor out ksft C helpers Joe Damato
@ 2025-04-17  1:32 ` Joe Damato
  2025-04-17  1:32 ` [PATCH net-next v2 4/4] selftests: drv-net: Test that NAPI ID is non-zero Joe Damato
  3 siblings, 0 replies; 12+ messages in thread
From: Joe Damato @ 2025-04-17  1:32 UTC (permalink / raw)
  To: netdev
  Cc: kuba, Joe Damato, David S. Miller, Eric Dumazet, Paolo Abeni,
	Simon Horman, Shuah Khan, Xiao Liang, Willem de Bruijn,
	open list:KERNEL SELFTEST FRAMEWORK, open list

Extend NetNSEnter to allow custom paths in order to support, for
example, /proc/self/ns/net.

Signed-off-by: Joe Damato <jdamato@fastly.com>
---
 tools/testing/selftests/net/lib/py/netns.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/lib/py/netns.py b/tools/testing/selftests/net/lib/py/netns.py
index 8e9317044eef..8d5c26317cb0 100644
--- a/tools/testing/selftests/net/lib/py/netns.py
+++ b/tools/testing/selftests/net/lib/py/netns.py
@@ -35,8 +35,8 @@ class NetNS:
 
 
 class NetNSEnter:
-    def __init__(self, ns_name):
-        self.ns_path = f"/run/netns/{ns_name}"
+    def __init__(self, ns_name, ns_path="/run/netns/"):
+        self.ns_path = f"{ns_path}{ns_name}"
 
     def __enter__(self):
         self.saved = open("/proc/thread-self/ns/net")
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next v2 4/4] selftests: drv-net: Test that NAPI ID is non-zero
  2025-04-17  1:32 [PATCH net-next v2 0/4] Fix netdevim to correctly mark NAPI IDs Joe Damato
                   ` (2 preceding siblings ...)
  2025-04-17  1:32 ` [PATCH net-next v2 3/4] selftests: net: Allow custom net ns paths Joe Damato
@ 2025-04-17  1:32 ` Joe Damato
  2025-04-17  7:26   ` Paolo Abeni
                     ` (2 more replies)
  3 siblings, 3 replies; 12+ messages in thread
From: Joe Damato @ 2025-04-17  1:32 UTC (permalink / raw)
  To: netdev
  Cc: kuba, Joe Damato, Andrew Lunn, David S. Miller, Eric Dumazet,
	Paolo Abeni, Shuah Khan, Alexei Starovoitov, Daniel Borkmann,
	Jesper Dangaard Brouer, John Fastabend, open list,
	open list:KERNEL SELFTEST FRAMEWORK,
	open list:XDP (eXpress Data Path):Keyword:(?:b|_)xdp(?:b|_)

Test that the SO_INCOMING_NAPI_ID of a network file descriptor is
non-zero. This ensures that either the core networking stack or, in some
cases like netdevsim, the driver correctly sets the NAPI ID.

Signed-off-by: Joe Damato <jdamato@fastly.com>
---
 .../testing/selftests/drivers/net/.gitignore  |  1 +
 tools/testing/selftests/drivers/net/Makefile  |  6 +-
 .../testing/selftests/drivers/net/napi_id.py  | 24 ++++++
 .../selftests/drivers/net/napi_id_helper.c    | 83 +++++++++++++++++++
 4 files changed, 113 insertions(+), 1 deletion(-)
 create mode 100755 tools/testing/selftests/drivers/net/napi_id.py
 create mode 100644 tools/testing/selftests/drivers/net/napi_id_helper.c

diff --git a/tools/testing/selftests/drivers/net/.gitignore b/tools/testing/selftests/drivers/net/.gitignore
index ec746f374e85..71bd7d651233 100644
--- a/tools/testing/selftests/drivers/net/.gitignore
+++ b/tools/testing/selftests/drivers/net/.gitignore
@@ -1,2 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0-only
 xdp_helper
+napi_id_helper
diff --git a/tools/testing/selftests/drivers/net/Makefile b/tools/testing/selftests/drivers/net/Makefile
index 0c95bd944d56..47247c2ef948 100644
--- a/tools/testing/selftests/drivers/net/Makefile
+++ b/tools/testing/selftests/drivers/net/Makefile
@@ -6,9 +6,13 @@ TEST_INCLUDES := $(wildcard lib/py/*.py) \
 		 ../../net/net_helper.sh \
 		 ../../net/lib.sh \
 
-TEST_GEN_FILES := xdp_helper
+TEST_GEN_FILES := \
+	napi_id_helper \
+	xdp_helper \
+# end of TEST_GEN_FILES
 
 TEST_PROGS := \
+	napi_id.py \
 	netcons_basic.sh \
 	netcons_fragmented_msg.sh \
 	netcons_overflow.sh \
diff --git a/tools/testing/selftests/drivers/net/napi_id.py b/tools/testing/selftests/drivers/net/napi_id.py
new file mode 100755
index 000000000000..aee6f90be49b
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/napi_id.py
@@ -0,0 +1,24 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+
+from lib.py import ksft_run, ksft_exit
+from lib.py import ksft_eq, NetDrvEpEnv
+from lib.py import bkg, cmd, rand_port, NetNSEnter
+
+def test_napi_id(cfg) -> None:
+    port = rand_port()
+    listen_cmd = f'{cfg.test_dir / "napi_id_helper"} {cfg.addr_v['4']} {port}'
+
+    with bkg(listen_cmd, ksft_wait=3) as server:
+        with NetNSEnter('net', '/proc/self/ns/'):
+          cmd(f"echo a | socat - TCP:{cfg.addr_v['4']}:{port}", host=cfg.remote, shell=True)
+
+    ksft_eq(0, server.ret)
+
+def main() -> None:
+    with NetDrvEpEnv(__file__) as cfg:
+        ksft_run([test_napi_id], args=(cfg,))
+    ksft_exit()
+
+if __name__ == "__main__":
+    main()
diff --git a/tools/testing/selftests/drivers/net/napi_id_helper.c b/tools/testing/selftests/drivers/net/napi_id_helper.c
new file mode 100644
index 000000000000..7e8e7d373b61
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/napi_id_helper.c
@@ -0,0 +1,83 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <errno.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <arpa/inet.h>
+#include <sys/socket.h>
+
+#include "ksft.h"
+
+int main(int argc, char *argv[])
+{
+	struct sockaddr_in address;
+	unsigned int napi_id;
+	unsigned int port;
+	socklen_t optlen;
+	char buf[1024];
+	int opt = 1;
+	int server;
+	int client;
+	int ret;
+
+	server = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
+	if (server < 0) {
+		perror("socket creation failed");
+		if (errno == EAFNOSUPPORT)
+			return -1;
+		return 1;
+	}
+
+	port = atoi(argv[2]);
+
+	if (setsockopt(server, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(opt))) {
+		perror("setsockopt");
+		return 1;
+	}
+
+	address.sin_family = AF_INET;
+	inet_pton(AF_INET, argv[1], &address.sin_addr);
+	address.sin_port = htons(port);
+
+	if (bind(server, (struct sockaddr *)&address, sizeof(address)) < 0) {
+		perror("bind failed");
+		return 1;
+	}
+
+	if (listen(server, 1) < 0) {
+		perror("listen");
+		return 1;
+	}
+
+	ksft_ready();
+
+	client = accept(server, NULL, 0);
+	if (client < 0) {
+		perror("accept");
+		return 1;
+	}
+
+	optlen = sizeof(napi_id);
+	ret = getsockopt(client, SOL_SOCKET, SO_INCOMING_NAPI_ID, &napi_id,
+			 &optlen);
+	if (ret != 0) {
+		perror("getsockopt");
+		return 1;
+	}
+
+	read(client, buf, 1024);
+
+	ksft_wait();
+
+	if (napi_id == 0) {
+		fprintf(stderr, "napi ID is 0\n");
+		return 1;
+	}
+
+	close(client);
+	close(server);
+
+	return 0;
+}
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 4/4] selftests: drv-net: Test that NAPI ID is non-zero
  2025-04-17  1:32 ` [PATCH net-next v2 4/4] selftests: drv-net: Test that NAPI ID is non-zero Joe Damato
@ 2025-04-17  7:26   ` Paolo Abeni
  2025-04-17 16:33     ` Joe Damato
  2025-04-17 10:21   ` Xiao Liang
  2025-04-17 13:46   ` Jakub Kicinski
  2 siblings, 1 reply; 12+ messages in thread
From: Paolo Abeni @ 2025-04-17  7:26 UTC (permalink / raw)
  To: Joe Damato, netdev
  Cc: kuba, Andrew Lunn, David S. Miller, Eric Dumazet, Shuah Khan,
	Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
	John Fastabend, open list, open list:KERNEL SELFTEST FRAMEWORK,
	open list:XDP (eXpress Data Path):Keyword:(?:b|_)xdp(?:b|_)

On 4/17/25 3:32 AM, Joe Damato wrote:
> diff --git a/tools/testing/selftests/drivers/net/napi_id.py b/tools/testing/selftests/drivers/net/napi_id.py
> new file mode 100755
> index 000000000000..aee6f90be49b
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/net/napi_id.py
> @@ -0,0 +1,24 @@
> +#!/usr/bin/env python3
> +# SPDX-License-Identifier: GPL-2.0
> +
> +from lib.py import ksft_run, ksft_exit
> +from lib.py import ksft_eq, NetDrvEpEnv
> +from lib.py import bkg, cmd, rand_port, NetNSEnter
> +
> +def test_napi_id(cfg) -> None:
> +    port = rand_port()
> +    listen_cmd = f'{cfg.test_dir / "napi_id_helper"} {cfg.addr_v['4']} {port}'

Not really a full review, but this is apparently causing self-tests
failures:

# selftests: drivers/net: napi_id.py
#   File
"/home/virtme/testing-17/tools/testing/selftests/drivers/net/./napi_id.py",
line 10
#     listen_cmd = f'{cfg.test_dir / "napi_id_helper"} {cfg.addr_v['4']}
{port}'
#                                                                   ^
# SyntaxError: f-string: unmatched '['
not ok 1 selftests: drivers/net: napi_id.py # exit=1

the second "'" char is closing the python format string, truncating the
cfg.addr_v['4'] expression.

Please run the self test locally before the next submission, thanks!

/P


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 4/4] selftests: drv-net: Test that NAPI ID is non-zero
  2025-04-17  1:32 ` [PATCH net-next v2 4/4] selftests: drv-net: Test that NAPI ID is non-zero Joe Damato
  2025-04-17  7:26   ` Paolo Abeni
@ 2025-04-17 10:21   ` Xiao Liang
  2025-04-17 13:46   ` Jakub Kicinski
  2 siblings, 0 replies; 12+ messages in thread
From: Xiao Liang @ 2025-04-17 10:21 UTC (permalink / raw)
  To: Joe Damato
  Cc: netdev, kuba, Andrew Lunn, David S. Miller, Eric Dumazet,
	Paolo Abeni, Shuah Khan, Alexei Starovoitov, Daniel Borkmann,
	Jesper Dangaard Brouer, John Fastabend, open list,
	open list:KERNEL SELFTEST FRAMEWORK,
	open list:XDP (eXpress Data Path):Keyword:(?:b|_)xdp(?:b|_)

On Thu, Apr 17, 2025 at 9:33 AM Joe Damato <jdamato@fastly.com> wrote:
>
[...]
> diff --git a/tools/testing/selftests/drivers/net/napi_id.py b/tools/testing/selftests/drivers/net/napi_id.py
> new file mode 100755
> index 000000000000..aee6f90be49b
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/net/napi_id.py
> @@ -0,0 +1,24 @@
> +#!/usr/bin/env python3
> +# SPDX-License-Identifier: GPL-2.0
> +
> +from lib.py import ksft_run, ksft_exit
> +from lib.py import ksft_eq, NetDrvEpEnv
> +from lib.py import bkg, cmd, rand_port, NetNSEnter
> +
> +def test_napi_id(cfg) -> None:
> +    port = rand_port()
> +    listen_cmd = f'{cfg.test_dir / "napi_id_helper"} {cfg.addr_v['4']} {port}'
> +
> +    with bkg(listen_cmd, ksft_wait=3) as server:
> +        with NetNSEnter('net', '/proc/self/ns/'):
> +          cmd(f"echo a | socat - TCP:{cfg.addr_v['4']}:{port}", host=cfg.remote, shell=True)

There's no need to reenter /proc/self/ns/net. And I think
passing `host=cfg.remote` should be sufficient to run
the command in the other netns.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 4/4] selftests: drv-net: Test that NAPI ID is non-zero
  2025-04-17  1:32 ` [PATCH net-next v2 4/4] selftests: drv-net: Test that NAPI ID is non-zero Joe Damato
  2025-04-17  7:26   ` Paolo Abeni
  2025-04-17 10:21   ` Xiao Liang
@ 2025-04-17 13:46   ` Jakub Kicinski
  2025-04-17 16:43     ` Joe Damato
  2 siblings, 1 reply; 12+ messages in thread
From: Jakub Kicinski @ 2025-04-17 13:46 UTC (permalink / raw)
  To: Joe Damato
  Cc: netdev, Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
	Shuah Khan, Alexei Starovoitov, Daniel Borkmann,
	Jesper Dangaard Brouer, John Fastabend, open list,
	open list:KERNEL SELFTEST FRAMEWORK,
	open list:XDP (eXpress Data Path):Keyword:(?:b|_)xdp(?:b|_)

On Thu, 17 Apr 2025 01:32:42 +0000 Joe Damato wrote:
> Test that the SO_INCOMING_NAPI_ID of a network file descriptor is
> non-zero. This ensures that either the core networking stack or, in some
> cases like netdevsim, the driver correctly sets the NAPI ID.
> 
> Signed-off-by: Joe Damato <jdamato@fastly.com>
> ---
>  .../testing/selftests/drivers/net/.gitignore  |  1 +
>  tools/testing/selftests/drivers/net/Makefile  |  6 +-
>  .../testing/selftests/drivers/net/napi_id.py  | 24 ++++++
>  .../selftests/drivers/net/napi_id_helper.c    | 83 +++++++++++++++++++
>  4 files changed, 113 insertions(+), 1 deletion(-)
>  create mode 100755 tools/testing/selftests/drivers/net/napi_id.py
>  create mode 100644 tools/testing/selftests/drivers/net/napi_id_helper.c
> 
> diff --git a/tools/testing/selftests/drivers/net/.gitignore b/tools/testing/selftests/drivers/net/.gitignore
> index ec746f374e85..71bd7d651233 100644
> --- a/tools/testing/selftests/drivers/net/.gitignore
> +++ b/tools/testing/selftests/drivers/net/.gitignore
> @@ -1,2 +1,3 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  xdp_helper
> +napi_id_helper

sort alphabetically, pls

> diff --git a/tools/testing/selftests/drivers/net/Makefile b/tools/testing/selftests/drivers/net/Makefile
> index 0c95bd944d56..47247c2ef948 100644
> --- a/tools/testing/selftests/drivers/net/Makefile
> +++ b/tools/testing/selftests/drivers/net/Makefile
> @@ -6,9 +6,13 @@ TEST_INCLUDES := $(wildcard lib/py/*.py) \
>  		 ../../net/net_helper.sh \
>  		 ../../net/lib.sh \
>  
> -TEST_GEN_FILES := xdp_helper
> +TEST_GEN_FILES := \
> +	napi_id_helper \
> +	xdp_helper \

like you did here

> +# end of TEST_GEN_FILES
>  
>  TEST_PROGS := \
> +	napi_id.py \
>  	netcons_basic.sh \
>  	netcons_fragmented_msg.sh \
>  	netcons_overflow.sh \
> diff --git a/tools/testing/selftests/drivers/net/napi_id.py b/tools/testing/selftests/drivers/net/napi_id.py
> new file mode 100755
> index 000000000000..aee6f90be49b
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/net/napi_id.py
> @@ -0,0 +1,24 @@
> +#!/usr/bin/env python3
> +# SPDX-License-Identifier: GPL-2.0
> +
> +from lib.py import ksft_run, ksft_exit
> +from lib.py import ksft_eq, NetDrvEpEnv
> +from lib.py import bkg, cmd, rand_port, NetNSEnter
> +
> +def test_napi_id(cfg) -> None:
> +    port = rand_port()
> +    listen_cmd = f'{cfg.test_dir / "napi_id_helper"} {cfg.addr_v['4']} {port}'

you need to deploy, in case test is running with a real remote machine
and the binary has to be copied over:

	bin_remote = cfg.remote.deploy(cfg.test_dir / "napi_id_helper")
	listen_cmd = f'{bin_remote} {cfg.addr_v['4']} {port}' 

> +    with bkg(listen_cmd, ksft_wait=3) as server:
> +        with NetNSEnter('net', '/proc/self/ns/'):
> +          cmd(f"echo a | socat - TCP:{cfg.addr_v['4']}:{port}", host=cfg.remote, shell=True)

Like Xiao Liang said, just host=cfg.remote should work.

> +    ksft_eq(0, server.ret)
> +

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 4/4] selftests: drv-net: Test that NAPI ID is non-zero
  2025-04-17  7:26   ` Paolo Abeni
@ 2025-04-17 16:33     ` Joe Damato
  0 siblings, 0 replies; 12+ messages in thread
From: Joe Damato @ 2025-04-17 16:33 UTC (permalink / raw)
  To: Paolo Abeni
  Cc: netdev, kuba, Andrew Lunn, David S. Miller, Eric Dumazet,
	Shuah Khan, Alexei Starovoitov, Daniel Borkmann,
	Jesper Dangaard Brouer, John Fastabend, open list,
	open list:KERNEL SELFTEST FRAMEWORK,
	open list:XDP (eXpress Data Path):Keyword:(?:b|_)xdp(?:b|_)

On Thu, Apr 17, 2025 at 09:26:22AM +0200, Paolo Abeni wrote:
> On 4/17/25 3:32 AM, Joe Damato wrote:
> > diff --git a/tools/testing/selftests/drivers/net/napi_id.py b/tools/testing/selftests/drivers/net/napi_id.py
> > new file mode 100755
> > index 000000000000..aee6f90be49b
> > --- /dev/null
> > +++ b/tools/testing/selftests/drivers/net/napi_id.py
> > @@ -0,0 +1,24 @@
> > +#!/usr/bin/env python3
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +from lib.py import ksft_run, ksft_exit
> > +from lib.py import ksft_eq, NetDrvEpEnv
> > +from lib.py import bkg, cmd, rand_port, NetNSEnter
> > +
> > +def test_napi_id(cfg) -> None:
> > +    port = rand_port()
> > +    listen_cmd = f'{cfg.test_dir / "napi_id_helper"} {cfg.addr_v['4']} {port}'
> 
> Not really a full review, but this is apparently causing self-tests
> failures:
> 
> # selftests: drivers/net: napi_id.py
> #   File
> "/home/virtme/testing-17/tools/testing/selftests/drivers/net/./napi_id.py",
> line 10
> #     listen_cmd = f'{cfg.test_dir / "napi_id_helper"} {cfg.addr_v['4']}
> {port}'
> #                                                                   ^
> # SyntaxError: f-string: unmatched '['
> not ok 1 selftests: drivers/net: napi_id.py # exit=1
> 
> the second "'" char is closing the python format string, truncating the
> cfg.addr_v['4'] expression.
> 
> Please run the self test locally before the next submission, thanks!

I did run it locally, many times, and it works for me:

$ sudo ./tools/testing/selftests/drivers/net/napi_id.py
TAP version 13
1..1
ok 1 napi_id.test_napi_id
# Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0

Maybe this has something to do with the Python version on my system
vs yours/the test host?

I am using Python 3.13.1 from Ubuntu 24.04.

Please let me know what Python version you are using so I can try to
reproduce this locally ?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 4/4] selftests: drv-net: Test that NAPI ID is non-zero
  2025-04-17 13:46   ` Jakub Kicinski
@ 2025-04-17 16:43     ` Joe Damato
  2025-04-17 16:53       ` Jakub Kicinski
  0 siblings, 1 reply; 12+ messages in thread
From: Joe Damato @ 2025-04-17 16:43 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: netdev, Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
	Shuah Khan, Alexei Starovoitov, Daniel Borkmann,
	Jesper Dangaard Brouer, John Fastabend, open list,
	open list:KERNEL SELFTEST FRAMEWORK,
	open list:XDP (eXpress Data Path):Keyword:(?:b|_)xdp(?:b|_)

On Thu, Apr 17, 2025 at 06:46:15AM -0700, Jakub Kicinski wrote:
> On Thu, 17 Apr 2025 01:32:42 +0000 Joe Damato wrote:
> > Test that the SO_INCOMING_NAPI_ID of a network file descriptor is
> > non-zero. This ensures that either the core networking stack or, in some
> > cases like netdevsim, the driver correctly sets the NAPI ID.
> > 
> > Signed-off-by: Joe Damato <jdamato@fastly.com>
> > ---
> >  .../testing/selftests/drivers/net/.gitignore  |  1 +
> >  tools/testing/selftests/drivers/net/Makefile  |  6 +-
> >  .../testing/selftests/drivers/net/napi_id.py  | 24 ++++++
> >  .../selftests/drivers/net/napi_id_helper.c    | 83 +++++++++++++++++++
> >  4 files changed, 113 insertions(+), 1 deletion(-)
> >  create mode 100755 tools/testing/selftests/drivers/net/napi_id.py
> >  create mode 100644 tools/testing/selftests/drivers/net/napi_id_helper.c
> > 
> > diff --git a/tools/testing/selftests/drivers/net/.gitignore b/tools/testing/selftests/drivers/net/.gitignore
> > index ec746f374e85..71bd7d651233 100644
> > --- a/tools/testing/selftests/drivers/net/.gitignore
> > +++ b/tools/testing/selftests/drivers/net/.gitignore
> > @@ -1,2 +1,3 @@
> >  # SPDX-License-Identifier: GPL-2.0-only
> >  xdp_helper
> > +napi_id_helper
> 
> sort alphabetically, pls

Thanks, fixed.

> > diff --git a/tools/testing/selftests/drivers/net/napi_id.py b/tools/testing/selftests/drivers/net/napi_id.py
> > new file mode 100755
> > index 000000000000..aee6f90be49b
> > --- /dev/null
> > +++ b/tools/testing/selftests/drivers/net/napi_id.py
> > @@ -0,0 +1,24 @@
> > +#!/usr/bin/env python3
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +from lib.py import ksft_run, ksft_exit
> > +from lib.py import ksft_eq, NetDrvEpEnv
> > +from lib.py import bkg, cmd, rand_port, NetNSEnter
> > +
> > +def test_napi_id(cfg) -> None:
> > +    port = rand_port()
> > +    listen_cmd = f'{cfg.test_dir / "napi_id_helper"} {cfg.addr_v['4']} {port}'
> 
> you need to deploy, in case test is running with a real remote machine
> and the binary has to be copied over:
> 
> 	bin_remote = cfg.remote.deploy(cfg.test_dir / "napi_id_helper")
> 	listen_cmd = f'{bin_remote} {cfg.addr_v['4']} {port}' 

Thanks, fixed.

> > +    with bkg(listen_cmd, ksft_wait=3) as server:
> > +        with NetNSEnter('net', '/proc/self/ns/'):
> > +          cmd(f"echo a | socat - TCP:{cfg.addr_v['4']}:{port}", host=cfg.remote, shell=True)
> 
> Like Xiao Liang said, just host=cfg.remote should work.

You are both correct; sorry about the noise. I thought I tried this
last night and it was failing, but clearly I was wrong/something
else was broken.

I've fixed this locally and dropped patch 3 which is now
unnecessary.

I think the main outstanding thing is Paolo's feedback which maybe
(?) is due to a Python version difference? If you have any guidance
on how to proceed on that, I'd appreciate it [1].

My guess is that I could rewrite that line to concat the strings
instead of interpolate and it would work both on Paolo's system and
mine. Would that be the right way to go?

[1]: https://lore.kernel.org/netdev/aAEtSppgCFNd8vr4@LQ3V64L9R2/

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 4/4] selftests: drv-net: Test that NAPI ID is non-zero
  2025-04-17 16:43     ` Joe Damato
@ 2025-04-17 16:53       ` Jakub Kicinski
  2025-04-17 17:06         ` Joe Damato
  0 siblings, 1 reply; 12+ messages in thread
From: Jakub Kicinski @ 2025-04-17 16:53 UTC (permalink / raw)
  To: Joe Damato
  Cc: netdev, Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
	Shuah Khan, Alexei Starovoitov, Daniel Borkmann,
	Jesper Dangaard Brouer, John Fastabend, open list,
	open list:KERNEL SELFTEST FRAMEWORK,
	open list:XDP (eXpress Data Path):Keyword:(?:b|_)xdp(?:b|_)

On Thu, 17 Apr 2025 09:43:23 -0700 Joe Damato wrote:
> I think the main outstanding thing is Paolo's feedback which maybe
> (?) is due to a Python version difference? If you have any guidance
> on how to proceed on that, I'd appreciate it [1].

yes, it's a Python version, I made the same mistake in the past.
Older Pythons terminate an fstring too early.
Just switch from ' to " inside the fstring, like you would in bash
if you wanted to quote a quote character. The two are functionally
equivalent.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 4/4] selftests: drv-net: Test that NAPI ID is non-zero
  2025-04-17 16:53       ` Jakub Kicinski
@ 2025-04-17 17:06         ` Joe Damato
  0 siblings, 0 replies; 12+ messages in thread
From: Joe Damato @ 2025-04-17 17:06 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: netdev, Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
	Shuah Khan, Alexei Starovoitov, Daniel Borkmann,
	Jesper Dangaard Brouer, John Fastabend, open list,
	open list:KERNEL SELFTEST FRAMEWORK,
	open list:XDP (eXpress Data Path):Keyword:(?:b|_)xdp(?:b|_)

On Thu, Apr 17, 2025 at 09:53:10AM -0700, Jakub Kicinski wrote:
> On Thu, 17 Apr 2025 09:43:23 -0700 Joe Damato wrote:
> > I think the main outstanding thing is Paolo's feedback which maybe
> > (?) is due to a Python version difference? If you have any guidance
> > on how to proceed on that, I'd appreciate it [1].
> 
> yes, it's a Python version, I made the same mistake in the past.
> Older Pythons terminate an fstring too early.
> Just switch from ' to " inside the fstring, like you would in bash
> if you wanted to quote a quote character. The two are functionally
> equivalent.

OK thanks for the details. Sorry that I am learning Python via
netdev ;)

I did this so it matches the style of the other fstring a few lines
below:

-    listen_cmd = f'{bin_remote} {cfg.addr_v['4']} {port}'
+    listen_cmd = f"{bin_remote} {cfg.addr_v['4']} {port}"

Test works and passes for me on my system, so I'll send the v3
tonight when I've hit 24 hrs.

Thanks!

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-04-17 17:06 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-17  1:32 [PATCH net-next v2 0/4] Fix netdevim to correctly mark NAPI IDs Joe Damato
2025-04-17  1:32 ` [PATCH net-next v2 1/4] netdevsim: Mark NAPI ID on skb in nsim_rcv Joe Damato
2025-04-17  1:32 ` [PATCH net-next v2 2/4] selftests: drv-net: Factor out ksft C helpers Joe Damato
2025-04-17  1:32 ` [PATCH net-next v2 3/4] selftests: net: Allow custom net ns paths Joe Damato
2025-04-17  1:32 ` [PATCH net-next v2 4/4] selftests: drv-net: Test that NAPI ID is non-zero Joe Damato
2025-04-17  7:26   ` Paolo Abeni
2025-04-17 16:33     ` Joe Damato
2025-04-17 10:21   ` Xiao Liang
2025-04-17 13:46   ` Jakub Kicinski
2025-04-17 16:43     ` Joe Damato
2025-04-17 16:53       ` Jakub Kicinski
2025-04-17 17:06         ` Joe Damato

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).