[PATCH v8 03/10] ethdev: support setting and querying RSS algorithm

[Ferruh Yigit]

On 11/1/2023 7:40 AM, Jie Hai wrote:
> 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 at huawei.com>
> Signed-off-by: Dongdong Liu <liudongdong3 at huawei.com>
> Acked-by: Huisong Li <lihuisong at huawei.com>
> ---
>  doc/guides/rel_notes/release_23_11.rst |  5 +++++
>  lib/ethdev/rte_ethdev.c                | 26 +++++++++++++++++++++++
>  lib/ethdev/rte_ethdev.h                | 29 ++++++++++++++++++++++++++
>  lib/ethdev/rte_flow.c                  |  1 -
>  lib/ethdev/rte_flow.h                  | 26 ++---------------------
>  5 files changed, 62 insertions(+), 25 deletions(-)
> 
> diff --git a/doc/guides/rel_notes/release_23_11.rst b/doc/guides/rel_notes/release_23_11.rst
> index 95db98d098d8..e207786044f9 100644
> --- a/doc/guides/rel_notes/release_23_11.rst
> +++ b/doc/guides/rel_notes/release_23_11.rst
> @@ -372,6 +372,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.
>  

As well as ABI change, can you also update the "New Features", to
document getting hash algorithm capability and setting hash algorithm
support added?

Also please add an empty line here.

>  Known Issues
>  ------------
> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> index 07bb35833ba6..f9bd99d07eb1 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;
> @@ -1510,6 +1511,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 (RTE_ETH_HASH_ALGO_TO_CAPA(algorithm) == 0 ||
>

"RTE_ETH_HASH_ALGO_TO_CAPA(algorithm)" can't be zero for valid "enum
rte_eth_hash_function" values, I assume above check is for the case
algorith > 31, as it will result zero.
My concern is, this is undefined behaviour (shift left >= 32) and some
compiler can complain about it, instead of relying this can you please
add explicit "0 <= algorithm < 32" check?