On 09/08, Joe Damato wrote: > 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? Yes, the suggestion here is to drop patch #2 from your series and keep napi_id as a user facing 'id' for the persistent storage. But, obviously, this requires persistent napi_id(s) that survive device resets. The function that allocates new napi_id is napi_hash_add from netif_napi_add_weight. So we can do either of the following: 1. Keep everything as is, but add the napi_rehash somewhere around napi_enable to 'takeover' previously allocated napi_id. 2. (preferred) Separate napi_hash_add out of netif_napi_add_weight. And have some new napi_hash_with_id(previous_napi_id) to expose it to the userspace. Then convert mlx5 to this new interface.
