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

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

[Joe Damato]

Greetings

Welcome to v5. No functional changes; removed an unused variable from
patch 2.

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/

v5:
  - 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                        | 11 +++
 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, 177 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: 233a2b1480a0bdf6b40d4debf58a07084e9921ff
-- 
2.43.0

[PATCH net-next v5 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 v5 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>
---
 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                  | 11 +++++++++++
 tools/include/uapi/linux/netdev.h       |  6 ++++++
 4 files changed, 35 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..b5a93a449af9 100644
--- a/net/core/netdev-genl.c
+++ b/net/core/netdev-genl.c
@@ -400,11 +400,22 @@ 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;
+
+		if (rxq->pool)
+			if (nla_put_empty_nest(rsp, NETDEV_A_QUEUE_XSK))
+				goto nla_put_failure;
+
 		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;
+
+		if (txq->pool)
+			if (nla_put_empty_nest(rsp, NETDEV_A_QUEUE_XSK))
+				goto nla_put_failure;
+
+		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 v5 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 c7f1c443f2af..81961c6e059d 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 v5 2/3] netdev-genl: Add an XSK attribute to queues

[Stanislav Fomichev]

On 02/08, Joe Damato wrote:
> 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>
> ---
>  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                  | 11 +++++++++++
>  tools/include/uapi/linux/netdev.h       |  6 ++++++
>  4 files changed, 35 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..b5a93a449af9 100644
> --- a/net/core/netdev-genl.c
> +++ b/net/core/netdev-genl.c
> @@ -400,11 +400,22 @@ 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;
> +
> +		if (rxq->pool)
> +			if (nla_put_empty_nest(rsp, NETDEV_A_QUEUE_XSK))
> +				goto nla_put_failure;

Needs to be guarded by ifdef CONFIG_XDP_SOCKETS?


net/core/netdev-genl.c: In function ‘netdev_nl_queue_fill_one’:
net/core/netdev-genl.c:404:24: error: ‘struct netdev_rx_queue’ has no member named ‘pool’
  404 |                 if (rxq->pool)
      |                        ^~
net/core/netdev-genl.c:414:24: error: ‘struct netdev_queue’ has no member named ‘pool’
  414 |                 if (txq->pool)
      |                        ^~


---
pw-bot: cr

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

[kernel test robot]

Hi Joe,

kernel test robot noticed the following build errors:

[auto build test ERROR on 233a2b1480a0bdf6b40d4debf58a07084e9921ff]

url:    https://github.com/intel-lab-lkp/linux/commits/Joe-Damato/netlink-Add-nla_put_empty_nest-helper/20250208-121856
base:   233a2b1480a0bdf6b40d4debf58a07084e9921ff
patch link:    https://lore.kernel.org/r/20250208041248.111118-3-jdamato%40fastly.com
patch subject: [PATCH net-next v5 2/3] netdev-genl: Add an XSK attribute to queues
config: x86_64-kexec (https://download.01.org/0day-ci/archive/20250209/202502091844.PyraqTPE-lkp@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250209/202502091844.PyraqTPE-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202502091844.PyraqTPE-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from net/core/netdev-genl.c:3:
   In file included from include/linux/netdevice.h:38:
   In file included from include/net/net_namespace.h:43:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:8:
   In file included from include/linux/cacheflush.h:5:
   In file included from arch/x86/include/asm/cacheflush.h:5:
   In file included from include/linux/mm.h:2224:
   include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     504 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     505 |                            item];
         |                            ~~~~
   include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     511 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     512 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   include/linux/vmstat.h:524:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     524 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     525 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
>> net/core/netdev-genl.c:404:12: error: no member named 'pool' in 'struct netdev_rx_queue'
     404 |                 if (rxq->pool)
         |                     ~~~  ^
>> net/core/netdev-genl.c:414:12: error: no member named 'pool' in 'struct netdev_queue'
     414 |                 if (txq->pool)
         |                     ~~~  ^
   3 warnings and 2 errors generated.


