Slight refactor to prepare the code for NAPI to queue mapping. No
functional changes.

Signed-off-by: Joe Damato <jdamato@fastly.com>
Reviewed-by: Gerhard Engleder <gerhard@engleder-embedded.com>
Tested-by: Lei Yang <leiyang@redhat.com>
---
 v2:
   - Previously patch 1 in the v1.
   - Added Reviewed-by and Tested-by tags to commit message. No
     functional changes.

 drivers/net/virtio_net.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 7646ddd9bef7..cff18c66b54a 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2789,7 +2789,8 @@ static void skb_recv_done(struct virtqueue *rvq)
 	virtqueue_napi_schedule(&rq->napi, rvq);
 }
 
-static void virtnet_napi_enable(struct virtqueue *vq, struct napi_struct *napi)
+static void virtnet_napi_do_enable(struct virtqueue *vq,
+				   struct napi_struct *napi)
 {
 	napi_enable(napi);
 
@@ -2802,6 +2803,11 @@ static void virtnet_napi_enable(struct virtqueue *vq, struct napi_struct *napi)
 	local_bh_enable();
 }
 
+static void virtnet_napi_enable(struct virtqueue *vq, struct napi_struct *napi)
+{
+	virtnet_napi_do_enable(vq, napi);
+}
+
 static void virtnet_napi_tx_enable(struct virtnet_info *vi,
 				   struct virtqueue *vq,
 				   struct napi_struct *napi)
@@ -2817,7 +2823,7 @@ static void virtnet_napi_tx_enable(struct virtnet_info *vi,
 		return;
 	}
 
-	return virtnet_napi_enable(vq, napi);
+	virtnet_napi_do_enable(vq, napi);
 }
 
 static void virtnet_napi_tx_disable(struct napi_struct *napi)
-- 
2.25.1

On Wed, Jan 22, 2025 at 3:11 AM Joe Damato <jdamato@fastly.com> wrote:
>
> Slight refactor to prepare the code for NAPI to queue mapping. No
> functional changes.
>
> Signed-off-by: Joe Damato <jdamato@fastly.com>
> Reviewed-by: Gerhard Engleder <gerhard@engleder-embedded.com>
> Tested-by: Lei Yang <leiyang@redhat.com>
> ---
>  v2:
>    - Previously patch 1 in the v1.
>    - Added Reviewed-by and Tested-by tags to commit message. No
>      functional changes.
>
>  drivers/net/virtio_net.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 7646ddd9bef7..cff18c66b54a 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -2789,7 +2789,8 @@ static void skb_recv_done(struct virtqueue *rvq)
>         virtqueue_napi_schedule(&rq->napi, rvq);
>  }
>
> -static void virtnet_napi_enable(struct virtqueue *vq, struct napi_struct *napi)
> +static void virtnet_napi_do_enable(struct virtqueue *vq,
> +                                  struct napi_struct *napi)
>  {
>         napi_enable(napi);

Nit: it might be better to not have this helper to avoid a misuse of
this function directly.

Other than this.

Acked-by: Jason Wang <jasowang@redhat.com>

Thanks

>
> @@ -2802,6 +2803,11 @@ static void virtnet_napi_enable(struct virtqueue *vq, struct napi_struct *napi)
>         local_bh_enable();
>  }
>
> +static void virtnet_napi_enable(struct virtqueue *vq, struct napi_struct *napi)
> +{
> +       virtnet_napi_do_enable(vq, napi);
> +}
> +
>  static void virtnet_napi_tx_enable(struct virtnet_info *vi,
>                                    struct virtqueue *vq,
>                                    struct napi_struct *napi)
> @@ -2817,7 +2823,7 @@ static void virtnet_napi_tx_enable(struct virtnet_info *vi,
>                 return;
>         }
>
> -       return virtnet_napi_enable(vq, napi);
> +       virtnet_napi_do_enable(vq, napi);
>  }
>
>  static void virtnet_napi_tx_disable(struct napi_struct *napi)
> --
> 2.25.1
>

