On 10/03, Joe Damato wrote: > On Thu, Oct 03, 2024 at 04:53:13PM -0700, Joe Damato wrote: > > On Thu, Oct 03, 2024 at 04:29:37PM -0700, Stanislav Fomichev wrote: > > > On 10/01, Joe Damato wrote: > > > > [...] > > > > > > 2. This revision seems to work (see below for a full walk through). Is > > > > this the behavior we want? Am I missing some use case or some > > > > behavioral thing other folks need? > > > > > > The walk through looks good! > > > > Thanks for taking a look. > > > > > > 3. Re a previous point made by Stanislav regarding "taking over a NAPI > > > > ID" when the channel count changes: mlx5 seems to call napi_disable > > > > followed by netif_napi_del for the old queues and then calls > > > > napi_enable for the new ones. In this RFC, the NAPI ID generation > > > > is deferred to napi_enable. This means we won't end up with two of > > > > the same NAPI IDs added to the hash at the same time (I am pretty > > > > sure). > > > > > > > > > [..] > > > > > > > Can we assume all drivers will napi_disable the old queues before > > > > napi_enable the new ones? If yes, we might not need to worry about > > > > a NAPI ID takeover function. > > > > > > With the explicit driver opt-in via netif_napi_add_config, this > > > shouldn't matter? When somebody gets to converting the drivers that > > > don't follow this common pattern they'll have to solve the takeover > > > part :-) > > > > That is true; that's a good point. > > Actually, sorry, that isn't strictly true. NAPI ID generation is > moved for everything to napi_enable; they just are (or are not) > persisted depending on whether the driver opted in to add_config or > not. > > So, the change does affect all drivers. NAPI IDs won't be generated > and added to the hash until napi_enable and they will be removed > from the hash in napi_disable... even if you didn't opt-in to having > storage. > > Opt-ing in to storage via netif_napi_add_config just means that your > NAPI IDs (and other settings) will be persistent. > > Sorry about my confusion when replying earlier. AFAIA, all control operations (ethtool or similar ones via netlink), should grab rtnl lock. So as long as both enable/disable happen under rtnl (and in my mind they should), I don't think there is gonna be any user-visible side-effects of your change. But I might be wrong, let's see if others can come up with some corner cases..
