Greetings
Welcome to v6. Added ifdefs to patch 2 and built with CONFIG_XDP_SOCKETS both enabled and disabled to confirm it builds cleanly. No other changes.
This is an attempt to followup on something Jakub asked me about [1], adding an xsk attribute to queues and more clearly documenting which queues are linked to NAPIs...
After the RFC [2], Jakub suggested creating an empty nest for queues which have a pool, so I've adjusted this version to work that way.
The nest can be extended in the future to express attributes about XSK as needed. Queues which are not used for AF_XDP do not have the xsk attribute present.
I've run the included test on: - my mlx5 machine (via NETIF=) - without setting NETIF
And the test seems to pass in both cases.
Thanks, Joe
[1]: https://lore.kernel.org/netdev/20250113143109.60afa59a@kernel.org/ [2]: https://lore.kernel.org/netdev/20250129172431.65773-1-jdamato@fastly.com/
v6: - Added ifdefs for CONFIG_XDP_SOCKETS in patch 2 as Stanislav suggested.
v5: https://lore.kernel.org/bpf/20250208041248.111118-1-jdamato@fastly.com/ - Removed unused ret variable from patch 2 as Simon suggested.
v4: https://lore.kernel.org/lkml/20250207030916.32751-1-jdamato@fastly.com/ - Add patch 1, as suggested by Jakub, which adds an empty nest helper. - Use the helper in patch 2, which makes the code cleaner and prevents a possible bug.
v3: https://lore.kernel.org/netdev/20250204191108.161046-1-jdamato@fastly.com/ - Change comment format in patch 2 to avoid kdoc warnings. No other changes.
v2: https://lore.kernel.org/all/20250203185828.19334-1-jdamato@fastly.com/ - Switched from RFC to actual submission now that net-next is open - Adjusted patch 1 to include an empty nest as suggested by Jakub - Adjusted patch 2 to update the test based on changes to patch 1, and to incorporate some Python feedback from Jakub :)
rfc: https://lore.kernel.org/netdev/20250129172431.65773-1-jdamato@fastly.com/
Joe Damato (3): netlink: Add nla_put_empty_nest helper netdev-genl: Add an XSK attribute to queues selftests: drv-net: Test queue xsk attribute
Documentation/netlink/specs/netdev.yaml | 13 ++- include/net/netlink.h | 15 ++++ include/uapi/linux/netdev.h | 6 ++ net/core/netdev-genl.c | 12 +++ tools/include/uapi/linux/netdev.h | 6 ++ .../testing/selftests/drivers/net/.gitignore | 2 + tools/testing/selftests/drivers/net/Makefile | 3 + tools/testing/selftests/drivers/net/queues.py | 35 +++++++- .../selftests/drivers/net/xdp_helper.c | 89 +++++++++++++++++++ 9 files changed, 178 insertions(+), 3 deletions(-) create mode 100644 tools/testing/selftests/drivers/net/.gitignore create mode 100644 tools/testing/selftests/drivers/net/xdp_helper.c
base-commit: 39f54262ba499d862420a97719d2f0eea0cbd394
Test that queues which are used for AF_XDP have the xsk nest attribute. The attribute is currently empty, but its existence means the AF_XDP is being used for the queue.
Signed-off-by: Joe Damato jdamato@fastly.com Suggested-by: Jakub Kicinski kuba@kernel.org --- v3: - Change comment style of helper C program to avoid kdoc warnings as suggested by Jakub. No other changes.
v2: - Updated the Python test after changes to patch 1 which expose an empty nest - Updated Python test with general Python coding feedback
.../testing/selftests/drivers/net/.gitignore | 2 + tools/testing/selftests/drivers/net/Makefile | 3 + tools/testing/selftests/drivers/net/queues.py | 35 +++++++- .../selftests/drivers/net/xdp_helper.c | 89 +++++++++++++++++++ 4 files changed, 127 insertions(+), 2 deletions(-) create mode 100644 tools/testing/selftests/drivers/net/.gitignore create mode 100644 tools/testing/selftests/drivers/net/xdp_helper.c
diff --git a/tools/testing/selftests/drivers/net/.gitignore b/tools/testing/selftests/drivers/net/.gitignore new file mode 100644 index 000000000000..ec746f374e85 --- /dev/null +++ b/tools/testing/selftests/drivers/net/.gitignore @@ -0,0 +1,2 @@ +# SPDX-License-Identifier: GPL-2.0-only +xdp_helper diff --git a/tools/testing/selftests/drivers/net/Makefile b/tools/testing/selftests/drivers/net/Makefile index 28b6d47f812d..68127c449c24 100644 --- a/tools/testing/selftests/drivers/net/Makefile +++ b/tools/testing/selftests/drivers/net/Makefile @@ -1,10 +1,13 @@ # SPDX-License-Identifier: GPL-2.0 +CFLAGS += $(KHDR_INCLUDES)
TEST_INCLUDES := $(wildcard lib/py/*.py) \ $(wildcard lib/sh/*.sh) \ ../../net/net_helper.sh \ ../../net/lib.sh \
+TEST_GEN_PROGS := xdp_helper + TEST_PROGS := \ netcons_basic.sh \ netcons_fragmented_msg.sh \ diff --git a/tools/testing/selftests/drivers/net/queues.py b/tools/testing/selftests/drivers/net/queues.py index 38303da957ee..55c2b296ad3c 100755 --- a/tools/testing/selftests/drivers/net/queues.py +++ b/tools/testing/selftests/drivers/net/queues.py @@ -8,7 +8,10 @@ from lib.py import NetDrvEnv from lib.py import 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}-*') @@ -21,6 +24,34 @@ def nl_get_queues(cfg, nl, qtype='rx'): return len([q for q in queues if q['type'] == qtype]) 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}"], + stdin=subprocess.PIPE, stdout=subprocess.PIPE, bufsize=1, + text=True) + defer(xdp.kill) + + stdout, stderr = xdp.communicate(timeout=10) + rx = tx = False + + 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 + + ksft_eq(q['xsk'], {}) + else: + if 'xsk' in q: + _fail("Check failed: xsk attribute set.") + + ksft_eq(rx, True) + ksft_eq(tx, True)
def get_queues(cfg, nl) -> None: snl = NetdevFamily(recv_size=4096) @@ -81,7 +112,7 @@ def check_down(cfg, nl) -> None:
def main() -> None: with NetDrvEnv(__file__, queue_count=100) as cfg: - ksft_run([get_queues, addremove_queues, check_down], args=(cfg, NetdevFamily())) + ksft_run([get_queues, addremove_queues, check_down, check_xdp], args=(cfg, NetdevFamily())) ksft_exit()
diff --git a/tools/testing/selftests/drivers/net/xdp_helper.c b/tools/testing/selftests/drivers/net/xdp_helper.c new file mode 100644 index 000000000000..b04d4e0ea30a --- /dev/null +++ b/tools/testing/selftests/drivers/net/xdp_helper.c @@ -0,0 +1,89 @@ +// SPDX-License-Identifier: GPL-2.0 +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <unistd.h> +#include <sys/mman.h> +#include <sys/socket.h> +#include <linux/if_xdp.h> +#include <linux/if_link.h> +#include <net/if.h> +#include <inttypes.h> + +#define UMEM_SZ (1U << 16) +#define NUM_DESC (UMEM_SZ / 2048) + +/* this is a simple helper program that creates an XDP socket and does the + * minimum necessary to get bind() to succeed. + * + * this test program is not intended to actually process packets, but could be + * extended in the future if that is actually needed. + * + * it is used by queues.py to ensure the xsk netlinux attribute is set + * correctly. + */ +int main(int argc, char **argv) +{ + struct xdp_umem_reg umem_reg = { 0 }; + struct sockaddr_xdp sxdp = { 0 }; + int num_desc = NUM_DESC; + void *umem_area; + int ifindex; + int sock_fd; + int queue; + char byte; + + if (argc != 3) { + fprintf(stderr, "Usage: %s ifindex queue_id", argv[0]); + return 1; + } + + sock_fd = socket(AF_XDP, SOCK_RAW, 0); + if (sock_fd < 0) { + perror("socket creation failed"); + return 1; + } + + ifindex = atoi(argv[1]); + queue = atoi(argv[2]); + + umem_area = mmap(NULL, UMEM_SZ, PROT_READ | PROT_WRITE, MAP_PRIVATE | + MAP_ANONYMOUS, -1, 0); + if (umem_area == MAP_FAILED) + return -1; + + umem_reg.addr = (uintptr_t)umem_area; + umem_reg.len = UMEM_SZ; + umem_reg.chunk_size = 2048; + umem_reg.headroom = 0; + + setsockopt(sock_fd, SOL_XDP, XDP_UMEM_REG, &umem_reg, + sizeof(umem_reg)); + setsockopt(sock_fd, SOL_XDP, XDP_UMEM_FILL_RING, &num_desc, + sizeof(num_desc)); + setsockopt(sock_fd, SOL_XDP, XDP_UMEM_COMPLETION_RING, &num_desc, + sizeof(num_desc)); + setsockopt(sock_fd, SOL_XDP, XDP_RX_RING, &num_desc, sizeof(num_desc)); + + sxdp.sxdp_family = AF_XDP; + sxdp.sxdp_ifindex = ifindex; + sxdp.sxdp_queue_id = queue; + sxdp.sxdp_flags = 0; + + if (bind(sock_fd, (struct sockaddr *)&sxdp, sizeof(sxdp)) != 0) { + perror("bind failed"); + close(sock_fd); + return 1; + } + + /* give the parent program some data when the socket is ready*/ + fprintf(stdout, "%d\n", sock_fd); + + /* 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; +}
On 2/10/25 8:38 PM, Joe Damato wrote:
+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}"],
stdin=subprocess.PIPE, stdout=subprocess.PIPE, bufsize=1,
text=True)
- defer(xdp.kill)
- stdout, stderr = xdp.communicate(timeout=10)
- rx = tx = False
- 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
ksft_eq(q['xsk'], {})
else:
if 'xsk' in q:
_fail("Check failed: xsk attribute set.")
- ksft_eq(rx, True)
- ksft_eq(tx, True)
This causes self-test failures:
https://netdev-3.bots.linux.dev/vmksft-net-drv/results/987742/4-queues-py/st...
but I really haven't done any real investigation here.
/P
def get_queues(cfg, nl) -> None: snl = NetdevFamily(recv_size=4096) @@ -81,7 +112,7 @@ def check_down(cfg, nl) -> None: def main() -> None: with NetDrvEnv(__file__, queue_count=100) as cfg:
ksft_run([get_queues, addremove_queues, check_down], args=(cfg, NetdevFamily()))
ksft_exit()ksft_run([get_queues, addremove_queues, check_down, check_xdp], args=(cfg, NetdevFamily()))
diff --git a/tools/testing/selftests/drivers/net/xdp_helper.c b/tools/testing/selftests/drivers/net/xdp_helper.c new file mode 100644 index 000000000000..b04d4e0ea30a --- /dev/null +++ b/tools/testing/selftests/drivers/net/xdp_helper.c @@ -0,0 +1,89 @@ +// SPDX-License-Identifier: GPL-2.0 +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <unistd.h> +#include <sys/mman.h> +#include <sys/socket.h> +#include <linux/if_xdp.h> +#include <linux/if_link.h> +#include <net/if.h> +#include <inttypes.h>
+#define UMEM_SZ (1U << 16) +#define NUM_DESC (UMEM_SZ / 2048)
+/* this is a simple helper program that creates an XDP socket and does the
- minimum necessary to get bind() to succeed.
- this test program is not intended to actually process packets, but could be
- extended in the future if that is actually needed.
- it is used by queues.py to ensure the xsk netlinux attribute is set
- correctly.
- */
+int main(int argc, char **argv) +{
- struct xdp_umem_reg umem_reg = { 0 };
- struct sockaddr_xdp sxdp = { 0 };
- int num_desc = NUM_DESC;
- void *umem_area;
- int ifindex;
- int sock_fd;
- int queue;
- char byte;
- if (argc != 3) {
fprintf(stderr, "Usage: %s ifindex queue_id", argv[0]);
return 1;
- }
- sock_fd = socket(AF_XDP, SOCK_RAW, 0);
- if (sock_fd < 0) {
perror("socket creation failed");
return 1;
- }
- ifindex = atoi(argv[1]);
- queue = atoi(argv[2]);
- umem_area = mmap(NULL, UMEM_SZ, PROT_READ | PROT_WRITE, MAP_PRIVATE |
MAP_ANONYMOUS, -1, 0);
- if (umem_area == MAP_FAILED)
return -1;
- umem_reg.addr = (uintptr_t)umem_area;
- umem_reg.len = UMEM_SZ;
- umem_reg.chunk_size = 2048;
- umem_reg.headroom = 0;
- setsockopt(sock_fd, SOL_XDP, XDP_UMEM_REG, &umem_reg,
sizeof(umem_reg));
- setsockopt(sock_fd, SOL_XDP, XDP_UMEM_FILL_RING, &num_desc,
sizeof(num_desc));
- setsockopt(sock_fd, SOL_XDP, XDP_UMEM_COMPLETION_RING, &num_desc,
sizeof(num_desc));
- setsockopt(sock_fd, SOL_XDP, XDP_RX_RING, &num_desc, sizeof(num_desc));
- sxdp.sxdp_family = AF_XDP;
- sxdp.sxdp_ifindex = ifindex;
- sxdp.sxdp_queue_id = queue;
- sxdp.sxdp_flags = 0;
- if (bind(sock_fd, (struct sockaddr *)&sxdp, sizeof(sxdp)) != 0) {
perror("bind failed");
close(sock_fd);
return 1;
- }
- /* give the parent program some data when the socket is ready*/
- fprintf(stdout, "%d\n", sock_fd);
- /* 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;
+}
On Tue, Feb 11, 2025 at 12:09:50PM +0100, Paolo Abeni wrote:
On 2/10/25 8:38 PM, Joe Damato wrote:
+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}"],
stdin=subprocess.PIPE, stdout=subprocess.PIPE, bufsize=1,
text=True)
- defer(xdp.kill)
- stdout, stderr = xdp.communicate(timeout=10)
- rx = tx = False
- 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
ksft_eq(q['xsk'], {})
else:
if 'xsk' in q:
_fail("Check failed: xsk attribute set.")
- ksft_eq(rx, True)
- ksft_eq(tx, True)
This causes self-test failures:
https://netdev-3.bots.linux.dev/vmksft-net-drv/results/987742/4-queues-py/st...
but I really haven't done any real investigation here.
I think it's because the test kernel in this case has CONFIG_XDP_SOCKETS undefined [1].
The error printed in the link you mentioned:
socket creation failed: Address family not supported by protocol
is coming from the C program, which fails to create the AF_XDP socket.
I think the immediate reaction is to add more error checking to the python to make sure that the subprocess succeeded and if it failed, skip.
But, we may want it to fail for other error states instead of skipping? Not sure if there's general guidance on this, but my plan was to have the AF_XDP socket creation failure return a different error code (I dunno maybe -1?) and only skip the test in that case.
Will that work or is there a better way? I only want to skip if AF_XDP doesn't exist in the test kernel.
[1]: https://netdev-3.bots.linux.dev/vmksft-net-drv/results/987742/config
On Tue, Feb 11, 2025 at 09:45:56AM -0800, Joe Damato wrote:
On Tue, Feb 11, 2025 at 12:09:50PM +0100, Paolo Abeni wrote:
On 2/10/25 8:38 PM, Joe Damato wrote:
+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}"],
stdin=subprocess.PIPE, stdout=subprocess.PIPE, bufsize=1,
text=True)
- defer(xdp.kill)
- stdout, stderr = xdp.communicate(timeout=10)
- rx = tx = False
- 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
ksft_eq(q['xsk'], {})
else:
if 'xsk' in q:
_fail("Check failed: xsk attribute set.")
- ksft_eq(rx, True)
- ksft_eq(tx, True)
This causes self-test failures:
https://netdev-3.bots.linux.dev/vmksft-net-drv/results/987742/4-queues-py/st...
but I really haven't done any real investigation here.
I think it's because the test kernel in this case has CONFIG_XDP_SOCKETS undefined [1].
The error printed in the link you mentioned:
socket creation failed: Address family not supported by protocol
is coming from the C program, which fails to create the AF_XDP socket.
I think the immediate reaction is to add more error checking to the python to make sure that the subprocess succeeded and if it failed, skip.
But, we may want it to fail for other error states instead of skipping? Not sure if there's general guidance on this, but my plan was to have the AF_XDP socket creation failure return a different error code (I dunno maybe -1?) and only skip the test in that case.
Will that work or is there a better way? I only want to skip if AF_XDP doesn't exist in the test kernel.
I'll give it a few more hours incase anyone has comments before I resend, but I got something working (tested on kernels with and without XDP sockets).
xdp_helper returns -1 if (errno == EAFNOSUPPORT). All other error cases return 1.
Updated the python to do this:
if xdp.returncode == 255: raise KsftSkipEx('AF_XDP unsupported') elif xdp.returncode > 0: raise KsftFailEx('unable to create AF_XDP socket')
Which seems to work on both types of kernels?
Happy to take feedback; will hold off on respinning for a bit just incase there's a better way I don't know about.
On 02/11, Joe Damato wrote:
On Tue, Feb 11, 2025 at 09:45:56AM -0800, Joe Damato wrote:
On Tue, Feb 11, 2025 at 12:09:50PM +0100, Paolo Abeni wrote:
On 2/10/25 8:38 PM, Joe Damato wrote:
+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}"],
stdin=subprocess.PIPE, stdout=subprocess.PIPE, bufsize=1,
text=True)
- defer(xdp.kill)
- stdout, stderr = xdp.communicate(timeout=10)
- rx = tx = False
- 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
ksft_eq(q['xsk'], {})
else:
if 'xsk' in q:
_fail("Check failed: xsk attribute set.")
- ksft_eq(rx, True)
- ksft_eq(tx, True)
This causes self-test failures:
https://netdev-3.bots.linux.dev/vmksft-net-drv/results/987742/4-queues-py/st...
but I really haven't done any real investigation here.
I think it's because the test kernel in this case has CONFIG_XDP_SOCKETS undefined [1].
The error printed in the link you mentioned:
socket creation failed: Address family not supported by protocol
is coming from the C program, which fails to create the AF_XDP socket.
I think the immediate reaction is to add more error checking to the python to make sure that the subprocess succeeded and if it failed, skip.
But, we may want it to fail for other error states instead of skipping? Not sure if there's general guidance on this, but my plan was to have the AF_XDP socket creation failure return a different error code (I dunno maybe -1?) and only skip the test in that case.
Will that work or is there a better way? I only want to skip if AF_XDP doesn't exist in the test kernel.
I'll give it a few more hours incase anyone has comments before I resend, but I got something working (tested on kernels with and without XDP sockets).
xdp_helper returns -1 if (errno == EAFNOSUPPORT). All other error cases return 1.
Updated the python to do this:
if xdp.returncode == 255: raise KsftSkipEx('AF_XDP unsupported') elif xdp.returncode > 0: raise KsftFailEx('unable to create AF_XDP socket')
Which seems to work on both types of kernels?
Happy to take feedback; will hold off on respinning for a bit just incase there's a better way I don't know about.
Any reason not to enable CONFIG_XDP_SOCKETS on NIPA kernels? Seems a bit surprising that we run networking tests without XSKs enabled.
On Tue, Feb 11, 2025 at 12:00:38PM -0800, Stanislav Fomichev wrote:
On 02/11, Joe Damato wrote:
On Tue, Feb 11, 2025 at 09:45:56AM -0800, Joe Damato wrote:
On Tue, Feb 11, 2025 at 12:09:50PM +0100, Paolo Abeni wrote:
On 2/10/25 8:38 PM, Joe Damato wrote:
[...]
This causes self-test failures:
https://netdev-3.bots.linux.dev/vmksft-net-drv/results/987742/4-queues-py/st...
but I really haven't done any real investigation here.
I think it's because the test kernel in this case has CONFIG_XDP_SOCKETS undefined [1].
The error printed in the link you mentioned:
socket creation failed: Address family not supported by protocol
is coming from the C program, which fails to create the AF_XDP socket.
I think the immediate reaction is to add more error checking to the python to make sure that the subprocess succeeded and if it failed, skip.
But, we may want it to fail for other error states instead of skipping? Not sure if there's general guidance on this, but my plan was to have the AF_XDP socket creation failure return a different error code (I dunno maybe -1?) and only skip the test in that case.
Will that work or is there a better way? I only want to skip if AF_XDP doesn't exist in the test kernel.
I'll give it a few more hours incase anyone has comments before I resend, but I got something working (tested on kernels with and without XDP sockets).
xdp_helper returns -1 if (errno == EAFNOSUPPORT). All other error cases return 1.
Updated the python to do this:
if xdp.returncode == 255: raise KsftSkipEx('AF_XDP unsupported') elif xdp.returncode > 0: raise KsftFailEx('unable to create AF_XDP socket')
Which seems to work on both types of kernels?
Happy to take feedback; will hold off on respinning for a bit just incase there's a better way I don't know about.
Any reason not to enable CONFIG_XDP_SOCKETS on NIPA kernels? Seems a bit surprising that we run networking tests without XSKs enabled.
I can't comment on NIPA because I have no idea how it works. Maybe there is a kernel with some options enabled and other kernels with various options disabled?
I wonder if that's a separate issue though?
In other words: maybe writing the test as I've mentioned above so it works regardless of whether CONFIG_XDP_SOCKETS is set or not is a good idea just on its own?
I'm just not sure if there's some other pattern I should be following other than what I proposed above. I'm hesitant to re-spin until I get feedback on the proposed approach.
On 02/11, Joe Damato wrote:
On Tue, Feb 11, 2025 at 12:00:38PM -0800, Stanislav Fomichev wrote:
On 02/11, Joe Damato wrote:
On Tue, Feb 11, 2025 at 09:45:56AM -0800, Joe Damato wrote:
On Tue, Feb 11, 2025 at 12:09:50PM +0100, Paolo Abeni wrote:
On 2/10/25 8:38 PM, Joe Damato wrote:
[...]
This causes self-test failures:
https://netdev-3.bots.linux.dev/vmksft-net-drv/results/987742/4-queues-py/st...
but I really haven't done any real investigation here.
I think it's because the test kernel in this case has CONFIG_XDP_SOCKETS undefined [1].
The error printed in the link you mentioned:
socket creation failed: Address family not supported by protocol
is coming from the C program, which fails to create the AF_XDP socket.
I think the immediate reaction is to add more error checking to the python to make sure that the subprocess succeeded and if it failed, skip.
But, we may want it to fail for other error states instead of skipping? Not sure if there's general guidance on this, but my plan was to have the AF_XDP socket creation failure return a different error code (I dunno maybe -1?) and only skip the test in that case.
Will that work or is there a better way? I only want to skip if AF_XDP doesn't exist in the test kernel.
I'll give it a few more hours incase anyone has comments before I resend, but I got something working (tested on kernels with and without XDP sockets).
xdp_helper returns -1 if (errno == EAFNOSUPPORT). All other error cases return 1.
Updated the python to do this:
if xdp.returncode == 255: raise KsftSkipEx('AF_XDP unsupported') elif xdp.returncode > 0: raise KsftFailEx('unable to create AF_XDP socket')
Which seems to work on both types of kernels?
Happy to take feedback; will hold off on respinning for a bit just incase there's a better way I don't know about.
Any reason not to enable CONFIG_XDP_SOCKETS on NIPA kernels? Seems a bit surprising that we run networking tests without XSKs enabled.
I can't comment on NIPA because I have no idea how it works. Maybe there is a kernel with some options enabled and other kernels with various options disabled?
Sorry, should've been more clear. My suggestion is to add CONFIG_XDP_SOCKETS to tools/testing/selftests/drivers/net/config to make your new testcase run in a proper environment with XSKs enabled.
I wonder if that's a separate issue though?
In other words: maybe writing the test as I've mentioned above so it works regardless of whether CONFIG_XDP_SOCKETS is set or not is a good idea just on its own?
I'm just not sure if there's some other pattern I should be following other than what I proposed above. I'm hesitant to re-spin until I get feedback on the proposed approach.
I'd keep your test as is (fail hard if XSK is not there), but let's see if Paolo/Jakub have any other suggestions.
On Tue, 11 Feb 2025 15:10:52 -0800 Stanislav Fomichev wrote:
I can't comment on NIPA because I have no idea how it works. Maybe there is a kernel with some options enabled and other kernels with various options disabled?
Sorry, should've been more clear. My suggestion is to add CONFIG_XDP_SOCKETS to tools/testing/selftests/drivers/net/config to make your new testcase run in a proper environment with XSKs enabled.
+1 this we need for sure
I wonder if that's a separate issue though?
In other words: maybe writing the test as I've mentioned above so it works regardless of whether CONFIG_XDP_SOCKETS is set or not is a good idea just on its own?
I'm just not sure if there's some other pattern I should be following other than what I proposed above. I'm hesitant to re-spin until I get feedback on the proposed approach.
I'd keep your test as is (fail hard if XSK is not there), but let's see if Paolo/Jakub have any other suggestions.
No strong preference. Stan is right that validating the environment is definitely a non-goal for the upstream tests. But if you already added and tested the checks Joe you can keep them, up to you.
On Tue, Feb 11, 2025 at 06:37:06PM -0800, Jakub Kicinski wrote:
On Tue, 11 Feb 2025 15:10:52 -0800 Stanislav Fomichev wrote:
I can't comment on NIPA because I have no idea how it works. Maybe there is a kernel with some options enabled and other kernels with various options disabled?
Sorry, should've been more clear. My suggestion is to add CONFIG_XDP_SOCKETS to tools/testing/selftests/drivers/net/config to make your new testcase run in a proper environment with XSKs enabled.
+1 this we need for sure
OK will do.
I wonder if that's a separate issue though?
In other words: maybe writing the test as I've mentioned above so it works regardless of whether CONFIG_XDP_SOCKETS is set or not is a good idea just on its own?
I'm just not sure if there's some other pattern I should be following other than what I proposed above. I'm hesitant to re-spin until I get feedback on the proposed approach.
I'd keep your test as is (fail hard if XSK is not there), but let's see if Paolo/Jakub have any other suggestions.
No strong preference. Stan is right that validating the environment is definitely a non-goal for the upstream tests. But if you already added and tested the checks Joe you can keep them, up to you.
OK. I guess I'll just leave them? They are as described earlier in the thread.
linux-kselftest-mirror@lists.linaro.org