On Wed, Jan 22, 2025 at 02:12:46PM +0800, Jason Wang wrote:
> On Wed, Jan 22, 2025 at 3:11 AM Joe Damato <jdamato@fastly.com> wrote:
> >
> > Slight refactor to prepare the code for NAPI to queue mapping. No
> > functional changes.
> >
> > Signed-off-by: Joe Damato <jdamato@fastly.com>
> > Reviewed-by: Gerhard Engleder <gerhard@engleder-embedded.com>
> > Tested-by: Lei Yang <leiyang@redhat.com>
> > ---
> >  v2:
> >    - Previously patch 1 in the v1.
> >    - Added Reviewed-by and Tested-by tags to commit message. No
> >      functional changes.
> >
> >  drivers/net/virtio_net.c | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 7646ddd9bef7..cff18c66b54a 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -2789,7 +2789,8 @@ static void skb_recv_done(struct virtqueue *rvq)
> >         virtqueue_napi_schedule(&rq->napi, rvq);
> >  }
> >
> > -static void virtnet_napi_enable(struct virtqueue *vq, struct napi_struct *napi)
> > +static void virtnet_napi_do_enable(struct virtqueue *vq,
> > +                                  struct napi_struct *napi)
> >  {
> >         napi_enable(napi);
> 
> Nit: it might be better to not have this helper to avoid a misuse of
> this function directly.

Sorry, I'm probably missing something here.

Both virtnet_napi_enable and virtnet_napi_tx_enable need the logic
in virtnet_napi_do_enable.

Are you suggesting that I remove virtnet_napi_do_enable and repeat
the block of code in there twice (in virtnet_napi_enable and
virtnet_napi_tx_enable)?

Just seemed like a lot of code to repeat twice and a helper would be
cleaner?

Let me know; since net-next is closed there is a plenty of time to
get this to where you'd like it to be before new code is accepted.

> Other than this.
> 
> Acked-by: Jason Wang <jasowang@redhat.com>

Thanks.

On Thu, Jan 23, 2025 at 1:41 AM Joe Damato <jdamato@fastly.com> wrote:
>
> On Wed, Jan 22, 2025 at 02:12:46PM +0800, Jason Wang wrote:
> > On Wed, Jan 22, 2025 at 3:11 AM Joe Damato <jdamato@fastly.com> wrote:
> > >
> > > Slight refactor to prepare the code for NAPI to queue mapping. No
> > > functional changes.
> > >
> > > Signed-off-by: Joe Damato <jdamato@fastly.com>
> > > Reviewed-by: Gerhard Engleder <gerhard@engleder-embedded.com>
> > > Tested-by: Lei Yang <leiyang@redhat.com>
> > > ---
> > >  v2:
> > >    - Previously patch 1 in the v1.
> > >    - Added Reviewed-by and Tested-by tags to commit message. No
> > >      functional changes.
> > >
> > >  drivers/net/virtio_net.c | 10 ++++++++--
> > >  1 file changed, 8 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > index 7646ddd9bef7..cff18c66b54a 100644
> > > --- a/drivers/net/virtio_net.c
> > > +++ b/drivers/net/virtio_net.c
> > > @@ -2789,7 +2789,8 @@ static void skb_recv_done(struct virtqueue *rvq)
> > >         virtqueue_napi_schedule(&rq->napi, rvq);
> > >  }
> > >
> > > -static void virtnet_napi_enable(struct virtqueue *vq, struct napi_struct *napi)
> > > +static void virtnet_napi_do_enable(struct virtqueue *vq,
> > > +                                  struct napi_struct *napi)
> > >  {
> > >         napi_enable(napi);
> >
> > Nit: it might be better to not have this helper to avoid a misuse of
> > this function directly.
>
> Sorry, I'm probably missing something here.
>
> Both virtnet_napi_enable and virtnet_napi_tx_enable need the logic
> in virtnet_napi_do_enable.
>
> Are you suggesting that I remove virtnet_napi_do_enable and repeat
> the block of code in there twice (in virtnet_napi_enable and
> virtnet_napi_tx_enable)?

I think I miss something here, it looks like virtnet_napi_tx_enable()
calls virtnet_napi_do_enable() directly.

I would like to know why we don't call netif_queue_set_napi() for TX NAPI here?

Thanks

>
> Just seemed like a lot of code to repeat twice and a helper would be
> cleaner?
>
> Let me know; since net-next is closed there is a plenty of time to
> get this to where you'd like it to be before new code is accepted.
>
> > Other than this.
> >
> > Acked-by: Jason Wang <jasowang@redhat.com>
>
> Thanks.
>

On Thu, Jan 23, 2025 at 10:40:43AM +0800, Jason Wang wrote:
> On Thu, Jan 23, 2025 at 1:41 AM Joe Damato <jdamato@fastly.com> wrote:
> >
> > On Wed, Jan 22, 2025 at 02:12:46PM +0800, Jason Wang wrote:
> > > On Wed, Jan 22, 2025 at 3:11 AM Joe Damato <jdamato@fastly.com> wrote:
> > > >
> > > > Slight refactor to prepare the code for NAPI to queue mapping. No
> > > > functional changes.
> > > >
> > > > Signed-off-by: Joe Damato <jdamato@fastly.com>
> > > > Reviewed-by: Gerhard Engleder <gerhard@engleder-embedded.com>
> > > > Tested-by: Lei Yang <leiyang@redhat.com>
> > > > ---
> > > >  v2:
> > > >    - Previously patch 1 in the v1.
> > > >    - Added Reviewed-by and Tested-by tags to commit message. No
> > > >      functional changes.
> > > >
> > > >  drivers/net/virtio_net.c | 10 ++++++++--
> > > >  1 file changed, 8 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > > index 7646ddd9bef7..cff18c66b54a 100644
> > > > --- a/drivers/net/virtio_net.c
> > > > +++ b/drivers/net/virtio_net.c
> > > > @@ -2789,7 +2789,8 @@ static void skb_recv_done(struct virtqueue *rvq)
> > > >         virtqueue_napi_schedule(&rq->napi, rvq);
> > > >  }
> > > >
> > > > -static void virtnet_napi_enable(struct virtqueue *vq, struct napi_struct *napi)
> > > > +static void virtnet_napi_do_enable(struct virtqueue *vq,
> > > > +                                  struct napi_struct *napi)
> > > >  {
> > > >         napi_enable(napi);
> > >
> > > Nit: it might be better to not have this helper to avoid a misuse of
> > > this function directly.
> >
> > Sorry, I'm probably missing something here.
> >
> > Both virtnet_napi_enable and virtnet_napi_tx_enable need the logic
> > in virtnet_napi_do_enable.
> >
> > Are you suggesting that I remove virtnet_napi_do_enable and repeat
> > the block of code in there twice (in virtnet_napi_enable and
> > virtnet_napi_tx_enable)?
> 
> I think I miss something here, it looks like virtnet_napi_tx_enable()
> calls virtnet_napi_do_enable() directly.
> 
> I would like to know why we don't call netif_queue_set_napi() for TX NAPI here?

Please see both the cover letter and the commit message of the next
commit which addresses this question.

TX-only NAPIs do not have NAPI IDs so there is nothing to map.

On Thu, Jan 23, 2025 at 10:47 AM Joe Damato <jdamato@fastly.com> wrote:
>
> On Thu, Jan 23, 2025 at 10:40:43AM +0800, Jason Wang wrote:
> > On Thu, Jan 23, 2025 at 1:41 AM Joe Damato <jdamato@fastly.com> wrote:
> > >
> > > On Wed, Jan 22, 2025 at 02:12:46PM +0800, Jason Wang wrote:
> > > > On Wed, Jan 22, 2025 at 3:11 AM Joe Damato <jdamato@fastly.com> wrote:
> > > > >
> > > > > Slight refactor to prepare the code for NAPI to queue mapping. No
> > > > > functional changes.
> > > > >
> > > > > Signed-off-by: Joe Damato <jdamato@fastly.com>
> > > > > Reviewed-by: Gerhard Engleder <gerhard@engleder-embedded.com>
> > > > > Tested-by: Lei Yang <leiyang@redhat.com>
> > > > > ---
> > > > >  v2:
> > > > >    - Previously patch 1 in the v1.
> > > > >    - Added Reviewed-by and Tested-by tags to commit message. No
> > > > >      functional changes.
> > > > >
> > > > >  drivers/net/virtio_net.c | 10 ++++++++--
> > > > >  1 file changed, 8 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > > > index 7646ddd9bef7..cff18c66b54a 100644
> > > > > --- a/drivers/net/virtio_net.c
> > > > > +++ b/drivers/net/virtio_net.c
> > > > > @@ -2789,7 +2789,8 @@ static void skb_recv_done(struct virtqueue *rvq)
> > > > >         virtqueue_napi_schedule(&rq->napi, rvq);
> > > > >  }
> > > > >
> > > > > -static void virtnet_napi_enable(struct virtqueue *vq, struct napi_struct *napi)
> > > > > +static void virtnet_napi_do_enable(struct virtqueue *vq,
> > > > > +                                  struct napi_struct *napi)
> > > > >  {
> > > > >         napi_enable(napi);
> > > >
> > > > Nit: it might be better to not have this helper to avoid a misuse of
> > > > this function directly.
> > >
> > > Sorry, I'm probably missing something here.
> > >
> > > Both virtnet_napi_enable and virtnet_napi_tx_enable need the logic
> > > in virtnet_napi_do_enable.
> > >
> > > Are you suggesting that I remove virtnet_napi_do_enable and repeat
> > > the block of code in there twice (in virtnet_napi_enable and
> > > virtnet_napi_tx_enable)?
> >
> > I think I miss something here, it looks like virtnet_napi_tx_enable()
> > calls virtnet_napi_do_enable() directly.
> >
> > I would like to know why we don't call netif_queue_set_napi() for TX NAPI here?
>
> Please see both the cover letter and the commit message of the next
> commit which addresses this question.
>
> TX-only NAPIs do not have NAPI IDs so there is nothing to map.

Interesting, but I have more questions

1) why need a driver to know the NAPI implementation like this?

