From: Joe Damato <jdamato@fastly.com>
To: Vinicius Costa Gomes <vinicius.gomes@intel.com>
Cc: netdev@vger.kernel.org, kurt@linutronix.de,
Tony Nguyen <anthony.l.nguyen@intel.com>,
Przemek Kitszel <przemyslaw.kitszel@intel.com>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Jesper Dangaard Brouer <hawk@kernel.org>,
John Fastabend <john.fastabend@gmail.com>,
"moderated list:INTEL ETHERNET DRIVERS"
<intel-wired-lan@lists.osuosl.org>,
open list <linux-kernel@vger.kernel.org>,
"open list:XDP (eXpress Data Path)" <bpf@vger.kernel.org>
Subject: Re: [RFC net-next v2 2/2] igc: Link queues to NAPI instances
Date: Mon, 14 Oct 2024 20:44:28 -0700 [thread overview]
Message-ID: <Zw3lHHEuMt_llNt8@LQ3V64L9R2> (raw)
In-Reply-To: <87ldyqnneq.fsf@intel.com>
On Mon, Oct 14, 2024 at 06:51:57PM -0700, Vinicius Costa Gomes wrote:
> Joe Damato <jdamato@fastly.com> writes:
[...]
> > +void igc_set_queue_napi(struct igc_adapter *adapter, int q_idx,
> > + struct napi_struct *napi)
> > +{
> > + if (adapter->flags & IGC_FLAG_QUEUE_PAIRS) {
> > + netif_queue_set_napi(adapter->netdev, q_idx,
> > + NETDEV_QUEUE_TYPE_RX, napi);
> > + netif_queue_set_napi(adapter->netdev, q_idx,
> > + NETDEV_QUEUE_TYPE_TX, napi);
> > + } else {
> > + if (q_idx < adapter->num_rx_queues) {
> > + netif_queue_set_napi(adapter->netdev, q_idx,
> > + NETDEV_QUEUE_TYPE_RX, napi);
> > + } else {
> > + q_idx -= adapter->num_rx_queues;
> > + netif_queue_set_napi(adapter->netdev, q_idx,
> > + NETDEV_QUEUE_TYPE_TX, napi);
> > + }
> > + }
> > +}
> > +
> > +void igc_unset_queue_napi(struct igc_adapter *adapter, int q_idx)
> > +{
> > + struct net_device *netdev = adapter->netdev;
> > +
> > + if (adapter->flags & IGC_FLAG_QUEUE_PAIRS) {
> > + netif_queue_set_napi(netdev, q_idx, NETDEV_QUEUE_TYPE_RX,
> > + NULL);
> > + netif_queue_set_napi(netdev, q_idx, NETDEV_QUEUE_TYPE_TX,
> > + NULL);
> > + } else {
> > + if (q_idx < adapter->num_rx_queues) {
> > + netif_queue_set_napi(adapter->netdev, q_idx,
> > + NETDEV_QUEUE_TYPE_RX, NULL);
> > + } else {
> > + q_idx -= adapter->num_rx_queues;
> > + netif_queue_set_napi(adapter->netdev, q_idx,
> > + NETDEV_QUEUE_TYPE_TX, NULL);
> > + }
> > + }
> > +}
>
> It seems that igc_unset_queue_napi() is igc_set_queue_napi(x, y, NULL),
> so I would suggest either implementing "unset" in terms of "set", or
> using igc_set_queue_napi(x, y, NULL) directly in the "unlink" case (I
> have a slight preference for the second option).
Ah, yes, of course. That is much simpler; I'll go with the second
option for the v3. Thank you for catching that in your review.
Unless any other significant feedback comes in, I'll likely send the
v3 as a PATCH instead of an RFC later this week.
WARNING: multiple messages have this Message-ID (diff)
From: Joe Damato <jdamato@fastly.com>
To: Vinicius Costa Gomes <vinicius.gomes@intel.com>
Cc: Jesper Dangaard Brouer <hawk@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
netdev@vger.kernel.org, kurt@linutronix.de,
John Fastabend <john.fastabend@gmail.com>,
Alexei Starovoitov <ast@kernel.org>,
open list <linux-kernel@vger.kernel.org>,
Eric Dumazet <edumazet@google.com>,
Tony Nguyen <anthony.l.nguyen@intel.com>,
Przemek Kitszel <przemyslaw.kitszel@intel.com>,
Jakub Kicinski <kuba@kernel.org>,
"open list:XDP \(eXpress Data Path\)" <bpf@vger.kernel.org>,
Paolo Abeni <pabeni@redhat.com>,
"David S. Miller" <davem@davemloft.net>,
"moderated list:INTEL ETHERNET DRIVERS"
<intel-wired-lan@lists.osuosl.org>
Subject: Re: [Intel-wired-lan] [RFC net-next v2 2/2] igc: Link queues to NAPI instances
Date: Mon, 14 Oct 2024 20:44:28 -0700 [thread overview]
Message-ID: <Zw3lHHEuMt_llNt8@LQ3V64L9R2> (raw)
In-Reply-To: <87ldyqnneq.fsf@intel.com>
On Mon, Oct 14, 2024 at 06:51:57PM -0700, Vinicius Costa Gomes wrote:
> Joe Damato <jdamato@fastly.com> writes:
[...]
> > +void igc_set_queue_napi(struct igc_adapter *adapter, int q_idx,
> > + struct napi_struct *napi)
> > +{
> > + if (adapter->flags & IGC_FLAG_QUEUE_PAIRS) {
> > + netif_queue_set_napi(adapter->netdev, q_idx,
> > + NETDEV_QUEUE_TYPE_RX, napi);
> > + netif_queue_set_napi(adapter->netdev, q_idx,
> > + NETDEV_QUEUE_TYPE_TX, napi);
> > + } else {
> > + if (q_idx < adapter->num_rx_queues) {
> > + netif_queue_set_napi(adapter->netdev, q_idx,
> > + NETDEV_QUEUE_TYPE_RX, napi);
> > + } else {
> > + q_idx -= adapter->num_rx_queues;
> > + netif_queue_set_napi(adapter->netdev, q_idx,
> > + NETDEV_QUEUE_TYPE_TX, napi);
> > + }
> > + }
> > +}
> > +
> > +void igc_unset_queue_napi(struct igc_adapter *adapter, int q_idx)
> > +{
> > + struct net_device *netdev = adapter->netdev;
> > +
> > + if (adapter->flags & IGC_FLAG_QUEUE_PAIRS) {
> > + netif_queue_set_napi(netdev, q_idx, NETDEV_QUEUE_TYPE_RX,
> > + NULL);
> > + netif_queue_set_napi(netdev, q_idx, NETDEV_QUEUE_TYPE_TX,
> > + NULL);
> > + } else {
> > + if (q_idx < adapter->num_rx_queues) {
> > + netif_queue_set_napi(adapter->netdev, q_idx,
> > + NETDEV_QUEUE_TYPE_RX, NULL);
> > + } else {
> > + q_idx -= adapter->num_rx_queues;
> > + netif_queue_set_napi(adapter->netdev, q_idx,
> > + NETDEV_QUEUE_TYPE_TX, NULL);
> > + }
> > + }
> > +}
>
> It seems that igc_unset_queue_napi() is igc_set_queue_napi(x, y, NULL),
> so I would suggest either implementing "unset" in terms of "set", or
> using igc_set_queue_napi(x, y, NULL) directly in the "unlink" case (I
> have a slight preference for the second option).
Ah, yes, of course. That is much simpler; I'll go with the second
option for the v3. Thank you for catching that in your review.
Unless any other significant feedback comes in, I'll likely send the
v3 as a PATCH instead of an RFC later this week.
next prev parent reply other threads:[~2024-10-15 3:44 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-10-14 21:30 [RFC net-next v2 0/2] igc: Link IRQs and queues to NAPIs Joe Damato
2024-10-14 21:30 ` [Intel-wired-lan] " Joe Damato
2024-10-14 21:30 ` [Intel-wired-lan] [RFC net-next v2 1/2] igc: Link IRQs to NAPI instances Joe Damato
2024-10-14 21:30 ` Joe Damato
2024-10-14 21:30 ` [RFC net-next v2 2/2] igc: Link queues " Joe Damato
2024-10-14 21:30 ` [Intel-wired-lan] " Joe Damato
2024-10-15 1:51 ` Vinicius Costa Gomes
2024-10-15 1:51 ` [Intel-wired-lan] " Vinicius Costa Gomes
2024-10-15 3:44 ` Joe Damato [this message]
2024-10-15 3:44 ` Joe Damato
2024-10-15 10:27 ` Kurt Kanzenbach
2024-10-15 10:27 ` [Intel-wired-lan] " Kurt Kanzenbach
2024-10-16 1:01 ` Joe Damato
2024-10-16 1:01 ` [Intel-wired-lan] " Joe Damato
2024-10-18 17:04 ` Joe Damato
2024-10-18 17:04 ` [Intel-wired-lan] " Joe Damato
2024-10-21 5:42 ` Kurt Kanzenbach
2024-10-21 5:42 ` [Intel-wired-lan] " Kurt Kanzenbach
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=Zw3lHHEuMt_llNt8@LQ3V64L9R2 \
--to=jdamato@fastly.com \
--cc=anthony.l.nguyen@intel.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=hawk@kernel.org \
--cc=intel-wired-lan@lists.osuosl.org \
--cc=john.fastabend@gmail.com \
--cc=kuba@kernel.org \
--cc=kurt@linutronix.de \
--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.