patches for DPDK stable branches
 help / color / mirror / Atom feed
* [PATCH 0/5] support setting and querying RSS algorithms
@ 2023-03-15 11:00 Dongdong Liu
  2023-03-15 11:00 ` [PATCH 1/5] ethdev: support setting and querying rss algorithm Dongdong Liu
                   ` (5 more replies)
  0 siblings, 6 replies; 27+ messages in thread
From: Dongdong Liu @ 2023-03-15 11:00 UTC (permalink / raw)
  To: dev, ferruh.yigit, thomas, andrew.rybchenko, reshma.pattan
  Cc: stable, yisen.zhuang, liudongdong3

This patchset is to support setting and querying RSS algorithms.

Huisong Li (1):
  net/hns3: support setting and querying RSS hash function

Jie Hai (4):
  ethdev: support setting and querying rss algorithm
  app/proc-info: fix never show RSS info
  app/proc-info: show RSS types with strings
  app/proc-info: support querying RSS hash algorithm

 app/proc-info/main.c                   | 145 ++++++++++++++++++++++++-
 doc/guides/rel_notes/release_23_03.rst |   4 +-
 drivers/net/hns3/hns3_rss.c            |  47 ++++----
 lib/ethdev/rte_ethdev.c                |  18 +++
 lib/ethdev/rte_ethdev.h                |   5 +
 5 files changed, 191 insertions(+), 28 deletions(-)

--
2.22.0


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH 1/5] ethdev: support setting and querying rss algorithm
  2023-03-15 11:00 [PATCH 0/5] support setting and querying RSS algorithms Dongdong Liu
@ 2023-03-15 11:00 ` Dongdong Liu
  2023-03-15 11:28   ` Ivan Malov
  2023-03-15 13:43   ` Thomas Monjalon
  2023-03-15 11:00 ` [PATCH 2/5] net/hns3: support setting and querying RSS hash function Dongdong Liu
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 27+ messages in thread
From: Dongdong Liu @ 2023-03-15 11:00 UTC (permalink / raw)
  To: dev, ferruh.yigit, thomas, andrew.rybchenko, reshma.pattan
  Cc: stable, yisen.zhuang, liudongdong3, Jie Hai

From: Jie Hai <haijie1@huawei.com>

Currently, rte_eth_rss_conf supports configuring rss hash
functions, rss key and it's length, but not rss hash algorithm.

The structure ``rte_eth_rss_conf`` is extended by adding a new field,
"func". This represents the RSS algorithms to apply. The following
API is affected:
	- rte_eth_dev_configure
	- rte_eth_dev_rss_hash_update
	- rte_eth_dev_rss_hash_conf_get

To prevent configuration failures caused by incorrect func input, check
this parameter in advance. If it's incorrect, a warning is generated
and the default value is set. Do the same for rte_eth_dev_rss_hash_update
and rte_eth_dev_configure.

To check whether the drivers report the func field, it is set to default
value before querying.

Signed-off-by: Jie Hai <haijie1@huawei.com>
Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
---
 doc/guides/rel_notes/release_23_03.rst |  4 ++--
 lib/ethdev/rte_ethdev.c                | 18 ++++++++++++++++++
 lib/ethdev/rte_ethdev.h                |  5 +++++
 3 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/doc/guides/rel_notes/release_23_03.rst b/doc/guides/rel_notes/release_23_03.rst
index af6f37389c..7879567427 100644
--- a/doc/guides/rel_notes/release_23_03.rst
+++ b/doc/guides/rel_notes/release_23_03.rst
@@ -284,8 +284,8 @@ ABI Changes
    Also, make sure to start the actual text at the margin.
    =======================================================
 
-* No ABI change that would break compatibility with 22.11.
-
+* ethdev: Added "func" 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 4d03255683..db561026bd 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -1368,6 +1368,15 @@ 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.func >= RTE_ETH_HASH_FUNCTION_MAX) {
+		RTE_ETHDEV_LOG(WARNING,
+			"Ethdev port_id=%u invalid rss hash function (%u), modified to default value (%u)\n",
+			port_id, dev_conf->rx_adv_conf.rss_conf.func,
+			RTE_ETH_HASH_FUNCTION_DEFAULT);
+		dev->data->dev_conf.rx_adv_conf.rss_conf.func =
+			RTE_ETH_HASH_FUNCTION_DEFAULT;
+	}
+
 	/* Check if Rx RSS distribution is disabled but RSS hash is enabled. */
 	if (((dev_conf->rxmode.mq_mode & RTE_ETH_MQ_RX_RSS_FLAG) == 0) &&
 	    (dev_conf->rxmode.offloads & RTE_ETH_RX_OFFLOAD_RSS_HASH)) {
@@ -4553,6 +4562,13 @@ rte_eth_dev_rss_hash_update(uint16_t port_id,
 		return -ENOTSUP;
 	}
 
+	if (rss_conf->func >= RTE_ETH_HASH_FUNCTION_MAX) {
+		RTE_ETHDEV_LOG(NOTICE,
+			"Ethdev port_id=%u invalid rss hash function (%u), modified to default value (%u)\n",
+			port_id, rss_conf->func, RTE_ETH_HASH_FUNCTION_DEFAULT);
+		rss_conf->func = RTE_ETH_HASH_FUNCTION_DEFAULT;
+	}
+
 	if (*dev->dev_ops->rss_hash_update == NULL)
 		return -ENOTSUP;
 	ret = eth_err(port_id, (*dev->dev_ops->rss_hash_update)(dev,
@@ -4580,6 +4596,8 @@ rte_eth_dev_rss_hash_conf_get(uint16_t port_id,
 		return -EINVAL;
 	}
 
+	rss_conf->func = 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 99fe9e238b..5abe2cb36d 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -174,6 +174,7 @@ extern "C" {
 
 #include "rte_ethdev_trace_fp.h"
 #include "rte_dev_info.h"
+#include "rte_flow.h"
 
 extern int rte_eth_dev_logtype;
 
@@ -461,11 +462,15 @@ struct rte_vlan_filter_conf {
  * 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.
+ *
+ * The *func* field of the *rss_conf* structure indicates the different
+ * types of hash algorithms applied by the RSS hashing.
  */
 struct rte_eth_rss_conf {
 	uint8_t *rss_key;    /**< If not NULL, 40-byte hash key. */
 	uint8_t rss_key_len; /**< hash key length in bytes. */
 	uint64_t rss_hf;     /**< Hash functions to apply - see below. */
+	enum rte_eth_hash_function func;	/**< Hash algorithm to apply. */
 };
 
 /*
-- 
2.22.0


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH 2/5] net/hns3: support setting and querying RSS hash function
  2023-03-15 11:00 [PATCH 0/5] support setting and querying RSS algorithms Dongdong Liu
  2023-03-15 11:00 ` [PATCH 1/5] ethdev: support setting and querying rss algorithm Dongdong Liu
@ 2023-03-15 11:00 ` Dongdong Liu
  2023-03-15 11:00 ` [PATCH 3/5] app/proc-info: fix never show RSS info Dongdong Liu
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 27+ messages in thread
From: Dongdong Liu @ 2023-03-15 11:00 UTC (permalink / raw)
  To: dev, ferruh.yigit, thomas, andrew.rybchenko, reshma.pattan
  Cc: stable, yisen.zhuang, liudongdong3, Huisong Li

From: Huisong Li <lihuisong@huawei.com>

Support setting and querying RSS hash function by ethdev ops.

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
---
 drivers/net/hns3/hns3_rss.c | 47 +++++++++++++++++++++----------------
 1 file changed, 27 insertions(+), 20 deletions(-)

diff --git a/drivers/net/hns3/hns3_rss.c b/drivers/net/hns3/hns3_rss.c
index 6126512bd7..c8346d43d1 100644
--- a/drivers/net/hns3/hns3_rss.c
+++ b/drivers/net/hns3/hns3_rss.c
@@ -646,14 +646,14 @@ hns3_dev_rss_hash_update(struct rte_eth_dev *dev,
 	if (ret)
 		goto set_tuple_fail;
 
-	if (key) {
-		ret = hns3_rss_set_algo_key(hw, hw->rss_info.hash_algo,
-					    key, hw->rss_key_size);
-		if (ret)
-			goto set_algo_key_fail;
-		/* Update the shadow RSS key with user specified */
+	ret = hns3_update_rss_algo_key(hw, rss_conf->func, key, key_len);
+	if (ret != 0)
+		goto set_algo_key_fail;
+
+	if (rss_conf->func != RTE_ETH_HASH_FUNCTION_DEFAULT)
+		hw->rss_info.hash_algo = hns3_hash_func_map[rss_conf->func];
+	if (key != NULL)
 		memcpy(hw->rss_info.key, key, hw->rss_key_size);
-	}
 	hw->rss_info.rss_hf = rss_hf;
 	rte_spinlock_unlock(&hw->lock);
 
@@ -769,7 +769,13 @@ int
 hns3_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
 			   struct rte_eth_rss_conf *rss_conf)
 {
+	const uint8_t hash_func_map[] = {
+		[HNS3_RSS_HASH_ALGO_TOEPLITZ] = RTE_ETH_HASH_FUNCTION_TOEPLITZ,
+		[HNS3_RSS_HASH_ALGO_SIMPLE] = RTE_ETH_HASH_FUNCTION_SIMPLE_XOR,
+		[HNS3_RSS_HASH_ALGO_SYMMETRIC_TOEP] = RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ,
+	};
 	struct hns3_adapter *hns = dev->data->dev_private;
+	uint8_t rss_key[HNS3_RSS_KEY_SIZE_MAX] = {0};
 	struct hns3_hw *hw = &hns->hw;
 	uint8_t hash_algo;
 	int ret;
@@ -777,26 +783,27 @@ hns3_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
 	rte_spinlock_lock(&hw->lock);
 	ret = hns3_rss_hash_get_rss_hf(hw, &rss_conf->rss_hf);
 	if (ret != 0) {
+		rte_spinlock_unlock(&hw->lock);
 		hns3_err(hw, "obtain hash tuples failed, ret = %d", ret);
-		goto out;
+		return ret;
+	}
+
+	ret = hns3_rss_get_algo_key(hw, &hash_algo, rss_key, hw->rss_key_size);
+	if (ret != 0) {
+		rte_spinlock_unlock(&hw->lock);
+		hns3_err(hw, "obtain hash algo and key failed, ret = %d", ret);
+		return ret;
 	}
+	rte_spinlock_unlock(&hw->lock);
 
-	/* Get the RSS Key required by the user */
+	/* Get the RSS Key if user required. */
 	if (rss_conf->rss_key && rss_conf->rss_key_len >= hw->rss_key_size) {
-		ret = hns3_rss_get_algo_key(hw, &hash_algo, rss_conf->rss_key,
-					    hw->rss_key_size);
-		if (ret != 0) {
-			hns3_err(hw, "obtain hash algo and key failed, ret = %d",
-				 ret);
-			goto out;
-		}
+		memcpy(rss_conf->rss_key, rss_key, hw->rss_key_size);
 		rss_conf->rss_key_len = hw->rss_key_size;
 	}
+	rss_conf->func = hash_func_map[hash_algo];
 
-out:
-	rte_spinlock_unlock(&hw->lock);
-
-	return ret;
+	return 0;
 }
 
 /*
-- 
2.22.0


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH 3/5] app/proc-info: fix never show RSS info
  2023-03-15 11:00 [PATCH 0/5] support setting and querying RSS algorithms Dongdong Liu
  2023-03-15 11:00 ` [PATCH 1/5] ethdev: support setting and querying rss algorithm Dongdong Liu
  2023-03-15 11:00 ` [PATCH 2/5] net/hns3: support setting and querying RSS hash function Dongdong Liu
@ 2023-03-15 11:00 ` Dongdong Liu
  2023-06-02 20:19   ` Ferruh Yigit
  2023-06-02 21:19   ` Stephen Hemminger
  2023-03-15 11:00 ` [PATCH 4/5] app/proc-info: show RSS types with strings Dongdong Liu
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 27+ messages in thread
From: Dongdong Liu @ 2023-03-15 11:00 UTC (permalink / raw)
  To: dev, ferruh.yigit, thomas, andrew.rybchenko, reshma.pattan
  Cc: stable, yisen.zhuang, liudongdong3, Jie Hai, Maryam Tahhan,
	Vipin Varghese, John McNamara

From: Jie Hai <haijie1@huawei.com>

Command show-port shows rss info only if rss_conf.rss_key
is not null but it will never be true. This patch allocates
memory for rss_conf.rss_key and makes it possible to show
rss info.

Fixes: 8a37f37fc243 ("app/procinfo: add --show-port")
Cc: stable@dpdk.org

Signed-off-by: Jie Hai <haijie1@huawei.com>
Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
---
 app/proc-info/main.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index 53e852a07c..878ce37e8b 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -823,6 +823,7 @@ show_port(void)
 		struct rte_eth_fc_conf fc_conf;
 		struct rte_ether_addr mac;
 		struct rte_eth_dev_owner owner;
+		uint8_t *rss_key;
 
 		/* Skip if port is not in mask */
 		if ((enabled_port_mask & (1ul << i)) == 0)
@@ -981,19 +982,26 @@ show_port(void)
 			printf("\n");
 		}
 
+		rss_key = rte_malloc(NULL,
+			dev_info.hash_key_size * sizeof(uint8_t), 0);
+		if (rss_key == NULL)
+			return;
+
+		rss_conf.rss_key = rss_key;
+		rss_conf.rss_key_len = dev_info.hash_key_size;
 		ret = rte_eth_dev_rss_hash_conf_get(i, &rss_conf);
 		if (ret == 0) {
-			if (rss_conf.rss_key) {
-				printf("  - RSS\n");
-				printf("\t  -- RSS len %u key (hex):",
-						rss_conf.rss_key_len);
-				for (k = 0; k < rss_conf.rss_key_len; k++)
-					printf(" %x", rss_conf.rss_key[k]);
-				printf("\t  -- hf 0x%"PRIx64"\n",
-						rss_conf.rss_hf);
-			}
+			printf("  - RSS\n");
+			printf("\t  -- RSS len %u key (hex):",
+					rss_conf.rss_key_len);
+			for (k = 0; k < rss_conf.rss_key_len; k++)
+				printf(" %x", rss_conf.rss_key[k]);
+			printf("\t  -- hf 0x%"PRIx64"\n",
+					rss_conf.rss_hf);
 		}
 
+		rte_free(rss_key);
+
 #ifdef RTE_LIB_SECURITY
 		show_security_context(i, true);
 #endif
-- 
2.22.0


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH 4/5] app/proc-info: show RSS types with strings
  2023-03-15 11:00 [PATCH 0/5] support setting and querying RSS algorithms Dongdong Liu
                   ` (2 preceding siblings ...)
  2023-03-15 11:00 ` [PATCH 3/5] app/proc-info: fix never show RSS info Dongdong Liu
@ 2023-03-15 11:00 ` Dongdong Liu
  2023-06-02 20:22   ` Ferruh Yigit
  2023-03-15 11:00 ` [PATCH 5/5] app/proc-info: support querying RSS hash algorithm Dongdong Liu
  2023-08-26  7:00 ` [PATCH v2 0/5] support setting and querying RSS algorithms Jie Hai
  5 siblings, 1 reply; 27+ messages in thread
From: Dongdong Liu @ 2023-03-15 11:00 UTC (permalink / raw)
  To: dev, ferruh.yigit, thomas, andrew.rybchenko, reshma.pattan
  Cc: stable, yisen.zhuang, liudongdong3, Jie Hai, Maryam Tahhan

From: Jie Hai <haijie1@huawei.com>

show RSS types details and adjust RSS info display format as following:

  - RSS info
	  -- hf:
		ipv4  ipv4-frag  ipv4-other  ipv6  ipv6-frag  ipv6-other
	  -- key len: 40
	  -- key (hex): 6d5a56da255b0ec24167253d43a38fb0d0ca2bcbae7b30b477cb2da38030f20c6a42b73bbeac01fa

Signed-off-by: Jie Hai <haijie1@huawei.com>
Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
---
 app/proc-info/main.c | 120 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 113 insertions(+), 7 deletions(-)

diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index 878ce37e8b..7b6f8f1228 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -54,6 +54,9 @@
 #define STATS_BDR_STR(w, s) printf("%.*s%s%.*s\n", w, \
 	STATS_BDR_FMT, s, w, STATS_BDR_FMT)
 
