[PATCH 0/5] support setting and querying RSS algorithms

[PATCH 0/5] support setting and querying RSS algorithms

[Dongdong Liu]

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

[PATCH 1/5] ethdev: support setting and querying rss algorithm

[Dongdong Liu]

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

[PATCH 2/5] net/hns3: support setting and querying RSS hash function

[Dongdong Liu]

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

[PATCH 3/5] app/proc-info: fix never show RSS info

[Dongdong Liu]

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

[PATCH 4/5] app/proc-info: show RSS types with strings

[Dongdong Liu]

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

[PATCH 5/5] app/proc-info: support querying RSS hash algorithm

[Dongdong Liu]

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

Re: [PATCH 1/5] ethdev: support setting and querying rss algorithm

[Ivan Malov]

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.

Re: [PATCH 1/5] ethdev: support setting and querying rss algorithm

[Thomas Monjalon]

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.

Re: [PATCH 1/5] ethdev: support setting and querying rss algorithm

[Dongdong Liu]

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.
>
> .
>

Re: [PATCH 1/5] ethdev: support setting and querying rss algorithm

[Dongdong Liu]

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
>
>
>
> .
>

Re: [PATCH 1/5] ethdev: support setting and querying rss algorithm

[Ivan Malov]

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.

Re: [PATCH 1/5] ethdev: support setting and querying rss algorithm

[Ferruh Yigit]

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
>>
>>
>>
>> .
>>

Re: [PATCH 3/5] app/proc-info: fix never show RSS info

[Ferruh Yigit]

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
> 

Re: [PATCH 4/5] app/proc-info: show RSS types with strings

[Ferruh Yigit]

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);

Re: [PATCH 3/5] app/proc-info: fix never show RSS info

[Stephen Hemminger]

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().

Re: [PATCH 1/5] ethdev: support setting and querying rss algorithm

[Dongdong Liu]

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
>>>
>>>
>>>
>>> .
>>>
>
> .
>

Re: [PATCH 3/5] app/proc-info: fix never show RSS info

[Dongdong Liu]

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
>>
>
> .
>

Re: [PATCH 3/5] app/proc-info: fix never show RSS info

[Dongdong Liu]

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
> .
>

Re: [PATCH 4/5] app/proc-info: show RSS types with strings

[Dongdong Liu]

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);
>
> .
>

[PATCH v2 0/5] support setting and querying RSS algorithms

[Jie Hai]

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

[PATCH v2 1/5] ethdev: support setting and querying rss algorithm

[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

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

[PATCH v2 2/5] net/hns3: support setting and querying RSS hash function

[Jie Hai]

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

[PATCH v2 3/5] app/proc-info: fix never show RSS info

[Jie Hai]

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

[PATCH v2 4/5] app/proc-info: adjust the display format of RSS info

[Jie Hai]

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

[PATCH v2 5/5] app/proc-info: support querying RSS hash algorithm

[Jie Hai]

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

Re: [PATCH v2 0/5] support setting and querying RSS algorithms

[Jie Hai]

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(-)
> 

Re: [PATCH v2 1/5] ethdev: support setting and querying rss algorithm

[Thomas Monjalon]

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".