Re: [PATCH v7 2/9] ethdev: support setting and querying RSS algorithm

["lihuisong (C)" <lihuisong@huawei.com>]

With belows to changes,
Acked-by: Huisong Li <lihuisong@huawei.com>


在 2023/10/28 9:46, Jie Hai 写道:
> Currently, rte_eth_rss_conf supports configuring and querying
> RSS hash functions, rss key and it's length, but not RSS hash
> algorithm.
>
> The structure ``rte_eth_dev_info`` is extended by adding a new
> field "rss_algo_capa". Drivers are responsible for reporting this
> capa and configurations of RSS hash algorithm can be verified based
> on the capability. The default value of "rss_algo_capa" is
> RTE_ETH_HASH_ALGO_CAPA_MASK(DEFAULT) if drivers do not report it.
>
> The structure ``rte_eth_rss_conf`` is extended by adding a new
> field "algorithm". This represents the RSS algorithms to apply.
> If the value of "algorithm" used for configuration is a gibberish
> value, drivers should report the error.
>
> To check whether the drivers report valid "algorithm", it is set
> to default value before querying in rte_eth_dev_rss_hash_conf_get().
>
> Signed-off-by: Jie Hai <haijie1@huawei.com>
> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> ---
>   doc/guides/rel_notes/release_23_11.rst |  5 ++++
>   lib/ethdev/rte_ethdev.c                | 26 ++++++++++++++++++++
>   lib/ethdev/rte_ethdev.h                | 33 +++++++++++++++++++++++++-
>   lib/ethdev/rte_flow.c                  |  1 -
>   lib/ethdev/rte_flow.h                  | 26 ++------------------
>   5 files changed, 65 insertions(+), 26 deletions(-)
>
> diff --git a/doc/guides/rel_notes/release_23_11.rst b/doc/guides/rel_notes/release_23_11.rst
> index 0a6fc76a9d02..a35d729d2cc7 100644
> --- a/doc/guides/rel_notes/release_23_11.rst
> +++ b/doc/guides/rel_notes/release_23_11.rst
> @@ -360,6 +360,11 @@ ABI Changes
>   * security: struct ``rte_security_ipsec_sa_options`` was updated
>     due to inline out-of-place feature addition.
>   
> +* ethdev: Added "rss_algo_capa" field to ``rte_eth_dev_info`` structure for
> +* reporting RSS hash algorithm capability.
> +
> +* ethdev: Added "algorithm" field to ``rte_eth_rss_conf`` structure for RSS
> +  hash algorithm.
>   
>   Known Issues
>   ------------
> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> index 9dabcb5ae28e..90bfbf14d1f7 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -1269,6 +1269,7 @@ int
>   rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
>   		      const struct rte_eth_conf *dev_conf)
>   {
> +	enum rte_eth_hash_function algorithm;
>   	struct rte_eth_dev *dev;
>   	struct rte_eth_dev_info dev_info;
>   	struct rte_eth_conf orig_conf;
> @@ -1500,6 +1501,18 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
>   		goto rollback;
>   	}
>   
> +	algorithm = dev_conf->rx_adv_conf.rss_conf.algorithm;
> +	if ((dev_info.rss_algo_capa &
> +	     RTE_ETH_HASH_ALGO_TO_CAPA(algorithm)) == 0) {
need to check the algorithm.
its value should be in range of 0 to 31.
> +		RTE_ETHDEV_LOG(ERR,
> +			"Ethdev port_id=%u config unsupported RSS hash algorithm: %u "
> +			"with rss_algo_capa: %x\n",
It seems that this log is not friendly to user.
Configured RSS hash algorithm (%u) is not in the algorithm capability ().
Anything ok like that.

%x --> 0x%" PRIx32 "
> +			port_id, algorithm,
> +			dev_info.rss_algo_capa);
> +		ret = -EINVAL;
> +		goto rollback;
> +	}
> +
>   	/*
>   	 * Setup new number of Rx/Tx queues and reconfigure device.
>   	 */
> @@ -3757,6 +3770,7 @@ rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info)
>   	dev_info->min_mtu = RTE_ETHER_MIN_LEN - RTE_ETHER_HDR_LEN -
>   		RTE_ETHER_CRC_LEN;
>   	dev_info->max_mtu = UINT16_MAX;
> +	dev_info->rss_algo_capa = RTE_ETH_HASH_ALGO_CAPA_MASK(DEFAULT);
>   
>   	if (*dev->dev_ops->dev_infos_get == NULL)
>   		return -ENOTSUP;
> @@ -4698,6 +4712,16 @@ rte_eth_dev_rss_hash_update(uint16_t port_id,
>   		return -ENOTSUP;
>   	}
>   
> +	if ((dev_info.rss_algo_capa &
> +	     RTE_ETH_HASH_ALGO_TO_CAPA(rss_conf->algorithm)) == 0) {
> +		RTE_ETHDEV_LOG(ERR,
> +			"Ethdev port_id=%u config unsupported RSS hash algorithm: %u "
> +			"with rss_algo_capa: %x\n",
> +			port_id, rss_conf->algorithm,
> +			dev_info.rss_algo_capa);
> +		return -EINVAL;
> +	}
> +
>   	if (*dev->dev_ops->rss_hash_update == NULL)
>   		return -ENOTSUP;
>   	ret = eth_err(port_id, (*dev->dev_ops->rss_hash_update)(dev,
> @@ -4725,6 +4749,8 @@ rte_eth_dev_rss_hash_conf_get(uint16_t port_id,
>   		return -EINVAL;
>   	}
>   
> +	rss_conf->algorithm = RTE_ETH_HASH_FUNCTION_DEFAULT;
> +
>   	if (*dev->dev_ops->rss_hash_conf_get == NULL)
>   		return -ENOTSUP;
>   	ret = eth_err(port_id, (*dev->dev_ops->rss_hash_conf_get)(dev,
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index 37fd5afef48a..2f639edd8218 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -445,6 +445,33 @@ struct rte_vlan_filter_conf {
>   	uint64_t ids[64];
>   };
>   
> +/**
> + * Hash function types.
> + */
> +enum rte_eth_hash_function {
> +	/** DEFAULT means driver decides which hash algorithm to pick. */
> +	RTE_ETH_HASH_FUNCTION_DEFAULT = 0,
> +	RTE_ETH_HASH_FUNCTION_TOEPLITZ, /**< Toeplitz */
> +	RTE_ETH_HASH_FUNCTION_SIMPLE_XOR, /**< Simple XOR */
> +	/**
> +	 * Symmetric Toeplitz: src, dst will be replaced by
> +	 * xor(src, dst). For the case with src/dst only,
> +	 * src or dst address will xor with zero pair.
> +	 */
> +	RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ,
> +	/**
> +	 * Symmetric Toeplitz: L3 and L4 fields are sorted prior to
> +	 * the hash function.
> +	 *  If src_ip > dst_ip, swap src_ip and dst_ip.
> +	 *  If src_port > dst_port, swap src_port and dst_port.
> +	 */
> +	RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ_SORT,
> +	RTE_ETH_HASH_FUNCTION_MAX,
> +};
> +
> +#define RTE_ETH_HASH_ALGO_TO_CAPA(x) RTE_BIT32(x)
> +#define RTE_ETH_HASH_ALGO_CAPA_MASK(x) RTE_BIT32(RTE_ETH_HASH_FUNCTION_ ## x)
> +
>   /**
>    * A structure used to configure the Receive Side Scaling (RSS) feature
>    * of an Ethernet port.
> @@ -469,6 +496,7 @@ struct rte_eth_rss_conf {
>   	 * which RSS hashing is to be applied.
>   	 */
>   	uint64_t rss_hf;
> +	enum rte_eth_hash_function algorithm;	/**< Hash algorithm. */
>   };
>   
>   /*
> @@ -1783,7 +1811,10 @@ struct rte_eth_dev_info {
>   	/** Supported error handling mode. */
>   	enum rte_eth_err_handle_mode err_handle_mode;
>   
> -	uint64_t reserved_64s[2]; /**< Reserved for future fields */
> +	/** RSS hash algorithms capabilities */
> +	uint32_t rss_algo_capa;
Please move this new field to "hash_key_size" and 
"flow_type_rss_offloads" in this struct.
Because this version allows ABI break.
> +
> +	uint32_t reserved_32s[3]; /**< Reserved for future fields */
>   	void *reserved_ptrs[2];   /**< Reserved for future fields */
>   };
>   
> diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
> index 3a67f1aaba9d..c13a2a391c37 100644
> --- a/lib/ethdev/rte_flow.c
> +++ b/lib/ethdev/rte_flow.c
> @@ -13,7 +13,6 @@
>   #include <rte_branch_prediction.h>
>   #include <rte_string_fns.h>
>   #include <rte_mbuf_dyn.h>
> -#include "rte_ethdev.h"
>   #include "rte_flow_driver.h"
>   #include "rte_flow.h"
>   
> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> index 25f1dffd1f30..c94f553ae06f 100644
> --- a/lib/ethdev/rte_flow.h
> +++ b/lib/ethdev/rte_flow.h
> @@ -40,6 +40,8 @@
>   #include <rte_macsec.h>
>   #include <rte_ib.h>
>   
> +#include "rte_ethdev.h"
> +
>   #ifdef __cplusplus
>   extern "C" {
>   #endif
> @@ -3222,30 +3224,6 @@ struct rte_flow_query_count {
>   	uint64_t bytes; /**< Number of bytes through this rule [out]. */
>   };
>   
> -/**
> - * Hash function types.
> - */
> -enum rte_eth_hash_function {
> -	/** DEFAULT means driver decides which hash algorithm to pick. */
> -	RTE_ETH_HASH_FUNCTION_DEFAULT = 0,
> -	RTE_ETH_HASH_FUNCTION_TOEPLITZ, /**< Toeplitz */
> -	RTE_ETH_HASH_FUNCTION_SIMPLE_XOR, /**< Simple XOR */
> -	/**
> -	 * Symmetric Toeplitz: src, dst will be replaced by
> -	 * xor(src, dst). For the case with src/dst only,
> -	 * src or dst address will xor with zero pair.
> -	 */
> -	RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ,
> -	/**
> -	 * Symmetric Toeplitz: L3 and L4 fields are sorted prior to
> -	 * the hash function.
> -	 *  If src_ip > dst_ip, swap src_ip and dst_ip.
> -	 *  If src_port > dst_port, swap src_port and dst_port.
> -	 */
> -	RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ_SORT,
> -	RTE_ETH_HASH_FUNCTION_MAX,
> -};
> -
>   /**
>    * RTE_FLOW_ACTION_TYPE_RSS
>    *