Re: [PATCH net v2 2/3] net: update NAPI threaded config even for disabled NAPIs

[Joe Damato <joe@dama.to>]

On Fri, Aug 08, 2025 at 05:12:04PM -0700, Jakub Kicinski wrote:

> We chose not to have an "unset" state for threaded, and not to wipe
> the NAPI config clean when channels are explicitly disabled.

Yea... I wonder if we could change that now or if it's too late? I think this
is the thing you mentioned that I couldn't recall in my response to the cover
letter.

> This means the persistent config structs "exist" even when their NAPIs
> are not instantiated.
> 
> Differently put - the NAPI persistent state lives in the net_device
> (ncfg == struct napi_config):
> 
>     ,--- [napi 0] - [napi 1]
>  [dev]      |          |
>     `--- [ncfg 0] - [ncfg 1]
> 
> so say we a device with 2 queues but only 1 enabled:
> 
>     ,--- [napi 0]
>  [dev]      |
>     `--- [ncfg 0] - [ncfg 1]
> 
> now we set the device to threaded=1:
> 
>     ,---------- [napi 0 (thr:1)]
>  [dev(thr:1)]      |
>     `---------- [ncfg 0 (thr:1)] - [ncfg 1 (thr:?)]
> 
> Since [ncfg 1] was not attached to a NAPI during configuration we
> skipped it. If we create a NAPI for it later it will have the old
> setting (presumably disabled). One could argue if this is right
> or not "in principle", but it's definitely not how things worked
> before per-NAPI config..

Thanks for the detailed commit message. I agree that it should probably work
the same now.

> Fixes: 2677010e7793 ("Add support to set NAPI threaded for individual NAPI")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> v2: add missing kdoc
> ---
>  include/linux/netdevice.h | 5 ++++-
>  net/core/dev.c            | 7 ++++++-
>  2 files changed, 10 insertions(+), 2 deletions(-)

Reviewed-by: Joe Damato <joe@dama.to>