2) does NAPI know (or why it needs to know) whether or not it's a TX
or not? I only see the following code in napi_hash_add():

static void napi_hash_add(struct napi_struct *napi)
{
        unsigned long flags;

        if (test_bit(NAPI_STATE_NO_BUSY_POLL, &napi->state))
                return;

...

        __napi_hash_add_with_id(napi, napi_gen_id);

        spin_unlock_irqrestore(&napi_hash_lock, flags);
}

It seems it only matters with NAPI_STATE_NO_BUSY_POLL.

And if NAPI knows everything, should it be better to just do the
linking in napi_enable/disable() instead of letting each driver do it
by itself?

Thanks

>

On Fri, Jan 24, 2025 at 09:14:54AM +0800, Jason Wang wrote:
> On Thu, Jan 23, 2025 at 10:47 AM Joe Damato <jdamato@fastly.com> wrote:
> >
> > On Thu, Jan 23, 2025 at 10:40:43AM +0800, Jason Wang wrote:
> > > On Thu, Jan 23, 2025 at 1:41 AM Joe Damato <jdamato@fastly.com> wrote:
> > > >
> > > > On Wed, Jan 22, 2025 at 02:12:46PM +0800, Jason Wang wrote:
> > > > > On Wed, Jan 22, 2025 at 3:11 AM Joe Damato <jdamato@fastly.com> wrote:
> > > > > >
> > > > > > Slight refactor to prepare the code for NAPI to queue mapping. No
> > > > > > functional changes.
> > > > > >
> > > > > > Signed-off-by: Joe Damato <jdamato@fastly.com>
> > > > > > Reviewed-by: Gerhard Engleder <gerhard@engleder-embedded.com>
> > > > > > Tested-by: Lei Yang <leiyang@redhat.com>
> > > > > > ---
> > > > > >  v2:
> > > > > >    - Previously patch 1 in the v1.
> > > > > >    - Added Reviewed-by and Tested-by tags to commit message. No
> > > > > >      functional changes.
> > > > > >
> > > > > >  drivers/net/virtio_net.c | 10 ++++++++--
> > > > > >  1 file changed, 8 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > > > > index 7646ddd9bef7..cff18c66b54a 100644
> > > > > > --- a/drivers/net/virtio_net.c
> > > > > > +++ b/drivers/net/virtio_net.c
> > > > > > @@ -2789,7 +2789,8 @@ static void skb_recv_done(struct virtqueue *rvq)
> > > > > >         virtqueue_napi_schedule(&rq->napi, rvq);
> > > > > >  }
> > > > > >
> > > > > > -static void virtnet_napi_enable(struct virtqueue *vq, struct napi_struct *napi)
> > > > > > +static void virtnet_napi_do_enable(struct virtqueue *vq,
> > > > > > +                                  struct napi_struct *napi)
> > > > > >  {
> > > > > >         napi_enable(napi);
> > > > >
> > > > > Nit: it might be better to not have this helper to avoid a misuse of
> > > > > this function directly.
> > > >
> > > > Sorry, I'm probably missing something here.
> > > >
> > > > Both virtnet_napi_enable and virtnet_napi_tx_enable need the logic
> > > > in virtnet_napi_do_enable.
> > > >
> > > > Are you suggesting that I remove virtnet_napi_do_enable and repeat
> > > > the block of code in there twice (in virtnet_napi_enable and
> > > > virtnet_napi_tx_enable)?
> > >
> > > I think I miss something here, it looks like virtnet_napi_tx_enable()
> > > calls virtnet_napi_do_enable() directly.
> > >
> > > I would like to know why we don't call netif_queue_set_napi() for TX NAPI here?
> >
> > Please see both the cover letter and the commit message of the next
> > commit which addresses this question.
> >
> > TX-only NAPIs do not have NAPI IDs so there is nothing to map.
> 
> Interesting, but I have more questions
> 
> 1) why need a driver to know the NAPI implementation like this?

I'm not sure I understand the question, but I'll try to give an
answer and please let me know if you have another question.

Mapping the NAPI IDs to queue IDs is useful for applications that
use epoll based busy polling (which relies on the NAPI ID, see also
SO_INCOMING_NAPI_ID and [1]), IRQ suspension [2], and generally
per-NAPI configuration [3].

Without this code added to the driver, the user application can get
the NAPI ID of an incoming connection, but has no way to know which
queue (or NIC) that NAPI ID is associated with or to set per-NAPI
configuration settings.

