diff mbox series

[net-next,v3] netdev-genl: Elide napi_id when not present

Message ID 20250204192724.199209-1-jdamato@fastly.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series [net-next,v3] netdev-genl: Elide napi_id when not present | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit fail Errors and warnings before: 4 this patch: 5
netdev/build_tools success Errors and warnings before: 26 (+1) this patch: 26 (+1)
netdev/cc_maintainers success CCed 6 of 6 maintainers
netdev/build_clang fail Errors and warnings before: 161 this patch: 164
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn fail Errors and warnings before: 135 this patch: 136
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 42 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Joe Damato Feb. 4, 2025, 7:27 p.m. UTC 
There are at least two cases where napi_id may not present and the
napi_id should be elided:

1. Queues could be created, but napi_enable may not have been called
   yet. In this case, there may be a NAPI but it may not have an ID and
   output of a napi_id should be elided.

2. TX-only NAPIs currently do not have NAPI IDs. If a TX queue happens
   to be linked with a TX-only NAPI, elide the NAPI ID from the netlink
   output as a NAPI ID of 0 is not useful for users.

Signed-off-by: Joe Damato <jdamato@fastly.com>
---
 v3:
  - Took Eric's suggested patch to refactor the code to use helpers,
    which improves the code quality significantly.

 v2: https://lore.kernel.org/netdev/20250203191714.155526-1-jdamato@fastly.com/
   - Updated to elide NAPI IDs for RX queues which may have not called
     napi_enable yet.

 rfc: https://lore.kernel.org/lkml/20250128163038.429864-1-jdamato@fastly.com/

 include/net/busy_poll.h |  5 +++++
 net/core/netdev-genl.c  | 13 +++++++++----
 2 files changed, 14 insertions(+), 4 deletions(-)


base-commit: c2933b2befe25309f4c5cfbea0ca80909735fd76

Comments

kernel test robot Feb. 5, 2025, 9:22 a.m. UTC | #1 
Hi Joe,

kernel test robot noticed the following build warnings:

[auto build test WARNING on c2933b2befe25309f4c5cfbea0ca80909735fd76]

