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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <680cf086aec78_193a062946c@willemb.c.googlers.com.notmuch>
Date: Sat, 26 Apr 2025 10:41:10 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Joe Damato <jdamato@...tly.com>, 
 Jakub Kicinski <kuba@...nel.org>
Cc: Samiullah Khawaja <skhawaja@...gle.com>, 
 "David S . Miller" <davem@...emloft.net>, 
 Eric Dumazet <edumazet@...gle.com>, 
 Paolo Abeni <pabeni@...hat.com>, 
 almasrymina@...gle.com, 
 willemb@...gle.com, 
 mkarsten@...terloo.ca, 
 netdev@...r.kernel.org
Subject: Re: [PATCH net-next v5] Add support to set napi threaded for
 individual napi

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 ;)

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

> 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.

This also reminds me of /proc/sys/net/ipv4/conf/{all, default, .. }
API. Which confuses me to this day.

>From the PoV of path of least surprise, minor preference for Joe's
example of having no sticky state, but just last write wins.

> 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 :)