[1]: https://lore.kernel.org/all/20240213061652.6342-1-jdamato@fastly.com/
[2]: https://lore.kernel.org/netdev/20241109050245.191288-5-jdamato@fastly.com/T/
[3]: https://lore.kernel.org/lkml/20241011184527.16393-1-jdamato@fastly.com/

> 2) does NAPI know (or why it needs to know) whether or not it's a TX
> or not? I only see the following code in napi_hash_add():

Note that I did not write the original implementation of NAPI IDs or
epoll-based busy poll, so I can only comment on what I know :)

I don't know why TX-only NAPIs do not have NAPI IDs. My guess is
that in the original implementation, the code was designed only for
RX busy polling, so TX-only NAPIs were not assigned NAPI IDs.

Perhaps in the future, TX-only NAPIs will be assigned NAPI IDs, but
currently they do not have NAPI IDs.

> static void napi_hash_add(struct napi_struct *napi)
> {
>         unsigned long flags;
> 
>         if (test_bit(NAPI_STATE_NO_BUSY_POLL, &napi->state))
>                 return;
> 
> ...
> 
>         __napi_hash_add_with_id(napi, napi_gen_id);
> 
>         spin_unlock_irqrestore(&napi_hash_lock, flags);
> }
> 
> It seems it only matters with NAPI_STATE_NO_BUSY_POLL.
> 
> And if NAPI knows everything, should it be better to just do the
> linking in napi_enable/disable() instead of letting each driver do it
> by itself?

It would be nice if this were possible, I agree. Perhaps in the
future some work could be done to make this possible.

I believe that this is not currently possible because the NAPI does
not know which queue ID it is associated with. That mapping of which
queue is associated with which NAPI is established in patch 3
(please see the commit message of patch 3 to see an example of the
output).

The driver knows both the queue ID and the NAPI for that queue, so
the mapping can be established only by the driver.

Let me know if that helps.

- Joe

On Sat, Jan 25, 2025 at 4:19 AM Joe Damato <jdamato@fastly.com> wrote:
>
> On Fri, Jan 24, 2025 at 09:14:54AM +0800, Jason Wang wrote:
> > On Thu, Jan 23, 2025 at 10:47 AM Joe Damato <jdamato@fastly.com> wrote:
> > >
> > > On Thu, Jan 23, 2025 at 10:40:43AM +0800, Jason Wang wrote:
> > > > On Thu, Jan 23, 2025 at 1:41 AM Joe Damato <jdamato@fastly.com> wrote:
> > > > >
> > > > > On Wed, Jan 22, 2025 at 02:12:46PM +0800, Jason Wang wrote:
> > > > > > On Wed, Jan 22, 2025 at 3:11 AM Joe Damato <jdamato@fastly.com> wrote:
> > > > > > >
> > > > > > > Slight refactor to prepare the code for NAPI to queue mapping. No
> > > > > > > functional changes.
> > > > > > >
> > > > > > > Signed-off-by: Joe Damato <jdamato@fastly.com>
> > > > > > > Reviewed-by: Gerhard Engleder <gerhard@engleder-embedded.com>
> > > > > > > Tested-by: Lei Yang <leiyang@redhat.com>
> > > > > > > ---
> > > > > > >  v2:
> > > > > > >    - Previously patch 1 in the v1.
> > > > > > >    - Added Reviewed-by and Tested-by tags to commit message. No
> > > > > > >      functional changes.
> > > > > > >
> > > > > > >  drivers/net/virtio_net.c | 10 ++++++++--
> > > > > > >  1 file changed, 8 insertions(+), 2 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > > > > > index 7646ddd9bef7..cff18c66b54a 100644
> > > > > > > --- a/drivers/net/virtio_net.c
> > > > > > > +++ b/drivers/net/virtio_net.c
> > > > > > > @@ -2789,7 +2789,8 @@ static void skb_recv_done(struct virtqueue *rvq)
> > > > > > >         virtqueue_napi_schedule(&rq->napi, rvq);
> > > > > > >  }
> > > > > > >
> > > > > > > -static void virtnet_napi_enable(struct virtqueue *vq, struct napi_struct *napi)
> > > > > > > +static void virtnet_napi_do_enable(struct virtqueue *vq,
> > > > > > > +                                  struct napi_struct *napi)
> > > > > > >  {
> > > > > > >         napi_enable(napi);
> > > > > >
> > > > > > Nit: it might be better to not have this helper to avoid a misuse of
> > > > > > this function directly.
> > > > >
> > > > > Sorry, I'm probably missing something here.
> > > > >
> > > > > Both virtnet_napi_enable and virtnet_napi_tx_enable need the logic
> > > > > in virtnet_napi_do_enable.
> > > > >
> > > > > Are you suggesting that I remove virtnet_napi_do_enable and repeat
> > > > > the block of code in there twice (in virtnet_napi_enable and
> > > > > virtnet_napi_tx_enable)?
> > > >
> > > > I think I miss something here, it looks like virtnet_napi_tx_enable()
> > > > calls virtnet_napi_do_enable() directly.
> > > >
> > > > I would like to know why we don't call netif_queue_set_napi() for TX NAPI here?
> > >
> > > Please see both the cover letter and the commit message of the next
> > > commit which addresses this question.
> > >
> > > TX-only NAPIs do not have NAPI IDs so there is nothing to map.
> >
> > Interesting, but I have more questions
> >
> > 1) why need a driver to know the NAPI implementation like this?
>
> I'm not sure I understand the question, but I'll try to give an
> answer and please let me know if you have another question.
>
> Mapping the NAPI IDs to queue IDs is useful for applications that
> use epoll based busy polling (which relies on the NAPI ID, see also
> SO_INCOMING_NAPI_ID and [1]), IRQ suspension [2], and generally
> per-NAPI configuration [3].
>
> Without this code added to the driver, the user application can get
> the NAPI ID of an incoming connection, but has no way to know which
> queue (or NIC) that NAPI ID is associated with or to set per-NAPI
> configuration settings.
>
> [1]: https://lore.kernel.org/all/20240213061652.6342-1-jdamato@fastly.com/
> [2]: https://lore.kernel.org/netdev/20241109050245.191288-5-jdamato@fastly.com/T/
> [3]: https://lore.kernel.org/lkml/20241011184527.16393-1-jdamato@fastly.com/

