| Message ID | 20250417013301.39228-5-jdamato@fastly.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Fix netdevim to correctly mark NAPI IDs | expand |
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 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 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 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!
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; +}
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