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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<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