netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v4 0/3] netdev-genl: Add an xsk attribute to queues
@ 2025-02-07  3:08 Joe Damato
  2025-02-07  3:08 ` [PATCH net-next v4 1/3] netlink: Add nla_put_empty_nest helper Joe Damato
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Joe Damato @ 2025-02-07  3:08 UTC (permalink / raw)
  To: netdev
  Cc: pabeni, edumazet, sridhar.samudrala, Joe Damato,
	Alexei Starovoitov, Andrew Lunn,
	open list:XDP (eXpress Data Path):Keyword:(?:b|_)xdp(?:b|_),
	Daniel Borkmann, Daniel Jurgens, David S. Miller, David Wei,
	Donald Hunter, Jakub Kicinski, Jesper Dangaard Brouer,
	John Fastabend, open list, open list:KERNEL SELFTEST FRAMEWORK,
	Martin Karsten, Mina Almasry, Shuah Khan, Simon Horman,
	Stanislav Fomichev, Xuan Zhuo

Greetings:

Welcome to v4. Small functional change, which makes the code cleaner
(see changelog) and tests pass on my machine with mlx5 and netdevsim.

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/

v4:
  - 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: f3eba8edd885db439f4bfaa2cf9d766bad1ae6c5
-- 
2.43.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH net-next v4 1/3] netlink: Add nla_put_empty_nest helper
  2025-02-07  3:08 [PATCH net-next v4 0/3] netdev-genl: Add an xsk attribute to queues Joe Damato
@ 2025-02-07  3:08 ` Joe Damato
  2025-02-07  3:08 ` [PATCH net-next v4 2/3] netdev-genl: Add an XSK attribute to queues Joe Damato
  2025-02-07  3:08 ` [PATCH net-next v4 3/3] selftests: drv-net: Test queue xsk attribute Joe Damato
  2 siblings, 0 replies; 7+ messages in thread
From: Joe Damato @ 2025-02-07  3:08 UTC (permalink / raw)
  To: netdev
  Cc: pabeni, edumazet, sridhar.samudrala, Joe Damato, Jakub Kicinski,
	David S. Miller, Simon Horman, open list

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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net-next v4 2/3] netdev-genl: Add an XSK attribute to queues
  2025-02-07  3:08 [PATCH net-next v4 0/3] netdev-genl: Add an xsk attribute to queues Joe Damato
  2025-02-07  3:08 ` [PATCH net-next v4 1/3] netlink: Add nla_put_empty_nest helper Joe Damato
@ 2025-02-07  3:08 ` Joe Damato
  2025-02-07 13:30   ` Simon Horman
  2025-02-08  4:02   ` kernel test robot
  2025-02-07  3:08 ` [PATCH net-next v4 3/3] selftests: drv-net: Test queue xsk attribute Joe Damato
  2 siblings, 2 replies; 7+ messages in thread
From: Joe Damato @ 2025-02-07  3:08 UTC (permalink / raw)
  To: netdev
  Cc: pabeni, edumazet, sridhar.samudrala, Joe Damato, Jakub Kicinski,
	Donald Hunter, David S. Miller, Simon Horman, Andrew Lunn,
	Stanislav Fomichev, Xuan Zhuo, Mina Almasry, Daniel Jurgens,
	Amritha Nambiar, David Wei, open list

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>
---
 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..75ca111aa591 100644
--- a/net/core/netdev-genl.c
+++ b/net/core/netdev-genl.c
@@ -380,6 +380,7 @@ netdev_nl_queue_fill_one(struct sk_buff *rsp, struct net_device *netdev,
 	struct netdev_rx_queue *rxq;
 	struct netdev_queue *txq;
 	void *hdr;
+	int ret;
 
 	hdr = genlmsg_iput(rsp, info);
 	if (!hdr)
@@ -400,11 +401,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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net-next v4 3/3] selftests: drv-net: Test queue xsk attribute
  2025-02-07  3:08 [PATCH net-next v4 0/3] netdev-genl: Add an xsk attribute to queues Joe Damato
  2025-02-07  3:08 ` [PATCH net-next v4 1/3] netlink: Add nla_put_empty_nest helper Joe Damato
  2025-02-07  3:08 ` [PATCH net-next v4 2/3] netdev-genl: Add an XSK attribute to queues Joe Damato
@ 2025-02-07  3:08 ` Joe Damato
  2 siblings, 0 replies; 7+ messages in thread
