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]

Date: Wed, 5 Jun 2024 20:42:32 +0300
From: Gal Pressman <gal@...dia.com>
To: Ahmed Zaki <ahmed.zaki@...el.com>, 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, 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 27/11/2023 16:14, Ahmed Zaki wrote:
> 
> 
> 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.

Hi Ahmed,

Sorry for reviving this old thread, I noticed you kept the extack in the
set_rxfh callback eventually. Was that on purpose?
It's weird that we have a parameter that is always passed as NULL.