All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joe Damato <jdamato@fastly.com>
To: Jakub Kicinski <kuba@kernel.org>
Cc: Samiullah Khawaja <skhawaja@google.com>,
	"David S . Miller " <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Paolo Abeni <pabeni@redhat.com>,
	almasrymina@google.com, willemb@google.com,
	mkarsten@uwaterloo.ca, netdev@vger.kernel.org
Subject: Re: [PATCH net-next v5] Add support to set napi threaded for individual napi
Date: Fri, 25 Apr 2025 19:31:52 -0700	[thread overview]
Message-ID: <aAxFmKo2cmLUmqAJ@LQ3V64L9R2> (raw)
In-Reply-To: <20250425174251.59d7a45d@kernel.org>

On Fri, Apr 25, 2025 at 05:42:51PM -0700, Jakub Kicinski wrote:
> On Wed, 23 Apr 2025 20:14:13 +0000 Samiullah Khawaja wrote:
> > A net device has a threaded sysctl that can be used to enable threaded
> > napi polling on all of the NAPI contexts under that device. Allow
> > enabling threaded napi polling at individual napi level using netlink.
> > 
> > Extend the netlink operation `napi-set` and allow setting the threaded
> > attribute of a NAPI. This will enable the threaded polling on a napi
> > context.
> 
> I think I haven't replied to you on the config recommendation about
> how global vs per-object config should behave. I implemented the
> suggested scheme for rx-buf-len to make sure its not a crazy ask:
> https://lore.kernel.org/all/20250421222827.283737-1-kuba@kernel.org/
> and I do like it more.
> 
> Joe, Stanislav and Mina all read that series and are CCed here.
> What do y'all think? Should we make the threaded config work like
> the rx-buf-len, if user sets it on a NAPI it takes precedence
> over global config? Or stick to the simplistic thing of last
> write wins?

For the per-NAPI defer-hard-irqs (for example):
  - writing to the NIC-wide sysfs path overwrites all of the
    individual NAPI settings to be the global setting written
  - writing to an individual NAPI, though, the setting takes
    precedence over the global

So, if you wrote 100 to the global path, then 5 to a specific NAPI,
then 200 again to the global path, IIRC the NAPI would go through:
  - being set to 100 (from the global path write)
  - being set to 5 (for its NAPI specific write)
  - being set to 200 (from the final global path write)

The individual NAPI setting takes precedence over the global
setting; but the individual setting is re-written when the global
value is adjusted.

Can't tell if that's clear or if I just made it worse ;)

Anyway: I have a preference for consistency when possible, so IMHO,
it would be nice if:
  - Writing to NIC-wide threaded set all NAPIs to the value written
    to the NIC-wide setting
  - Individual NAPIs can have threaded enabled/disabled, which takes
    precedence over global

But IDK if that's realistic/desirable/or even what everyone else
prefers :)

  reply	other threads:[~2025-04-26  2:31 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-23 20:14 [PATCH net-next v5] Add support to set napi threaded for individual napi Samiullah Khawaja
2025-04-24 23:13 ` Joe Damato
2025-04-25 18:28   ` Samiullah Khawaja
2025-04-25 22:24     ` Joe Damato
2025-04-25 22:52       ` Samiullah Khawaja
2025-04-26  0:37         ` Jakub Kicinski
2025-04-26  2:34           ` Joe Damato
2025-04-26  2:47             ` Jakub Kicinski
2025-04-26  3:12               ` Jakub Kicinski
2025-04-26  3:53                 ` Samiullah Khawaja
2025-04-28 18:23                   ` Jakub Kicinski
2025-04-28 19:25                     ` Samiullah Khawaja
2025-04-25 23:06     ` Samiullah Khawaja
2025-04-26  0:42 ` Jakub Kicinski
2025-04-26  2:31   ` Joe Damato [this message]
2025-04-26 14:41     ` Willem de Bruijn
2025-04-28 18:12       ` Joe Damato
2025-04-28 18:38         ` Jakub Kicinski
2025-04-28 21:29           ` Joe Damato
2025-04-28 22:32             ` Jakub Kicinski
2025-04-30  0:16               ` Joe Damato
2025-05-03  2:10                 ` Jakub Kicinski
2025-05-03  3:04                   ` Joe Damato
2025-05-05 18:56                     ` Jakub Kicinski

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=aAxFmKo2cmLUmqAJ@LQ3V64L9R2 \
    --to=jdamato@fastly.com \
    --cc=almasrymina@google.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=mkarsten@uwaterloo.ca \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=skhawaja@google.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 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.