From: Joe Damato @ 2025-02-07  3:08 UTC (permalink / raw)
  To: netdev
  Cc: pabeni, edumazet, sridhar.samudrala, Joe Damato, Jakub Kicinski,
	Andrew Lunn, David S. Miller, Shuah Khan, Alexei Starovoitov,
	Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
	open list, open list:KERNEL SELFTEST FRAMEWORK,
	open list:XDP (eXpress Data Path):Keyword:(?:b|_)xdp(?:b|_)

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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next v4 2/3] netdev-genl: Add an XSK attribute to queues
  2025-02-07  3:08 ` [PATCH net-next v4 2/3] netdev-genl: Add an XSK attribute to queues Joe Damato
@ 2025-02-07 13:30   ` Simon Horman
  2025-02-07 17:24     ` Joe Damato
  2025-02-08  4:02   ` kernel test robot
  1 sibling, 1 reply; 7+ messages in thread
From: Simon Horman @ 2025-02-07 13:30 UTC (permalink / raw)
  To: Joe Damato
  Cc: netdev, pabeni, edumazet, sridhar.samudrala, Jakub Kicinski,
	Donald Hunter, David S. Miller, Andrew Lunn, Stanislav Fomichev,
	Xuan Zhuo, Mina Almasry, Daniel Jurgens, Amritha Nambiar,
	David Wei, open list

On Fri, Feb 07, 2025 at 03:08:54AM +0000, 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>

...

> diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
> index 0dcd4faefd8d..75ca111aa591 100644
> --- a/net/core/netdev-genl.c
> +++ b/net/core/netdev-genl.c
> @@ -380,6 +380,7 @@ netdev_nl_queue_fill_one(struct sk_buff *rsp, struct net_device *netdev,
>  	struct netdev_rx_queue *rxq;
>  	struct netdev_queue *txq;
>  	void *hdr;
> +	int ret;
>  

Hi Joe,

Perhaps this got left behind after some revisions elsewhere.
But as it stands ret is unused in this function and should be removed.

>  	hdr = genlmsg_iput(rsp, info);
>  	if (!hdr)

...

-- 
pw-bot: changes-requested

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next v4 2/3] netdev-genl: Add an XSK attribute to queues
  2025-02-07 13:30   ` Simon Horman
@ 2025-02-07 17:24     ` Joe Damato
  0 siblings, 0 replies; 7+ messages in thread
From: Joe Damato @ 2025-02-07 17:24 UTC (permalink / raw)
  To: Simon Horman
  Cc: netdev, pabeni, edumazet, sridhar.samudrala, Jakub Kicinski,
	Donald Hunter, David S. Miller, Andrew Lunn, Stanislav Fomichev,
	Xuan Zhuo, Mina Almasry, Daniel Jurgens, Amritha Nambiar,
	David Wei, open list

On Fri, Feb 07, 2025 at 01:30:55PM +0000, Simon Horman wrote:
> On Fri, Feb 07, 2025 at 03:08:54AM +0000, 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>
> 
> ...
> 
> > diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
> > index 0dcd4faefd8d..75ca111aa591 100644
> > --- a/net/core/netdev-genl.c
> > +++ b/net/core/netdev-genl.c
> > @@ -380,6 +380,7 @@ netdev_nl_queue_fill_one(struct sk_buff *rsp, struct net_device *netdev,
> >  	struct netdev_rx_queue *rxq;
> >  	struct netdev_queue *txq;
> >  	void *hdr;
> > +	int ret;
> >  
> 
> Hi Joe,
> 
> Perhaps this got left behind after some revisions elsewhere.
> But as it stands ret is unused in this function and should be removed.
> 
> >  	hdr = genlmsg_iput(rsp, info);
> >  	if (!hdr)
> 
> ...

Yes, you are right. I originally added it for the empty nest and
then didn't use it.

Sorry that I missed that and caused unnecessary churn due to my own
negligence.

Thanks for the review / catching it.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next v4 2/3] netdev-genl: Add an XSK attribute to queues
  2025-02-07  3:08 ` [PATCH net-next v4 2/3] netdev-genl: Add an XSK attribute to queues Joe Damato
  2025-02-07 13:30   ` Simon Horman
