* Re: [PATCH net-next 1/2] netdev-genl: Add ifname for queue and NAPI APIs
@ 2024-02-23 10:47 kernel test robot
0 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2024-02-23 10:47 UTC (permalink / raw)
To: oe-kbuild; +Cc: lkp, Dan Carpenter
BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <1708531057-67392-2-git-send-email-jdamato@fastly.com>
References: <1708531057-67392-2-git-send-email-jdamato@fastly.com>
TO: Joe Damato <jdamato@fastly.com>
TO: netdev@vger.kernel.org
TO: linux-kernel@vger.kernel.org
CC: Joe Damato <jdamato@fastly.com>
CC: Eric Dumazet <edumazet@google.com>
CC: Jakub Kicinski <kuba@kernel.org>
CC: Paolo Abeni <pabeni@redhat.com>
CC: Jesper Dangaard Brouer <hawk@kernel.org>
CC: Stanislav Fomichev <sdf@google.com>
CC: Amritha Nambiar <amritha.nambiar@intel.com>
CC: Larysa Zaremba <larysa.zaremba@intel.com>
CC: Lorenzo Bianconi <lorenzo@kernel.org>
CC: Tariq Toukan <tariqt@nvidia.com>
CC: Sridhar Samudrala <sridhar.samudrala@intel.com>
CC: Alexei Starovoitov <ast@kernel.org>
CC: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Hi Joe,
kernel test robot noticed the following build warnings:
[auto build test WARNING on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/Joe-Damato/netdev-genl-Add-ifname-for-queue-and-NAPI-APIs/20240222-000134
base: net-next/main
patch link: https://lore.kernel.org/r/1708531057-67392-2-git-send-email-jdamato%40fastly.com
patch subject: [PATCH net-next 1/2] netdev-genl: Add ifname for queue and NAPI APIs
:::::: branch date: 2 days ago
:::::: commit date: 2 days ago
config: i386-randconfig-141-20240222 (https://download.01.org/0day-ci/archive/20240223/202402231851.2NeORqwi-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
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>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Closes: https://lore.kernel.org/r/202402231851.2NeORqwi-lkp@intel.com/
New smatch warnings:
net/core/netdev-genl.c:388 netdev_nl_queue_get_doit() error: uninitialized symbol 'ifname'.
Old smatch warnings:
net/core/netdev-genl.c:45 netdev_nl_dev_fill() warn: inconsistent indenting
vim +/ifname +388 net/core/netdev-genl.c
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 370
bc877956272f05 Amritha Nambiar 2023-12-01 371 int netdev_nl_queue_get_doit(struct sk_buff *skb, struct genl_info *info)
bc877956272f05 Amritha Nambiar 2023-12-01 372 {
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 373 u32 q_id, q_type, ifindex;
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 374 struct net_device *netdev;
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 375 struct sk_buff *rsp;
f340b224321fa6 Joe Damato 2024-02-21 376 char *ifname;
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 377 int err;
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 378
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 379 if (GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_ID) ||
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 380 GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_TYPE) ||
f340b224321fa6 Joe Damato 2024-02-21 381 GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_IFINDEX) ||
f340b224321fa6 Joe Damato 2024-02-21 382 GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_IFNAME))
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 383 return -EINVAL;
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 384
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 385 q_id = nla_get_u32(info->attrs[NETDEV_A_QUEUE_ID]);
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 386 q_type = nla_get_u32(info->attrs[NETDEV_A_QUEUE_TYPE]);
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 387 ifindex = nla_get_u32(info->attrs[NETDEV_A_QUEUE_IFINDEX]);
f340b224321fa6 Joe Damato 2024-02-21 @388 nla_strscpy(ifname, info->attrs[NETDEV_A_QUEUE_IFNAME], IFNAMSIZ);
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 389
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 390 rsp = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 391 if (!rsp)
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 392 return -ENOMEM;
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 393
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 394 rtnl_lock();
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 395
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 396 netdev = __dev_get_by_index(genl_info_net(info), ifindex);
f340b224321fa6 Joe Damato 2024-02-21 397
f340b224321fa6 Joe Damato 2024-02-21 398 if (strcmp(netdev->name, ifname)) {
f340b224321fa6 Joe Damato 2024-02-21 399 err = -ENODEV;
f340b224321fa6 Joe Damato 2024-02-21 400 } else {
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 401 if (netdev)
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 402 err = netdev_nl_queue_fill(rsp, netdev, q_id, q_type, info);
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 403 else
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 404 err = -ENODEV;
f340b224321fa6 Joe Damato 2024-02-21 405 }
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 406
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 407 rtnl_unlock();
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 408
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 409 if (err)
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 410 goto err_free_msg;
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 411
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 412 return genlmsg_reply(rsp, info);
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 413
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 414 err_free_msg:
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 415 nlmsg_free(rsp);
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 416 return err;
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 417 }
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 418
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 1/2] netdev-genl: Add ifname for queue and NAPI APIs
2024-02-21 15:57 ` [PATCH net-next 1/2] netdev-genl: Add ifname for queue and NAPI APIs Joe Damato
` (2 preceding siblings ...)
2024-02-22 20:55 ` kernel test robot
@ 2024-02-23 11:21 ` Dan Carpenter
3 siblings, 0 replies; 8+ messages in thread
From: Dan Carpenter @ 2024-02-23 11:21 UTC (permalink / raw)
To: oe-kbuild, Joe Damato, netdev, linux-kernel
Cc: lkp, oe-kbuild-all, Joe Damato, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Jesper Dangaard Brouer, Stanislav Fomichev,
Amritha Nambiar, Larysa Zaremba, Lorenzo Bianconi, Tariq Toukan,
Sridhar Samudrala, Alexei Starovoitov, Maciej Fijalkowski
Hi Joe,
kernel test robot noticed the following build warnings:
url: https://github.com/intel-lab-lkp/linux/commits/Joe-Damato/netdev-genl-Add-ifname-for-queue-and-NAPI-APIs/20240222-000134
base: net-next/main
patch link: https://lore.kernel.org/r/1708531057-67392-2-git-send-email-jdamato%40fastly.com
patch subject: [PATCH net-next 1/2] netdev-genl: Add ifname for queue and NAPI APIs
config: i386-randconfig-141-20240222 (https://download.01.org/0day-ci/archive/20240223/202402231851.2NeORqwi-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
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>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202402231851.2NeORqwi-lkp@intel.com/
New smatch warnings:
net/core/netdev-genl.c:388 netdev_nl_queue_get_doit() error: uninitialized symbol 'ifname'.
vim +/ifname +388 net/core/netdev-genl.c
bc877956272f05 Amritha Nambiar 2023-12-01 371 int netdev_nl_queue_get_doit(struct sk_buff *skb, struct genl_info *info)
bc877956272f05 Amritha Nambiar 2023-12-01 372 {
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 373 u32 q_id, q_type, ifindex;
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 374 struct net_device *netdev;
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 375 struct sk_buff *rsp;
f340b224321fa6 Joe Damato 2024-02-21 376 char *ifname;
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 377 int err;
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 378
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 379 if (GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_ID) ||
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 380 GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_TYPE) ||
f340b224321fa6 Joe Damato 2024-02-21 381 GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_IFINDEX) ||
f340b224321fa6 Joe Damato 2024-02-21 382 GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_IFNAME))
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 383 return -EINVAL;
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 384
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 385 q_id = nla_get_u32(info->attrs[NETDEV_A_QUEUE_ID]);
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 386 q_type = nla_get_u32(info->attrs[NETDEV_A_QUEUE_TYPE]);
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 387 ifindex = nla_get_u32(info->attrs[NETDEV_A_QUEUE_IFINDEX]);
f340b224321fa6 Joe Damato 2024-02-21 @388 nla_strscpy(ifname, info->attrs[NETDEV_A_QUEUE_IFNAME], IFNAMSIZ);
^^^^^^
missing initialization
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 389
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 390 rsp = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 391 if (!rsp)
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 392 return -ENOMEM;
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 393
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 394 rtnl_lock();
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 395
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 396 netdev = __dev_get_by_index(genl_info_net(info), ifindex);
f340b224321fa6 Joe Damato 2024-02-21 397
f340b224321fa6 Joe Damato 2024-02-21 398 if (strcmp(netdev->name, ifname)) {
f340b224321fa6 Joe Damato 2024-02-21 399 err = -ENODEV;
f340b224321fa6 Joe Damato 2024-02-21 400 } else {
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 401 if (netdev)
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 402 err = netdev_nl_queue_fill(rsp, netdev, q_id, q_type, info);
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 403 else
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 404 err = -ENODEV;
f340b224321fa6 Joe Damato 2024-02-21 405 }
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 406
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 407 rtnl_unlock();
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 408
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 409 if (err)
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 410 goto err_free_msg;
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 411
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 412 return genlmsg_reply(rsp, info);
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 413
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 414 err_free_msg:
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 415 nlmsg_free(rsp);
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 416 return err;
6b6171db7fc8f7 Amritha Nambiar 2023-12-01 417 }
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 1/2] netdev-genl: Add ifname for queue and NAPI APIs
2024-02-21 15:57 ` [PATCH net-next 1/2] netdev-genl: Add ifname for queue and NAPI APIs Joe Damato
2024-02-21 19:12 ` Jakub Kicinski
2024-02-21 19:12 ` Nambiar, Amritha
@ 2024-02-22 20:55 ` kernel test robot
2024-02-23 11:21 ` Dan Carpenter
3 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2024-02-22 20:55 UTC (permalink / raw)
To: Joe Damato, netdev, linux-kernel
Cc: llvm, oe-kbuild-all, Joe Damato, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Jesper Dangaard Brouer, Stanislav Fomichev,
Amritha Nambiar, Larysa Zaremba, Lorenzo Bianconi, Tariq Toukan,
Sridhar Samudrala, Alexei Starovoitov, Maciej Fijalkowski
Hi Joe,
kernel test robot noticed the following build warnings:
[auto build test WARNING on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/Joe-Damato/netdev-genl-Add-ifname-for-queue-and-NAPI-APIs/20240222-000134
base: net-next/main
patch link: https://lore.kernel.org/r/1708531057-67392-2-git-send-email-jdamato%40fastly.com
patch subject: [PATCH net-next 1/2] netdev-genl: Add ifname for queue and NAPI APIs
config: powerpc-acadia_defconfig (https://download.01.org/0day-ci/archive/20240223/202402230429.g2qC2zMK-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project edd4aee4dd9b5b98b2576a6f783e4086173d902a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240223/202402230429.g2qC2zMK-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/202402230429.g2qC2zMK-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> net/core/netdev-genl.c:388:14: warning: variable 'ifname' is uninitialized when used here [-Wuninitialized]
388 | nla_strscpy(ifname, info->attrs[NETDEV_A_QUEUE_IFNAME], IFNAMSIZ);
| ^~~~~~
net/core/netdev-genl.c:376:14: note: initialize the variable 'ifname' to silence this warning
376 | char *ifname;
| ^
| = NULL
1 warning generated.
vim +/ifname +388 net/core/netdev-genl.c
370
371 int netdev_nl_queue_get_doit(struct sk_buff *skb, struct genl_info *info)
372 {
373 u32 q_id, q_type, ifindex;
374 struct net_device *netdev;
375 struct sk_buff *rsp;
376 char *ifname;
377 int err;
378
379 if (GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_ID) ||
380 GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_TYPE) ||
381 GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_IFINDEX) ||
382 GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_IFNAME))
383 return -EINVAL;
384
385 q_id = nla_get_u32(info->attrs[NETDEV_A_QUEUE_ID]);
386 q_type = nla_get_u32(info->attrs[NETDEV_A_QUEUE_TYPE]);
387 ifindex = nla_get_u32(info->attrs[NETDEV_A_QUEUE_IFINDEX]);
> 388 nla_strscpy(ifname, info->attrs[NETDEV_A_QUEUE_IFNAME], IFNAMSIZ);
389
390 rsp = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
391 if (!rsp)
392 return -ENOMEM;
393
394 rtnl_lock();
395
396 netdev = __dev_get_by_index(genl_info_net(info), ifindex);
397
398 if (strcmp(netdev->name, ifname)) {
399 err = -ENODEV;
400 } else {
401 if (netdev)
402 err = netdev_nl_queue_fill(rsp, netdev, q_id, q_type, info);
403 else
404 err = -ENODEV;
405 }
406
407 rtnl_unlock();
408
409 if (err)
410 goto err_free_msg;
411
412 return genlmsg_reply(rsp, info);
413
414 err_free_msg:
415 nlmsg_free(rsp);
416 return err;
417 }
418
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 1/2] netdev-genl: Add ifname for queue and NAPI APIs
2024-02-21 19:12 ` Nambiar, Amritha
@ 2024-02-21 19:23 ` Joe Damato
0 siblings, 0 replies; 8+ messages in thread
From: Joe Damato @ 2024-02-21 19:23 UTC (permalink / raw)
To: Nambiar, Amritha
Cc: netdev, linux-kernel, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Jesper Dangaard Brouer,
Stanislav Fomichev, Larysa Zaremba, Lorenzo Bianconi,
Tariq Toukan, Sridhar Samudrala, Alexei Starovoitov,
Maciej Fijalkowski
On Wed, Feb 21, 2024 at 11:12:47AM -0800, Nambiar, Amritha wrote:
> On 2/21/2024 7:57 AM, Joe Damato wrote:
> >Expose the netdevice name when queue and NAPI netdev-genl APIs are used
> >
> >Signed-off-by: Joe Damato <jdamato@fastly.com>
> >---
> > include/uapi/linux/netdev.h | 2 ++
> > net/core/netdev-genl.c | 22 +++++++++++++++++-----
> > 2 files changed, 19 insertions(+), 5 deletions(-)
> >
> >diff --git a/include/uapi/linux/netdev.h b/include/uapi/linux/netdev.h
> >index 93cb411..80762bc 100644
> >--- a/include/uapi/linux/netdev.h
> >+++ b/include/uapi/linux/netdev.h
> >@@ -117,6 +117,7 @@ enum {
> > NETDEV_A_NAPI_ID,
> > NETDEV_A_NAPI_IRQ,
> > NETDEV_A_NAPI_PID,
> >+ NETDEV_A_NAPI_IFNAME,
> > __NETDEV_A_NAPI_MAX,
> > NETDEV_A_NAPI_MAX = (__NETDEV_A_NAPI_MAX - 1)
> >@@ -127,6 +128,7 @@ enum {
> > NETDEV_A_QUEUE_IFINDEX,
> > NETDEV_A_QUEUE_TYPE,
> > NETDEV_A_QUEUE_NAPI_ID,
> >+ NETDEV_A_QUEUE_IFNAME,
> > __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 fd98936..a886e6a 100644
> >--- a/net/core/netdev-genl.c
> >+++ b/net/core/netdev-genl.c
> >@@ -181,6 +181,9 @@ netdev_nl_napi_fill_one(struct sk_buff *rsp, struct napi_struct *napi,
> > if (nla_put_u32(rsp, NETDEV_A_NAPI_IFINDEX, napi->dev->ifindex))
> > goto nla_put_failure;
> >+ if (nla_put_string(rsp, NETDEV_A_NAPI_IFNAME, napi->dev->name))
> >+ goto nla_put_failure;
> >+
> > if (napi->irq >= 0 && nla_put_u32(rsp, NETDEV_A_NAPI_IRQ, napi->irq))
> > goto nla_put_failure;
> >@@ -307,7 +310,8 @@ netdev_nl_queue_fill_one(struct sk_buff *rsp, struct net_device *netdev,
> > if (nla_put_u32(rsp, NETDEV_A_QUEUE_ID, q_idx) ||
> > nla_put_u32(rsp, NETDEV_A_QUEUE_TYPE, q_type) ||
> >- nla_put_u32(rsp, NETDEV_A_QUEUE_IFINDEX, netdev->ifindex))
> >+ nla_put_u32(rsp, NETDEV_A_QUEUE_IFINDEX, netdev->ifindex) ||
> >+ nla_put_string(rsp, NETDEV_A_QUEUE_IFNAME, netdev->name))
> > goto nla_put_failure;
> > switch (q_type) {
> >@@ -369,16 +373,19 @@ int netdev_nl_queue_get_doit(struct sk_buff *skb, struct genl_info *info)
> > u32 q_id, q_type, ifindex;
> > struct net_device *netdev;
> > struct sk_buff *rsp;
> >+ char *ifname;
> > int err;
> > if (GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_ID) ||
> > GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_TYPE) ||
> >- GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_IFINDEX))
> >+ GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_IFINDEX) ||
> >+ GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_IFNAME))
> > return -EINVAL;
> > q_id = nla_get_u32(info->attrs[NETDEV_A_QUEUE_ID]);
> > q_type = nla_get_u32(info->attrs[NETDEV_A_QUEUE_TYPE]);
> > ifindex = nla_get_u32(info->attrs[NETDEV_A_QUEUE_IFINDEX]);
> >+ nla_strscpy(ifname, info->attrs[NETDEV_A_QUEUE_IFNAME], IFNAMSIZ);
> > rsp = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
> > if (!rsp)
> >@@ -387,10 +394,15 @@ int netdev_nl_queue_get_doit(struct sk_buff *skb, struct genl_info *info)
> > rtnl_lock();
> > netdev = __dev_get_by_index(genl_info_net(info), ifindex);
> >- if (netdev)
> >- err = netdev_nl_queue_fill(rsp, netdev, q_id, q_type, info);
> >- else
> >+
> >+ if (strcmp(netdev->name, ifname)) {
> > err = -ENODEV;
> >+ } else {
> >+ if (netdev)
> >+ err = netdev_nl_queue_fill(rsp, netdev, q_id, q_type, info);
> >+ else
> >+ err = -ENODEV;
> >+ }
>
> This looks bit incorrect to me that the netdev is checked after netdev->name
> is accessed. Shouldn't this be something like:
>
> if (netdev && !strcmp(netdev->name, ifname))
> err = netdev_nl_queue_fill(rsp, netdev, q_id, q_type, info);
> else
> err = -ENODEV;
Yes, you are right. Thanks.
Based on Jakub's comment re exposing names, though, it seems that perhaps
this change is not desirable overall.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 1/2] netdev-genl: Add ifname for queue and NAPI APIs
2024-02-21 19:12 ` Jakub Kicinski
@ 2024-02-21 19:17 ` Joe Damato
0 siblings, 0 replies; 8+ messages in thread
From: Joe Damato @ 2024-02-21 19:17 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, linux-kernel, David S. Miller, Eric Dumazet, Paolo Abeni,
Jesper Dangaard Brouer, Stanislav Fomichev, Amritha Nambiar,
Larysa Zaremba, Lorenzo Bianconi, Tariq Toukan,
Sridhar Samudrala, Alexei Starovoitov, Maciej Fijalkowski
On Wed, Feb 21, 2024 at 11:12:20AM -0800, Jakub Kicinski wrote:
> On Wed, 21 Feb 2024 07:57:29 -0800 Joe Damato wrote:
> > if (GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_ID) ||
> > GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_TYPE) ||
> > - GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_IFINDEX))
> > + GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_IFINDEX) ||
> > + GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_IFNAME))
>
> This means user always has to provide both ifindex and ifname,
> right?
That's right. I'm OK with omitting this requirement, though. I feel like to
your earlier point on name changes, maybe ifindex is enough as far as
required params go.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 1/2] netdev-genl: Add ifname for queue and NAPI APIs
2024-02-21 15:57 ` [PATCH net-next 1/2] netdev-genl: Add ifname for queue and NAPI APIs Joe Damato
2024-02-21 19:12 ` Jakub Kicinski
@ 2024-02-21 19:12 ` Nambiar, Amritha
2024-02-21 19:23 ` Joe Damato
2024-02-22 20:55 ` kernel test robot
2024-02-23 11:21 ` Dan Carpenter
3 siblings, 1 reply; 8+ messages in thread
From: Nambiar, Amritha @ 2024-02-21 19:12 UTC (permalink / raw)
To: Joe Damato, netdev, linux-kernel
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Jesper Dangaard Brouer, Stanislav Fomichev, Larysa Zaremba,
Lorenzo Bianconi, Tariq Toukan, Sridhar Samudrala,
Alexei Starovoitov, Maciej Fijalkowski
On 2/21/2024 7:57 AM, Joe Damato wrote:
> Expose the netdevice name when queue and NAPI netdev-genl APIs are used
>
> Signed-off-by: Joe Damato <jdamato@fastly.com>
> ---
> include/uapi/linux/netdev.h | 2 ++
> net/core/netdev-genl.c | 22 +++++++++++++++++-----
> 2 files changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/include/uapi/linux/netdev.h b/include/uapi/linux/netdev.h
> index 93cb411..80762bc 100644
> --- a/include/uapi/linux/netdev.h
> +++ b/include/uapi/linux/netdev.h
> @@ -117,6 +117,7 @@ enum {
> NETDEV_A_NAPI_ID,
> NETDEV_A_NAPI_IRQ,
> NETDEV_A_NAPI_PID,
> + NETDEV_A_NAPI_IFNAME,
>
> __NETDEV_A_NAPI_MAX,
> NETDEV_A_NAPI_MAX = (__NETDEV_A_NAPI_MAX - 1)
> @@ -127,6 +128,7 @@ enum {
> NETDEV_A_QUEUE_IFINDEX,
> NETDEV_A_QUEUE_TYPE,
> NETDEV_A_QUEUE_NAPI_ID,
> + NETDEV_A_QUEUE_IFNAME,
>
> __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 fd98936..a886e6a 100644
> --- a/net/core/netdev-genl.c
> +++ b/net/core/netdev-genl.c
> @@ -181,6 +181,9 @@ netdev_nl_napi_fill_one(struct sk_buff *rsp, struct napi_struct *napi,
> if (nla_put_u32(rsp, NETDEV_A_NAPI_IFINDEX, napi->dev->ifindex))
> goto nla_put_failure;
>
> + if (nla_put_string(rsp, NETDEV_A_NAPI_IFNAME, napi->dev->name))
> + goto nla_put_failure;
> +
> if (napi->irq >= 0 && nla_put_u32(rsp, NETDEV_A_NAPI_IRQ, napi->irq))
> goto nla_put_failure;
>
> @@ -307,7 +310,8 @@ netdev_nl_queue_fill_one(struct sk_buff *rsp, struct net_device *netdev,
>
> if (nla_put_u32(rsp, NETDEV_A_QUEUE_ID, q_idx) ||
> nla_put_u32(rsp, NETDEV_A_QUEUE_TYPE, q_type) ||
> - nla_put_u32(rsp, NETDEV_A_QUEUE_IFINDEX, netdev->ifindex))
> + nla_put_u32(rsp, NETDEV_A_QUEUE_IFINDEX, netdev->ifindex) ||
> + nla_put_string(rsp, NETDEV_A_QUEUE_IFNAME, netdev->name))
> goto nla_put_failure;
>
> switch (q_type) {
> @@ -369,16 +373,19 @@ int netdev_nl_queue_get_doit(struct sk_buff *skb, struct genl_info *info)
> u32 q_id, q_type, ifindex;
> struct net_device *netdev;
> struct sk_buff *rsp;
> + char *ifname;
> int err;
>
> if (GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_ID) ||
> GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_TYPE) ||
> - GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_IFINDEX))
> + GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_IFINDEX) ||
> + GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_IFNAME))
> return -EINVAL;
>
> q_id = nla_get_u32(info->attrs[NETDEV_A_QUEUE_ID]);
> q_type = nla_get_u32(info->attrs[NETDEV_A_QUEUE_TYPE]);
> ifindex = nla_get_u32(info->attrs[NETDEV_A_QUEUE_IFINDEX]);
> + nla_strscpy(ifname, info->attrs[NETDEV_A_QUEUE_IFNAME], IFNAMSIZ);
>
> rsp = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
> if (!rsp)
> @@ -387,10 +394,15 @@ int netdev_nl_queue_get_doit(struct sk_buff *skb, struct genl_info *info)
> rtnl_lock();
>
> netdev = __dev_get_by_index(genl_info_net(info), ifindex);
> - if (netdev)
> - err = netdev_nl_queue_fill(rsp, netdev, q_id, q_type, info);
> - else
> +
> + if (strcmp(netdev->name, ifname)) {
> err = -ENODEV;
> + } else {
> + if (netdev)
> + err = netdev_nl_queue_fill(rsp, netdev, q_id, q_type, info);
> + else
> + err = -ENODEV;
> + }
>
This looks bit incorrect to me that the netdev is checked after
netdev->name is accessed. Shouldn't this be something like:
if (netdev && !strcmp(netdev->name, ifname))
err = netdev_nl_queue_fill(rsp, netdev, q_id, q_type, info);
else
err = -ENODEV;
> rtnl_unlock();
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 1/2] netdev-genl: Add ifname for queue and NAPI APIs
2024-02-21 15:57 ` [PATCH net-next 1/2] netdev-genl: Add ifname for queue and NAPI APIs Joe Damato
@ 2024-02-21 19:12 ` Jakub Kicinski
2024-02-21 19:17 ` Joe Damato
2024-02-21 19:12 ` Nambiar, Amritha
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Jakub Kicinski @ 2024-02-21 19:12 UTC (permalink / raw)
To: Joe Damato
Cc: netdev, linux-kernel, David S. Miller, Eric Dumazet, Paolo Abeni,
Jesper Dangaard Brouer, Stanislav Fomichev, Amritha Nambiar,
Larysa Zaremba, Lorenzo Bianconi, Tariq Toukan,
Sridhar Samudrala, Alexei Starovoitov, Maciej Fijalkowski
On Wed, 21 Feb 2024 07:57:29 -0800 Joe Damato wrote:
> if (GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_ID) ||
> GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_TYPE) ||
> - GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_IFINDEX))
> + GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_IFINDEX) ||
> + GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_IFNAME))
This means user always has to provide both ifindex and ifname,
right?
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net-next 1/2] netdev-genl: Add ifname for queue and NAPI APIs
2024-02-21 15:57 [PATCH net-next 0/2] Expose netdev name in netdev netlink APIs Joe Damato
@ 2024-02-21 15:57 ` Joe Damato
2024-02-21 19:12 ` Jakub Kicinski
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Joe Damato @ 2024-02-21 15:57 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: Joe Damato, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Jesper Dangaard Brouer, Stanislav Fomichev,
Amritha Nambiar, Larysa Zaremba, Lorenzo Bianconi, Tariq Toukan,
Sridhar Samudrala, Alexei Starovoitov, Maciej Fijalkowski
Expose the netdevice name when queue and NAPI netdev-genl APIs are used
Signed-off-by: Joe Damato <jdamato@fastly.com>
---
include/uapi/linux/netdev.h | 2 ++
net/core/netdev-genl.c | 22 +++++++++++++++++-----
2 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/include/uapi/linux/netdev.h b/include/uapi/linux/netdev.h
index 93cb411..80762bc 100644
--- a/include/uapi/linux/netdev.h
+++ b/include/uapi/linux/netdev.h
@@ -117,6 +117,7 @@ enum {
NETDEV_A_NAPI_ID,
NETDEV_A_NAPI_IRQ,
NETDEV_A_NAPI_PID,
+ NETDEV_A_NAPI_IFNAME,
__NETDEV_A_NAPI_MAX,
NETDEV_A_NAPI_MAX = (__NETDEV_A_NAPI_MAX - 1)
@@ -127,6 +128,7 @@ enum {
NETDEV_A_QUEUE_IFINDEX,
NETDEV_A_QUEUE_TYPE,
NETDEV_A_QUEUE_NAPI_ID,
+ NETDEV_A_QUEUE_IFNAME,
__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 fd98936..a886e6a 100644
--- a/net/core/netdev-genl.c
+++ b/net/core/netdev-genl.c
@@ -181,6 +181,9 @@ netdev_nl_napi_fill_one(struct sk_buff *rsp, struct napi_struct *napi,
if (nla_put_u32(rsp, NETDEV_A_NAPI_IFINDEX, napi->dev->ifindex))
goto nla_put_failure;
+ if (nla_put_string(rsp, NETDEV_A_NAPI_IFNAME, napi->dev->name))
+ goto nla_put_failure;
+
if (napi->irq >= 0 && nla_put_u32(rsp, NETDEV_A_NAPI_IRQ, napi->irq))
goto nla_put_failure;
@@ -307,7 +310,8 @@ netdev_nl_queue_fill_one(struct sk_buff *rsp, struct net_device *netdev,
if (nla_put_u32(rsp, NETDEV_A_QUEUE_ID, q_idx) ||
nla_put_u32(rsp, NETDEV_A_QUEUE_TYPE, q_type) ||
- nla_put_u32(rsp, NETDEV_A_QUEUE_IFINDEX, netdev->ifindex))
+ nla_put_u32(rsp, NETDEV_A_QUEUE_IFINDEX, netdev->ifindex) ||
+ nla_put_string(rsp, NETDEV_A_QUEUE_IFNAME, netdev->name))
goto nla_put_failure;
switch (q_type) {
@@ -369,16 +373,19 @@ int netdev_nl_queue_get_doit(struct sk_buff *skb, struct genl_info *info)
u32 q_id, q_type, ifindex;
struct net_device *netdev;
struct sk_buff *rsp;
+ char *ifname;
int err;
if (GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_ID) ||
GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_TYPE) ||
- GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_IFINDEX))
+ GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_IFINDEX) ||
+ GENL_REQ_ATTR_CHECK(info, NETDEV_A_QUEUE_IFNAME))
return -EINVAL;
q_id = nla_get_u32(info->attrs[NETDEV_A_QUEUE_ID]);
q_type = nla_get_u32(info->attrs[NETDEV_A_QUEUE_TYPE]);
ifindex = nla_get_u32(info->attrs[NETDEV_A_QUEUE_IFINDEX]);
+ nla_strscpy(ifname, info->attrs[NETDEV_A_QUEUE_IFNAME], IFNAMSIZ);
rsp = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
if (!rsp)
@@ -387,10 +394,15 @@ int netdev_nl_queue_get_doit(struct sk_buff *skb, struct genl_info *info)
rtnl_lock();
netdev = __dev_get_by_index(genl_info_net(info), ifindex);
- if (netdev)
- err = netdev_nl_queue_fill(rsp, netdev, q_id, q_type, info);
- else
+
+ if (strcmp(netdev->name, ifname)) {
err = -ENODEV;
+ } else {
+ if (netdev)
+ err = netdev_nl_queue_fill(rsp, netdev, q_id, q_type, info);
+ else
+ err = -ENODEV;
+ }
rtnl_unlock();
--
2.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-02-23 11:21 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-23 10:47 [PATCH net-next 1/2] netdev-genl: Add ifname for queue and NAPI APIs kernel test robot
-- strict thread matches above, loose matches on Subject: below --
2024-02-21 15:57 [PATCH net-next 0/2] Expose netdev name in netdev netlink APIs Joe Damato
2024-02-21 15:57 ` [PATCH net-next 1/2] netdev-genl: Add ifname for queue and NAPI APIs Joe Damato
2024-02-21 19:12 ` Jakub Kicinski
2024-02-21 19:17 ` Joe Damato
2024-02-21 19:12 ` Nambiar, Amritha
2024-02-21 19:23 ` Joe Damato
2024-02-22 20:55 ` kernel test robot
2024-02-23 11:21 ` Dan Carpenter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.