netdev - Re: [PATCH v5 net-next 4/7] net: ethtool: let the core choose RSS context IDs

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] [day] [month] [year] [list]

Date: Thu, 20 Jun 2024 05:42:49 +0100
From: Edward Cree <ecree.xilinx@...il.com>
To: Jakub Kicinski <kuba@...nel.org>, edward.cree@....com
Cc: linux-net-drivers@....com, davem@...emloft.com, edumazet@...gle.com,
 pabeni@...hat.com, netdev@...r.kernel.org, habetsm.xilinx@...il.com,
 sudheer.mogilappagari@...el.com, jdamato@...tly.com, mw@...ihalf.com,
 linux@...linux.org.uk, sgoutham@...vell.com, gakula@...vell.com,
 sbhatta@...vell.com, hkelam@...vell.com, saeedm@...dia.com, leon@...nel.org,
 jacob.e.keller@...el.com, andrew@...n.ch, ahmed.zaki@...el.com
Subject: Re: [PATCH v5 net-next 4/7] net: ethtool: let the core choose RSS
 context IDs

On 19/06/2024 18:24, Jakub Kicinski wrote:
> On Tue, 18 Jun 2024 23:44:24 +0100 edward.cree@....com wrote:
>> + * @create_rxfh_context: Create a new RSS context with the specified RX flow
>> + *	hash indirection table, hash key, and hash function.
>> + *	Parameters which are set to %NULL or zero will be populated to
>> + *	appropriate defaults by the driver.
> 
> The defaults will most likely "inherit" whatever is set in context 0.
> So the driver _may_ init the values according to its preferences
> but they will not be used by the core (specifically not reported to
> user space via ethtool netlink)
> 
> Does that match your thinking?

Yes, that was what I had in mind.

> Indirection table needs to get reported.

Okay, I'll alter the documentation to say so, that notwithstanding
 this bit...

>> + *	The &struct ethtool_rxfh_context for this context is passed in @ctx;
>> + *	note that the indir table, hkey and hfunc are not yet populated as
>> + *	of this call.  The driver does not need to update these; the core
>> + *	will do so if this op succeeds.

... at least indir MUST, and the others MAY, be filled in by the
 driver if they weren't specified in params.
(sfc does this already, because it uses the ctx as a place to store
 the new table and/or key if it has to generate them.)

>> +	int	(*remove_rxfh_context)(struct net_device *,
>> +				       struct ethtool_rxfh_context *ctx,
>> +				       u32 rss_context);
> 
> Can we make remove void? It's sort of a cleanup, cleanups which can
> fail make life hard.

At least on sfc it's possible for it to fail.  Apart from anything
 else, I don't think there's anything in the core to catch the case
 of trying to remove a context while there are still ntuple filters
 targeting it; I believe that gets all the way down to the firmware,
 which responds EBUSY or something.  If that happens, we don't want
 to pretend it worked and delete the context from the xarray.

-ed