[v9,1/9] ethdev: overwrite some comment related to RSS

Message ID 20231102082020.2588392-2-haijie1@huawei.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers
Series support setting and querying RSS algorithms |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-testing warning apply patch failure

Commit Message

Jie Hai Nov. 2, 2023, 8:20 a.m. UTC
  In rte_eth_dev_rss_hash_conf_get(), the "rss_key_len" should be
greater than or equal to the "hash_key_size" which get from
rte_eth_dev_info_get() API. And the "rss_key" should contain at
least "hash_key_size" bytes. If these requirements are not met,
the query unreliable.

In rte_eth_dev_rss_hash_update() or rte_eth_dev_configure(), the
"rss_key_len" indicates the length of the "rss_key" in bytes of
the array pointed by "rss_key", it should be equal to the
"hash_key_size" if "rss_key" is not NULL.

This patch overwrites the comments of fields of "rte_eth_rss_conf"
and "RTE_ETH_HASH_FUNCTION_DEFAULT", checks "rss_key_len" in
ethdev level, and documents these changes.

Signed-off-by: Jie Hai <haijie1@huawei.com>
Acked-by: Huisong Li <lihuisong@huawei.com>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
---
 doc/guides/rel_notes/release_23_11.rst |  4 ++++
 lib/ethdev/rte_ethdev.c                | 31 ++++++++++++++++++++++++
 lib/ethdev/rte_ethdev.h                | 33 ++++++++++++++------------
 lib/ethdev/rte_flow.h                  |  1 +
 4 files changed, 54 insertions(+), 15 deletions(-)
  

Comments

Andrew Rybchenko Nov. 6, 2023, 10:57 a.m. UTC | #1
On 11/2/23 11:20, Jie Hai wrote:
> In rte_eth_dev_rss_hash_conf_get(), the "rss_key_len" should be
> greater than or equal to the "hash_key_size" which get from
> rte_eth_dev_info_get() API. And the "rss_key" should contain at
> least "hash_key_size" bytes. If these requirements are not met,
> the query unreliable.
> 
> In rte_eth_dev_rss_hash_update() or rte_eth_dev_configure(), the
> "rss_key_len" indicates the length of the "rss_key" in bytes of
> the array pointed by "rss_key", it should be equal to the
> "hash_key_size" if "rss_key" is not NULL.
> 
> This patch overwrites the comments of fields of "rte_eth_rss_conf"
> and "RTE_ETH_HASH_FUNCTION_DEFAULT", checks "rss_key_len" in
> ethdev level, and documents these changes.
> 
> Signed-off-by: Jie Hai <haijie1@huawei.com>
> Acked-by: Huisong Li <lihuisong@huawei.com>
> Acked-by: Chengwen Feng <fengchengwen@huawei.com>


<snip>

> @@ -4712,6 +4730,7 @@ int
>   rte_eth_dev_rss_hash_conf_get(uint16_t port_id,
>   			      struct rte_eth_rss_conf *rss_conf)
>   {
> +	struct rte_eth_dev_info dev_info = { 0 };

There is no poiint to init dev_info here. Get functoin does it anyway.
  

Patch

diff --git a/doc/guides/rel_notes/release_23_11.rst b/doc/guides/rel_notes/release_23_11.rst
index 95db98d098d8..0b67f31dc385 100644
--- a/doc/guides/rel_notes/release_23_11.rst
+++ b/doc/guides/rel_notes/release_23_11.rst
@@ -345,6 +345,10 @@  API Changes
   Updated ``rte_ml_op``, ``rte_ml_io_quantize`` and ``rte_ml_io_dequantize``
   to support an array of ``rte_ml_buff_seg``.
 
+* ethdev: When ``rte_eth_dev_configure`` or ``rte_eth_dev_rss_hash_update`` are
+  called, the ``rss_key_len`` of structure ``rte_eth_rss_conf`` should be provided
+  by user for the case ``rss_key != NULL``, it won't be taken as default 40
+  bytes anymore.
 
 ABI Changes
 -----------
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index af23ac0ad00f..6727aca12dce 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -1500,6 +1500,16 @@  rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
 		goto rollback;
 	}
 