Yes, exactly. Sorry for being unclear, what I want to ask is actually:

1) TX NAPI doesn't have a NAPI ID, this seems more like a NAPI
implementation details which should be hidden from the driver.
2) If 1 is true, in the netif_queue_set_napi(), should it be better to
add and check for whether or not NAPI has an ID and return early if it
doesn't have one
3) Then driver doesn't need to know NAPI implementation details like
NAPI stuffs?

>
> > 2) does NAPI know (or why it needs to know) whether or not it's a TX
> > or not? I only see the following code in napi_hash_add():
>
> Note that I did not write the original implementation of NAPI IDs or
> epoll-based busy poll, so I can only comment on what I know :)
>
> I don't know why TX-only NAPIs do not have NAPI IDs. My guess is
> that in the original implementation, the code was designed only for
> RX busy polling, so TX-only NAPIs were not assigned NAPI IDs.
>
> Perhaps in the future, TX-only NAPIs will be assigned NAPI IDs, but
> currently they do not have NAPI IDs.

Jakub, could you please help to clarify this part?

>
> > static void napi_hash_add(struct napi_struct *napi)
> > {
> >         unsigned long flags;
> >
> >         if (test_bit(NAPI_STATE_NO_BUSY_POLL, &napi->state))
> >                 return;
> >
> > ...
> >
> >         __napi_hash_add_with_id(napi, napi_gen_id);
> >
> >         spin_unlock_irqrestore(&napi_hash_lock, flags);
> > }
> >
> > It seems it only matters with NAPI_STATE_NO_BUSY_POLL.
> >
> > And if NAPI knows everything, should it be better to just do the
> > linking in napi_enable/disable() instead of letting each driver do it
> > by itself?
>
> It would be nice if this were possible, I agree. Perhaps in the
> future some work could be done to make this possible.
>
> I believe that this is not currently possible because the NAPI does
> not know which queue ID it is associated with. That mapping of which
> queue is associated with which NAPI is established in patch 3
> (please see the commit message of patch 3 to see an example of the
> output).
>
> The driver knows both the queue ID and the NAPI for that queue, so
> the mapping can be established only by the driver.
>
> Let me know if that helps.

Yes, definitely.

Let's see Jakub's comment.

Thanks

>
> - Joe
>

On Sun, Jan 26, 2025 at 04:04:02PM +0800, Jason Wang wrote:
> On Sat, Jan 25, 2025 at 4:19 AM Joe Damato <jdamato@fastly.com> wrote:
> >
> > On Fri, Jan 24, 2025 at 09:14:54AM +0800, Jason Wang wrote:
> > > On Thu, Jan 23, 2025 at 10:47 AM Joe Damato <jdamato@fastly.com> wrote:
> > > >
> > > > On Thu, Jan 23, 2025 at 10:40:43AM +0800, Jason Wang wrote:
> > > > > On Thu, Jan 23, 2025 at 1:41 AM Joe Damato <jdamato@fastly.com> wrote:
> > > > > >
> > > > > > On Wed, Jan 22, 2025 at 02:12:46PM +0800, Jason Wang wrote:
> > > > > > > On Wed, Jan 22, 2025 at 3:11 AM Joe Damato <jdamato@fastly.com> wrote:

[...]

> > > > > > > >
> > > > > > > > -static void virtnet_napi_enable(struct virtqueue *vq, struct napi_struct *napi)
> > > > > > > > +static void virtnet_napi_do_enable(struct virtqueue *vq,
> > > > > > > > +                                  struct napi_struct *napi)
> > > > > > > >  {
> > > > > > > >         napi_enable(napi);
> > > > > > >
> > > > > > > Nit: it might be better to not have this helper to avoid a misuse of
> > > > > > > this function directly.
> > > > > >
> > > > > > Sorry, I'm probably missing something here.
> > > > > >
> > > > > > Both virtnet_napi_enable and virtnet_napi_tx_enable need the logic
> > > > > > in virtnet_napi_do_enable.
> > > > > >
> > > > > > Are you suggesting that I remove virtnet_napi_do_enable and repeat
> > > > > > the block of code in there twice (in virtnet_napi_enable and
> > > > > > virtnet_napi_tx_enable)?
> > > > >
> > > > > I think I miss something here, it looks like virtnet_napi_tx_enable()
> > > > > calls virtnet_napi_do_enable() directly.
> > > > >
> > > > > I would like to know why we don't call netif_queue_set_napi() for TX NAPI here?
> > > >
> > > > Please see both the cover letter and the commit message of the next
> > > > commit which addresses this question.
> > > >
> > > > TX-only NAPIs do not have NAPI IDs so there is nothing to map.
> > >
> > > Interesting, but I have more questions
> > >
> > > 1) why need a driver to know the NAPI implementation like this?
> >
> > I'm not sure I understand the question, but I'll try to give an
> > answer and please let me know if you have another question.
> >
> > Mapping the NAPI IDs to queue IDs is useful for applications that
> > use epoll based busy polling (which relies on the NAPI ID, see also
> > SO_INCOMING_NAPI_ID and [1]), IRQ suspension [2], and generally
> > per-NAPI configuration [3].
> >
> > Without this code added to the driver, the user application can get
> > the NAPI ID of an incoming connection, but has no way to know which
> > queue (or NIC) that NAPI ID is associated with or to set per-NAPI
> > configuration settings.
> >
> > [1]: https://lore.kernel.org/all/20240213061652.6342-1-jdamato@fastly.com/
> > [2]: https://lore.kernel.org/netdev/20241109050245.191288-5-jdamato@fastly.com/T/
> > [3]: https://lore.kernel.org/lkml/20241011184527.16393-1-jdamato@fastly.com/
> 
> Yes, exactly. Sorry for being unclear, what I want to ask is actually:
> 
> 1) TX NAPI doesn't have a NAPI ID, this seems more like a NAPI
> implementation details which should be hidden from the driver.
> 2) If 1 is true, in the netif_queue_set_napi(), should it be better to
> add and check for whether or not NAPI has an ID and return early if it
> doesn't have one
> 3) Then driver doesn't need to know NAPI implementation details like
> NAPI stuffs?

Sorry it just feels like this conversation is getting off track.

