Re: [PATCH v5 net-next 4/7] net: ethtool: let the core choose RSS context IDs

[Jakub Kicinski <kuba@kernel.org>]

On Tue, 18 Jun 2024 23:44:24 +0100 edward.cree@amd.com wrote:
> + * @create_rxfh_context: Create a new RSS context with the specified RX flow
> + *	hash indirection table, hash key, and hash function.
> + *	Parameters which are set to %NULL or zero will be populated to
> + *	appropriate defaults by the driver.

The defaults will most likely "inherit" whatever is set in context 0.
So the driver _may_ init the values according to its preferences
but they will not be used by the core (specifically not reported to
user space via ethtool netlink)

Does that match your thinking?

> + *	The &struct ethtool_rxfh_context for this context is passed in @ctx;
> + *	note that the indir table, hkey and hfunc are not yet populated as
> + *	of this call.  The driver does not need to update these; the core
> + *	will do so if this op succeeds.
> + *	If the driver provides this method, it must also provide
> + *	@modify_rxfh_context and @remove_rxfh_context.
> + *	Returns a negative error code or zero.
> + * @modify_rxfh_context: Reconfigure the specified RSS context.  Allows setting
> + *	the contents of the RX flow hash indirection table, hash key, and/or
> + *	hash function associated with the given context.
> + *	Parameters which are set to %NULL or zero will remain unchanged.
> + *	The &struct ethtool_rxfh_context for this context is passed in @ctx;
> + *	note that it will still contain the *old* settings.  The driver does
> + *	not need to update these; the core will do so if this op succeeds.
> + *	Returns a negative error code or zero. An error code must be returned
> + *	if at least one unsupported change was requested.
> + * @remove_rxfh_context: Remove the specified RSS context.
> + *	The &struct ethtool_rxfh_context for this context is passed in @ctx.
> + *	Returns a negative error code or zero.
>   * @get_channels: Get number of channels.
>   * @set_channels: Set number of channels.  Returns a negative error code or
>   *	zero.
> @@ -906,6 +933,7 @@ struct ethtool_ops {
>  	u32     cap_rss_ctx_supported:1;
>  	u32	cap_rss_sym_xor_supported:1;
>  	u16	rxfh_priv_size;
> +	u32	rxfh_max_context_id;
>  	u32	supported_coalesce_params;
>  	u32	supported_ring_params;
>  	void	(*get_drvinfo)(struct net_device *, struct ethtool_drvinfo *);
> @@ -968,6 +996,15 @@ struct ethtool_ops {
>  	int	(*get_rxfh)(struct net_device *, struct ethtool_rxfh_param *);
>  	int	(*set_rxfh)(struct net_device *, struct ethtool_rxfh_param *,
>  			    struct netlink_ext_ack *extack);
> +	int	(*create_rxfh_context)(struct net_device *,
> +				       struct ethtool_rxfh_context *ctx,
> +				       const struct ethtool_rxfh_param *rxfh);
> +	int	(*modify_rxfh_context)(struct net_device *,
> +				       struct ethtool_rxfh_context *ctx,
> +				       const struct ethtool_rxfh_param *rxfh);
> +	int	(*remove_rxfh_context)(struct net_device *,
> +				       struct ethtool_rxfh_context *ctx,
> +				       u32 rss_context);

Can we make remove void? It's sort of a cleanup, cleanups which can
fail make life hard.