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.
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Reply via email to