@ 2025-02-08  4:02   ` kernel test robot
  1 sibling, 0 replies; 7+ messages in thread
From: kernel test robot @ 2025-02-08  4:02 UTC (permalink / raw)
  To: Joe Damato, netdev
  Cc: oe-kbuild-all, pabeni, edumazet, sridhar.samudrala, Joe Damato,
	Jakub Kicinski, Donald Hunter, Simon Horman, Andrew Lunn,
	Stanislav Fomichev, Xuan Zhuo, Mina Almasry, Daniel Jurgens,
	Amritha Nambiar, David Wei, linux-kernel

Hi Joe,

kernel test robot noticed the following build warnings:

[auto build test WARNING on f3eba8edd885db439f4bfaa2cf9d766bad1ae6c5]

url:    https://github.com/intel-lab-lkp/linux/commits/Joe-Damato/netlink-Add-nla_put_empty_nest-helper/20250207-111126
base:   f3eba8edd885db439f4bfaa2cf9d766bad1ae6c5
patch link:    https://lore.kernel.org/r/20250207030916.32751-3-jdamato%40fastly.com
patch subject: [PATCH net-next v4 2/3] netdev-genl: Add an XSK attribute to queues
config: i386-buildonly-randconfig-001-20250208 (https://download.01.org/0day-ci/archive/20250208/202502081141.R4zMr9v1-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-12) 11.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250208/202502081141.R4zMr9v1-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/202502081141.R4zMr9v1-lkp@intel.com/

All warnings (new ones prefixed by >>):

   net/core/netdev-genl.c: In function 'netdev_nl_queue_fill_one':
>> net/core/netdev-genl.c:383:13: warning: unused variable 'ret' [-Wunused-variable]
     383 |         int ret;
         |             ^~~


vim +/ret +383 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		int ret;
   384	
   385		hdr = genlmsg_iput(rsp, info);
   386		if (!hdr)
   387			return -EMSGSIZE;
   388	
   389		if (nla_put_u32(rsp, NETDEV_A_QUEUE_ID, q_idx) ||
   390		    nla_put_u32(rsp, NETDEV_A_QUEUE_TYPE, q_type) ||
   391		    nla_put_u32(rsp, NETDEV_A_QUEUE_IFINDEX, netdev->ifindex))
   392			goto nla_put_failure;
   393	
   394		switch (q_type) {
   395		case NETDEV_QUEUE_TYPE_RX:
   396			rxq = __netif_get_rx_queue(netdev, q_idx);
   397			if (nla_put_napi_id(rsp, rxq->napi))
   398				goto nla_put_failure;
   399	
   400			params = &rxq->mp_params;
   401			if (params->mp_ops &&
   402			    params->mp_ops->nl_fill(params->mp_priv, rsp, rxq))
   403				goto nla_put_failure;
   404	
   405			if (rxq->pool)
   406				if (nla_put_empty_nest(rsp, NETDEV_A_QUEUE_XSK))
   407					goto nla_put_failure;
   408	
   409			break;
   410		case NETDEV_QUEUE_TYPE_TX:
   411			txq = netdev_get_tx_queue(netdev, q_idx);
   412			if (nla_put_napi_id(rsp, txq->napi))
   413				goto nla_put_failure;
   414	
   415			if (txq->pool)
   416				if (nla_put_empty_nest(rsp, NETDEV_A_QUEUE_XSK))
   417					goto nla_put_failure;
   418	
   419			break;
   420		}
   421	
   422		genlmsg_end(rsp, hdr);
   423	
   424		return 0;
   425	
   426	nla_put_failure:
   427		genlmsg_cancel(rsp, hdr);
   428		return -EMSGSIZE;
   429	}
   430	

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-02-08  4:03 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-07  3:08 [PATCH net-next v4 0/3] netdev-genl: Add an xsk attribute to queues Joe Damato
2025-02-07  3:08 ` [PATCH net-next v4 1/3] netlink: Add nla_put_empty_nest helper Joe Damato
2025-02-07  3:08 ` [PATCH net-next v4 2/3] netdev-genl: Add an XSK attribute to queues Joe Damato
2025-02-07 13:30   ` Simon Horman
2025-02-07 17:24     ` Joe Damato
2025-02-08  4:02   ` kernel test robot
2025-02-07  3:08 ` [PATCH net-next v4 3/3] selftests: drv-net: Test queue xsk attribute Joe Damato

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).