+	if (dev_conf->rx_adv_conf.rss_conf.rss_key != NULL &&
+	    dev_conf->rx_adv_conf.rss_conf.rss_key_len != dev_info.hash_key_size) {
+		RTE_ETHDEV_LOG(ERR,
+			"Ethdev port_id=%u invalid RSS key len: %u, valid value: %u\n",
+			port_id, dev_conf->rx_adv_conf.rss_conf.rss_key_len,
+			dev_info.hash_key_size);
+		ret = -EINVAL;
+		goto rollback;
+	}
+
 	/*
 	 * Setup new number of Rx/Tx queues and reconfigure device.
 	 */
@@ -4698,6 +4708,14 @@  rte_eth_dev_rss_hash_update(uint16_t port_id,
 		return -ENOTSUP;
 	}
 
+	if (rss_conf->rss_key != NULL &&
+	    rss_conf->rss_key_len != dev_info.hash_key_size) {
+		RTE_ETHDEV_LOG(ERR,
+			"Ethdev port_id=%u invalid RSS key len: %u, valid value: %u\n",
+			port_id, rss_conf->rss_key_len, dev_info.hash_key_size);
+		return -EINVAL;
+	}
+
 	if (*dev->dev_ops->rss_hash_update == NULL)
 		return -ENOTSUP;
 	ret = eth_err(port_id, (*dev->dev_ops->rss_hash_update)(dev,
@@ -4712,6 +4730,7 @@  int
 rte_eth_dev_rss_hash_conf_get(uint16_t port_id,
 			      struct rte_eth_rss_conf *rss_conf)
 {
+	struct rte_eth_dev_info dev_info = { 0 };
 	struct rte_eth_dev *dev;
 	int ret;
 
@@ -4725,6 +4744,18 @@  rte_eth_dev_rss_hash_conf_get(uint16_t port_id,
 		return -EINVAL;
 	}
 
+	ret = rte_eth_dev_info_get(port_id, &dev_info);
+	if (ret != 0)
+		return ret;
+
+	if (rss_conf->rss_key != NULL &&
+	    rss_conf->rss_key_len < dev_info.hash_key_size) {
+		RTE_ETHDEV_LOG(ERR,
+			"Ethdev port_id=%u invalid RSS key len: %u, should not be less than: %u\n",
+			port_id, rss_conf->rss_key_len, dev_info.hash_key_size);
+		return -EINVAL;
+	}
+
 	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 a53dd5a1efec..fb1a1f89c67f 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -448,24 +448,27 @@  struct rte_vlan_filter_conf {
 /**
  * A structure used to configure the Receive Side Scaling (RSS) feature
  * of an Ethernet port.
- * If not NULL, the *rss_key* pointer of the *rss_conf* structure points
- * to an array holding the RSS key to use for hashing specific header
- * fields of received packets. The length of this array should be indicated
- * by *rss_key_len* below. Otherwise, a default random hash key is used by
- * the device driver.
- *
- * The *rss_key_len* field of the *rss_conf* structure indicates the length
- * in bytes of the array pointed by *rss_key*. To be compatible, this length
- * will be checked in i40e only. Others assume 40 bytes to be used as before.
- *
- * The *rss_hf* field of the *rss_conf* structure indicates the different
- * types of IPv4/IPv6 packets to which the RSS hashing must be applied.
- * Supplying an *rss_hf* equal to zero disables the RSS feature.
  */
 struct rte_eth_rss_conf {
-	uint8_t *rss_key;    /**< If not NULL, 40-byte hash key. */
+	/**
+	 * In rte_eth_dev_rss_hash_conf_get(), the *rss_key_len* should be
+	 * greater than or equal to the *hash_key_size* which get from
+	 * rte_eth_dev_info_get() API. And the *rss_key* should contain at least
+	 * *hash_key_size* bytes. If not meet these requirements, the query
+	 * result is unreliable even if the operation returns success.
+	 *
+	 * In rte_eth_dev_rss_hash_update() or rte_eth_dev_configure(), if
+	 * *rss_key* is not NULL, the *rss_key_len* indicates the length of the
+	 * *rss_key* in bytes and it should be equal to *hash_key_size*.
+	 * If *rss_key* is NULL, drivers are free to use a random or a default key.
+	 */
+	uint8_t *rss_key;
 	uint8_t rss_key_len; /**< hash key length in bytes. */
-	uint64_t rss_hf;     /**< Hash functions to apply - see below. */
+	/**
+	 * Indicates the type of packets or the specific part of packets to
+	 * which RSS hashing is to be applied.
+	 */
+	uint64_t rss_hf;
 };
 
 /*
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index c16fe8c21f2f..751c29a0f3f3 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -3226,6 +3226,7 @@  struct rte_flow_query_count {
  * 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 */