+/* It is used to print the RSS types. */
+#define RSS_TYPES_CHAR_NUM_PER_LINE 64
+
 /* mask of enabled ports */
 static unsigned long enabled_port_mask;
 /* Enable stats. */
@@ -132,6 +135,76 @@ struct desc_param {
 static struct desc_param rx_desc_param;
 static struct desc_param tx_desc_param;
 
+/* Information for a given RSS type. */
+struct rss_type_info {
+	const char *str; /* Type name. */
+	uint64_t rss_type; /* Type value. */
+};
+
+static const struct rss_type_info rss_type_table[] = {
+	/* Group types */
+	{ "all", RTE_ETH_RSS_ETH | RTE_ETH_RSS_VLAN | RTE_ETH_RSS_IP | RTE_ETH_RSS_TCP |
+		RTE_ETH_RSS_UDP | RTE_ETH_RSS_SCTP | RTE_ETH_RSS_L2_PAYLOAD |
+		RTE_ETH_RSS_L2TPV3 | RTE_ETH_RSS_ESP | RTE_ETH_RSS_AH | RTE_ETH_RSS_PFCP |
+		RTE_ETH_RSS_GTPU | RTE_ETH_RSS_ECPRI | RTE_ETH_RSS_MPLS | RTE_ETH_RSS_L2TPV2},
+	{ "none", 0 },
+	{ "ip", RTE_ETH_RSS_IP },
+	{ "udp", RTE_ETH_RSS_UDP },
+	{ "tcp", RTE_ETH_RSS_TCP },
+	{ "sctp", RTE_ETH_RSS_SCTP },
+	{ "tunnel", RTE_ETH_RSS_TUNNEL },
+	{ "vlan", RTE_ETH_RSS_VLAN },
+
+	/* Individual type */
+	{ "ipv4", RTE_ETH_RSS_IPV4 },
+	{ "ipv4-frag", RTE_ETH_RSS_FRAG_IPV4 },
+	{ "ipv4-tcp", RTE_ETH_RSS_NONFRAG_IPV4_TCP },
+	{ "ipv4-udp", RTE_ETH_RSS_NONFRAG_IPV4_UDP },
+	{ "ipv4-sctp", RTE_ETH_RSS_NONFRAG_IPV4_SCTP },
+	{ "ipv4-other", RTE_ETH_RSS_NONFRAG_IPV4_OTHER },
+	{ "ipv6", RTE_ETH_RSS_IPV6 },
+	{ "ipv6-frag", RTE_ETH_RSS_FRAG_IPV6 },
+	{ "ipv6-tcp", RTE_ETH_RSS_NONFRAG_IPV6_TCP },
+	{ "ipv6-udp", RTE_ETH_RSS_NONFRAG_IPV6_UDP },
+	{ "ipv6-sctp", RTE_ETH_RSS_NONFRAG_IPV6_SCTP },
+	{ "ipv6-other", RTE_ETH_RSS_NONFRAG_IPV6_OTHER },
+	{ "l2-payload", RTE_ETH_RSS_L2_PAYLOAD },
+	{ "ipv6-ex", RTE_ETH_RSS_IPV6_EX },
+	{ "ipv6-tcp-ex", RTE_ETH_RSS_IPV6_TCP_EX },
+	{ "ipv6-udp-ex", RTE_ETH_RSS_IPV6_UDP_EX },
+	{ "port", RTE_ETH_RSS_PORT },
+	{ "vxlan", RTE_ETH_RSS_VXLAN },
+	{ "geneve", RTE_ETH_RSS_GENEVE },
+	{ "nvgre", RTE_ETH_RSS_NVGRE },
+	{ "gtpu", RTE_ETH_RSS_GTPU },
+	{ "eth", RTE_ETH_RSS_ETH },
+	{ "s-vlan", RTE_ETH_RSS_S_VLAN },
+	{ "c-vlan", RTE_ETH_RSS_C_VLAN },
+	{ "esp", RTE_ETH_RSS_ESP },
+	{ "ah", RTE_ETH_RSS_AH },
+	{ "l2tpv3", RTE_ETH_RSS_L2TPV3 },
+	{ "pfcp", RTE_ETH_RSS_PFCP },
+	{ "pppoe", RTE_ETH_RSS_PPPOE },
+	{ "ecpri", RTE_ETH_RSS_ECPRI },
+	{ "mpls", RTE_ETH_RSS_MPLS },
+	{ "ipv4-chksum", RTE_ETH_RSS_IPV4_CHKSUM },
+	{ "l4-chksum", RTE_ETH_RSS_L4_CHKSUM },
+	{ "l2tpv2", RTE_ETH_RSS_L2TPV2 },
+	{ "l3-pre96", RTE_ETH_RSS_L3_PRE96 },
+	{ "l3-pre64", RTE_ETH_RSS_L3_PRE64 },
+	{ "l3-pre56", RTE_ETH_RSS_L3_PRE56 },
+	{ "l3-pre48", RTE_ETH_RSS_L3_PRE48 },
+	{ "l3-pre40", RTE_ETH_RSS_L3_PRE40 },
+	{ "l3-pre32", RTE_ETH_RSS_L3_PRE32 },
+	{ "l2-dst-only", RTE_ETH_RSS_L2_DST_ONLY },
+	{ "l2-src-only", RTE_ETH_RSS_L2_SRC_ONLY },
+	{ "l4-dst-only", RTE_ETH_RSS_L4_DST_ONLY },
+	{ "l4-src-only", RTE_ETH_RSS_L4_SRC_ONLY },
+	{ "l3-dst-only", RTE_ETH_RSS_L3_DST_ONLY },
+	{ "l3-src-only", RTE_ETH_RSS_L3_SRC_ONLY },
+	{ NULL, 0},
+};
+
 /* display usage */
 static void
 proc_info_usage(const char *prgname)
@@ -806,6 +879,33 @@ show_offloads(uint64_t offloads,
 	}
 }
 
+static void
+show_rss_types(uint64_t rss_types)
+{
+	uint16_t total_len = 0;
+	uint16_t str_len;
+	uint16_t i;
+
+	printf("\t\t");
+	for (i = 0; rss_type_table[i].str != NULL; i++) {
+		if (rss_type_table[i].rss_type == 0)
+			continue;
+
+		if ((rss_type_table[i].rss_type & rss_types) ==
+					rss_type_table[i].rss_type) {
+			/* Contain two spaces */
+			str_len = strlen(rss_type_table[i].str) + 2;
+			if (total_len + str_len > RSS_TYPES_CHAR_NUM_PER_LINE) {
+				printf("\n\t\t");
+				total_len = 0;
+			}
+			printf("%s  ", rss_type_table[i].str);
+			total_len += str_len;
+		}
+	}
+	printf("\n");
+}
+
 static void
 show_port(void)
 {
@@ -991,13 +1091,19 @@ show_port(void)
 		rss_conf.rss_key_len = dev_info.hash_key_size;
 		ret = rte_eth_dev_rss_hash_conf_get(i, &rss_conf);
 		if (ret == 0) {
-			printf("  - RSS\n");
-			printf("\t  -- RSS len %u key (hex):",
-					rss_conf.rss_key_len);
-			for (k = 0; k < rss_conf.rss_key_len; k++)
-				printf(" %x", rss_conf.rss_key[k]);
-			printf("\t  -- hf 0x%"PRIx64"\n",
-					rss_conf.rss_hf);
+			printf("  - RSS info\n");
+			if (rss_conf.rss_hf == 0) {
+				printf("\tRSS disabled\n");
+			} else {
+				printf("\t  -- hf:\n");
+				show_rss_types(rss_conf.rss_hf);
+				printf("\t  -- key len: %u\n",
+						rss_conf.rss_key_len);
+				printf("\t  -- key (hex): ");
+				for (k = 0; k < rss_conf.rss_key_len; k++)
+					printf("%02x", rss_conf.rss_key[k]);
+				printf("\n");
+			}
 		}
 
 		rte_free(rss_key);
-- 
2.22.0


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH 5/5] app/proc-info: support querying RSS hash algorithm
  2023-03-15 11:00 [PATCH 0/5] support setting and querying RSS algorithms Dongdong Liu
                   ` (3 preceding siblings ...)
  2023-03-15 11:00 ` [PATCH 4/5] app/proc-info: show RSS types with strings Dongdong Liu
@ 2023-03-15 11:00 ` Dongdong Liu
  2023-08-26  7:00 ` [PATCH v2 0/5] support setting and querying RSS algorithms Jie Hai
  5 siblings, 0 replies; 27+ messages in thread
From: Dongdong Liu @ 2023-03-15 11:00 UTC (permalink / raw)
  To: dev, ferruh.yigit, thomas, andrew.rybchenko, reshma.pattan
  Cc: stable, yisen.zhuang, liudongdong3, Jie Hai, Maryam Tahhan

From: Jie Hai <haijie1@huawei.com>

Display RSS hash algorithm with command show-port as below.
  - RSS info
	  -- hash algorithm: toeplitz

Signed-off-by: Jie Hai <haijie1@huawei.com>
Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
---
 app/proc-info/main.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index 7b6f8f1228..4c2a113a1a 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -879,6 +879,23 @@ show_offloads(uint64_t offloads,
 	}
 }
 
+static const char *
+rss_func_to_str(enum rte_eth_hash_function func)
+{
+	switch (func) {
+	case RTE_ETH_HASH_FUNCTION_SIMPLE_XOR:
+		return "simple_xor";
+	case RTE_ETH_HASH_FUNCTION_TOEPLITZ:
+		return "toeplitz";
+	case RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ:
+		return "symmetric_toeplitz";
+	case RTE_ETH_HASH_FUNCTION_DEFAULT:
+		return "default";
+	default:
+		return "unknown";
+	}
+}
+
 static void
 show_rss_types(uint64_t rss_types)
 {
@@ -1097,6 +1114,8 @@ show_port(void)
 			} else {
 				printf("\t  -- hf:\n");
 				show_rss_types(rss_conf.rss_hf);
+				printf("\t  -- hash algorithm: %s\n",
+					rss_func_to_str(rss_conf.func));
 				printf("\t  -- key len: %u\n",
 						rss_conf.rss_key_len);
 				printf("\t  -- key (hex): ");
-- 
2.22.0


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 1/5] ethdev: support setting and querying rss algorithm
  2023-03-15 11:00 ` [PATCH 1/5] ethdev: support setting and querying rss algorithm Dongdong Liu
@ 2023-03-15 11:28   ` Ivan Malov
  2023-03-16 13:10     ` Dongdong Liu
  2023-03-15 13:43   ` Thomas Monjalon
  1 sibling, 1 reply; 27+ messages in thread
From: Ivan Malov @ 2023-03-15 11:28 UTC (permalink / raw)
  To: Dongdong Liu
  Cc: dev, ferruh.yigit, thomas, andrew.rybchenko, reshma.pattan,
	stable, yisen.zhuang, Jie Hai

Hi,

On Wed, 15 Mar 2023, Dongdong Liu wrote:

