[PATCH net-next v6 0/3] netdev-genl: Add an xsk attribute to queues

[PATCH net-next v6 0/3] netdev-genl: Add an xsk attribute to queues

[Joe Damato]

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
-- 
2.43.0

[PATCH net-next v6 1/3] netlink: Add nla_put_empty_nest helper

[Joe Damato]

Creating empty nests is helpful when the exact attributes to be exposed
in the future are not known. Encapsulate the logic in a helper.

Signed-off-by: Joe Damato <jdamato@fastly.com>
Suggested-by: Jakub Kicinski <kuba@kernel.org>
---
 v4:
   - new in v4

 include/net/netlink.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/include/net/netlink.h b/include/net/netlink.h
index e015ffbed819..29e0db940382 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -118,6 +118,7 @@
  *   nla_nest_start(skb, type)		start a nested attribute
  *   nla_nest_end(skb, nla)		finalize a nested attribute
  *   nla_nest_cancel(skb, nla)		cancel nested attribute construction
+ *   nla_put_empty_nest(skb, type)	create an empty nest
  *
  * Attribute Length Calculations:
  *   nla_attr_size(payload)		length of attribute w/o padding
@@ -2240,6 +2241,20 @@ static inline void nla_nest_cancel(struct sk_buff *skb, struct nlattr *start)
 	nlmsg_trim(skb, start);
 }
 
