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
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.
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")
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; +}
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
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 ?
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.
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)
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/
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.
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!
linux-kselftest-mirror@lists.linaro.org
-
Jakub Kicinski -
Joe Damato -
Paolo Abeni -
Xiao Liang