This change is about mapping virtio_net RX queues to NAPI IDs to
allow for RX busy polling, per-NAPI config settings, etc.

If you try to use netif_queue_set_napi with a TX-only NAPI, it will
set the NAPI ID to 0.

I already addressed this in the cover letter, would you mind
carefully re-reading my cover letter and commit messages?

If your main concern is that you want me to call
netif_queue_set_napi for TX-only NAPIs in addition to the RX NAPIs
in virtio_net, I can do that and resend an RFC.

In that case, the output will show "0" for NAPI ID for the TX-only
NAPIs. See the commit message of patch 3 and imagine that the output
shows this instead:

$ ./tools/net/ynl/pyynl/cli.py \
       --spec Documentation/netlink/specs/netdev.yaml \
       --dump queue-get --json='{"ifindex": 2}'
[{'id': 0, 'ifindex': 2, 'napi-id': 8289, 'type': 'rx'},
 {'id': 1, 'ifindex': 2, 'napi-id': 8290, 'type': 'rx'},
 {'id': 2, 'ifindex': 2, 'napi-id': 8291, 'type': 'rx'},
 {'id': 3, 'ifindex': 2, 'napi-id': 8292, 'type': 'rx'},
 {'id': 0, 'ifindex': 2, 'napi-id': 0, 'type': 'tx'},
 {'id': 1, 'ifindex': 2, 'napi-id': 0, 'type': 'tx'},
 {'id': 2, 'ifindex': 2, 'napi-id': 0, 'type': 'tx'},
 {'id': 3, 'ifindex': 2, 'napi-id': 0, 'type': 'tx'}]

If in the future the TX-only NAPIs get NAPI IDs, then nothing would
need to be updated in the driver and the NAPI IDs would "just work"
and appear.

> >
> > > 2) does NAPI know (or why it needs to know) whether or not it's a TX
> > > or not? I only see the following code in napi_hash_add():
> >
> > Note that I did not write the original implementation of NAPI IDs or
> > epoll-based busy poll, so I can only comment on what I know :)
> >
> > I don't know why TX-only NAPIs do not have NAPI IDs. My guess is
> > that in the original implementation, the code was designed only for
> > RX busy polling, so TX-only NAPIs were not assigned NAPI IDs.
> >
> > Perhaps in the future, TX-only NAPIs will be assigned NAPI IDs, but
> > currently they do not have NAPI IDs.
> 
> Jakub, could you please help to clarify this part?

Can you please explain what part needs clarification?

Regardless of TX-only NAPIs, we can still set NAPI IDs for
virtio_net RX queues and that would be immensely useful for users.

There's two options for virtio_net as I've outlined above and in my
cover letter and commit messages:

1. This implementation as-is. Then if one day in the future
   TX-only NAPIs get NAPI IDs, this driver (and others like mlx4)
   can be updated.

- OR -

2. Calling netif_queue_set_napi for all NAPIs, which results in the
   TX-only NAPIs displaying "0" as shown above.

Please let me know which option you'd like to see; I don't have a
preference, I just want to get support for this API in virtio_net.

> >
> > > static void napi_hash_add(struct napi_struct *napi)
> > > {
> > >         unsigned long flags;
> > >
> > >         if (test_bit(NAPI_STATE_NO_BUSY_POLL, &napi->state))
> > >                 return;
> > >
> > > ...
> > >
> > >         __napi_hash_add_with_id(napi, napi_gen_id);
> > >
> > >         spin_unlock_irqrestore(&napi_hash_lock, flags);
> > > }
> > >
> > > It seems it only matters with NAPI_STATE_NO_BUSY_POLL.
> > >
> > > And if NAPI knows everything, should it be better to just do the
> > > linking in napi_enable/disable() instead of letting each driver do it
> > > by itself?
> >
> > It would be nice if this were possible, I agree. Perhaps in the
> > future some work could be done to make this possible.
> >
> > I believe that this is not currently possible because the NAPI does
> > not know which queue ID it is associated with. That mapping of which
> > queue is associated with which NAPI is established in patch 3
> > (please see the commit message of patch 3 to see an example of the
> > output).
> >
> > The driver knows both the queue ID and the NAPI for that queue, so
> > the mapping can be established only by the driver.
> >
> > Let me know if that helps.
> 
> Yes, definitely.
> 
> Let's see Jakub's comment.

As mentioned above, I'm not sure if we need to worry about TX-only
NAPIs getting NAPI IDs.

That seems pretty unrelated to this change as I've explained above.

