[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e0d451be-8c22-332b-bd6b-09edc4d25c97@gmail.com>
Date: Tue, 4 Jun 2024 15:58:02 +0100
From: Edward Cree <ecree.xilinx@...il.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: Ahmed Zaki <ahmed.zaki@...el.com>, netdev@...r.kernel.org
Subject: Re: [PATCH net-next v6 1/7] net: ethtool: pass ethtool_rxfh to
get/set_rxfh ethtool ops
On 04/06/2024 00:17, Jakub Kicinski wrote:
> We add "supported" fields to the ethtool_ops (e.g.
> supported_coalesce_params) and reject settings in the core
> if the driver didn't opt in.
Ah yeah, good point. Will use params then.
> 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.)
So I guess context.indir_no_change should really be called
something like .indir_unspecified?
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?
-ed
Powered by blists - more mailing lists