Greetings:

This is an RFC to get feedback before submitting an actual series and
because I have a question for igc maintainers, see below.

This series addss support for netdev-genl to igc so that userland apps
can query IRQ, queue, and NAPI instance relationships. This is useful
because developers who have igc NICs (for example, in their Intel NUCs)
who are working on epoll-based busy polling apps and using
SO_INCOMING_NAPI_ID, need access to this API to map NAPI IDs back to
queues.

See the commit messages of each patch for example output I got on my igc
hardware.

My question for maintainers:

In patch 2, the linking should be avoided for XDP queues. Is there a way
to test that somehow in the driver? I looked around a bit, but didn't
notice anything. Sorry if I'm missing something obvious.

Thanks,
Joe

Joe Damato (2):
  igc: Link IRQs to NAPI instances
  igc: Link queues to NAPI instances

 drivers/net/ethernet/intel/igc/igc.h      |  1 +
 drivers/net/ethernet/intel/igc/igc_main.c | 33 ++++++++++++++++++++---
 2 files changed, 30 insertions(+), 4 deletions(-)

-- 
2.25.1

Joe Damato <jdamato@fastly.com> writes:

> Greetings:
>
> This is an RFC to get feedback before submitting an actual series and
> because I have a question for igc maintainers, see below.
>
> This series addss support for netdev-genl to igc so that userland apps
> can query IRQ, queue, and NAPI instance relationships. This is useful
> because developers who have igc NICs (for example, in their Intel NUCs)
> who are working on epoll-based busy polling apps and using
> SO_INCOMING_NAPI_ID, need access to this API to map NAPI IDs back to
> queues.
>
> See the commit messages of each patch for example output I got on my igc
> hardware.
>
> My question for maintainers:
>
> In patch 2, the linking should be avoided for XDP queues. Is there a way
> to test that somehow in the driver? I looked around a bit, but didn't
> notice anything. Sorry if I'm missing something obvious.
>

From a quick look, it seems that you could "unlink" the XDP queues in
igc_xdp_enable_pool() and (re-)link them in igc_xdp_disable_poll().

Or just the existence of the flag IGC_RING_FLAG_AF_XDP_ZC in the rings
associated with the queue is enough?

I still have to take a better look at your work to help more, sorry.

> Thanks,
> Joe
>
> Joe Damato (2):
>   igc: Link IRQs to NAPI instances
>   igc: Link queues to NAPI instances
>
>  drivers/net/ethernet/intel/igc/igc.h      |  1 +
>  drivers/net/ethernet/intel/igc/igc_main.c | 33 ++++++++++++++++++++---
>  2 files changed, 30 insertions(+), 4 deletions(-)
>
> -- 
> 2.25.1
>
>

Cheers,
-- 
Vinicius

On Mon, Oct 07, 2024 at 04:03:00PM -0700, Vinicius Costa Gomes wrote:
> Joe Damato <jdamato@fastly.com> writes:
> 
> > Greetings:
> >
> > This is an RFC to get feedback before submitting an actual series and
> > because I have a question for igc maintainers, see below.
> >
> > This series addss support for netdev-genl to igc so that userland apps
> > can query IRQ, queue, and NAPI instance relationships. This is useful
> > because developers who have igc NICs (for example, in their Intel NUCs)
> > who are working on epoll-based busy polling apps and using
> > SO_INCOMING_NAPI_ID, need access to this API to map NAPI IDs back to
> > queues.
> >
> > See the commit messages of each patch for example output I got on my igc
> > hardware.
> >
> > My question for maintainers:
> >
> > In patch 2, the linking should be avoided for XDP queues. Is there a way
> > to test that somehow in the driver? I looked around a bit, but didn't
> > notice anything. Sorry if I'm missing something obvious.
> >
> 
> From a quick look, it seems that you could "unlink" the XDP queues in
> igc_xdp_enable_pool() and (re-)link them in igc_xdp_disable_poll().

That approach seems reasonable to me, but I am not an igc expert by
any means :)

I checked and it seems that igc_xdp_enable_pool and
igc_xdp_disable_poll are only called while RTNL is held, which is
good because netif_queue_set_napi uses ASSERT_RTNL.
 
> Or just the existence of the flag IGC_RING_FLAG_AF_XDP_ZC in the rings
> associated with the queue is enough?

I didn't notice that flag, thanks for pointing that out.

It might be better to go the link/unlink route as you described
above, though.

> I still have to take a better look at your work to help more, sorry.

No worries, thanks for taking a look.

I'll implement what you suggested above and send another RFC.