* [PATCH net-next v2] netdev-genl: Elide napi_id when not present
@ 2025-02-03 19:17 Joe Damato
2025-02-03 19:50 ` Samudrala, Sridhar
2025-02-04 5:41 ` Eric Dumazet
0 siblings, 2 replies; 8+ messages in thread
From: Joe Damato @ 2025-02-03 19:17 UTC (permalink / raw)
To: netdev
Cc: kuba, Joe Damato, David S. Miller, Eric Dumazet, Paolo Abeni,
Simon Horman, Amritha Nambiar, Mina Almasry, open list
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>
---
v2:
- 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/
net/core/netdev-genl.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
index 715f85c6b62e..a97d3b99f6cd 100644
--- a/net/core/netdev-genl.c
+++ b/net/core/netdev-genl.c
@@ -385,9 +385,10 @@ 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))
- goto nla_put_failure;
+ if (rxq->napi && rxq->napi->napi_id >= MIN_NAPI_ID)
+ if (nla_put_u32(rsp, NETDEV_A_QUEUE_NAPI_ID,
+ rxq->napi->napi_id))
+ goto nla_put_failure;
binding = rxq->mp_params.mp_priv;
if (binding &&
@@ -397,9 +398,10 @@ 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))
- goto nla_put_failure;
+ if (txq->napi && txq->napi->napi_id >= MIN_NAPI_ID)
+ if (nla_put_u32(rsp, NETDEV_A_QUEUE_NAPI_ID,
+ txq->napi->napi_id))
+ goto nla_put_failure;
}
genlmsg_end(rsp, hdr);
base-commit: c2933b2befe25309f4c5cfbea0ca80909735fd76
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v2] netdev-genl: Elide napi_id when not present
2025-02-03 19:17 [PATCH net-next v2] netdev-genl: Elide napi_id when not present Joe Damato
@ 2025-02-03 19:50 ` Samudrala, Sridhar
2025-02-04 5:41 ` Eric Dumazet
1 sibling, 0 replies; 8+ messages in thread
From: Samudrala, Sridhar @ 2025-02-03 19:50 UTC (permalink / raw)
To: Joe Damato, netdev
Cc: kuba, David S. Miller, Eric Dumazet, Paolo Abeni, Simon Horman,
Amritha Nambiar, Mina Almasry, open list
On 2/3/2025 11:17 AM, Joe Damato wrote:
> 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>
Reviewed-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
> ---
> v2:
> - 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/
> net/core/netdev-genl.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
> index 715f85c6b62e..a97d3b99f6cd 100644
> --- a/net/core/netdev-genl.c
> +++ b/net/core/netdev-genl.c
> @@ -385,9 +385,10 @@ 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))
> - goto nla_put_failure;
> + if (rxq->napi && rxq->napi->napi_id >= MIN_NAPI_ID)
> + if (nla_put_u32(rsp, NETDEV_A_QUEUE_NAPI_ID,
> + rxq->napi->napi_id))
> + goto nla_put_failure;
>
> binding = rxq->mp_params.mp_priv;
> if (binding &&
> @@ -397,9 +398,10 @@ 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))
> - goto nla_put_failure;
> + if (txq->napi && txq->napi->napi_id >= MIN_NAPI_ID)
> + if (nla_put_u32(rsp, NETDEV_A_QUEUE_NAPI_ID,
> + txq->napi->napi_id))
> + goto nla_put_failure;
> }
>
> genlmsg_end(rsp, hdr);
>
> base-commit: c2933b2befe25309f4c5cfbea0ca80909735fd76
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v2] netdev-genl: Elide napi_id when not present
2025-02-03 19:17 [PATCH net-next v2] netdev-genl: Elide napi_id when not present Joe Damato
2025-02-03 19:50 ` Samudrala, Sridhar
@ 2025-02-04 5:41 ` Eric Dumazet
2025-02-04 13:02 ` Paolo Abeni
` (2 more replies)
1 sibling, 3 replies; 8+ messages in thread
From: Eric Dumazet @ 2025-02-04 5:41 UTC (permalink / raw)
To: Joe Damato
Cc: netdev, kuba, David S. Miller, Paolo Abeni, Simon Horman,
Amritha Nambiar, Mina Almasry, open list
On Mon, Feb 3, 2025 at 8:17 PM Joe Damato <jdamato@fastly.com> wrote:
>
> 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>
> ---
> v2:
> - 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/
> net/core/netdev-genl.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
> index 715f85c6b62e..a97d3b99f6cd 100644
> --- a/net/core/netdev-genl.c
> +++ b/net/core/netdev-genl.c
> @@ -385,9 +385,10 @@ 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))
> - goto nla_put_failure;
> + if (rxq->napi && rxq->napi->napi_id >= MIN_NAPI_ID)
> + if (nla_put_u32(rsp, NETDEV_A_QUEUE_NAPI_ID,
> + rxq->napi->napi_id))
> + goto nla_put_failure;
>
> binding = rxq->mp_params.mp_priv;
> if (binding &&
> @@ -397,9 +398,10 @@ 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))
> - goto nla_put_failure;
> + if (txq->napi && txq->napi->napi_id >= MIN_NAPI_ID)
> + if (nla_put_u32(rsp, NETDEV_A_QUEUE_NAPI_ID,
> + txq->napi->napi_id))
> + goto nla_put_failure;
> }
Hi Joe
This might be time to add helpers, we now have these checks about
MIN_NAPI_ID all around the places.
diff --git a/include/net/busy_poll.h b/include/net/busy_poll.h
index c39a426ebf52038b493b145e65e2d3d7f0c74e4c..741fa77547001aa5d966763c81540ee9210c949e
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 715f85c6b62eabcc61a10065f98cc914f5f0d975..b5c4e42351e67d73c0aa879156eea431d4baf7aa
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, txq->napi))
goto nla_put_failure;
}
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v2] netdev-genl: Elide napi_id when not present
2025-02-04 5:41 ` Eric Dumazet
@ 2025-02-04 13:02 ` Paolo Abeni
2025-02-04 18:13 ` Joe Damato
2025-02-04 18:28 ` Joe Damato
2 siblings, 0 replies; 8+ messages in thread
From: Paolo Abeni @ 2025-02-04 13:02 UTC (permalink / raw)
To: Eric Dumazet, Joe Damato
Cc: netdev, kuba, David S. Miller, Simon Horman, Amritha Nambiar,
Mina Almasry, open list
On 2/4/25 6:41 AM, Eric Dumazet wrote:
> On Mon, Feb 3, 2025 at 8:17 PM Joe Damato <jdamato@fastly.com> wrote:
>>
>> 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>
>> ---
>> v2:
>> - 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/
>> net/core/netdev-genl.c | 14 ++++++++------
>> 1 file changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
>> index 715f85c6b62e..a97d3b99f6cd 100644
>> --- a/net/core/netdev-genl.c
>> +++ b/net/core/netdev-genl.c
>> @@ -385,9 +385,10 @@ 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))
>> - goto nla_put_failure;
>> + if (rxq->napi && rxq->napi->napi_id >= MIN_NAPI_ID)
>> + if (nla_put_u32(rsp, NETDEV_A_QUEUE_NAPI_ID,
>> + rxq->napi->napi_id))
>> + goto nla_put_failure;
>>
>> binding = rxq->mp_params.mp_priv;
>> if (binding &&
>> @@ -397,9 +398,10 @@ 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))
>> - goto nla_put_failure;
>> + if (txq->napi && txq->napi->napi_id >= MIN_NAPI_ID)
>> + if (nla_put_u32(rsp, NETDEV_A_QUEUE_NAPI_ID,
>> + txq->napi->napi_id))
>> + goto nla_put_failure;
>> }
>
> Hi Joe
>
> This might be time to add helpers, we now have these checks about
> MIN_NAPI_ID all around the places.
+1.
IMHO a follow-up patch could additionally replace the existing
open-coded tests with the new helper.
/P
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v2] netdev-genl: Elide napi_id when not present
2025-02-04 5:41 ` Eric Dumazet
2025-02-04 13:02 ` Paolo Abeni
@ 2025-02-04 18:13 ` Joe Damato
2025-02-04 18:28 ` Joe Damato
2 siblings, 0 replies; 8+ messages in thread
From: Joe Damato @ 2025-02-04 18:13 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev, kuba, David S. Miller, Paolo Abeni, Simon Horman,
Amritha Nambiar, Mina Almasry, open list
On Tue, Feb 04, 2025 at 06:41:34AM +0100, Eric Dumazet wrote:
> On Mon, Feb 3, 2025 at 8:17 PM Joe Damato <jdamato@fastly.com> wrote:
> >
> > 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>
> > ---
> > v2:
> > - 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/
> > net/core/netdev-genl.c | 14 ++++++++------
> > 1 file changed, 8 insertions(+), 6 deletions(-)
> >
> > diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
> > index 715f85c6b62e..a97d3b99f6cd 100644
> > --- a/net/core/netdev-genl.c
> > +++ b/net/core/netdev-genl.c
> > @@ -385,9 +385,10 @@ 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))
> > - goto nla_put_failure;
> > + if (rxq->napi && rxq->napi->napi_id >= MIN_NAPI_ID)
> > + if (nla_put_u32(rsp, NETDEV_A_QUEUE_NAPI_ID,
> > + rxq->napi->napi_id))
> > + goto nla_put_failure;
> >
> > binding = rxq->mp_params.mp_priv;
> > if (binding &&
> > @@ -397,9 +398,10 @@ 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))
> > - goto nla_put_failure;
> > + if (txq->napi && txq->napi->napi_id >= MIN_NAPI_ID)
> > + if (nla_put_u32(rsp, NETDEV_A_QUEUE_NAPI_ID,
> > + txq->napi->napi_id))
> > + goto nla_put_failure;
> > }
>
> Hi Joe
>
> This might be time to add helpers, we now have these checks about
> MIN_NAPI_ID all around the places.
Thanks that's a good idea. I'll add that for the v3.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v2] netdev-genl: Elide napi_id when not present
2025-02-04 5:41 ` Eric Dumazet
2025-02-04 13:02 ` Paolo Abeni
2025-02-04 18:13 ` Joe Damato
@ 2025-02-04 18:28 ` Joe Damato
2025-02-04 18:53 ` Eric Dumazet
2 siblings, 1 reply; 8+ messages in thread
From: Joe Damato @ 2025-02-04 18:28 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev, kuba, David S. Miller, Paolo Abeni, Simon Horman,
Amritha Nambiar, Mina Almasry, open list
On Tue, Feb 04, 2025 at 06:41:34AM +0100, Eric Dumazet wrote:
> On Mon, Feb 3, 2025 at 8:17 PM Joe Damato <jdamato@fastly.com> wrote:
> >
> > 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>
> > ---
> > v2:
> > - 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/
> > net/core/netdev-genl.c | 14 ++++++++------
> > 1 file changed, 8 insertions(+), 6 deletions(-)
> >
> > diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
> > index 715f85c6b62e..a97d3b99f6cd 100644
> > --- a/net/core/netdev-genl.c
> > +++ b/net/core/netdev-genl.c
> > @@ -385,9 +385,10 @@ 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))
> > - goto nla_put_failure;
> > + if (rxq->napi && rxq->napi->napi_id >= MIN_NAPI_ID)
> > + if (nla_put_u32(rsp, NETDEV_A_QUEUE_NAPI_ID,
> > + rxq->napi->napi_id))
> > + goto nla_put_failure;
> >
> > binding = rxq->mp_params.mp_priv;
> > if (binding &&
> > @@ -397,9 +398,10 @@ 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))
> > - goto nla_put_failure;
> > + if (txq->napi && txq->napi->napi_id >= MIN_NAPI_ID)
> > + if (nla_put_u32(rsp, NETDEV_A_QUEUE_NAPI_ID,
> > + txq->napi->napi_id))
> > + goto nla_put_failure;
> > }
>
> Hi Joe
>
> This might be time to add helpers, we now have these checks about
> MIN_NAPI_ID all around the places.
I'm not sure what the right etiquette is; I was thinking of just
taking the patch you proposed below and submitting it with you as
the author with my Reviewed-by.
Is that OK and if so, are you OK with the commit message?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v2] netdev-genl: Elide napi_id when not present
2025-02-04 18:28 ` Joe Damato
@ 2025-02-04 18:53 ` Eric Dumazet
2025-02-04 18:56 ` Joe Damato
0 siblings, 1 reply; 8+ messages in thread
From: Eric Dumazet @ 2025-02-04 18:53 UTC (permalink / raw)
To: Joe Damato, Eric Dumazet, netdev, kuba, David S. Miller,
Paolo Abeni, Simon Horman, Amritha Nambiar, Mina Almasry,
open list
On Tue, Feb 4, 2025 at 7:28 PM Joe Damato <jdamato@fastly.com> wrote:
>
> On Tue, Feb 04, 2025 at 06:41:34AM +0100, Eric Dumazet wrote:
> > On Mon, Feb 3, 2025 at 8:17 PM Joe Damato <jdamato@fastly.com> wrote:
> > >
> > > 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>
> > > ---
> > > v2:
> > > - 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/
> > > net/core/netdev-genl.c | 14 ++++++++------
> > > 1 file changed, 8 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
> > > index 715f85c6b62e..a97d3b99f6cd 100644
> > > --- a/net/core/netdev-genl.c
> > > +++ b/net/core/netdev-genl.c
> > > @@ -385,9 +385,10 @@ 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))
> > > - goto nla_put_failure;
> > > + if (rxq->napi && rxq->napi->napi_id >= MIN_NAPI_ID)
> > > + if (nla_put_u32(rsp, NETDEV_A_QUEUE_NAPI_ID,
> > > + rxq->napi->napi_id))
> > > + goto nla_put_failure;
> > >
> > > binding = rxq->mp_params.mp_priv;
> > > if (binding &&
> > > @@ -397,9 +398,10 @@ 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))
> > > - goto nla_put_failure;
> > > + if (txq->napi && txq->napi->napi_id >= MIN_NAPI_ID)
> > > + if (nla_put_u32(rsp, NETDEV_A_QUEUE_NAPI_ID,
> > > + txq->napi->napi_id))
> > > + goto nla_put_failure;
> > > }
> >
> > Hi Joe
> >
> > This might be time to add helpers, we now have these checks about
> > MIN_NAPI_ID all around the places.
>
> I'm not sure what the right etiquette is; I was thinking of just
> taking the patch you proposed below and submitting it with you as
> the author with my Reviewed-by.
>
> Is that OK and if so, are you OK with the commit message?
Oh no worries, please grab the patch completely, I will add my usual
'Reviewed-by' tag.
Thanks
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next v2] netdev-genl: Elide napi_id when not present
2025-02-04 18:53 ` Eric Dumazet
@ 2025-02-04 18:56 ` Joe Damato
0 siblings, 0 replies; 8+ messages in thread
From: Joe Damato @ 2025-02-04 18:56 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev, kuba, David S. Miller, Paolo Abeni, Simon Horman,
Amritha Nambiar, Mina Almasry, open list
On Tue, Feb 04, 2025 at 07:53:51PM +0100, Eric Dumazet wrote:
> On Tue, Feb 4, 2025 at 7:28 PM Joe Damato <jdamato@fastly.com> wrote:
> >
> > On Tue, Feb 04, 2025 at 06:41:34AM +0100, Eric Dumazet wrote:
> > > On Mon, Feb 3, 2025 at 8:17 PM Joe Damato <jdamato@fastly.com> wrote:
[...]
> > >
> > > Hi Joe
> > >
> > > This might be time to add helpers, we now have these checks about
> > > MIN_NAPI_ID all around the places.
> >
> > I'm not sure what the right etiquette is; I was thinking of just
> > taking the patch you proposed below and submitting it with you as
> > the author with my Reviewed-by.
> >
> > Is that OK and if so, are you OK with the commit message?
>
> Oh no worries, please grab the patch completely, I will add my usual
> 'Reviewed-by' tag.
OK, will do. Thanks for the guidance.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-02-04 18:56 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-03 19:17 [PATCH net-next v2] netdev-genl: Elide napi_id when not present Joe Damato
2025-02-03 19:50 ` Samudrala, Sridhar
2025-02-04 5:41 ` Eric Dumazet
2025-02-04 13:02 ` Paolo Abeni
2025-02-04 18:13 ` Joe Damato
2025-02-04 18:28 ` Joe Damato
2025-02-04 18:53 ` Eric Dumazet
2025-02-04 18:56 ` 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).