Re: [PATCH net-next v5] Add support to set napi threaded for individual napi

[Joe Damato <jdamato@fastly.com>]

On Sat, Apr 26, 2025 at 10:41:10AM -0400, Willem de Bruijn wrote:
> Joe Damato wrote:
> > 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 ;)
> 
> That does not sound like precedence to me ;)

Sounds like you are focusing on the usage of a word both out of
context and without considering the behavior of the system ;)

> I interpret precedence as a value being sticky. The NAPI would stay
> at 5 even after the global write of 200.

The individual NAPI config value is always used before the global
value is consulted. One might say it precedes the global value when
used in the networking stack.

That individual NAPI value may be rewritten by writes to the
NIC-wide path, though, which does not affect the precedence with
which the values are consulted by the code.

> > Anyway: I have a preference for consistency
> 
> +1
> 
> I don't think either solution is vastly better than the other, as
> long as it is the path of least surprise. Different behavior for
> different options breaks that rule.

I agree and my feedback on the previous revision was that all NAPI
config settings should work similarly. Whether that's what I already
implemented for defer-hard-irq/gro-flush-timeout or something else I
don't really have a strong preference.

Implementing something other than what already exists for
defer-hard-irq/gro-flush-timeout, though, would probably mean you'll
need to update how both of those work, for consistency.