Re: [PATCH net-next v6 1/7] net: ethtool: pass ethtool_rxfh to get/set_rxfh ethtool ops

[Jakub Kicinski <kuba@kernel.org>]

On Tue, 4 Jun 2024 15:58:02 +0100 Edward Cree wrote:
> > Can we avoid the confusion by careful wording of the related kdoc?
> > "context" is the current state, while "params" describe the intended
> > configuration. If we move the "no_change" bits over to "params", 
> > I hope it wouldn't be all that confusing.  
> 
> I think "no_change" should stay in "context", but be renamed.
> ("params" has them implicitly via setting indir_size to
>  ETH_RXFH_INDIR_NO_CHANGE or key_size to zero.)
> The bits in "context" mean that indir or key has *never* been
>  configured for this context, and therefore the driver should
>  make up a default.  In that case, if the context has to be
>  recreated (e.g. after a device reset, or maybe an ethtool -L
>  changing the number of RXQs), the driver could generate a
>  different table.  (Also, unless the driver decides to write
>  the generated default table back into "context" by hand, the
>  core won't be able to show it to userspace in netlink dumps
>  when those get added.)

Ah, great point!

> So I guess context.indir_no_change should really be called
>  something like .indir_unspecified?

/me looks at the code
We already have IFF_RXFH_CONFIGURED, and corresponding
netif_is_rxfh_configured(). Should we stick to "${field}_configured"?

> Or should the core just insist on handling default generation
>  itself (but then it can't be sure of producing defaults that
>  a device with limited resources can honour), or have yet
>  another op to populate the defaults into params when the
>  user didn't specify them?

Thinking this over during breakfast I concluded we should leave out
feeding the defaults into drivers for now.

The only useful fields we could pre-populate are indir table and
key (useful because it'd save drivers calling some ethtool_default*
helpers). But both of those are fairly complex. Key may not be
populated for dynamically created contexts at all. Indir table
may have different sizes and has to be re-calculated when queue
count changes.