On Mon, Jan 27, 2025 at 12:52:06PM -0500, Joe Damato wrote:
> On Sun, Jan 26, 2025 at 04:04:02PM +0800, Jason Wang wrote:
> > On Sat, Jan 25, 2025 at 4:19 AM Joe Damato <jdamato@fastly.com> wrote:
> > >
> > > On Fri, Jan 24, 2025 at 09:14:54AM +0800, Jason Wang wrote:
> > > > On Thu, Jan 23, 2025 at 10:47 AM Joe Damato <jdamato@fastly.com> wrote:
> > > > >
> > > > > On Thu, Jan 23, 2025 at 10:40:43AM +0800, Jason Wang wrote:
> > > > > > On Thu, Jan 23, 2025 at 1:41 AM Joe Damato <jdamato@fastly.com> wrote:
> > > > > > >
> > > > > > > On Wed, Jan 22, 2025 at 02:12:46PM +0800, Jason Wang wrote:
> > > > > > > > On Wed, Jan 22, 2025 at 3:11 AM Joe Damato <jdamato@fastly.com> wrote:
> 
> [...]
> 
> > > > > > > > >
> > > > > > > > > -static void virtnet_napi_enable(struct virtqueue *vq, struct napi_struct *napi)
> > > > > > > > > +static void virtnet_napi_do_enable(struct virtqueue *vq,
> > > > > > > > > +                                  struct napi_struct *napi)
> > > > > > > > >  {
> > > > > > > > >         napi_enable(napi);
> > > > > > > >
> > > > > > > > Nit: it might be better to not have this helper to avoid a misuse of
> > > > > > > > this function directly.
> > > > > > >
> > > > > > > Sorry, I'm probably missing something here.
> > > > > > >
> > > > > > > Both virtnet_napi_enable and virtnet_napi_tx_enable need the logic
> > > > > > > in virtnet_napi_do_enable.
> > > > > > >
> > > > > > > Are you suggesting that I remove virtnet_napi_do_enable and repeat
> > > > > > > the block of code in there twice (in virtnet_napi_enable and
> > > > > > > virtnet_napi_tx_enable)?
> > > > > >
> > > > > > I think I miss something here, it looks like virtnet_napi_tx_enable()
> > > > > > calls virtnet_napi_do_enable() directly.
> > > > > >
> > > > > > I would like to know why we don't call netif_queue_set_napi() for TX NAPI here?
> > > > >
> > > > > Please see both the cover letter and the commit message of the next
> > > > > commit which addresses this question.
> > > > >
> > > > > TX-only NAPIs do not have NAPI IDs so there is nothing to map.
> > > >
> > > > Interesting, but I have more questions
> > > >
> > > > 1) why need a driver to know the NAPI implementation like this?
> > >
> > > I'm not sure I understand the question, but I'll try to give an
> > > answer and please let me know if you have another question.
> > >
> > > Mapping the NAPI IDs to queue IDs is useful for applications that
> > > use epoll based busy polling (which relies on the NAPI ID, see also
> > > SO_INCOMING_NAPI_ID and [1]), IRQ suspension [2], and generally
> > > per-NAPI configuration [3].
> > >
> > > Without this code added to the driver, the user application can get
> > > the NAPI ID of an incoming connection, but has no way to know which
> > > queue (or NIC) that NAPI ID is associated with or to set per-NAPI
> > > configuration settings.
> > >
> > > [1]: https://lore.kernel.org/all/20240213061652.6342-1-jdamato@fastly.com/
> > > [2]: https://lore.kernel.org/netdev/20241109050245.191288-5-jdamato@fastly.com/T/
> > > [3]: https://lore.kernel.org/lkml/20241011184527.16393-1-jdamato@fastly.com/
> > 
> > Yes, exactly. Sorry for being unclear, what I want to ask is actually:
> > 
> > 1) TX NAPI doesn't have a NAPI ID, this seems more like a NAPI
> > implementation details which should be hidden from the driver.
> > 2) If 1 is true, in the netif_queue_set_napi(), should it be better to
> > add and check for whether or not NAPI has an ID and return early if it
> > doesn't have one
> > 3) Then driver doesn't need to know NAPI implementation details like
> > NAPI stuffs?
> 
> Sorry it just feels like this conversation is getting off track.
> 
> This change is about mapping virtio_net RX queues to NAPI IDs to
> allow for RX busy polling, per-NAPI config settings, etc.
> 
> If you try to use netif_queue_set_napi with a TX-only NAPI, it will
> set the NAPI ID to 0.
> 
> I already addressed this in the cover letter, would you mind
> carefully re-reading my cover letter and commit messages?
> 
> If your main concern is that you want me to call
> netif_queue_set_napi for TX-only NAPIs in addition to the RX NAPIs
> in virtio_net, I can do that and resend an RFC.
> 
> In that case, the output will show "0" for NAPI ID for the TX-only
> NAPIs. See the commit message of patch 3 and imagine that the output
> shows this instead:
> 
> $ ./tools/net/ynl/pyynl/cli.py \
>        --spec Documentation/netlink/specs/netdev.yaml \
>        --dump queue-get --json='{"ifindex": 2}'
> [{'id': 0, 'ifindex': 2, 'napi-id': 8289, 'type': 'rx'},
>  {'id': 1, 'ifindex': 2, 'napi-id': 8290, 'type': 'rx'},
>  {'id': 2, 'ifindex': 2, 'napi-id': 8291, 'type': 'rx'},
>  {'id': 3, 'ifindex': 2, 'napi-id': 8292, 'type': 'rx'},
>  {'id': 0, 'ifindex': 2, 'napi-id': 0, 'type': 'tx'},
>  {'id': 1, 'ifindex': 2, 'napi-id': 0, 'type': 'tx'},
>  {'id': 2, 'ifindex': 2, 'napi-id': 0, 'type': 'tx'},
>  {'id': 3, 'ifindex': 2, 'napi-id': 0, 'type': 'tx'}]
> 
> If in the future the TX-only NAPIs get NAPI IDs, then nothing would
> need to be updated in the driver and the NAPI IDs would "just work"
> and appear.

Actually, I missed a patch Jakub submit to net [1], which prevents
dumping TX-only NAPIs.

So, I think this RFC as-is (only calling netif_queue_set_napi
for RX NAPIs) should be fine without changes.

Please let me know.

[1]: https://lore.kernel.org/netdev/20250103183207.1216004-1-kuba@kernel.org/

On Mon, 27 Jan 2025 14:31:21 -0500 Joe Damato wrote:
> Actually, I missed a patch Jakub submit to net [1], which prevents
> dumping TX-only NAPIs.

That patch only addresses NAPI ops, here I think we're talking about
attributes of the queue object.

> So, I think this RFC as-is (only calling netif_queue_set_napi
> for RX NAPIs) should be fine without changes.

Weak preference towards making netdev_nl_queue_fill_one() "do the right
thing" when NAPI does not have ID assigned. And right thing IMO would
be to skip reporting the NAPI_ID attribute.

Tx NAPIs are one aspect, whether they have ID or not we may want direct
access to the struct somewhere in the core, via txq, at some point, and
then people may forget the linking has an unintended effect of also
changing the netlink attrs. The other aspect is that driver may link
queue to a Rx NAPI instance before napi_enable(), so before ID is
assigned. Again, we don't want to report ID of 0 in that case.

On Mon, Jan 27, 2025 at 01:33:04PM -0800, Jakub Kicinski wrote:
> On Mon, 27 Jan 2025 14:31:21 -0500 Joe Damato wrote:
> > Actually, I missed a patch Jakub submit to net [1], which prevents
> > dumping TX-only NAPIs.
> 
> That patch only addresses NAPI ops, here I think we're talking about
> attributes of the queue object.

Yea, that's true.

> > So, I think this RFC as-is (only calling netif_queue_set_napi
> > for RX NAPIs) should be fine without changes.
> 
> Weak preference towards making netdev_nl_queue_fill_one() "do the right
> thing" when NAPI does not have ID assigned. And right thing IMO would
> be to skip reporting the NAPI_ID attribute.