url:    https://github.com/intel-lab-lkp/linux/commits/Joe-Damato/netdev-genl-Elide-napi_id-when-not-present/20250205-033024
base:   c2933b2befe25309f4c5cfbea0ca80909735fd76
patch link:    https://lore.kernel.org/r/20250204192724.199209-1-jdamato%40fastly.com
patch subject: [PATCH net-next v3] netdev-genl: Elide napi_id when not present
config: i386-buildonly-randconfig-006-20250205 (https://download.01.org/0day-ci/archive/20250205/202502051721.hL1sw85A-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/20250205/202502051721.hL1sw85A-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/202502051721.hL1sw85A-lkp@intel.com/

All warnings (new ones prefixed by >>):

   net/core/netdev-genl.c:380:23: warning: variable 'txq' set but not used [-Wunused-but-set-variable]
     380 |         struct netdev_queue *txq;
         |                              ^
>> net/core/netdev-genl.c:406:28: warning: variable 'rxq' is uninitialized when used here [-Wuninitialized]
     406 |                 if (nla_put_napi_id(rsp, rxq->napi))
         |                                          ^~~
   net/core/netdev-genl.c:379:29: note: initialize the variable 'rxq' to silence this warning
     379 |         struct netdev_rx_queue *rxq;
         |                                    ^
         |                                     = NULL
   2 warnings generated.


vim +/rxq +406 net/core/netdev-genl.c

   373	
   374	static int
   375	netdev_nl_queue_fill_one(struct sk_buff *rsp, struct net_device *netdev,
   376				 u32 q_idx, u32 q_type, const struct genl_info *info)
   377	{
   378		struct net_devmem_dmabuf_binding *binding;
   379		struct netdev_rx_queue *rxq;
   380		struct netdev_queue *txq;
   381		void *hdr;
   382	
   383		hdr = genlmsg_iput(rsp, info);
   384		if (!hdr)
   385			return -EMSGSIZE;
   386	
   387		if (nla_put_u32(rsp, NETDEV_A_QUEUE_ID, q_idx) ||
   388		    nla_put_u32(rsp, NETDEV_A_QUEUE_TYPE, q_type) ||
   389		    nla_put_u32(rsp, NETDEV_A_QUEUE_IFINDEX, netdev->ifindex))
   390			goto nla_put_failure;
   391	
   392		switch (q_type) {
   393		case NETDEV_QUEUE_TYPE_RX:
   394			rxq = __netif_get_rx_queue(netdev, q_idx);
   395			if (nla_put_napi_id(rsp, rxq->napi))
   396				goto nla_put_failure;
   397	
   398			binding = rxq->mp_params.mp_priv;
   399			if (binding &&
   400			    nla_put_u32(rsp, NETDEV_A_QUEUE_DMABUF, binding->id))
   401				goto nla_put_failure;
   402	
   403			break;
   404		case NETDEV_QUEUE_TYPE_TX:
   405			txq = netdev_get_tx_queue(netdev, q_idx);
 > 406			if (nla_put_napi_id(rsp, rxq->napi))
   407				goto nla_put_failure;
   408		}
   409	
   410		genlmsg_end(rsp, hdr);
   411	
   412		return 0;
   413	
   414	nla_put_failure:
   415		genlmsg_cancel(rsp, hdr);
   416		return -EMSGSIZE;
   417	}
   418
kernel test robot Feb. 5, 2025, 9:22 a.m. UTC | #2 
Hi Joe,

kernel test robot noticed the following build warnings:

[auto build test WARNING on c2933b2befe25309f4c5cfbea0ca80909735fd76]

url:    https://github.com/intel-lab-lkp/linux/commits/Joe-Damato/netdev-genl-Elide-napi_id-when-not-present/20250205-033024
base:   c2933b2befe25309f4c5cfbea0ca80909735fd76
patch link:    https://lore.kernel.org/r/20250204192724.199209-1-jdamato%40fastly.com
patch subject: [PATCH net-next v3] netdev-genl: Elide napi_id when not present
config: i386-buildonly-randconfig-004-20250205 (https://download.01.org/0day-ci/archive/20250205/202502051756.62x7XJ1n-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/20250205/202502051756.62x7XJ1n-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/202502051756.62x7XJ1n-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:380:30: warning: variable 'txq' set but not used [-Wunused-but-set-variable]
     380 |         struct netdev_queue *txq;
         |                              ^~~


vim +/txq +380 net/core/netdev-genl.c

4e43e696a7aeb2 Joe Damato      2025-02-04  373  
6b6171db7fc8f7 Amritha Nambiar 2023-12-01  374  static int
6b6171db7fc8f7 Amritha Nambiar 2023-12-01  375  netdev_nl_queue_fill_one(struct sk_buff *rsp, struct net_device *netdev,
6b6171db7fc8f7 Amritha Nambiar 2023-12-01  376  			 u32 q_idx, u32 q_type, const struct genl_info *info)
6b6171db7fc8f7 Amritha Nambiar 2023-12-01  377  {
d0caf9876a1c9f Mina Almasry    2024-09-10  378  	struct net_devmem_dmabuf_binding *binding;
6b6171db7fc8f7 Amritha Nambiar 2023-12-01  379  	struct netdev_rx_queue *rxq;
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 @380  	struct netdev_queue *txq;
6b6171db7fc8f7 Amritha Nambiar 2023-12-01  381  	void *hdr;
6b6171db7fc8f7 Amritha Nambiar 2023-12-01  382  
6b6171db7fc8f7 Amritha Nambiar 2023-12-01  383  	hdr = genlmsg_iput(rsp, info);
6b6171db7fc8f7 Amritha Nambiar 2023-12-01  384  	if (!hdr)
6b6171db7fc8f7 Amritha Nambiar 2023-12-01  385  		return -EMSGSIZE;
6b6171db7fc8f7 Amritha Nambiar 2023-12-01  386  
6b6171db7fc8f7 Amritha Nambiar 2023-12-01  387  	if (nla_put_u32(rsp, NETDEV_A_QUEUE_ID, q_idx) ||
6b6171db7fc8f7 Amritha Nambiar 2023-12-01  388  	    nla_put_u32(rsp, NETDEV_A_QUEUE_TYPE, q_type) ||
6b6171db7fc8f7 Amritha Nambiar 2023-12-01  389  	    nla_put_u32(rsp, NETDEV_A_QUEUE_IFINDEX, netdev->ifindex))
6b6171db7fc8f7 Amritha Nambiar 2023-12-01  390  		goto nla_put_failure;
6b6171db7fc8f7 Amritha Nambiar 2023-12-01  391  
6b6171db7fc8f7 Amritha Nambiar 2023-12-01  392  	switch (q_type) {
6b6171db7fc8f7 Amritha Nambiar 2023-12-01  393  	case NETDEV_QUEUE_TYPE_RX:
6b6171db7fc8f7 Amritha Nambiar 2023-12-01  394  		rxq = __netif_get_rx_queue(netdev, q_idx);
4e43e696a7aeb2 Joe Damato      2025-02-04  395  		if (nla_put_napi_id(rsp, rxq->napi))
6b6171db7fc8f7 Amritha Nambiar 2023-12-01  396  			goto nla_put_failure;
d0caf9876a1c9f Mina Almasry    2024-09-10  397  
d0caf9876a1c9f Mina Almasry    2024-09-10  398  		binding = rxq->mp_params.mp_priv;
d0caf9876a1c9f Mina Almasry    2024-09-10  399  		if (binding &&
d0caf9876a1c9f Mina Almasry    2024-09-10  400  		    nla_put_u32(rsp, NETDEV_A_QUEUE_DMABUF, binding->id))
d0caf9876a1c9f Mina Almasry    2024-09-10  401  			goto nla_put_failure;
d0caf9876a1c9f Mina Almasry    2024-09-10  402  
6b6171db7fc8f7 Amritha Nambiar 2023-12-01  403  		break;
6b6171db7fc8f7 Amritha Nambiar 2023-12-01  404  	case NETDEV_QUEUE_TYPE_TX:
6b6171db7fc8f7 Amritha Nambiar 2023-12-01  405  		txq = netdev_get_tx_queue(netdev, q_idx);
4e43e696a7aeb2 Joe Damato      2025-02-04  406  		if (nla_put_napi_id(rsp, rxq->napi))
6b6171db7fc8f7 Amritha Nambiar 2023-12-01  407  			goto nla_put_failure;
6b6171db7fc8f7 Amritha Nambiar 2023-12-01  408  	}
6b6171db7fc8f7 Amritha Nambiar 2023-12-01  409  
6b6171db7fc8f7 Amritha Nambiar 2023-12-01  410  	genlmsg_end(rsp, hdr);
6b6171db7fc8f7 Amritha Nambiar 2023-12-01  411  
6b6171db7fc8f7 Amritha Nambiar 2023-12-01  412  	return 0;
6b6171db7fc8f7 Amritha Nambiar 2023-12-01  413  
6b6171db7fc8f7 Amritha Nambiar 2023-12-01  414  nla_put_failure:
6b6171db7fc8f7 Amritha Nambiar 2023-12-01  415  	genlmsg_cancel(rsp, hdr);
6b6171db7fc8f7 Amritha Nambiar 2023-12-01  416  	return -EMSGSIZE;
6b6171db7fc8f7 Amritha Nambiar 2023-12-01  417  }
6b6171db7fc8f7 Amritha Nambiar 2023-12-01  418
diff mbox series

Patch

diff --git a/include/net/busy_poll.h b/include/net/busy_poll.h
index c39a426ebf52..741fa7754700 100644
--- a/include/net/busy_poll.h
+++ b/include/net/busy_poll.h
@@ -24,6 +24,11 @@ 
  */
 #define MIN_NAPI_ID ((unsigned int)(NR_CPUS + 1))
 
+static inline bool napi_id_valid(unsigned int napi_id)
+{
+	return napi_id >= MIN_NAPI_ID;
+}
+
 #define BUSY_POLL_BUDGET 8
 
 #ifdef CONFIG_NET_RX_BUSY_POLL
diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
index 715f85c6b62e..5aa3ed870b2c 100644
--- a/net/core/netdev-genl.c
+++ b/net/core/netdev-genl.c
@@ -364,6 +364,13 @@  int netdev_nl_napi_set_doit(struct sk_buff *skb, struct genl_info *info)
 	return err;
 }
 
+static int nla_put_napi_id(struct sk_buff *skb, const struct napi_struct *napi)
+{
+	if (napi && napi_id_valid(napi->napi_id))
+		return nla_put_u32(skb, NETDEV_A_QUEUE_NAPI_ID, napi->napi_id);
+	return 0;
+}
+
 static int
 netdev_nl_queue_fill_one(struct sk_buff *rsp, struct net_device *netdev,
 			 u32 q_idx, u32 q_type, const struct genl_info *info)
@@ -385,8 +392,7 @@  netdev_nl_queue_fill_one(struct sk_buff *rsp, struct net_device *netdev,
 	switch (q_type) {
 	case NETDEV_QUEUE_TYPE_RX:
 		rxq = __netif_get_rx_queue(netdev, q_idx);
-		if (rxq->napi && nla_put_u32(rsp, NETDEV_A_QUEUE_NAPI_ID,
-					     rxq->napi->napi_id))
+		if (nla_put_napi_id(rsp, rxq->napi))
 			goto nla_put_failure;
 
 		binding = rxq->mp_params.mp_priv;
@@ -397,8 +403,7 @@  netdev_nl_queue_fill_one(struct sk_buff *rsp, struct net_device *netdev,
 		break;
 	case NETDEV_QUEUE_TYPE_TX:
 		txq = netdev_get_tx_queue(netdev, q_idx);
-		if (txq->napi && nla_put_u32(rsp, NETDEV_A_QUEUE_NAPI_ID,
-					     txq->napi->napi_id))
+		if (nla_put_napi_id(rsp, rxq->napi))
 			goto nla_put_failure;
 	}