vim +404 net/core/netdev-genl.c

   374	
   375	static int
   376	netdev_nl_queue_fill_one(struct sk_buff *rsp, struct net_device *netdev,
   377				 u32 q_idx, u32 q_type, const struct genl_info *info)
   378	{
   379		struct pp_memory_provider_params *params;
   380		struct netdev_rx_queue *rxq;
   381		struct netdev_queue *txq;
   382		void *hdr;
   383	
   384		hdr = genlmsg_iput(rsp, info);
   385		if (!hdr)
   386			return -EMSGSIZE;
   387	
   388		if (nla_put_u32(rsp, NETDEV_A_QUEUE_ID, q_idx) ||
   389		    nla_put_u32(rsp, NETDEV_A_QUEUE_TYPE, q_type) ||
   390		    nla_put_u32(rsp, NETDEV_A_QUEUE_IFINDEX, netdev->ifindex))
   391			goto nla_put_failure;
   392	
   393		switch (q_type) {
   394		case NETDEV_QUEUE_TYPE_RX:
   395			rxq = __netif_get_rx_queue(netdev, q_idx);
   396			if (nla_put_napi_id(rsp, rxq->napi))
   397				goto nla_put_failure;
   398	
   399			params = &rxq->mp_params;
   400			if (params->mp_ops &&
   401			    params->mp_ops->nl_fill(params->mp_priv, rsp, rxq))
   402				goto nla_put_failure;
   403	
 > 404			if (rxq->pool)
   405				if (nla_put_empty_nest(rsp, NETDEV_A_QUEUE_XSK))
   406					goto nla_put_failure;
   407	
   408			break;
   409		case NETDEV_QUEUE_TYPE_TX:
   410			txq = netdev_get_tx_queue(netdev, q_idx);
   411			if (nla_put_napi_id(rsp, txq->napi))
   412				goto nla_put_failure;
   413	
 > 414			if (txq->pool)
   415				if (nla_put_empty_nest(rsp, NETDEV_A_QUEUE_XSK))
   416					goto nla_put_failure;
   417	
   418			break;
   419		}
   420	
   421		genlmsg_end(rsp, hdr);
   422	
   423		return 0;
   424	
   425	nla_put_failure:
   426		genlmsg_cancel(rsp, hdr);
   427		return -EMSGSIZE;
   428	}
   429	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

[Joe Damato]

