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
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
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
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?
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.
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();
>
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.
© 2016 - 2026 Red Hat, Inc.