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.