+/**
+ * nla_put_empty_nest - Create an empty nest
+ * @skb: socket buffer the message is stored in
+ * @attrtype: attribute type of the container
+ *
+ * This function is a helper for creating empty nests.
+ *
+ * Returns: 0 when successful or -EMSGSIZE on failure.
+ */
+static inline int nla_put_empty_nest(struct sk_buff *skb, int attrtype)
+{
+	return nla_nest_start(skb, attrtype) ? 0 : -EMSGSIZE;
+}
+
 /**
  * __nla_validate_nested - Validate a stream of nested attributes
  * @start: container attribute
-- 
2.43.0

[PATCH net-next v6 2/3] netdev-genl: Add an XSK attribute to queues

[Joe Damato]

Expose a new per-queue nest attribute, xsk, which will be present for
queues that are being used for AF_XDP. If the queue is not being used for
AF_XDP, the nest will not be present.

In the future, this attribute can be extended to include more data about
XSK as it is needed.

Signed-off-by: Joe Damato <jdamato@fastly.com>
Suggested-by: Jakub Kicinski <kuba@kernel.org>
---
 v6:
   - Added ifdefs to netdev_nl_queue_fill_one for CONFIG_XDP_SOCKETS.

 v5:
   - Removed unused variable, ret, from netdev_nl_queue_fill_one.

 v4:
   - Updated netdev_nl_queue_fill_one to use the empty nest helper added
     in patch 1.

 v2:
   - Patch adjusted to include an attribute, xsk, which is an empty nest
     and exposed for queues which have a pool.

 Documentation/netlink/specs/netdev.yaml | 13 ++++++++++++-
 include/uapi/linux/netdev.h             |  6 ++++++
 net/core/netdev-genl.c                  | 12 ++++++++++++
 tools/include/uapi/linux/netdev.h       |  6 ++++++
 4 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/Documentation/netlink/specs/netdev.yaml b/Documentation/netlink/specs/netdev.yaml
index 288923e965ae..85402a2e289c 100644
--- a/Documentation/netlink/specs/netdev.yaml
+++ b/Documentation/netlink/specs/netdev.yaml
@@ -276,6 +276,9 @@ attribute-sets:
         doc: The timeout, in nanoseconds, of how long to suspend irq
              processing, if event polling finds events
         type: uint
+  -
+    name: xsk-info
+    attributes: []
   -
     name: queue
     attributes:
@@ -294,6 +297,9 @@ attribute-sets:
       -
         name: type
         doc: Queue type as rx, tx. Each queue type defines a separate ID space.
+             XDP TX queues allocated in the kernel are not linked to NAPIs and
+             thus not listed. AF_XDP queues will have more information set in
+             the xsk attribute.
         type: u32
         enum: queue-type
       -
@@ -309,7 +315,11 @@ attribute-sets:
         doc: io_uring memory provider information.
         type: nest
         nested-attributes: io-uring-provider-info
-
+      -
+        name: xsk
+        doc: XSK information for this queue, if any.
+        type: nest
+        nested-attributes: xsk-info
   -
     name: qstats
     doc: |
@@ -652,6 +662,7 @@ operations:
             - ifindex
             - dmabuf
             - io-uring
+            - xsk
       dump:
         request:
           attributes:
diff --git a/include/uapi/linux/netdev.h b/include/uapi/linux/netdev.h
index 6c6ee183802d..4e82f3871473 100644
--- a/include/uapi/linux/netdev.h
+++ b/include/uapi/linux/netdev.h
@@ -136,6 +136,11 @@ enum {
 	NETDEV_A_NAPI_MAX = (__NETDEV_A_NAPI_MAX - 1)
 };
 
+enum {
+	__NETDEV_A_XSK_INFO_MAX,
+	NETDEV_A_XSK_INFO_MAX = (__NETDEV_A_XSK_INFO_MAX - 1)
+};
+
 enum {
 	NETDEV_A_QUEUE_ID = 1,
 	NETDEV_A_QUEUE_IFINDEX,
@@ -143,6 +148,7 @@ enum {
 	NETDEV_A_QUEUE_NAPI_ID,
 	NETDEV_A_QUEUE_DMABUF,
 	NETDEV_A_QUEUE_IO_URING,
+	NETDEV_A_QUEUE_XSK,
 
 	__NETDEV_A_QUEUE_MAX,
 	NETDEV_A_QUEUE_MAX = (__NETDEV_A_QUEUE_MAX - 1)
diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
index 0dcd4faefd8d..49b2b2248821 100644
--- a/net/core/netdev-genl.c
+++ b/net/core/netdev-genl.c
@@ -400,11 +400,23 @@ netdev_nl_queue_fill_one(struct sk_buff *rsp, struct net_device *netdev,
 		if (params->mp_ops &&
 		    params->mp_ops->nl_fill(params->mp_priv, rsp, rxq))
 			goto nla_put_failure;
+#ifdef CONFIG_XDP_SOCKETS
+		if (rxq->pool)
+			if (nla_put_empty_nest(rsp, NETDEV_A_QUEUE_XSK))
+				goto nla_put_failure;
+#endif
+
 		break;
 	case NETDEV_QUEUE_TYPE_TX:
 		txq = netdev_get_tx_queue(netdev, q_idx);
 		if (nla_put_napi_id(rsp, txq->napi))
 			goto nla_put_failure;
+#ifdef CONFIG_XDP_SOCKETS
+		if (txq->pool)
+			if (nla_put_empty_nest(rsp, NETDEV_A_QUEUE_XSK))
+				goto nla_put_failure;
+#endif
+		break;
 	}
 
 	genlmsg_end(rsp, hdr);
diff --git a/tools/include/uapi/linux/netdev.h b/tools/include/uapi/linux/netdev.h
index 6c6ee183802d..4e82f3871473 100644
--- a/tools/include/uapi/linux/netdev.h
+++ b/tools/include/uapi/linux/netdev.h
@@ -136,6 +136,11 @@ enum {
 	NETDEV_A_NAPI_MAX = (__NETDEV_A_NAPI_MAX - 1)
 };
 
+enum {
+	__NETDEV_A_XSK_INFO_MAX,
+	NETDEV_A_XSK_INFO_MAX = (__NETDEV_A_XSK_INFO_MAX - 1)
+};
+
 enum {
 	NETDEV_A_QUEUE_ID = 1,
 	NETDEV_A_QUEUE_IFINDEX,
@@ -143,6 +148,7 @@ enum {
 	NETDEV_A_QUEUE_NAPI_ID,
 	NETDEV_A_QUEUE_DMABUF,
 	NETDEV_A_QUEUE_IO_URING,
+	NETDEV_A_QUEUE_XSK,
 
 	__NETDEV_A_QUEUE_MAX,
 	NETDEV_A_QUEUE_MAX = (__NETDEV_A_QUEUE_MAX - 1)
-- 
2.43.0

[PATCH net-next v6 3/3] selftests: drv-net: Test queue xsk attribute

[Joe Damato]

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;
+}
-- 
2.43.0

Re: [PATCH net-next v6 3/3] selftests: drv-net: Test queue xsk attribute

[Paolo Abeni]

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/stdout

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_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;
> +}

Re: [PATCH net-next v6 3/3] selftests: drv-net: Test queue xsk attribute

[Joe Damato]

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/stdout
> 
> 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

Re: [PATCH net-next v6 3/3] selftests: drv-net: Test queue xsk attribute

[Joe Damato]

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/stdout
> > 
> > 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

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.

Re: [PATCH net-next v6 3/3] selftests: drv-net: Test queue xsk attribute

[Stanislav Fomichev]

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/stdout
> > > 
> > > 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
> 
> 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.

Re: [PATCH net-next v6 3/3] selftests: drv-net: Test queue xsk attribute

[Joe Damato]

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/stdout
> > > > 
> > > > 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
> > 
> > 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.

Re: [PATCH net-next v6 3/3] selftests: drv-net: Test queue xsk attribute

[Stanislav Fomichev]

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/stdout
> > > > > 
> > > > > 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
> > > 
> > > 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.

Re: [PATCH net-next v6 3/3] selftests: drv-net: Test queue xsk attribute

[Jakub Kicinski]

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.

Re: [PATCH net-next v6 3/3] selftests: drv-net: Test queue xsk attribute

[Joe Damato]

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.