On 2023/11/1 21:36, Ferruh Yigit wrote:
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.
thanks,will add.

  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?
yes, how about associate with "rss_algo_capa"?

+       if (algorithm >= CHAR_BIT * sizeof(dev_info.rss_algo_capa) ||
+ (dev_info.rss_algo_capa & RTE_ETH_HASH_ALGO_TO_CAPA(algorithm)) == 0) {




.

Reply via email to