netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: "Nambiar, Amritha" <amritha.nambiar@intel.com>,
	Willem de Bruijn <willemb@google.com>
Cc: <netdev@vger.kernel.org>, <pabeni@redhat.com>,
	<sridhar.samudrala@intel.com>
Subject: Re: [net-next PATCH v8 02/10] net: Add queue and napi association
Date: Tue, 21 Nov 2023 17:15:00 -0800	[thread overview]
Message-ID: <20231121171500.0068a5bb@kernel.org> (raw)
In-Reply-To: <d696c18b-c129-41c1-8a8a-f9273da1f215@intel.com>

On Tue, 21 Nov 2023 16:08:07 -0800 Nambiar, Amritha wrote:
> > To reiterate - the thing I find odd about the current situation is that
> > we hide the queues if they get disabled by lowering ethtool -L, but we
> > don't hide them when the entire interface is down. When the entire
> > interface is down there should be no queues, right?
> 
> "When the entire interface is down there should be no queues" - 
> currently, 'ethtool --show-channels' reports all the available queues 
> when interface is DOWN

That's not the same. ethtool -l shows the configuration not 
the instantiated objects. ethtool -a will also show you the
pause settings even when cable is not plugged in.
sysfs objects of the queues are still exposed for devices which 
are down, that's true. But again, that's to expose the config.

> > Differently put - what logic that'd make sense to the user do we apply
> > when trying to decide if the queue is visible? < real_num_queues is
> > an implementation detail.
> > 
> > We can list all the queues, always, too. No preference. I just want to
> > make sure that the rules are clear and not very dependent on current
> > implementation and not different driver to driver.  
> 
> I think currently, the queue dump results when the device is down aligns 
> for both APIs (netdev-genl queue-get and ethtool show-channels) for all 
> the drivers. If we decide to NOT show queues/NAPIs (with netdev-genl) 
> when the device is down, the user would see conflicting results, the 
> dump results with netdev-genl APIs would be different from what 'ethtool 
> --show-channels' and 'ps -aef | grep napi' reports.

We should make the distinction between configuration and state of
instantiated objects clear before we get too far. Say we support
setting ring length for a specific queue. Global setting is 512,
queue X wants 256. How do we remove the override for queue X?
By setting it to 512? What if we want 512, and the default shifts
to something else? We'll need an explicit "reset" command.

I think it may be cleaner to keep queue-get as state of queues,
and configuration / settings / rules completely separate.

Am I wrong? Willem?

  reply	other threads:[~2023-11-22  1:15 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-17  1:16 [net-next PATCH v8 00/10] Introduce queue and NAPI support in netdev-genl (Was: Introduce NAPI queues support) Amritha Nambiar
2023-11-17  1:16 ` [net-next PATCH v8 01/10] netdev-genl: spec: Extend netdev netlink spec in YAML for queue Amritha Nambiar
2023-11-17  1:16 ` [net-next PATCH v8 02/10] net: Add queue and napi association Amritha Nambiar
2023-11-20 23:54   ` Jakub Kicinski
2023-11-21 21:26     ` Nambiar, Amritha
2023-11-21 22:22       ` Jakub Kicinski
2023-11-22  0:08         ` Nambiar, Amritha
2023-11-22  1:15           ` Jakub Kicinski [this message]
2023-11-22 21:28             ` Nambiar, Amritha
2023-11-22 22:00               ` Jakub Kicinski
2023-11-23  0:56                 ` Nambiar, Amritha
2023-11-17  1:16 ` [net-next PATCH v8 03/10] ice: Add support in the driver for associating queue with napi Amritha Nambiar
2023-11-17  1:16 ` [net-next PATCH v8 04/10] netdev-genl: Add netlink framework functions for queue Amritha Nambiar
2023-11-17  1:17 ` [net-next PATCH v8 05/10] netdev-genl: spec: Extend netdev netlink spec in YAML for NAPI Amritha Nambiar
2023-11-17  1:17 ` [net-next PATCH v8 06/10] netdev-genl: Add netlink framework functions for napi Amritha Nambiar
2023-11-17  1:17 ` [net-next PATCH v8 07/10] netdev-genl: spec: Add irq in netdev netlink YAML spec Amritha Nambiar
2023-11-17  1:17 ` [net-next PATCH v8 08/10] net: Add NAPI IRQ support Amritha Nambiar
2023-11-17  1:17 ` [net-next PATCH v8 09/10] netdev-genl: spec: Add PID in netdev netlink YAML spec Amritha Nambiar
2023-11-17  1:17 ` [net-next PATCH v8 10/10] netdev-genl: Add PID for the NAPI thread Amritha Nambiar
2023-11-20 23:56 ` [PATCH 11/10] eth: bnxt: link NAPI instances to queues and IRQs Jakub Kicinski
2023-11-21 21:31   ` Nambiar, Amritha

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=20231121171500.0068a5bb@kernel.org \
    --to=kuba@kernel.org \
    --cc=amritha.nambiar@intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=sridhar.samudrala@intel.com \
    --cc=willemb@google.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 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).