Ah, right. Your patch was for netdev_nl_napi_fill_one, so presumably
a similar patch would need to be written for
netdev_nl_queue_fill_one.

> Tx NAPIs are one aspect, whether they have ID or not we may want direct
> access to the struct somewhere in the core, via txq, at some point, and
> then people may forget the linking has an unintended effect of also
> changing the netlink attrs. The other aspect is that driver may link
> queue to a Rx NAPI instance before napi_enable(), so before ID is
> assigned. Again, we don't want to report ID of 0 in that case.

I'm sorry I'm not sure I'm following what you are saying here; I
think there might be separate threads concurrently and I'm probably
just confused :)

I think you are saying that netdev_nl_napi_fill_one should not
report 0, which I think is fine but probably a separate patch?

I think, but am not sure, that Jason was asking for guidance on
TX-only NAPIs and linking them with calls to netif_queue_set_napi.
It seems that Jason may be suggesting that the driver shouldn't have
to know that TX-only NAPIs have a NAPI ID of 0 and thus should call
netif_queue_set_napi for all NAPIs and not have to deal think about
TX-only NAPIs at all.

From you've written, Jakub, I think you are suggesting you agree
with that, but with the caveat that netdev_nl_napi_fill_one should
not report 0.

Then, one day in the future, if TX-only NAPIs get an ID they will
magically start to show up.

Is that right?

If so, I'll re-spin the RFC to call netif_queue_set_napi for all
NAPIs in virtio_net, including TX-only NAPIs and see about including
a patch to tweak netdev_nl_napi_fill_one, if necessary.

On Mon, 27 Jan 2025 17:07:54 -0500 Joe Damato wrote:
> > Tx NAPIs are one aspect, whether they have ID or not we may want direct
> > access to the struct somewhere in the core, via txq, at some point, and
> > then people may forget the linking has an unintended effect of also
> > changing the netlink attrs. The other aspect is that driver may link
> > queue to a Rx NAPI instance before napi_enable(), so before ID is
> > assigned. Again, we don't want to report ID of 0 in that case.  
> 
> I'm sorry I'm not sure I'm following what you are saying here; I
> think there might be separate threads concurrently and I'm probably
> just confused :)
> 
> I think you are saying that netdev_nl_napi_fill_one should not
> report 0, which I think is fine but probably a separate patch?
> 
> I think, but am not sure, that Jason was asking for guidance on
> TX-only NAPIs and linking them with calls to netif_queue_set_napi.
> It seems that Jason may be suggesting that the driver shouldn't have
> to know that TX-only NAPIs have a NAPI ID of 0 and thus should call
> netif_queue_set_napi for all NAPIs and not have to deal think about
> TX-only NAPIs at all.
> 
> From you've written, Jakub, I think you are suggesting you agree
> with that, but with the caveat that netdev_nl_napi_fill_one should
> not report 0.

Right up to this point.

> Then, one day in the future, if TX-only NAPIs get an ID they will
> magically start to show up.
> 
> Is that right?

Sort of. I was trying to point out corner cases which would also
benefit from netdev_nl_queue_fill_one() being more careful about 
the NAPI IDs it reports. But the conclusion is the same.

> If so, I'll re-spin the RFC to call netif_queue_set_napi for all
> NAPIs in virtio_net, including TX-only NAPIs and see about including
> a patch to tweak netdev_nl_napi_fill_one, if necessary.

netdev_nl_queue_fill_one(), not netdev_nl_napi_fill_one()

Otherwise SG.

After net-next reopens I think the patch to netdev_nl_queue_fill_one()
could be posted separately. There may be drivers out there which already
link Tx NAPIs, we shouldn't delay making the reporting more careful.

On Mon, Jan 27, 2025 at 02:24:00PM -0800, Jakub Kicinski wrote:
> On Mon, 27 Jan 2025 17:07:54 -0500 Joe Damato wrote:
> > > Tx NAPIs are one aspect, whether they have ID or not we may want direct
> > > access to the struct somewhere in the core, via txq, at some point, and
> > > then people may forget the linking has an unintended effect of also
> > > changing the netlink attrs. The other aspect is that driver may link
> > > queue to a Rx NAPI instance before napi_enable(), so before ID is
> > > assigned. Again, we don't want to report ID of 0 in that case.  
> > 
> > I'm sorry I'm not sure I'm following what you are saying here; I
> > think there might be separate threads concurrently and I'm probably
> > just confused :)
> > 
> > I think you are saying that netdev_nl_napi_fill_one should not
> > report 0, which I think is fine but probably a separate patch?
> > 
> > I think, but am not sure, that Jason was asking for guidance on
> > TX-only NAPIs and linking them with calls to netif_queue_set_napi.
> > It seems that Jason may be suggesting that the driver shouldn't have
> > to know that TX-only NAPIs have a NAPI ID of 0 and thus should call
> > netif_queue_set_napi for all NAPIs and not have to deal think about
> > TX-only NAPIs at all.
> > 
> > From you've written, Jakub, I think you are suggesting you agree
> > with that, but with the caveat that netdev_nl_napi_fill_one should
> > not report 0.
> 
> Right up to this point.
> 
> > Then, one day in the future, if TX-only NAPIs get an ID they will
> > magically start to show up.
> > 
> > Is that right?
> 
> Sort of. I was trying to point out corner cases which would also
> benefit from netdev_nl_queue_fill_one() being more careful about 
> the NAPI IDs it reports. But the conclusion is the same.
> 
> > If so, I'll re-spin the RFC to call netif_queue_set_napi for all
> > NAPIs in virtio_net, including TX-only NAPIs and see about including
> > a patch to tweak netdev_nl_napi_fill_one, if necessary.
> 
> netdev_nl_queue_fill_one(), not netdev_nl_napi_fill_one()

Right, sorry for the typo/added confusion.
 
> Otherwise SG.
> 
> After net-next reopens I think the patch to netdev_nl_queue_fill_one()
> could be posted separately. There may be drivers out there which already
> link Tx NAPIs, we shouldn't delay making the reporting more careful.

OK, I'll start with that when net-next reopens while waiting on the
locking changes to come later and do the actual linking.