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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Mon, 27 Nov 2023 07:14:51 -0700
From: Ahmed Zaki <ahmed.zaki@...el.com>
To: Jakub Kicinski <kuba@...nel.org>
CC: <netdev@...r.kernel.org>, <intel-wired-lan@...ts.osuosl.org>,
	<corbet@....net>, <jesse.brandeburg@...el.com>, <anthony.l.nguyen@...el.com>,
	<davem@...emloft.net>, <edumazet@...gle.com>, <pabeni@...hat.com>,
	<vladimir.oltean@....com>, <andrew@...n.ch>, <horms@...nel.org>,
	<mkubecek@...e.cz>, <willemdebruijn.kernel@...il.com>, <gal@...dia.com>,
	<alexander.duyck@...il.com>, <linux-doc@...r.kernel.org>, Igor Bagnucki
	<igor.bagnucki@...el.com>, Jacob Keller <jacob.e.keller@...el.com>
Subject: Re: [PATCH net-next v6 1/7] net: ethtool: pass ethtool_rxfh to
 get/set_rxfh ethtool ops



On 2023-11-21 16:29, Jakub Kicinski wrote:
> On Mon, 20 Nov 2023 13:56:08 -0700 Ahmed Zaki wrote:
>>   	u32	(*get_rxfh_key_size)(struct net_device *);
>>   	u32	(*get_rxfh_indir_size)(struct net_device *);
>> -	int	(*get_rxfh)(struct net_device *, u32 *indir, u8 *key,
>> -			    u8 *hfunc);
>> -	int	(*set_rxfh)(struct net_device *, const u32 *indir,
>> -			    const u8 *key, const u8 hfunc);
>> +	int	(*get_rxfh)(struct net_device *, struct ethtool_rxfh *,
>> +			    u32 *indir, u8 *key);
>> +	int	(*set_rxfh)(struct net_device *, struct ethtool_rxfh *,
>> +			    const u32 *indir, const u8 *key);
>>   	int	(*get_rxfh_context)(struct net_device *, u32 *indir, u8 *key,
>>   				    u8 *hfunc, u32 rss_context);
>>   	int	(*set_rxfh_context)(struct net_device *, const u32 *indir,
> 
> This conversion looks 1/4th done. You should do the following:
> 
>   - First simplify the code by always providing a pointer to all params
>     (indir, key and func); the fact that some of them may be NULL seems
>     like a weird historic thing or a premature optimization.
>     It will simplify the drivers if all pointers are always present.
>     You don't have to remove the if () checks in the existing drivers.
> 
>   - Then make the functions take a dev pointer, and a pointer to a
>     single struct wrapping all arguments. The set_* should also take
>     an extack.

Can we skip the "extack" part for this series? There is no 
"ETHTOOL_MSG_RSS_SET" netlink message, which is needed for user-space to 
get the ACK and adding all the netlink stuff seems a bit out of scope.

I will do the rest in the next version.

Thanks.

> 
>   - Add a rss_context member to the argument struct and a capability
>     like cap_link_lanes_supported to indicate whether driver supports
>     rss contexts, then you can remove *et_rxfh_context functions,
>     and instead call *et_rxfh() with a non-zero rss_context.
> 
>   - Add your new member to the struct wrapping all params.
> 
> If you just expose struct ethtool_rxfh to the drivers (a) there are
> fields in there drivers shouldn't touch, and (b) that struct is uAPI
> so we can't add netlink-only fields easily.