Re: [RFC net-next v2 1/9] net: napi: Add napi_storage

[Joe Damato]

On Sun, Sep 08, 2024 at 04:06:35PM +0000, Joe Damato wrote:
> Add a persistent NAPI storage area for NAPI configuration to the core.
> Drivers opt-in to setting the storage for a NAPI by passing an index
> when calling netif_napi_add_storage.
> 
> napi_storage is allocated in alloc_netdev_mqs, freed in free_netdev
> (after the NAPIs are deleted), and set to 0 when napi_enable is called.
> 
> Signed-off-by: Joe Damato <jdamato@xxxxxxxxxx>
> ---
>  .../networking/net_cachelines/net_device.rst  |  1 +
>  include/linux/netdevice.h                     | 34 +++++++++++++++++++
>  net/core/dev.c                                | 18 +++++++++-
>  3 files changed, 52 insertions(+), 1 deletion(-)
> 

[...]

> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -6719,6 +6719,9 @@ void napi_enable(struct napi_struct *n)
>  		if (n->dev->threaded && n->thread)
>  			new |= NAPIF_STATE_THREADED;
>  	} while (!try_cmpxchg(&n->state, &val, new));
> +
> +	if (n->napi_storage)
> +		memset(n->napi_storage, 0, sizeof(*n->napi_storage));

This part is very obviously wrong ;)

I think when I was reading the other thread about resetting on
channel change [1] I got a bit confused.

Maybe what was intended was on napi_enable, do nothing / remove the
above code (which I suppose would give the persistence desired?).

But modify net/ethtool/channels.c to reset NAPIs to the global
(sysfs) settings? Not sure how to balance both persistence with
queue count changes in a way that makes sense for users of the API.

And, I didn't quite follow the bits about:
  1. The proposed ring to NAPI mapping
  2. The two step "takeover" which seemed to imply that we might
     pull napi_id into napi_storage? Or maybe I just read that part
     wrong?

I'll go back re-re-(re?)-read those emails to see if I can figure
out what the desired implementation is.

I'd welcome any/all feedback/clarifications on this part in
particular.

[1]: https://lore.kernel.org/netdev/20240903124008.4793c087@xxxxxxxxxx/