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