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 <haij...@huawei.com>
> Signed-off-by: Dongdong Liu <liudongdo...@huawei.com>
> Acked-by: Huisong Li <lihuis...@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?