> From: Jie Hai <haijie1@huawei.com>
>
> Currently, rte_eth_rss_conf supports configuring rss hash
> functions, rss key and it's length, but not rss hash algorithm.
>
> The structure ``rte_eth_rss_conf`` is extended by adding a new field,
> "func". This represents the RSS algorithms to apply. The following
> API is affected:
> 	- rte_eth_dev_configure
> 	- rte_eth_dev_rss_hash_update
> 	- rte_eth_dev_rss_hash_conf_get
>
> To prevent configuration failures caused by incorrect func input, check
> this parameter in advance. If it's incorrect, a warning is generated
> and the default value is set. Do the same for rte_eth_dev_rss_hash_update
> and rte_eth_dev_configure.
>
> To check whether the drivers report the func field, it is set to default
> value before querying.
>
> Signed-off-by: Jie Hai <haijie1@huawei.com>
> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> ---
> doc/guides/rel_notes/release_23_03.rst |  4 ++--
> lib/ethdev/rte_ethdev.c                | 18 ++++++++++++++++++
> lib/ethdev/rte_ethdev.h                |  5 +++++
> 3 files changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/doc/guides/rel_notes/release_23_03.rst b/doc/guides/rel_notes/release_23_03.rst
> index af6f37389c..7879567427 100644
> --- a/doc/guides/rel_notes/release_23_03.rst
> +++ b/doc/guides/rel_notes/release_23_03.rst
> @@ -284,8 +284,8 @@ ABI Changes
>    Also, make sure to start the actual text at the margin.
>    =======================================================
>
> -* No ABI change that would break compatibility with 22.11.
> -
> +* ethdev: Added "func" 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 4d03255683..db561026bd 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -1368,6 +1368,15 @@ 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.func >= RTE_ETH_HASH_FUNCTION_MAX) {
> +		RTE_ETHDEV_LOG(WARNING,
> +			"Ethdev port_id=%u invalid rss hash function (%u), modified to default value (%u)\n",
> +			port_id, dev_conf->rx_adv_conf.rss_conf.func,
> +			RTE_ETH_HASH_FUNCTION_DEFAULT);
> +		dev->data->dev_conf.rx_adv_conf.rss_conf.func =
> +			RTE_ETH_HASH_FUNCTION_DEFAULT;

I have no strong opinion, but, to me, this behaviour conceals
programming errors. For example, if an application intends
to enable hash algorithm A but, due to a programming error,
passes a gibberish value here, chances are the error will
end up unnoticed. Especially in case the application
sets the log level to such that warnings are omitted.

Why not just return the error the standard way?

> +	}
> +
> 	/* Check if Rx RSS distribution is disabled but RSS hash is enabled. */
> 	if (((dev_conf->rxmode.mq_mode & RTE_ETH_MQ_RX_RSS_FLAG) == 0) &&
> 	    (dev_conf->rxmode.offloads & RTE_ETH_RX_OFFLOAD_RSS_HASH)) {
> @@ -4553,6 +4562,13 @@ rte_eth_dev_rss_hash_update(uint16_t port_id,
> 		return -ENOTSUP;
> 	}
>
> +	if (rss_conf->func >= RTE_ETH_HASH_FUNCTION_MAX) {
> +		RTE_ETHDEV_LOG(NOTICE,
> +			"Ethdev port_id=%u invalid rss hash function (%u), modified to default value (%u)\n",
> +			port_id, rss_conf->func, RTE_ETH_HASH_FUNCTION_DEFAULT);
> +		rss_conf->func = RTE_ETH_HASH_FUNCTION_DEFAULT;
> +	}
> +
> 	if (*dev->dev_ops->rss_hash_update == NULL)
> 		return -ENOTSUP;
> 	ret = eth_err(port_id, (*dev->dev_ops->rss_hash_update)(dev,
> @@ -4580,6 +4596,8 @@ rte_eth_dev_rss_hash_conf_get(uint16_t port_id,
> 		return -EINVAL;
> 	}
>
> +	rss_conf->func = 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 99fe9e238b..5abe2cb36d 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -174,6 +174,7 @@ extern "C" {
>
> #include "rte_ethdev_trace_fp.h"
> #include "rte_dev_info.h"
> +#include "rte_flow.h"
>
> extern int rte_eth_dev_logtype;
>
> @@ -461,11 +462,15 @@ struct rte_vlan_filter_conf {
>  * 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.
> + *
> + * The *func* field of the *rss_conf* structure indicates the different
> + * types of hash algorithms applied by the RSS hashing.

Consider:

The *func* field of the *rss_conf* structure indicates the algorithm to
use when computing hash. Passing RTE_ETH_HASH_FUNCTION_DEFAULT allows
the PMD to use its best-effort algorithm rather than a specific one.

>  */
> struct rte_eth_rss_conf {
> 	uint8_t *rss_key;    /**< If not NULL, 40-byte hash key. */
> 	uint8_t rss_key_len; /**< hash key length in bytes. */
> 	uint64_t rss_hf;     /**< Hash functions to apply - see below. */
> +	enum rte_eth_hash_function func;	/**< Hash algorithm to apply. */
> };
>
> /*
> -- 
> 2.22.0
>
>

Thank you.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 1/5] ethdev: support setting and querying rss algorithm
  2023-03-15 11:00 ` [PATCH 1/5] ethdev: support setting and querying rss algorithm Dongdong Liu
  2023-03-15 11:28   ` Ivan Malov
@ 2023-03-15 13:43   ` Thomas Monjalon
  2023-03-16 13:16     ` Dongdong Liu
  1 sibling, 1 reply; 27+ messages in thread
From: Thomas Monjalon @ 2023-03-15 13:43 UTC (permalink / raw)
  To: Dongdong Liu, Jie Hai
  Cc: dev, ferruh.yigit, andrew.rybchenko, reshma.pattan, stable,
	yisen.zhuang, david.marchand

15/03/2023 12:00, Dongdong Liu:
> From: Jie Hai <haijie1@huawei.com>
> --- a/doc/guides/rel_notes/release_23_03.rst
> +++ b/doc/guides/rel_notes/release_23_03.rst
> -* No ABI change that would break compatibility with 22.11.
> -
> +* ethdev: Added "func" field to ``rte_eth_rss_conf`` structure for RSS hash
> +  algorithm.

We cannot break ABI compatibility until 23.11.




^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 1/5] ethdev: support setting and querying rss algorithm
  2023-03-15 11:28   ` Ivan Malov
@ 2023-03-16 13:10     ` Dongdong Liu
  2023-03-16 14:31       ` Ivan Malov
  0 siblings, 1 reply; 27+ messages in thread
From: Dongdong Liu @ 2023-03-16 13:10 UTC (permalink / raw)
  To: Ivan Malov
  Cc: dev, ferruh.yigit, thomas, andrew.rybchenko, reshma.pattan,
	stable, yisen.zhuang, Jie Hai

Hi Ivan

Many thanks for your review.

On 2023/3/15 19:28, Ivan Malov wrote:
> Hi,
>
> On Wed, 15 Mar 2023, Dongdong Liu wrote:
>
>> From: Jie Hai <haijie1@huawei.com>
>>
>> Currently, rte_eth_rss_conf supports configuring rss hash
>> functions, rss key and it's length, but not rss hash algorithm.
>>
>> The structure ``rte_eth_rss_conf`` is extended by adding a new field,
>> "func". This represents the RSS algorithms to apply. The following
>> API is affected:
>>     - rte_eth_dev_configure
>>     - rte_eth_dev_rss_hash_update
>>     - rte_eth_dev_rss_hash_conf_get
>>
>> To prevent configuration failures caused by incorrect func input, check
>> this parameter in advance. If it's incorrect, a warning is generated
>> and the default value is set. Do the same for rte_eth_dev_rss_hash_update
>> and rte_eth_dev_configure.
>>
>> To check whether the drivers report the func field, it is set to default
>> value before querying.
>>
>> Signed-off-by: Jie Hai <haijie1@huawei.com>
>> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
>> ---
>> doc/guides/rel_notes/release_23_03.rst |  4 ++--
>> lib/ethdev/rte_ethdev.c                | 18 ++++++++++++++++++
>> lib/ethdev/rte_ethdev.h                |  5 +++++
>> 3 files changed, 25 insertions(+), 2 deletions(-)
>>
>> diff --git a/doc/guides/rel_notes/release_23_03.rst
>> b/doc/guides/rel_notes/release_23_03.rst
>> index af6f37389c..7879567427 100644
>> --- a/doc/guides/rel_notes/release_23_03.rst
>> +++ b/doc/guides/rel_notes/release_23_03.rst
>> @@ -284,8 +284,8 @@ ABI Changes
>>    Also, make sure to start the actual text at the margin.
>>    =======================================================
>>
>> -* No ABI change that would break compatibility with 22.11.
>> -
>> +* ethdev: Added "func" 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 4d03255683..db561026bd 100644
>> --- a/lib/ethdev/rte_ethdev.c
>> +++ b/lib/ethdev/rte_ethdev.c
>> @@ -1368,6 +1368,15 @@ 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.func >=
>> RTE_ETH_HASH_FUNCTION_MAX) {
>> +        RTE_ETHDEV_LOG(WARNING,
>> +            "Ethdev port_id=%u invalid rss hash function (%u),
>> modified to default value (%u)\n",
>> +            port_id, dev_conf->rx_adv_conf.rss_conf.func,
>> +            RTE_ETH_HASH_FUNCTION_DEFAULT);
>> +        dev->data->dev_conf.rx_adv_conf.rss_conf.func =
>> +            RTE_ETH_HASH_FUNCTION_DEFAULT;
>
> I have no strong opinion, but, to me, this behaviour conceals
> programming errors. For example, if an application intends
> to enable hash algorithm A but, due to a programming error,
> passes a gibberish value here, chances are the error will
> end up unnoticed. Especially in case the application
> sets the log level to such that warnings are omitted.
Good point, will fix.
>
> Why not just return the error the standard way?

Aha, The original intention is not to break the ABI,
but I think it could not achieve that.
>
>> +    }
>> +
>>     /* Check if Rx RSS distribution is disabled but RSS hash is
>> enabled. */
>>     if (((dev_conf->rxmode.mq_mode & RTE_ETH_MQ_RX_RSS_FLAG) == 0) &&
>>         (dev_conf->rxmode.offloads & RTE_ETH_RX_OFFLOAD_RSS_HASH)) {
>> @@ -4553,6 +4562,13 @@ rte_eth_dev_rss_hash_update(uint16_t port_id,
>>         return -ENOTSUP;
>>     }
>>
>> +    if (rss_conf->func >= RTE_ETH_HASH_FUNCTION_MAX) {
>> +        RTE_ETHDEV_LOG(NOTICE,
>> +            "Ethdev port_id=%u invalid rss hash function (%u),
>> modified to default value (%u)\n",
>> +            port_id, rss_conf->func, RTE_ETH_HASH_FUNCTION_DEFAULT);
>> +        rss_conf->func = RTE_ETH_HASH_FUNCTION_DEFAULT;
>> +    }
>> +
>>     if (*dev->dev_ops->rss_hash_update == NULL)
>>         return -ENOTSUP;
>>     ret = eth_err(port_id, (*dev->dev_ops->rss_hash_update)(dev,
>> @@ -4580,6 +4596,8 @@ rte_eth_dev_rss_hash_conf_get(uint16_t port_id,
>>         return -EINVAL;
>>     }
>>
>> +    rss_conf->func = 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 99fe9e238b..5abe2cb36d 100644
>> --- a/lib/ethdev/rte_ethdev.h
>> +++ b/lib/ethdev/rte_ethdev.h
>> @@ -174,6 +174,7 @@ extern "C" {
>>
>> #include "rte_ethdev_trace_fp.h"
>> #include "rte_dev_info.h"
>> +#include "rte_flow.h"
>>
>> extern int rte_eth_dev_logtype;
>>
>> @@ -461,11 +462,15 @@ struct rte_vlan_filter_conf {
>>  * 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.
>> + *
>> + * The *func* field of the *rss_conf* structure indicates the different
>> + * types of hash algorithms applied by the RSS hashing.
>
> Consider:
>
> The *func* field of the *rss_conf* structure indicates the algorithm to
> use when computing hash. Passing RTE_ETH_HASH_FUNCTION_DEFAULT allows
> the PMD to use its best-effort algorithm rather than a specific one.

Look at some PMD drivers(i40e, hns3 etc), it seems the 
RTE_ETH_HASH_FUNCTION_DEFAULT consider as no rss algorithm is set.

Thanks,
Dongdong
>
>>  */
>> struct rte_eth_rss_conf {
>>     uint8_t *rss_key;    /**< If not NULL, 40-byte hash key. */
>>     uint8_t rss_key_len; /**< hash key length in bytes. */
>>     uint64_t rss_hf;     /**< Hash functions to apply - see below. */
>> +    enum rte_eth_hash_function func;    /**< Hash algorithm to apply. */
>> };
>>
>> /*
>> --
>> 2.22.0
>>
>>
>
> Thank you.
>
> .
>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 1/5] ethdev: support setting and querying rss algorithm
  2023-03-15 13:43   ` Thomas Monjalon
@ 2023-03-16 13:16     ` Dongdong Liu
  2023-06-02 20:19       ` Ferruh Yigit
  0 siblings, 1 reply; 27+ messages in thread
From: Dongdong Liu @ 2023-03-16 13:16 UTC (permalink / raw)
  To: Thomas Monjalon, Jie Hai
  Cc: dev, ferruh.yigit, andrew.rybchenko, reshma.pattan, stable,
	yisen.zhuang, david.marchand

Hi Thomas
On 2023/3/15 21:43, Thomas Monjalon wrote:
> 15/03/2023 12:00, Dongdong Liu:
>> From: Jie Hai <haijie1@huawei.com>
>> --- a/doc/guides/rel_notes/release_23_03.rst
>> +++ b/doc/guides/rel_notes/release_23_03.rst
>> -* No ABI change that would break compatibility with 22.11.
>> -
>> +* ethdev: Added "func" field to ``rte_eth_rss_conf`` structure for RSS hash
>> +  algorithm.
>
> We cannot break ABI compatibility until 23.11.
Got it. Thank you for reminding.

[PATCH 3/5] and [PATCH 4/5] do not relate with this ABI compatibility.
I will send them separately.

Thanks,
Dongdong
>
>
>
> .
>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 1/5] ethdev: support setting and querying rss algorithm
  2023-03-16 13:10     ` Dongdong Liu
@ 2023-03-16 14:31       ` Ivan Malov
  0 siblings, 0 replies; 27+ messages in thread
From: Ivan Malov @ 2023-03-16 14:31 UTC (permalink / raw)
  To: Dongdong Liu
  Cc: dev, ferruh.yigit, thomas, andrew.rybchenko, reshma.pattan,
	stable, yisen.zhuang, Jie Hai

Hi,

Thanks for responding and PSB.

On Thu, 16 Mar 2023, Dongdong Liu wrote:

> Hi Ivan
>
> Many thanks for your review.
>
> On 2023/3/15 19:28, Ivan Malov wrote:
>> Hi,
>> 
>> On Wed, 15 Mar 2023, Dongdong Liu wrote:
>> 
>>> From: Jie Hai <haijie1@huawei.com>
>>> 
>>> Currently, rte_eth_rss_conf supports configuring rss hash
>>> functions, rss key and it's length, but not rss hash algorithm.
>>> 
>>> The structure ``rte_eth_rss_conf`` is extended by adding a new field,
>>> "func". This represents the RSS algorithms to apply. The following
>>> API is affected:
>>>     - rte_eth_dev_configure
>>>     - rte_eth_dev_rss_hash_update
>>>     - rte_eth_dev_rss_hash_conf_get
>>> 
>>> To prevent configuration failures caused by incorrect func input, check
>>> this parameter in advance. If it's incorrect, a warning is generated
>>> and the default value is set. Do the same for rte_eth_dev_rss_hash_update
>>> and rte_eth_dev_configure.
>>> 
>>> To check whether the drivers report the func field, it is set to default
>>> value before querying.
>>> 
>>> Signed-off-by: Jie Hai <haijie1@huawei.com>
>>> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
>>> ---
>>> doc/guides/rel_notes/release_23_03.rst |  4 ++--
>>> lib/ethdev/rte_ethdev.c                | 18 ++++++++++++++++++
>>> lib/ethdev/rte_ethdev.h                |  5 +++++
>>> 3 files changed, 25 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/doc/guides/rel_notes/release_23_03.rst
>>> b/doc/guides/rel_notes/release_23_03.rst
>>> index af6f37389c..7879567427 100644
>>> --- a/doc/guides/rel_notes/release_23_03.rst
>>> +++ b/doc/guides/rel_notes/release_23_03.rst
>>> @@ -284,8 +284,8 @@ ABI Changes
>>>    Also, make sure to start the actual text at the margin.
>>>    =======================================================
>>> 
>>> -* No ABI change that would break compatibility with 22.11.
>>> -
>>> +* ethdev: Added "func" 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 4d03255683..db561026bd 100644
>>> --- a/lib/ethdev/rte_ethdev.c
>>> +++ b/lib/ethdev/rte_ethdev.c
>>> @@ -1368,6 +1368,15 @@ 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.func >=
>>> RTE_ETH_HASH_FUNCTION_MAX) {
>>> +        RTE_ETHDEV_LOG(WARNING,
>>> +            "Ethdev port_id=%u invalid rss hash function (%u),
>>> modified to default value (%u)\n",
>>> +            port_id, dev_conf->rx_adv_conf.rss_conf.func,
>>> +            RTE_ETH_HASH_FUNCTION_DEFAULT);
>>> +        dev->data->dev_conf.rx_adv_conf.rss_conf.func =
>>> +            RTE_ETH_HASH_FUNCTION_DEFAULT;
>> 
>> I have no strong opinion, but, to me, this behaviour conceals
>> programming errors. For example, if an application intends
>> to enable hash algorithm A but, due to a programming error,
>> passes a gibberish value here, chances are the error will
>> end up unnoticed. Especially in case the application
>> sets the log level to such that warnings are omitted.
> Good point, will fix.
>> 
>> Why not just return the error the standard way?
>
> Aha, The original intention is not to break the ABI,
> but I think it could not achieve that.
>> 
>>> +    }
>>> +
>>>     /* Check if Rx RSS distribution is disabled but RSS hash is
>>> enabled. */
>>>     if (((dev_conf->rxmode.mq_mode & RTE_ETH_MQ_RX_RSS_FLAG) == 0) &&
>>>         (dev_conf->rxmode.offloads & RTE_ETH_RX_OFFLOAD_RSS_HASH)) {
>>> @@ -4553,6 +4562,13 @@ rte_eth_dev_rss_hash_update(uint16_t port_id,
>>>         return -ENOTSUP;
>>>     }
>>> 
>>> +    if (rss_conf->func >= RTE_ETH_HASH_FUNCTION_MAX) {
>>> +        RTE_ETHDEV_LOG(NOTICE,
>>> +            "Ethdev port_id=%u invalid rss hash function (%u),
>>> modified to default value (%u)\n",
>>> +            port_id, rss_conf->func, RTE_ETH_HASH_FUNCTION_DEFAULT);
>>> +        rss_conf->func = RTE_ETH_HASH_FUNCTION_DEFAULT;
>>> +    }
>>> +
>>>     if (*dev->dev_ops->rss_hash_update == NULL)
>>>         return -ENOTSUP;
>>>     ret = eth_err(port_id, (*dev->dev_ops->rss_hash_update)(dev,
>>> @@ -4580,6 +4596,8 @@ rte_eth_dev_rss_hash_conf_get(uint16_t port_id,
>>>         return -EINVAL;
>>>     }
>>> 
>>> +    rss_conf->func = 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 99fe9e238b..5abe2cb36d 100644
>>> --- a/lib/ethdev/rte_ethdev.h
>>> +++ b/lib/ethdev/rte_ethdev.h
>>> @@ -174,6 +174,7 @@ extern "C" {
>>> 
>>> #include "rte_ethdev_trace_fp.h"
>>> #include "rte_dev_info.h"
>>> +#include "rte_flow.h"
>>> 
>>> extern int rte_eth_dev_logtype;
>>> 
>>> @@ -461,11 +462,15 @@ struct rte_vlan_filter_conf {
>>>  * 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.
>>> + *
>>> + * The *func* field of the *rss_conf* structure indicates the different
>>> + * types of hash algorithms applied by the RSS hashing.
>> 
>> Consider:
>> 
>> The *func* field of the *rss_conf* structure indicates the algorithm to
>> use when computing hash. Passing RTE_ETH_HASH_FUNCTION_DEFAULT allows
>> the PMD to use its best-effort algorithm rather than a specific one.
>
> Look at some PMD drivers(i40e, hns3 etc), it seems the 
> RTE_ETH_HASH_FUNCTION_DEFAULT consider as no rss algorithm is set.

This does not seem to contradict the suggested description.

If they, however, treat this as "no RSS at all", then
perhaps it is a mistake, because if the user requests
Rx MQ mode "RSS" and selects algorithm DEFAULT, this
is clearly not the same as "no RSS". Not by a long
shot. Because for "no RSS" the user would have
passed MQ mode choice "NONE", I take it.

>
> Thanks,
> Dongdong
>>
>>>  */
>>> struct rte_eth_rss_conf {
>>>     uint8_t *rss_key;    /**< If not NULL, 40-byte hash key. */
>>>     uint8_t rss_key_len; /**< hash key length in bytes. */
>>>     uint64_t rss_hf;     /**< Hash functions to apply - see below. */
>>> +    enum rte_eth_hash_function func;    /**< Hash algorithm to apply. */
>>> };
>>> 
>>> /*
>>> --
>>> 2.22.0
>>> 
>>> 
>> 
>> Thank you.
>> 
>> .
>> 
>

Thank you.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 1/5] ethdev: support setting and querying rss algorithm
  2023-03-16 13:16     ` Dongdong Liu
@ 2023-06-02 20:19       ` Ferruh Yigit
  2023-06-05 12:34         ` Dongdong Liu
  0 siblings, 1 reply; 27+ messages in thread
From: Ferruh Yigit @ 2023-06-02 20:19 UTC (permalink / raw)
  To: Dongdong Liu, Thomas Monjalon, Jie Hai
  Cc: dev, andrew.rybchenko, reshma.pattan, stable, yisen.zhuang,
	david.marchand

On 3/16/2023 1:16 PM, Dongdong Liu wrote:
> Hi Thomas
> On 2023/3/15 21:43, Thomas Monjalon wrote:
>> 15/03/2023 12:00, Dongdong Liu:
>>> From: Jie Hai <haijie1@huawei.com>
>>> --- a/doc/guides/rel_notes/release_23_03.rst
>>> +++ b/doc/guides/rel_notes/release_23_03.rst
>>> -* No ABI change that would break compatibility with 22.11.
>>> -
>>> +* ethdev: Added "func" field to ``rte_eth_rss_conf`` structure for
>>> RSS hash
>>> +  algorithm.
>>
>> We cannot break ABI compatibility until 23.11.
> Got it. Thank you for reminding.
> 

Hi Dongdong,

Please remember to send a deprecation notice for this release.
Deprecation notice should be merged in this release so that it can be
applied in v23.11


> [PATCH 3/5] and [PATCH 4/5] do not relate with this ABI compatibility.
> I will send them separately.
> 
> Thanks,
> Dongdong
>>
>>
>>
>> .
>>


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 3/5] app/proc-info: fix never show RSS info
  2023-03-15 11:00 ` [PATCH 3/5] app/proc-info: fix never show RSS info Dongdong Liu
@ 2023-06-02 20:19   ` Ferruh Yigit
  2023-06-05 13:04     ` Dongdong Liu
  2023-06-02 21:19   ` Stephen Hemminger
  1 sibling, 1 reply; 27+ messages in thread
From: Ferruh Yigit @ 2023-06-02 20:19 UTC (permalink / raw)
  To: Dongdong Liu, dev, thomas, andrew.rybchenko, reshma.pattan
  Cc: stable, yisen.zhuang, Jie Hai, Maryam Tahhan, Vipin Varghese,
	John McNamara

On 3/15/2023 11:00 AM, Dongdong Liu wrote:

> From: Jie Hai <haijie1@huawei.com>
> 
> Command show-port shows rss info only if rss_conf.rss_key
> is not null but it will never be true. This patch allocates
> memory for rss_conf.rss_key and makes it possible to show
> rss info.
> 

Why 'rss_conf.rss_key == NULL' case is never true?

'rss_key' is pointer and 'rte_eth_dev_rss_hash_conf_get()' doesn't
allocate it, so can't it be NULL?


> Fixes: 8a37f37fc243 ("app/procinfo: add --show-port")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Jie Hai <haijie1@huawei.com>
> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> ---
>  app/proc-info/main.c | 26 +++++++++++++++++---------
>  1 file changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/app/proc-info/main.c b/app/proc-info/main.c
> index 53e852a07c..878ce37e8b 100644
> --- a/app/proc-info/main.c
> +++ b/app/proc-info/main.c
> @@ -823,6 +823,7 @@ show_port(void)
>                 struct rte_eth_fc_conf fc_conf;
>                 struct rte_ether_addr mac;
>                 struct rte_eth_dev_owner owner;
> +               uint8_t *rss_key;
> 
>                 /* Skip if port is not in mask */
>                 if ((enabled_port_mask & (1ul << i)) == 0)
> @@ -981,19 +982,26 @@ show_port(void)
>                         printf("\n");
>                 }
> 
> +               rss_key = rte_malloc(NULL,
> +                       dev_info.hash_key_size * sizeof(uint8_t), 0);
> +               if (rss_key == NULL)
> +                       return;
> +
> +               rss_conf.rss_key = rss_key;
> +               rss_conf.rss_key_len = dev_info.hash_key_size;
>                 ret = rte_eth_dev_rss_hash_conf_get(i, &rss_conf);
>                 if (ret == 0) {
> -                       if (rss_conf.rss_key) {
> -                               printf("  - RSS\n");
> -                               printf("\t  -- RSS len %u key (hex):",
> -                                               rss_conf.rss_key_len);
> -                               for (k = 0; k < rss_conf.rss_key_len; k++)
> -                                       printf(" %x", rss_conf.rss_key[k]);
> -                               printf("\t  -- hf 0x%"PRIx64"\n",
> -                                               rss_conf.rss_hf);
> -                       }
> +                       printf("  - RSS\n");
> +                       printf("\t  -- RSS len %u key (hex):",
> +                                       rss_conf.rss_key_len);
> +                       for (k = 0; k < rss_conf.rss_key_len; k++)
> +                               printf(" %x", rss_conf.rss_key[k]);
> +                       printf("\t  -- hf 0x%"PRIx64"\n",
> +                                       rss_conf.rss_hf);
>                 }
> 
> +               rte_free(rss_key);
> +
>  #ifdef RTE_LIB_SECURITY
>                 show_security_context(i, true);
>  #endif
> --
> 2.22.0
> 


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 4/5] app/proc-info: show RSS types with strings
  2023-03-15 11:00 ` [PATCH 4/5] app/proc-info: show RSS types with strings Dongdong Liu
@ 2023-06-02 20:22   ` Ferruh Yigit
  2023-06-05 13:12     ` Dongdong Liu
  0 siblings, 1 reply; 27+ messages in thread
From: Ferruh Yigit @ 2023-06-02 20:22 UTC (permalink / raw)
  To: Dongdong Liu, dev, thomas, andrew.rybchenko, reshma.pattan
  Cc: stable, yisen.zhuang, Jie Hai, Maryam Tahhan

On 3/15/2023 11:00 AM, Dongdong Liu wrote:
> From: Jie Hai <haijie1@huawei.com>
> 
> show RSS types details and adjust RSS info display format as following:
> 
>   - RSS info
> 	  -- hf:
> 		ipv4  ipv4-frag  ipv4-other  ipv6  ipv6-frag  ipv6-other
> 	  -- key len: 40
> 	  -- key (hex): 6d5a56da255b0ec24167253d43a38fb0d0ca2bcbae7b30b477cb2da38030f20c6a42b73bbeac01fa
> 
> Signed-off-by: Jie Hai <haijie1@huawei.com>
> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> ---
>  app/proc-info/main.c | 120 ++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 113 insertions(+), 7 deletions(-)
> 
> diff --git a/app/proc-info/main.c b/app/proc-info/main.c
> index 878ce37e8b..7b6f8f1228 100644
> --- a/app/proc-info/main.c
> +++ b/app/proc-info/main.c
> @@ -54,6 +54,9 @@
>  #define STATS_BDR_STR(w, s) printf("%.*s%s%.*s\n", w, \
>  	STATS_BDR_FMT, s, w, STATS_BDR_FMT)
>  
> +/* It is used to print the RSS types. */
> +#define RSS_TYPES_CHAR_NUM_PER_LINE 64
> +
>  /* mask of enabled ports */
>  static unsigned long enabled_port_mask;
>  /* Enable stats. */
> @@ -132,6 +135,76 @@ struct desc_param {
>  static struct desc_param rx_desc_param;
>  static struct desc_param tx_desc_param;
>  
> +/* Information for a given RSS type. */
> +struct rss_type_info {
> +	const char *str; /* Type name. */
> +	uint64_t rss_type; /* Type value. */
> +};
> +
> +static const struct rss_type_info rss_type_table[] = {
> +	/* Group types */
> +	{ "all", RTE_ETH_RSS_ETH | RTE_ETH_RSS_VLAN | RTE_ETH_RSS_IP | RTE_ETH_RSS_TCP |
> +		RTE_ETH_RSS_UDP | RTE_ETH_RSS_SCTP | RTE_ETH_RSS_L2_PAYLOAD |
> +		RTE_ETH_RSS_L2TPV3 | RTE_ETH_RSS_ESP | RTE_ETH_RSS_AH | RTE_ETH_RSS_PFCP |
> +		RTE_ETH_RSS_GTPU | RTE_ETH_RSS_ECPRI | RTE_ETH_RSS_MPLS | RTE_ETH_RSS_L2TPV2},
> +	{ "none", 0 },
> +	{ "ip", RTE_ETH_RSS_IP },
> +	{ "udp", RTE_ETH_RSS_UDP },
> +	{ "tcp", RTE_ETH_RSS_TCP },
> +	{ "sctp", RTE_ETH_RSS_SCTP },
> +	{ "tunnel", RTE_ETH_RSS_TUNNEL },
> +	{ "vlan", RTE_ETH_RSS_VLAN },
> +
> +	/* Individual type */
> +	{ "ipv4", RTE_ETH_RSS_IPV4 },
> +	{ "ipv4-frag", RTE_ETH_RSS_FRAG_IPV4 },
> +	{ "ipv4-tcp", RTE_ETH_RSS_NONFRAG_IPV4_TCP },
> +	{ "ipv4-udp", RTE_ETH_RSS_NONFRAG_IPV4_UDP },
> +	{ "ipv4-sctp", RTE_ETH_RSS_NONFRAG_IPV4_SCTP },
> +	{ "ipv4-other", RTE_ETH_RSS_NONFRAG_IPV4_OTHER },
> +	{ "ipv6", RTE_ETH_RSS_IPV6 },
> +	{ "ipv6-frag", RTE_ETH_RSS_FRAG_IPV6 },
> +	{ "ipv6-tcp", RTE_ETH_RSS_NONFRAG_IPV6_TCP },
> +	{ "ipv6-udp", RTE_ETH_RSS_NONFRAG_IPV6_UDP },
> +	{ "ipv6-sctp", RTE_ETH_RSS_NONFRAG_IPV6_SCTP },
> +	{ "ipv6-other", RTE_ETH_RSS_NONFRAG_IPV6_OTHER },
> +	{ "l2-payload", RTE_ETH_RSS_L2_PAYLOAD },
> +	{ "ipv6-ex", RTE_ETH_RSS_IPV6_EX },
> +	{ "ipv6-tcp-ex", RTE_ETH_RSS_IPV6_TCP_EX },
> +	{ "ipv6-udp-ex", RTE_ETH_RSS_IPV6_UDP_EX },
> +	{ "port", RTE_ETH_RSS_PORT },
> +	{ "vxlan", RTE_ETH_RSS_VXLAN },
> +	{ "geneve", RTE_ETH_RSS_GENEVE },
> +	{ "nvgre", RTE_ETH_RSS_NVGRE },
> +	{ "gtpu", RTE_ETH_RSS_GTPU },
> +	{ "eth", RTE_ETH_RSS_ETH },
> +	{ "s-vlan", RTE_ETH_RSS_S_VLAN },
> +	{ "c-vlan", RTE_ETH_RSS_C_VLAN },
> +	{ "esp", RTE_ETH_RSS_ESP },
> +	{ "ah", RTE_ETH_RSS_AH },
> +	{ "l2tpv3", RTE_ETH_RSS_L2TPV3 },
> +	{ "pfcp", RTE_ETH_RSS_PFCP },
> +	{ "pppoe", RTE_ETH_RSS_PPPOE },
> +	{ "ecpri", RTE_ETH_RSS_ECPRI },
> +	{ "mpls", RTE_ETH_RSS_MPLS },
> +	{ "ipv4-chksum", RTE_ETH_RSS_IPV4_CHKSUM },
> +	{ "l4-chksum", RTE_ETH_RSS_L4_CHKSUM },
> +	{ "l2tpv2", RTE_ETH_RSS_L2TPV2 },
> +	{ "l3-pre96", RTE_ETH_RSS_L3_PRE96 },
> +	{ "l3-pre64", RTE_ETH_RSS_L3_PRE64 },
> +	{ "l3-pre56", RTE_ETH_RSS_L3_PRE56 },
> +	{ "l3-pre48", RTE_ETH_RSS_L3_PRE48 },
> +	{ "l3-pre40", RTE_ETH_RSS_L3_PRE40 },
> +	{ "l3-pre32", RTE_ETH_RSS_L3_PRE32 },
> +	{ "l2-dst-only", RTE_ETH_RSS_L2_DST_ONLY },
> +	{ "l2-src-only", RTE_ETH_RSS_L2_SRC_ONLY },
> +	{ "l4-dst-only", RTE_ETH_RSS_L4_DST_ONLY },
> +	{ "l4-src-only", RTE_ETH_RSS_L4_SRC_ONLY },
> +	{ "l3-dst-only", RTE_ETH_RSS_L3_DST_ONLY },
> +	{ "l3-src-only", RTE_ETH_RSS_L3_SRC_ONLY },
> +	{ NULL, 0},
> +};
> +

Agree this makes output more user friendly, but this brings something to
maintain in the application, and I am almost sure that it will become
outdated by time as new RSS types added.

One option is to add this as an API in the library, so it will be easier
to keep up to date, and use API from here. But not sure if it worths to
have new API for this?

>  /* display usage */
>  static void
>  proc_info_usage(const char *prgname)
> @@ -806,6 +879,33 @@ show_offloads(uint64_t offloads,
>  	}
>  }
>  
> +static void
> +show_rss_types(uint64_t rss_types)
> +{
> +	uint16_t total_len = 0;
> +	uint16_t str_len;
> +	uint16_t i;
> +
> +	printf("\t\t");
> +	for (i = 0; rss_type_table[i].str != NULL; i++) {
> +		if (rss_type_table[i].rss_type == 0)
> +			continue;
> +
> +		if ((rss_type_table[i].rss_type & rss_types) ==
> +					rss_type_table[i].rss_type) {
> +			/* Contain two spaces */
> +			str_len = strlen(rss_type_table[i].str) + 2;
> +			if (total_len + str_len > RSS_TYPES_CHAR_NUM_PER_LINE) {
> +				printf("\n\t\t");
> +				total_len = 0;
> +			}
> +			printf("%s  ", rss_type_table[i].str);
> +			total_len += str_len;
> +		}
> +	}
> +	printf("\n");
> +}
> +
>  static void
>  show_port(void)
>  {
> @@ -991,13 +1091,19 @@ show_port(void)
>  		rss_conf.rss_key_len = dev_info.hash_key_size;
>  		ret = rte_eth_dev_rss_hash_conf_get(i, &rss_conf);
>  		if (ret == 0) {
> -			printf("  - RSS\n");
> -			printf("\t  -- RSS len %u key (hex):",
> -					rss_conf.rss_key_len);
> -			for (k = 0; k < rss_conf.rss_key_len; k++)
> -				printf(" %x", rss_conf.rss_key[k]);
> -			printf("\t  -- hf 0x%"PRIx64"\n",
> -					rss_conf.rss_hf);
> +			printf("  - RSS info\n");
> +			if (rss_conf.rss_hf == 0) {
> +				printf("\tRSS disabled\n");
> +			} else {
> +				printf("\t  -- hf:\n");
> +				show_rss_types(rss_conf.rss_hf);
> +				printf("\t  -- key len: %u\n",
> +						rss_conf.rss_key_len);
> +				printf("\t  -- key (hex): ");
> +				for (k = 0; k < rss_conf.rss_key_len; k++)
> +					printf("%02x", rss_conf.rss_key[k]);
> +				printf("\n");
> +			}
>  		}
>  
>  		rte_free(rss_key);


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 3/5] app/proc-info: fix never show RSS info
  2023-03-15 11:00 ` [PATCH 3/5] app/proc-info: fix never show RSS info Dongdong Liu
  2023-06-02 20:19   ` Ferruh Yigit
@ 2023-06-02 21:19   ` Stephen Hemminger
  2023-06-05 13:07     ` Dongdong Liu
  1 sibling, 1 reply; 27+ messages in thread
From: Stephen Hemminger @ 2023-06-02 21:19 UTC (permalink / raw)
  To: Dongdong Liu
  Cc: dev, ferruh.yigit, thomas, andrew.rybchenko, reshma.pattan,
	stable, yisen.zhuang, Jie Hai, Maryam Tahhan, Vipin Varghese,
	John McNamara

On Wed, 15 Mar 2023 19:00:31 +0800
Dongdong Liu <liudongdong3@huawei.com> wrote:

> +		rss_key = rte_malloc(NULL,
> +			dev_info.hash_key_size * sizeof(uint8_t), 0);
> +		if (rss_key == NULL)
> +			return;

Don't use rte_malloc() unless this is a structure that needs to be
shared between primary/secondary. In this case it doesn't need to be shared;
so just use calloc().

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 1/5] ethdev: support setting and querying rss algorithm
  2023-06-02 20:19       ` Ferruh Yigit
@ 2023-06-05 12:34         ` Dongdong Liu
  0 siblings, 0 replies; 27+ messages in thread
From: Dongdong Liu @ 2023-06-05 12:34 UTC (permalink / raw)
  To: Ferruh Yigit, Thomas Monjalon, Jie Hai
  Cc: dev, andrew.rybchenko, reshma.pattan, stable, yisen.zhuang,
	david.marchand

Hi Ferruh

On 2023/6/3 4:19, Ferruh Yigit wrote:
> On 3/16/2023 1:16 PM, Dongdong Liu wrote:
>> Hi Thomas
>> On 2023/3/15 21:43, Thomas Monjalon wrote:
>>> 15/03/2023 12:00, Dongdong Liu:
>>>> From: Jie Hai <haijie1@huawei.com>
>>>> --- a/doc/guides/rel_notes/release_23_03.rst
>>>> +++ b/doc/guides/rel_notes/release_23_03.rst
>>>> -* No ABI change that would break compatibility with 22.11.
>>>> -
>>>> +* ethdev: Added "func" field to ``rte_eth_rss_conf`` structure for
>>>> RSS hash
>>>> +  algorithm.
>>>
>>> We cannot break ABI compatibility until 23.11.
>> Got it. Thank you for reminding.
>>
>
> Hi Dongdong,
>
> Please remember to send a deprecation notice for this release.
> Deprecation notice should be merged in this release so that it can be
> applied in v23.11
Thanks for pointing that.
Will do.

Thanks,
Dongdong
>
>
>> [PATCH 3/5] and [PATCH 4/5] do not relate with this ABI compatibility.
>> I will send them separately.
>>
>> Thanks,
>> Dongdong
>>>
>>>
>>>
>>> .
>>>
>
> .
>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 3/5] app/proc-info: fix never show RSS info
  2023-06-02 20:19   ` Ferruh Yigit
@ 2023-06-05 13:04     ` Dongdong Liu
  0 siblings, 0 replies; 27+ messages in thread
From: Dongdong Liu @ 2023-06-05 13:04 UTC (permalink / raw)
  To: Ferruh Yigit, dev, thomas, andrew.rybchenko, reshma.pattan
  Cc: stable, yisen.zhuang, Jie Hai, Maryam Tahhan, Vipin Varghese,
	John McNamara

Hi Ferruh

On 2023/6/3 4:19, Ferruh Yigit wrote:
> On 3/15/2023 11:00 AM, Dongdong Liu wrote:
>
>> From: Jie Hai <haijie1@huawei.com>
>>
>> Command show-port shows rss info only if rss_conf.rss_key
>> is not null but it will never be true. This patch allocates
>> memory for rss_conf.rss_key and makes it possible to show
>> rss info.
>>
>
> Why 'rss_conf.rss_key == NULL' case is never true?
>
> 'rss_key' is pointer and 'rte_eth_dev_rss_hash_conf_get()' doesn't
> allocate it, so can't it be NULL?

The code want to show rss info (rss_key, len and rss_hf),
but it does not allocate memory for rss_conf.rss_key,
so current code rss_conf.rss_key will be always NULL,
This patch fixes that.

Maybe the description of commit message is not correct,
will fix that.

Thanks,
Dongdong
>
>
>> Fixes: 8a37f37fc243 ("app/procinfo: add --show-port")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Jie Hai <haijie1@huawei.com>
>> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
>> ---
>>  app/proc-info/main.c | 26 +++++++++++++++++---------
>>  1 file changed, 17 insertions(+), 9 deletions(-)
>>
>> diff --git a/app/proc-info/main.c b/app/proc-info/main.c
>> index 53e852a07c..878ce37e8b 100644
>> --- a/app/proc-info/main.c
>> +++ b/app/proc-info/main.c
>> @@ -823,6 +823,7 @@ show_port(void)
>>                 struct rte_eth_fc_conf fc_conf;
>>                 struct rte_ether_addr mac;
>>                 struct rte_eth_dev_owner owner;
>> +               uint8_t *rss_key;
>>
>>                 /* Skip if port is not in mask */
>>                 if ((enabled_port_mask & (1ul << i)) == 0)
>> @@ -981,19 +982,26 @@ show_port(void)
>>                         printf("\n");
>>                 }
>>
>> +               rss_key = rte_malloc(NULL,
>> +                       dev_info.hash_key_size * sizeof(uint8_t), 0);
>> +               if (rss_key == NULL)
>> +                       return;
>> +
>> +               rss_conf.rss_key = rss_key;
>> +               rss_conf.rss_key_len = dev_info.hash_key_size;
>>                 ret = rte_eth_dev_rss_hash_conf_get(i, &rss_conf);
>>                 if (ret == 0) {
>> -                       if (rss_conf.rss_key) {
>> -                               printf("  - RSS\n");
>> -                               printf("\t  -- RSS len %u key (hex):",
>> -                                               rss_conf.rss_key_len);
>> -                               for (k = 0; k < rss_conf.rss_key_len; k++)
>> -                                       printf(" %x", rss_conf.rss_key[k]);
>> -                               printf("\t  -- hf 0x%"PRIx64"\n",
>> -                                               rss_conf.rss_hf);
>> -                       }
>> +                       printf("  - RSS\n");
>> +                       printf("\t  -- RSS len %u key (hex):",
>> +                                       rss_conf.rss_key_len);
>> +                       for (k = 0; k < rss_conf.rss_key_len; k++)
>> +                               printf(" %x", rss_conf.rss_key[k]);
>> +                       printf("\t  -- hf 0x%"PRIx64"\n",
>> +                                       rss_conf.rss_hf);
>>                 }
>>
>> +               rte_free(rss_key);
>> +
>>  #ifdef RTE_LIB_SECURITY
>>                 show_security_context(i, true);
>>  #endif
>> --
>> 2.22.0
>>
>
> .
>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 3/5] app/proc-info: fix never show RSS info
  2023-06-02 21:19   ` Stephen Hemminger
@ 2023-06-05 13:07     ` Dongdong Liu
  0 siblings, 0 replies; 27+ messages in thread
From: Dongdong Liu @ 2023-06-05 13:07 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: dev, ferruh.yigit, thomas, andrew.rybchenko, reshma.pattan,
	stable, yisen.zhuang, Jie Hai, Maryam Tahhan, Vipin Varghese,
	John McNamara


Hi Stephen
On 2023/6/3 5:19, Stephen Hemminger wrote:
> On Wed, 15 Mar 2023 19:00:31 +0800
> Dongdong Liu <liudongdong3@huawei.com> wrote:
>
>> +		rss_key = rte_malloc(NULL,
>> +			dev_info.hash_key_size * sizeof(uint8_t), 0);
>> +		if (rss_key == NULL)
>> +			return;
>
> Don't use rte_malloc() unless this is a structure that needs to be
> shared between primary/secondary. In this case it doesn't need to be shared;
> so just use calloc().
Thanks for pointing that,
Will do.

Thanks,
Dongdong
> .
>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH 4/5] app/proc-info: show RSS types with strings
  2023-06-02 20:22   ` Ferruh Yigit
@ 2023-06-05 13:12     ` Dongdong Liu
  0 siblings, 0 replies; 27+ messages in thread
From: Dongdong Liu @ 2023-06-05 13:12 UTC (permalink / raw)
  To: Ferruh Yigit, dev, thomas, andrew.rybchenko, reshma.pattan
  Cc: stable, yisen.zhuang, Jie Hai, Maryam Tahhan


Hi Ferruh
On 2023/6/3 4:22, Ferruh Yigit wrote:
> On 3/15/2023 11:00 AM, Dongdong Liu wrote:
>> From: Jie Hai <haijie1@huawei.com>
>>
>> show RSS types details and adjust RSS info display format as following:
>>
>>   - RSS info
>> 	  -- hf:
>> 		ipv4  ipv4-frag  ipv4-other  ipv6  ipv6-frag  ipv6-other
>> 	  -- key len: 40
>> 	  -- key (hex): 6d5a56da255b0ec24167253d43a38fb0d0ca2bcbae7b30b477cb2da38030f20c6a42b73bbeac01fa
>>
>> Signed-off-by: Jie Hai <haijie1@huawei.com>
>> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
>> ---
>>  app/proc-info/main.c | 120 ++++++++++++++++++++++++++++++++++++++++---
>>  1 file changed, 113 insertions(+), 7 deletions(-)
>>
>> diff --git a/app/proc-info/main.c b/app/proc-info/main.c
>> index 878ce37e8b..7b6f8f1228 100644
>> --- a/app/proc-info/main.c
>> +++ b/app/proc-info/main.c
>> @@ -54,6 +54,9 @@
>>  #define STATS_BDR_STR(w, s) printf("%.*s%s%.*s\n", w, \
>>  	STATS_BDR_FMT, s, w, STATS_BDR_FMT)
>>
>> +/* It is used to print the RSS types. */
>> +#define RSS_TYPES_CHAR_NUM_PER_LINE 64
>> +
>>  /* mask of enabled ports */
>>  static unsigned long enabled_port_mask;
>>  /* Enable stats. */
>> @@ -132,6 +135,76 @@ struct desc_param {
>>  static struct desc_param rx_desc_param;
>>  static struct desc_param tx_desc_param;
>>
>> +/* Information for a given RSS type. */
>> +struct rss_type_info {
>> +	const char *str; /* Type name. */
>> +	uint64_t rss_type; /* Type value. */
>> +};
>> +
>> +static const struct rss_type_info rss_type_table[] = {
>> +	/* Group types */
>> +	{ "all", RTE_ETH_RSS_ETH | RTE_ETH_RSS_VLAN | RTE_ETH_RSS_IP | RTE_ETH_RSS_TCP |
>> +		RTE_ETH_RSS_UDP | RTE_ETH_RSS_SCTP | RTE_ETH_RSS_L2_PAYLOAD |
>> +		RTE_ETH_RSS_L2TPV3 | RTE_ETH_RSS_ESP | RTE_ETH_RSS_AH | RTE_ETH_RSS_PFCP |
>> +		RTE_ETH_RSS_GTPU | RTE_ETH_RSS_ECPRI | RTE_ETH_RSS_MPLS | RTE_ETH_RSS_L2TPV2},
>> +	{ "none", 0 },
>> +	{ "ip", RTE_ETH_RSS_IP },
>> +	{ "udp", RTE_ETH_RSS_UDP },
>> +	{ "tcp", RTE_ETH_RSS_TCP },
>> +	{ "sctp", RTE_ETH_RSS_SCTP },
>> +	{ "tunnel", RTE_ETH_RSS_TUNNEL },
>> +	{ "vlan", RTE_ETH_RSS_VLAN },
>> +
>> +	/* Individual type */
>> +	{ "ipv4", RTE_ETH_RSS_IPV4 },
>> +	{ "ipv4-frag", RTE_ETH_RSS_FRAG_IPV4 },
>> +	{ "ipv4-tcp", RTE_ETH_RSS_NONFRAG_IPV4_TCP },
>> +	{ "ipv4-udp", RTE_ETH_RSS_NONFRAG_IPV4_UDP },
>> +	{ "ipv4-sctp", RTE_ETH_RSS_NONFRAG_IPV4_SCTP },
>> +	{ "ipv4-other", RTE_ETH_RSS_NONFRAG_IPV4_OTHER },
>> +	{ "ipv6", RTE_ETH_RSS_IPV6 },
>> +	{ "ipv6-frag", RTE_ETH_RSS_FRAG_IPV6 },
>> +	{ "ipv6-tcp", RTE_ETH_RSS_NONFRAG_IPV6_TCP },
>> +	{ "ipv6-udp", RTE_ETH_RSS_NONFRAG_IPV6_UDP },
>> +	{ "ipv6-sctp", RTE_ETH_RSS_NONFRAG_IPV6_SCTP },
>> +	{ "ipv6-other", RTE_ETH_RSS_NONFRAG_IPV6_OTHER },
>> +	{ "l2-payload", RTE_ETH_RSS_L2_PAYLOAD },
>> +	{ "ipv6-ex", RTE_ETH_RSS_IPV6_EX },
>> +	{ "ipv6-tcp-ex", RTE_ETH_RSS_IPV6_TCP_EX },
>> +	{ "ipv6-udp-ex", RTE_ETH_RSS_IPV6_UDP_EX },
>> +	{ "port", RTE_ETH_RSS_PORT },
>> +	{ "vxlan", RTE_ETH_RSS_VXLAN },
>> +	{ "geneve", RTE_ETH_RSS_GENEVE },
>> +	{ "nvgre", RTE_ETH_RSS_NVGRE },
>> +	{ "gtpu", RTE_ETH_RSS_GTPU },
>> +	{ "eth", RTE_ETH_RSS_ETH },
>> +	{ "s-vlan", RTE_ETH_RSS_S_VLAN },
>> +	{ "c-vlan", RTE_ETH_RSS_C_VLAN },
>> +	{ "esp", RTE_ETH_RSS_ESP },
>> +	{ "ah", RTE_ETH_RSS_AH },
>> +	{ "l2tpv3", RTE_ETH_RSS_L2TPV3 },
>> +	{ "pfcp", RTE_ETH_RSS_PFCP },
>> +	{ "pppoe", RTE_ETH_RSS_PPPOE },
>> +	{ "ecpri", RTE_ETH_RSS_ECPRI },
>> +	{ "mpls", RTE_ETH_RSS_MPLS },
>> +	{ "ipv4-chksum", RTE_ETH_RSS_IPV4_CHKSUM },
>> +	{ "l4-chksum", RTE_ETH_RSS_L4_CHKSUM },
>> +	{ "l2tpv2", RTE_ETH_RSS_L2TPV2 },
>> +	{ "l3-pre96", RTE_ETH_RSS_L3_PRE96 },
>> +	{ "l3-pre64", RTE_ETH_RSS_L3_PRE64 },
>> +	{ "l3-pre56", RTE_ETH_RSS_L3_PRE56 },
>> +	{ "l3-pre48", RTE_ETH_RSS_L3_PRE48 },
>> +	{ "l3-pre40", RTE_ETH_RSS_L3_PRE40 },
>> +	{ "l3-pre32", RTE_ETH_RSS_L3_PRE32 },
>> +	{ "l2-dst-only", RTE_ETH_RSS_L2_DST_ONLY },
>> +	{ "l2-src-only", RTE_ETH_RSS_L2_SRC_ONLY },
>> +	{ "l4-dst-only", RTE_ETH_RSS_L4_DST_ONLY },
>> +	{ "l4-src-only", RTE_ETH_RSS_L4_SRC_ONLY },
>> +	{ "l3-dst-only", RTE_ETH_RSS_L3_DST_ONLY },
>> +	{ "l3-src-only", RTE_ETH_RSS_L3_SRC_ONLY },
>> +	{ NULL, 0},
>> +};
>> +
>
> Agree this makes output more user friendly, but this brings something to
> maintain in the application, and I am almost sure that it will become
> outdated by time as new RSS types added.
That's true.
>
> One option is to add this as an API in the library, so it will be easier
> to keep up to date, and use API from here. But not sure if it worths to
> have new API for this?
I will consider that.

Thanks,
Dongdong.
>
>>  /* display usage */
>>  static void
>>  proc_info_usage(const char *prgname)
>> @@ -806,6 +879,33 @@ show_offloads(uint64_t offloads,
>>  	}
>>  }
>>
>> +static void
>> +show_rss_types(uint64_t rss_types)
>> +{
>> +	uint16_t total_len = 0;
>> +	uint16_t str_len;
>> +	uint16_t i;
>> +
>> +	printf("\t\t");
>> +	for (i = 0; rss_type_table[i].str != NULL; i++) {
>> +		if (rss_type_table[i].rss_type == 0)
>> +			continue;
>> +
>> +		if ((rss_type_table[i].rss_type & rss_types) ==
>> +					rss_type_table[i].rss_type) {
>> +			/* Contain two spaces */
>> +			str_len = strlen(rss_type_table[i].str) + 2;
>> +			if (total_len + str_len > RSS_TYPES_CHAR_NUM_PER_LINE) {
>> +				printf("\n\t\t");
>> +				total_len = 0;
>> +			}
>> +			printf("%s  ", rss_type_table[i].str);
>> +			total_len += str_len;
>> +		}
>> +	}
>> +	printf("\n");
>> +}
>> +
>>  static void
>>  show_port(void)
>>  {
>> @@ -991,13 +1091,19 @@ show_port(void)
>>  		rss_conf.rss_key_len = dev_info.hash_key_size;
>>  		ret = rte_eth_dev_rss_hash_conf_get(i, &rss_conf);
>>  		if (ret == 0) {
>> -			printf("  - RSS\n");
>> -			printf("\t  -- RSS len %u key (hex):",
>> -					rss_conf.rss_key_len);
>> -			for (k = 0; k < rss_conf.rss_key_len; k++)
>> -				printf(" %x", rss_conf.rss_key[k]);
>> -			printf("\t  -- hf 0x%"PRIx64"\n",
>> -					rss_conf.rss_hf);
>> +			printf("  - RSS info\n");
>> +			if (rss_conf.rss_hf == 0) {
>> +				printf("\tRSS disabled\n");
>> +			} else {
>> +				printf("\t  -- hf:\n");
>> +				show_rss_types(rss_conf.rss_hf);
>> +				printf("\t  -- key len: %u\n",
>> +						rss_conf.rss_key_len);
>> +				printf("\t  -- key (hex): ");
>> +				for (k = 0; k < rss_conf.rss_key_len; k++)
>> +					printf("%02x", rss_conf.rss_key[k]);
>> +				printf("\n");
>> +			}
>>  		}
>>
>>  		rte_free(rss_key);
>
> .
>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v2 0/5] support setting and querying RSS algorithms
  2023-03-15 11:00 [PATCH 0/5] support setting and querying RSS algorithms Dongdong Liu
                   ` (4 preceding siblings ...)
  2023-03-15 11:00 ` [PATCH 5/5] app/proc-info: support querying RSS hash algorithm Dongdong Liu
@ 2023-08-26  7:00 ` Jie Hai
  2023-08-26  7:00   ` [PATCH v2 1/5] ethdev: support setting and querying rss algorithm Jie Hai
                     ` (5 more replies)
  5 siblings, 6 replies; 27+ messages in thread
From: Jie Hai @ 2023-08-26  7:00 UTC (permalink / raw)
  To: ferruh.yigit, thomas, andrew.rybchenko, reshma.pattan
  Cc: liudongdong3, haijie1, yisen.zhuang, stable

This patchset is to support setting and querying RSS algorithms.

--
v2:
1. return error if "func" is invalid.
2. modify the comments of the "func" field.
3. modify commit log of patch [3/5].
4. use malloc instead of rte_malloc.
5. adjust display format of RSS info.
6. remove the string display of rss_hf.
--

Huisong Li (1):
  net/hns3: support setting and querying RSS hash function

Jie Hai (4):
  ethdev: support setting and querying rss algorithm
  app/proc-info: fix never show RSS info
  app/proc-info: adjust the display format of RSS info
  app/proc-info: support querying RSS hash algorithm

 app/proc-info/main.c                   | 45 +++++++++++++++++++-----
 doc/guides/rel_notes/release_23_11.rst |  2 ++
 drivers/net/hns3/hns3_rss.c            | 47 +++++++++++++++-----------
 lib/ethdev/rte_ethdev.c                | 17 ++++++++++
 lib/ethdev/rte_ethdev.h                |  6 ++++
 5 files changed, 88 insertions(+), 29 deletions(-)

-- 
2.33.0


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v2 1/5] ethdev: support setting and querying rss algorithm
  2023-08-26  7:00 ` [PATCH v2 0/5] support setting and querying RSS algorithms Jie Hai
@ 2023-08-26  7:00   ` Jie Hai
  2023-08-30 11:41     ` Thomas Monjalon
  2023-08-26  7:00   ` [PATCH v2 2/5] net/hns3: support setting and querying RSS hash function Jie Hai
                     ` (4 subsequent siblings)
  5 siblings, 1 reply; 27+ messages in thread
From: Jie Hai @ 2023-08-26  7:00 UTC (permalink / raw)
  To: ferruh.yigit, thomas, andrew.rybchenko, reshma.pattan
  Cc: liudongdong3, haijie1, yisen.zhuang, stable

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_rss_conf`` is extended by adding a new
field "func". This represents the RSS algorithms to apply. The
following API is affected:
	- rte_eth_dev_configure
	- rte_eth_dev_rss_hash_update
	- rte_eth_dev_rss_hash_conf_get

If the value of "func" used for configuration is a gibberish
value, report the error and return. Do the same for
rte_eth_dev_rss_hash_update and rte_eth_dev_configure.

To check whether the drivers report the "func" field, it is set
to default value before querying.

Signed-off-by: Jie Hai <haijie1@huawei.com>
Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
---
 doc/guides/rel_notes/release_23_11.rst |  2 ++
 lib/ethdev/rte_ethdev.c                | 17 +++++++++++++++++
 lib/ethdev/rte_ethdev.h                |  6 ++++++
 3 files changed, 25 insertions(+)

diff --git a/doc/guides/rel_notes/release_23_11.rst b/doc/guides/rel_notes/release_23_11.rst
index 4411bb32c195..3746436e8bc9 100644
--- a/doc/guides/rel_notes/release_23_11.rst
+++ b/doc/guides/rel_notes/release_23_11.rst
@@ -123,6 +123,8 @@ ABI Changes
    Also, make sure to start the actual text at the margin.
    =======================================================
 
+   * ethdev: Added "func" 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 0840d2b5942a..4cbcdb344cac 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -1445,6 +1445,14 @@ 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.func >= RTE_ETH_HASH_FUNCTION_MAX) {
+		RTE_ETHDEV_LOG(ERR,
+			"Ethdev port_id=%u invalid rss hash function (%u)\n",
+			port_id, dev_conf->rx_adv_conf.rss_conf.func);
+		ret = -EINVAL;
+		goto rollback;
+	}
+
 	/* Check if Rx RSS distribution is disabled but RSS hash is enabled. */
 	if (((dev_conf->rxmode.mq_mode & RTE_ETH_MQ_RX_RSS_FLAG) == 0) &&
 	    (dev_conf->rxmode.offloads & RTE_ETH_RX_OFFLOAD_RSS_HASH)) {
@@ -4630,6 +4638,13 @@ rte_eth_dev_rss_hash_update(uint16_t port_id,
 		return -ENOTSUP;
 	}
 
+	if (rss_conf->func >= RTE_ETH_HASH_FUNCTION_MAX) {
+		RTE_ETHDEV_LOG(ERR,
+			"Ethdev port_id=%u invalid rss hash function (%u)\n",
+			port_id, rss_conf->func);
+		return -EINVAL;
+	}
+
 	if (*dev->dev_ops->rss_hash_update == NULL)
 		return -ENOTSUP;
 	ret = eth_err(port_id, (*dev->dev_ops->rss_hash_update)(dev,
@@ -4657,6 +4672,8 @@ rte_eth_dev_rss_hash_conf_get(uint16_t port_id,
 		return -EINVAL;
 	}
 
+	rss_conf->func = 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 04a2564f222a..1bb5f23059ca 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -174,6 +174,7 @@ extern "C" {
 
 #include "rte_ethdev_trace_fp.h"
 #include "rte_dev_info.h"
+#include "rte_flow.h"
 
 extern int rte_eth_dev_logtype;
 
@@ -461,11 +462,16 @@ struct rte_vlan_filter_conf {
  * 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.
+ *
+ * The *func* field of the *rss_conf* structure indicates the hash algorithm
+ * applied by the RSS hashing. Passing RTE_ETH_HASH_FUNCTION_DEFAULT allows
+ * the PMD to use its best-effort algorithm rather than a specific one.
  */
 struct rte_eth_rss_conf {
 	uint8_t *rss_key;    /**< If not NULL, 40-byte hash key. */
 	uint8_t rss_key_len; /**< hash key length in bytes. */
 	uint64_t rss_hf;     /**< Hash functions to apply - see below. */
+	enum rte_eth_hash_function func;	/**< Hash algorithm to apply. */
 };
 
 /*
-- 
2.33.0


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v2 2/5] net/hns3: support setting and querying RSS hash function
  2023-08-26  7:00 ` [PATCH v2 0/5] support setting and querying RSS algorithms Jie Hai
  2023-08-26  7:00   ` [PATCH v2 1/5] ethdev: support setting and querying rss algorithm Jie Hai
@ 2023-08-26  7:00   ` Jie Hai
  2023-08-26  7:00   ` [PATCH v2 3/5] app/proc-info: fix never show RSS info Jie Hai
                     ` (3 subsequent siblings)
  5 siblings, 0 replies; 27+ messages in thread
From: Jie Hai @ 2023-08-26  7:00 UTC (permalink / raw)
  To: ferruh.yigit, thomas, andrew.rybchenko, reshma.pattan,
	Dongdong Liu, Yisen Zhuang
  Cc: haijie1, stable

From: Huisong Li <lihuisong@huawei.com>

Support setting and querying RSS hash function by ethdev ops.

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
---
 drivers/net/hns3/hns3_rss.c | 47 +++++++++++++++++++++----------------
 1 file changed, 27 insertions(+), 20 deletions(-)

diff --git a/drivers/net/hns3/hns3_rss.c b/drivers/net/hns3/hns3_rss.c
index 6126512bd780..c8346d43d15c 100644
--- a/drivers/net/hns3/hns3_rss.c
+++ b/drivers/net/hns3/hns3_rss.c
@@ -646,14 +646,14 @@ hns3_dev_rss_hash_update(struct rte_eth_dev *dev,
 	if (ret)
 		goto set_tuple_fail;
 
-	if (key) {
-		ret = hns3_rss_set_algo_key(hw, hw->rss_info.hash_algo,
-					    key, hw->rss_key_size);
-		if (ret)
-			goto set_algo_key_fail;
-		/* Update the shadow RSS key with user specified */
+	ret = hns3_update_rss_algo_key(hw, rss_conf->func, key, key_len);
+	if (ret != 0)
+		goto set_algo_key_fail;
+
+	if (rss_conf->func != RTE_ETH_HASH_FUNCTION_DEFAULT)
+		hw->rss_info.hash_algo = hns3_hash_func_map[rss_conf->func];
+	if (key != NULL)
 		memcpy(hw->rss_info.key, key, hw->rss_key_size);
-	}
 	hw->rss_info.rss_hf = rss_hf;
 	rte_spinlock_unlock(&hw->lock);
 
@@ -769,7 +769,13 @@ int
 hns3_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
 			   struct rte_eth_rss_conf *rss_conf)
 {
+	const uint8_t hash_func_map[] = {
+		[HNS3_RSS_HASH_ALGO_TOEPLITZ] = RTE_ETH_HASH_FUNCTION_TOEPLITZ,
+		[HNS3_RSS_HASH_ALGO_SIMPLE] = RTE_ETH_HASH_FUNCTION_SIMPLE_XOR,
+		[HNS3_RSS_HASH_ALGO_SYMMETRIC_TOEP] = RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ,
+	};
 	struct hns3_adapter *hns = dev->data->dev_private;
+	uint8_t rss_key[HNS3_RSS_KEY_SIZE_MAX] = {0};
 	struct hns3_hw *hw = &hns->hw;
 	uint8_t hash_algo;
 	int ret;
@@ -777,26 +783,27 @@ hns3_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
 	rte_spinlock_lock(&hw->lock);
 	ret = hns3_rss_hash_get_rss_hf(hw, &rss_conf->rss_hf);
 	if (ret != 0) {
+		rte_spinlock_unlock(&hw->lock);
 		hns3_err(hw, "obtain hash tuples failed, ret = %d", ret);
-		goto out;
+		return ret;
+	}
+
+	ret = hns3_rss_get_algo_key(hw, &hash_algo, rss_key, hw->rss_key_size);
+	if (ret != 0) {
+		rte_spinlock_unlock(&hw->lock);
+		hns3_err(hw, "obtain hash algo and key failed, ret = %d", ret);
+		return ret;
 	}
+	rte_spinlock_unlock(&hw->lock);
 
-	/* Get the RSS Key required by the user */
+	/* Get the RSS Key if user required. */
 	if (rss_conf->rss_key && rss_conf->rss_key_len >= hw->rss_key_size) {
-		ret = hns3_rss_get_algo_key(hw, &hash_algo, rss_conf->rss_key,
-					    hw->rss_key_size);
-		if (ret != 0) {
-			hns3_err(hw, "obtain hash algo and key failed, ret = %d",
-				 ret);
-			goto out;
-		}
+		memcpy(rss_conf->rss_key, rss_key, hw->rss_key_size);
 		rss_conf->rss_key_len = hw->rss_key_size;
 	}
+	rss_conf->func = hash_func_map[hash_algo];
 
-out:
-	rte_spinlock_unlock(&hw->lock);
-
-	return ret;
+	return 0;
 }
 
 /*
-- 
2.33.0


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v2 3/5] app/proc-info: fix never show RSS info
  2023-08-26  7:00 ` [PATCH v2 0/5] support setting and querying RSS algorithms Jie Hai
  2023-08-26  7:00   ` [PATCH v2 1/5] ethdev: support setting and querying rss algorithm Jie Hai
  2023-08-26  7:00   ` [PATCH v2 2/5] net/hns3: support setting and querying RSS hash function Jie Hai
@ 2023-08-26  7:00   ` Jie Hai
  2023-08-26  7:00   ` [PATCH v2 4/5] app/proc-info: adjust the display format of " Jie Hai
                     ` (2 subsequent siblings)
  5 siblings, 0 replies; 27+ messages in thread
From: Jie Hai @ 2023-08-26  7:00 UTC (permalink / raw)
  To: ferruh.yigit, thomas, andrew.rybchenko, reshma.pattan,
	John McNamara, Vipin Varghese
  Cc: liudongdong3, haijie1, yisen.zhuang, stable

Command show-port should show rss info (rss_key, len and rss_hf),
However, the information is showned only when rss_conf.rss_key is not
NULL. Since no memory is allocated for rss_conf.rss_key, rss_key
will always be NULL and the rss_info will never show. This patch
allocates memory for rss_conf.rss_key and makes it possible to show
rss info.

Fixes: 8a37f37fc243 ("app/procinfo: add --show-port")
Cc: stable@dpdk.org

Signed-off-by: Jie Hai <haijie1@huawei.com>
Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
---
 app/proc-info/main.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index 88cee0ca487b..f6b77a705dce 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -1013,6 +1013,7 @@ show_port(void)
 		struct rte_eth_fc_conf fc_conf;
 		struct rte_ether_addr mac;
 		struct rte_eth_dev_owner owner;
+		uint8_t *rss_key;
 
 		/* Skip if port is not in mask */
 		if ((enabled_port_mask & (1ul << i)) == 0)
@@ -1171,19 +1172,25 @@ show_port(void)
 			printf("\n");
 		}
 
+		rss_key = malloc(dev_info.hash_key_size * sizeof(uint8_t));
+		if (rss_key == NULL)
+			return;
+
+		rss_conf.rss_key = rss_key;
+		rss_conf.rss_key_len = dev_info.hash_key_size;
 		ret = rte_eth_dev_rss_hash_conf_get(i, &rss_conf);
 		if (ret == 0) {
-			if (rss_conf.rss_key) {
-				printf("  - RSS\n");
-				printf("\t  -- RSS len %u key (hex):",
-						rss_conf.rss_key_len);
-				for (k = 0; k < rss_conf.rss_key_len; k++)
-					printf(" %x", rss_conf.rss_key[k]);
-				printf("\t  -- hf 0x%"PRIx64"\n",
-						rss_conf.rss_hf);
-			}
+			printf("  - RSS\n");
+			printf("\t  -- RSS len %u key (hex):",
+					rss_conf.rss_key_len);
+			for (k = 0; k < rss_conf.rss_key_len; k++)
+				printf(" %x", rss_conf.rss_key[k]);
+			printf("\t  -- hf 0x%"PRIx64"\n",
+					rss_conf.rss_hf);
 		}
 
+		free(rss_key);
+
 #ifdef RTE_LIB_SECURITY
 		show_security_context(i, true);
 #endif
-- 
2.33.0


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v2 4/5] app/proc-info: adjust the display format of RSS info
  2023-08-26  7:00 ` [PATCH v2 0/5] support setting and querying RSS algorithms Jie Hai
                     ` (2 preceding siblings ...)
  2023-08-26  7:00   ` [PATCH v2 3/5] app/proc-info: fix never show RSS info Jie Hai
@ 2023-08-26  7:00   ` Jie Hai
  2023-08-26  7:00   ` [PATCH v2 5/5] app/proc-info: support querying RSS hash algorithm Jie Hai
  2023-08-26  8:52   ` [PATCH v2 0/5] support setting and querying RSS algorithms Jie Hai
  5 siblings, 0 replies; 27+ messages in thread
From: Jie Hai @ 2023-08-26  7:00 UTC (permalink / raw)
  To: ferruh.yigit, thomas, andrew.rybchenko, reshma.pattan,
	Vipin Varghese, John McNamara
  Cc: liudongdong3, haijie1, yisen.zhuang, stable

This patch splits the length and value of RSS key into two parts,
removes spaces between RSS keys, and adds line breaks between RSS
key and RSS hf.

Before the adjustment, RSS info is shown as:
  - RSS
	  -- RSS len 40 key (hex): 6d 5a 56 da 25 5b e c2 41 67 \
	     25 3d 43 a3 8f b0 d0 ca 2b cb ae 7b 30 b4 77 cb 2d \
	     a3 80 30 f2 c 6a 42 b7 3b be ac 1 fa -- hf 0x0
and after:
  - RSS info
	  -- RSS key len : 40
	  -- RSS key (hex) :
		6d5a56da255b0ec24167253d43a38fb0d0ca2bcbae7b30b4 \
		77cb2da38030f20c6a42b73bbeac01fa
	  -- RSS hf : 0x0

Fixes: 8a37f37fc243 ("app/procinfo: add --show-port")
Cc: stable@dpdk.org

Signed-off-by: Jie Hai <haijie1@huawei.com>
---
 app/proc-info/main.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index f6b77a705dce..6d2d77fea6ba 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -1180,12 +1180,13 @@ show_port(void)
 		rss_conf.rss_key_len = dev_info.hash_key_size;
 		ret = rte_eth_dev_rss_hash_conf_get(i, &rss_conf);
 		if (ret == 0) {
-			printf("  - RSS\n");
-			printf("\t  -- RSS len %u key (hex):",
+			printf("  - RSS info\n");
+			printf("\t  -- key len : %u\n",
 					rss_conf.rss_key_len);
+			printf("\t  -- key (hex) : ");
 			for (k = 0; k < rss_conf.rss_key_len; k++)
-				printf(" %x", rss_conf.rss_key[k]);
-			printf("\t  -- hf 0x%"PRIx64"\n",
+				printf("%02x", rss_conf.rss_key[k]);
+			printf("\n\t  -- hf : 0x%"PRIx64"\n",
 					rss_conf.rss_hf);
 		}
 
-- 
2.33.0


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v2 5/5] app/proc-info: support querying RSS hash algorithm
  2023-08-26  7:00 ` [PATCH v2 0/5] support setting and querying RSS algorithms Jie Hai
                     ` (3 preceding siblings ...)
  2023-08-26  7:00   ` [PATCH v2 4/5] app/proc-info: adjust the display format of " Jie Hai
@ 2023-08-26  7:00   ` Jie Hai
  2023-08-26  8:52   ` [PATCH v2 0/5] support setting and querying RSS algorithms Jie Hai
  5 siblings, 0 replies; 27+ messages in thread
From: Jie Hai @ 2023-08-26  7:00 UTC (permalink / raw)
  To: ferruh.yigit, thomas, andrew.rybchenko, reshma.pattan
  Cc: liudongdong3, haijie1, yisen.zhuang, stable

Display RSS hash algorithm with command show-port as below.
  - RSS info
	  -- hash algorithm : toeplitz

Signed-off-by: Jie Hai <haijie1@huawei.com>
Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
---
 app/proc-info/main.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index 6d2d77fea6ba..02b418a4c661 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -996,6 +996,23 @@ show_offloads(uint64_t offloads,
 	}
 }
 
+static const char *
+rss_func_to_str(enum rte_eth_hash_function func)
+{
+	switch (func) {
+	case RTE_ETH_HASH_FUNCTION_SIMPLE_XOR:
+		return "simple_xor";
+	case RTE_ETH_HASH_FUNCTION_TOEPLITZ:
+		return "toeplitz";
+	case RTE_ETH_HASH_FUNCTION_SYMMETRIC_TOEPLITZ:
+		return "symmetric_toeplitz";
+	case RTE_ETH_HASH_FUNCTION_DEFAULT:
+		return "default";
+	default:
+		return "unknown";
+	}
+}
+
 static void
 show_port(void)
 {
@@ -1188,6 +1205,8 @@ show_port(void)
 				printf("%02x", rss_conf.rss_key[k]);
 			printf("\n\t  -- hf : 0x%"PRIx64"\n",
 					rss_conf.rss_hf);
+			printf("\t  -- hash algorithm : %s\n",
+				rss_func_to_str(rss_conf.func));
 		}
 
 		free(rss_key);
-- 
2.33.0


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 0/5] support setting and querying RSS algorithms
  2023-08-26  7:00 ` [PATCH v2 0/5] support setting and querying RSS algorithms Jie Hai
                     ` (4 preceding siblings ...)
  2023-08-26  7:00   ` [PATCH v2 5/5] app/proc-info: support querying RSS hash algorithm Jie Hai
@ 2023-08-26  8:52   ` Jie Hai
  5 siblings, 0 replies; 27+ messages in thread
From: Jie Hai @ 2023-08-26  8:52 UTC (permalink / raw)
  To: ferruh.yigit, thomas, andrew.rybchenko, reshma.pattan
  Cc: liudongdong3, yisen.zhuang, stable

Hi, all,

Sorry, I misoperated this group of patches,
they should be sent to dev@dpdk.org instead of stable@dpdk.org.
Please see new emails:

https://inbox.dpdk.org/dev/20230826074607.16771-1-haijie1@huawei.com/

Thanks,
Jie Hai

On 2023/8/26 15:00, Jie Hai wrote:
> This patchset is to support setting and querying RSS algorithms.
> 
> --
> v2:
> 1. return error if "func" is invalid.
> 2. modify the comments of the "func" field.
> 3. modify commit log of patch [3/5].
> 4. use malloc instead of rte_malloc.
> 5. adjust display format of RSS info.
> 6. remove the string display of rss_hf.
> --
> 
> Huisong Li (1):
>    net/hns3: support setting and querying RSS hash function
> 
> Jie Hai (4):
>    ethdev: support setting and querying rss algorithm
>    app/proc-info: fix never show RSS info
>    app/proc-info: adjust the display format of RSS info
>    app/proc-info: support querying RSS hash algorithm
> 
>   app/proc-info/main.c                   | 45 +++++++++++++++++++-----
>   doc/guides/rel_notes/release_23_11.rst |  2 ++
>   drivers/net/hns3/hns3_rss.c            | 47 +++++++++++++++-----------
>   lib/ethdev/rte_ethdev.c                | 17 ++++++++++
>   lib/ethdev/rte_ethdev.h                |  6 ++++
>   5 files changed, 88 insertions(+), 29 deletions(-)
> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 1/5] ethdev: support setting and querying rss algorithm
  2023-08-26  7:00   ` [PATCH v2 1/5] ethdev: support setting and querying rss algorithm Jie Hai
@ 2023-08-30 11:41     ` Thomas Monjalon
  0 siblings, 0 replies; 27+ messages in thread
From: Thomas Monjalon @ 2023-08-30 11:41 UTC (permalink / raw)
  To: Jie Hai
  Cc: ferruh.yigit, andrew.rybchenko, reshma.pattan, liudongdong3,
	haijie1, yisen.zhuang, stable, orika, jerinj, ajit.khaparde

Hello,

Thanks for bringing a new capability.

26/08/2023 09:00, 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_rss_conf`` is extended by adding a new
> field "func". This represents the RSS algorithms to apply. The
> following API is affected:
> 	- rte_eth_dev_configure
> 	- rte_eth_dev_rss_hash_update
> 	- rte_eth_dev_rss_hash_conf_get

So far, the RSS algorithm was used only in flow RSS API.

> --- a/doc/guides/rel_notes/release_23_11.rst
> +++ b/doc/guides/rel_notes/release_23_11.rst
> @@ -123,6 +123,8 @@ ABI Changes
>     Also, make sure to start the actual text at the margin.
>     =======================================================
>  
> +   * ethdev: Added "func" field to ``rte_eth_rss_conf`` structure for RSS hash
> +     algorithm.

As written above, it should start at the margin.

> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> +#include "rte_flow.h"

It is strange to include rte_flow.h here.
It would be better to move the enum.

> + * The *func* field of the *rss_conf* structure indicates the hash algorithm
> + * applied by the RSS hashing. Passing RTE_ETH_HASH_FUNCTION_DEFAULT allows
> + * the PMD to use its best-effort algorithm rather than a specific one.

I don't like commenting a field on top of the structure.
By the way, the first sentence does not look helpful.
RTE_ETH_HASH_FUNCTION_DEFAULT may be explained in the enum.

>   */
>  struct rte_eth_rss_conf {
>  	uint8_t *rss_key;    /**< If not NULL, 40-byte hash key. */
>  	uint8_t rss_key_len; /**< hash key length in bytes. */
>  	uint64_t rss_hf;     /**< Hash functions to apply - see below. */
> +	enum rte_eth_hash_function func;	/**< Hash algorithm to apply. */

You can drop "to apply".



^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2023-08-30 11:41 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-15 11:00 [PATCH 0/5] support setting and querying RSS algorithms Dongdong Liu
2023-03-15 11:00 ` [PATCH 1/5] ethdev: support setting and querying rss algorithm Dongdong Liu
2023-03-15 11:28   ` Ivan Malov
2023-03-16 13:10     ` Dongdong Liu
2023-03-16 14:31       ` Ivan Malov
2023-03-15 13:43   ` Thomas Monjalon
2023-03-16 13:16     ` Dongdong Liu
2023-06-02 20:19       ` Ferruh Yigit
2023-06-05 12:34         ` Dongdong Liu
2023-03-15 11:00 ` [PATCH 2/5] net/hns3: support setting and querying RSS hash function Dongdong Liu
2023-03-15 11:00 ` [PATCH 3/5] app/proc-info: fix never show RSS info Dongdong Liu
2023-06-02 20:19   ` Ferruh Yigit
2023-06-05 13:04     ` Dongdong Liu
2023-06-02 21:19   ` Stephen Hemminger
2023-06-05 13:07     ` Dongdong Liu
2023-03-15 11:00 ` [PATCH 4/5] app/proc-info: show RSS types with strings Dongdong Liu
2023-06-02 20:22   ` Ferruh Yigit
2023-06-05 13:12     ` Dongdong Liu
2023-03-15 11:00 ` [PATCH 5/5] app/proc-info: support querying RSS hash algorithm Dongdong Liu
2023-08-26  7:00 ` [PATCH v2 0/5] support setting and querying RSS algorithms Jie Hai
2023-08-26  7:00   ` [PATCH v2 1/5] ethdev: support setting and querying rss algorithm Jie Hai
2023-08-30 11:41     ` Thomas Monjalon
2023-08-26  7:00   ` [PATCH v2 2/5] net/hns3: support setting and querying RSS hash function Jie Hai
2023-08-26  7:00   ` [PATCH v2 3/5] app/proc-info: fix never show RSS info Jie Hai
2023-08-26  7:00   ` [PATCH v2 4/5] app/proc-info: adjust the display format of " Jie Hai
2023-08-26  7:00   ` [PATCH v2 5/5] app/proc-info: support querying RSS hash algorithm Jie Hai
2023-08-26  8:52   ` [PATCH v2 0/5] support setting and querying RSS algorithms Jie Hai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).