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/
