All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joe Damato <jdamato@fastly.com>
To: Vinicius Costa Gomes <vinicius.gomes@intel.com>
Cc: Tony Nguyen <anthony.l.nguyen@intel.com>,
	netdev@vger.kernel.org, open list <linux-kernel@vger.kernel.org>,
	Eric Dumazet <edumazet@google.com>,
	"moderated list:INTEL ETHERNET DRIVERS"
	<intel-wired-lan@lists.osuosl.org>,
	Przemek Kitszel <przemyslaw.kitszel@intel.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	"David S. Miller" <davem@davemloft.net>
Subject: Re: [Intel-wired-lan] [RFC net-next 0/2] igc: Link IRQs and queues to NAPIs
Date: Wed, 9 Oct 2024 10:13:49 -0700	[thread overview]
Message-ID: <Zwa5zdhtxlqJxIj7@LQ3V64L9R2> (raw)
In-Reply-To: <87h69ntt23.fsf@intel.com>

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.

WARNING: multiple messages have this Message-ID (diff)
From: Joe Damato <jdamato@fastly.com>
To: Vinicius Costa Gomes <vinicius.gomes@intel.com>
Cc: netdev@vger.kernel.org, "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	"moderated list:INTEL ETHERNET DRIVERS"
	<intel-wired-lan@lists.osuosl.org>,
	Jakub Kicinski <kuba@kernel.org>,
	open list <linux-kernel@vger.kernel.org>,
	Paolo Abeni <pabeni@redhat.com>,
	Przemek Kitszel <przemyslaw.kitszel@intel.com>,
	Tony Nguyen <anthony.l.nguyen@intel.com>
Subject: Re: [RFC net-next 0/2] igc: Link IRQs and queues to NAPIs
Date: Wed, 9 Oct 2024 10:13:49 -0700	[thread overview]
Message-ID: <Zwa5zdhtxlqJxIj7@LQ3V64L9R2> (raw)
In-Reply-To: <87h69ntt23.fsf@intel.com>

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.

  reply	other threads:[~2024-10-09 17:13 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-03 23:38 [Intel-wired-lan] [RFC net-next 0/2] igc: Link IRQs and queues to NAPIs Joe Damato
2024-10-03 23:38 ` Joe Damato
2024-10-03 23:38 ` [Intel-wired-lan] [RFC net-next 1/2] igc: Link IRQs to NAPI instances Joe Damato
2024-10-03 23:38   ` Joe Damato
2024-10-03 23:38 ` [Intel-wired-lan] [RFC net-next 2/2] igc: Link queues " Joe Damato
2024-10-03 23:38   ` Joe Damato
2024-10-07  9:14   ` [Intel-wired-lan] " Kurt Kanzenbach
2024-10-07  9:14     ` Kurt Kanzenbach
2024-10-09 17:04     ` [Intel-wired-lan] " Joe Damato
2024-10-09 17:04       ` Joe Damato
2024-10-10  7:08       ` [Intel-wired-lan] " Kurt Kanzenbach
2024-10-10  7:08         ` Kurt Kanzenbach
2024-10-12  1:58         ` [Intel-wired-lan] " Joe Damato
2024-10-12  1:58           ` Joe Damato
2024-10-14 12:08           ` [Intel-wired-lan] " Kurt Kanzenbach
2024-10-14 12:08             ` Kurt Kanzenbach
2024-10-07 23:03 ` [Intel-wired-lan] [RFC net-next 0/2] igc: Link IRQs and queues to NAPIs Vinicius Costa Gomes
2024-10-07 23:03   ` Vinicius Costa Gomes
2024-10-09 17:13   ` Joe Damato [this message]
2024-10-09 17:13     ` Joe Damato

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Zwa5zdhtxlqJxIj7@LQ3V64L9R2 \
    --to=jdamato@fastly.com \
    --cc=anthony.l.nguyen@intel.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=przemyslaw.kitszel@intel.com \
    --cc=vinicius.gomes@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.