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.