On Sat, Feb 08, 2025 at 05:43:47PM -0800, Stanislav Fomichev wrote:
> On 02/08, Joe Damato wrote:
> > 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>
> > ---
> >  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                  | 11 +++++++++++
> >  tools/include/uapi/linux/netdev.h       |  6 ++++++
> >  4 files changed, 35 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..b5a93a449af9 100644
> > --- a/net/core/netdev-genl.c
> > +++ b/net/core/netdev-genl.c
> > @@ -400,11 +400,22 @@ 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;
> > +
> > +		if (rxq->pool)
> > +			if (nla_put_empty_nest(rsp, NETDEV_A_QUEUE_XSK))
> > +				goto nla_put_failure;
> 
> Needs to be guarded by ifdef CONFIG_XDP_SOCKETS?
> 
> 
> net/core/netdev-genl.c: In function `netdev_nl_queue_fill_one´:
> net/core/netdev-genl.c:404:24: error: `struct netdev_rx_queue´ has no member named `pool´
>   404 |                 if (rxq->pool)
>       |                        ^~
> net/core/netdev-genl.c:414:24: error: `struct netdev_queue´ has no member named `pool´
>   414 |                 if (txq->pool)
>       |                        ^~

Ah, thanks.

I'm trying to decide if it'll look better factored out into helpers
vs just dropping the #ifdefs in netdev_nl_queue_fill_one.

Open to opinions so that hopefully v6 will be the last one ;)

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

[Stanislav Fomichev]

On 02/10, Joe Damato wrote:
> On Sat, Feb 08, 2025 at 05:43:47PM -0800, Stanislav Fomichev wrote:
> > On 02/08, Joe Damato wrote:
> > > 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>
> > > ---
> > >  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                  | 11 +++++++++++
> > >  tools/include/uapi/linux/netdev.h       |  6 ++++++
> > >  4 files changed, 35 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..b5a93a449af9 100644
> > > --- a/net/core/netdev-genl.c
> > > +++ b/net/core/netdev-genl.c
> > > @@ -400,11 +400,22 @@ 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;
> > > +
> > > +		if (rxq->pool)
> > > +			if (nla_put_empty_nest(rsp, NETDEV_A_QUEUE_XSK))
> > > +				goto nla_put_failure;
> > 
> > Needs to be guarded by ifdef CONFIG_XDP_SOCKETS?
> > 
> > 
> > net/core/netdev-genl.c: In function `netdev_nl_queue_fill_one´:
> > net/core/netdev-genl.c:404:24: error: `struct netdev_rx_queue´ has no member named `pool´
> >   404 |                 if (rxq->pool)
> >       |                        ^~
> > net/core/netdev-genl.c:414:24: error: `struct netdev_queue´ has no member named `pool´
> >   414 |                 if (txq->pool)
> >       |                        ^~
> 
> Ah, thanks.
> 
> I'm trying to decide if it'll look better factored out into helpers
> vs just dropping the #ifdefs in netdev_nl_queue_fill_one.
> 
> Open to opinions so that hopefully v6 will be the last one ;)

Might be too much boilerplate for the helpers? (assuming you
want empty helpers for #else case). The only other place that tests
rxq->pool is devmem (net/core/devmem.c) and it uses simple ifdef
in place.

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

[Joe Damato]

On Mon, Feb 10, 2025 at 11:08:10AM -0800, Stanislav Fomichev wrote:
> On 02/10, Joe Damato wrote:
> > On Sat, Feb 08, 2025 at 05:43:47PM -0800, Stanislav Fomichev wrote:
> > > On 02/08, Joe Damato wrote:
> > > > 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>
> > > > ---
> > > >  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                  | 11 +++++++++++
> > > >  tools/include/uapi/linux/netdev.h       |  6 ++++++
> > > >  4 files changed, 35 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..b5a93a449af9 100644
> > > > --- a/net/core/netdev-genl.c
> > > > +++ b/net/core/netdev-genl.c
> > > > @@ -400,11 +400,22 @@ 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;
> > > > +
> > > > +		if (rxq->pool)
> > > > +			if (nla_put_empty_nest(rsp, NETDEV_A_QUEUE_XSK))
> > > > +				goto nla_put_failure;
> > > 
> > > Needs to be guarded by ifdef CONFIG_XDP_SOCKETS?
> > > 
> > > 
> > > net/core/netdev-genl.c: In function `netdev_nl_queue_fill_one´:
> > > net/core/netdev-genl.c:404:24: error: `struct netdev_rx_queue´ has no member named `pool´
> > >   404 |                 if (rxq->pool)
> > >       |                        ^~
> > > net/core/netdev-genl.c:414:24: error: `struct netdev_queue´ has no member named `pool´
> > >   414 |                 if (txq->pool)
> > >       |                        ^~
> > 
> > Ah, thanks.
> > 
> > I'm trying to decide if it'll look better factored out into helpers
> > vs just dropping the #ifdefs in netdev_nl_queue_fill_one.
> > 
> > Open to opinions so that hopefully v6 will be the last one ;)
> 
> Might be too much boilerplate for the helpers? (assuming you
> want empty helpers for #else case). The only other place that tests
> rxq->pool is devmem (net/core/devmem.c) and it uses simple ifdef
> in place.

Yea, I saw that. OK, I'll just drop the ifdefs in and see what
happens.

Thanks for the review; appreciate your time and energy.