Re: [PATCH net-next 5/5] netdev-genl: Support setting per-NAPI config values

[Jakub Kicinski <kuba@kernel.org>]

On Thu, 29 Aug 2024 13:12:01 +0000 Joe Damato wrote:
> +	napi = napi_by_id(napi_id);
> +	if (napi)
> +		err = netdev_nl_napi_set_config(napi, info);
> +	else
> +		err = -EINVAL;

if (napi) {
...
} else {
	NL_SET_BAD_ATTR(info->extack, info->attrs[NETDEV_A_NAPI_ID])
	err = -ENOENT;
}

> +      doc: Set configurable NAPI instance settings.

We should pause and think here how configuring NAPI params should
behave. NAPI instances are ephemeral, if you close and open the
device (or for some drivers change any BPF or ethtool setting)
the NAPIs may get wiped and recreated, discarding all configuration.

This is not how the sysfs API behaves, the sysfs settings on the device
survive close. It's (weirdly?) also not how queues behave, because we
have struct netdev{_rx,}_queue to store stuff persistently. Even tho
you'd think queues are as ephemeral as NAPIs if not more.

I guess we can either document this, and move on (which may be fine,
you have more practical experience than me). Or we can add an internal
concept of a "channel" (which perhaps maybe if you squint is what
ethtool -l calls NAPIs?) or just "napi_storage" as an array inside
net_device and store such config there. For simplicity of matching
config to NAPIs we can assume drivers add NAPI instances in order. 
If driver wants to do something more fancy we can add a variant of
netif_napi_add() which specifies the channel/storage to use.

Thoughts? I may be overly sensitive to the ephemeral thing, maybe
I work with unfortunate drivers...