[dpdk-dev] [PATCH] app/testpmd: fix RSS-hash-key size
Hi Awal, As I said in the offline discussion, here are few nits that I think need to be addressed. See below. Konstantin > > RSS hash-key-size is retrieved from device configuration instead of using a > fixed size of 40 bytes. > > Fixes: f79959ea1504 ("app/testpmd: allow to configure RSS hash key") > > Signed-off-by: Mohammad Abdul Awal > --- > app/test-pmd/cmdline.c | 24 +--- app/test-pmd/config.c > | 17 ++--- > 2 files changed, 31 insertions(+), 10 deletions(-) > > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index > f90befc..14412b4 100644 > --- a/app/test-pmd/cmdline.c > +++ b/app/test-pmd/cmdline.c > @@ -1608,7 +1608,6 @@ struct cmd_config_rss_hash_key { > cmdline_fixed_string_t key; > }; > > -#define RSS_HASH_KEY_LENGTH 40 > static uint8_t > hexa_digit_to_value(char hexa_digit) > { > @@ -1640,20 +1639,30 @@ cmd_config_rss_hash_key_parsed(void *parsed_result, > __attribute__((unused)) void *data) { > struct cmd_config_rss_hash_key *res = parsed_result; > - uint8_t hash_key[RSS_HASH_KEY_LENGTH]; > + uint8_t hash_key[16 * 4]; No need for hard-coded constants. I'd suggest to keep RSS_HASH_KEY_LENGTH, just increase it to 52 (or might be even bigger) value. > uint8_t xdgt0; > uint8_t xdgt1; > int i; > + struct rte_eth_dev_info dev_info; > + uint8_t hash_key_size; > > + memset(&dev_info, 0, sizeof(dev_info)); > + rte_eth_dev_info_get(res->port_id, &dev_info); > + if (dev_info.hash_key_size > 0) { && dev_info.hash_key_size <= sizeof(hash_key) { > + hash_key_size = dev_info.hash_key_size; > + } else { > + printf("dev_info did not provide a valid hash key size\n"); > + return; > + } > /* Check the length of the RSS hash key */ > - if (strlen(res->key) != (RSS_HASH_KEY_LENGTH * 2)) { > + if (strlen(res->key) != (hash_key_size * 2)) { > printf("key length: %d invalid - key must be a string of %d" > "hexa-decimal numbers\n", (int) strlen(res->key), > -RSS_HASH_KEY_LENGTH * 2); > +hash_key_size * 2); > return; > } > /* Translate RSS hash key into binary representation */ > - for (i = 0; i < RSS_HASH_KEY_LENGTH; i++) { > + for (i = 0; i < hash_key_size; i++) { > xdgt0 = parse_and_check_key_hexa_digit(res->key, (i * 2)); > if (xdgt0 == 0xFF) > return; > @@ -1663,7 +1672,7 @@ cmd_config_rss_hash_key_parsed(void *parsed_result, > hash_key[i] = (uint8_t) ((xdgt0 * 16) + xdgt1); > } > port_rss_hash_key_update(res->port_id, res->rss_type, hash_key, > - RSS_HASH_KEY_LENGTH); > + hash_key_size); > } > > cmdline_parse_token_string_t cmd_config_rss_hash_key_port = @@ -1692,7 > +1701,8 @@ cmdline_parse_inst_t > cmd_config_rss_hash_key = { > "port config X rss-hash-key ipv4|ipv4-frag|ipv4-tcp|ipv4-udp|" > "ipv4-sctp|ipv4-other|ipv6|ipv6-frag|ipv6-tcp|ipv6-udp|" > "ipv6-sctp|ipv6-other|l2-payload|" > - "ipv6-ex|ipv6-tcp-ex|ipv6-udp-ex 80 hexa digits\n", > + "ipv6-ex|ipv6-tcp-ex|ipv6-udp-ex " > + "80 hexa digits (104 hexa digits for fortville)\n", No need to mention particular NIC (Fortville) here. I'd say better: 'array of hex digits (variable size, NIC dependent)' or so. > .tokens = { > (void *)&cmd_config_rss_hash_key_port, > (void *)&cmd_config_rss_hash_key_config, > diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index > bfcbff9..851408b 100644 > --- a/app/test-pmd/config.c > +++ b/app/test-pmd/config.c > @@ -1012,14 +1012,25 @@ void > port_rss_hash_conf_show(portid_t port_id, char rss_info[], int show_rss_key) > { > struct rte_eth_rss_conf rss_conf; > - uint8_t rss_key[10 * 4] = ""; > + uint8_t rss_key[16 * 4] = ""; better rss_key[RSS_HASH_KEY_LENGTH ], and I think there is no need to put '0' into first element. > uint64_t rss_hf; > uint8_t i; > int diag; > + struct rte_eth_dev_info dev_info; > + uint8_t hash_key_size; > > if (port_id_is_invalid(port_id, ENABLED_WARN)) > return; > > + memset(&dev_info, 0, sizeof(dev_info)); > + rte_eth_dev_info_get(port_id, &dev_info); > + if (dev_info.hash_key_size > 0) { > + hash_key_size = dev_info.hash_key_size; > + } else { > + printf("dev_info did not provide a valid hash key size\n"); > + return; > + } > + > rss_conf.rss_hf = 0; > for (i = 0; i < RTE_DIM(rss_type_table); i++) { > if (!strcmp(rss_info, rss_type_table[i].str)) @@ -1028,7 > +1039,7 @@ port_rss_hash_conf_show(portid_t port_id, char > rss_info[], int show_rss_key) > > /* Get RSS hash key i
[dpdk-dev] [PATCH] app/testpmd: fix RSS-hash-key size
RSS hash-key-size is retrieved from device configuration instead of using a fixed size of 40 bytes. Fixes: f79959ea1504 ("app/testpmd: allow to configure RSS hash key") Signed-off-by: Mohammad Abdul Awal --- app/test-pmd/cmdline.c | 24 +--- app/test-pmd/config.c | 17 ++--- 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index f90befc..14412b4 100644 --- a/app/test-pmd/cmdline.c +++ b/app/test-pmd/cmdline.c @@ -1608,7 +1608,6 @@ struct cmd_config_rss_hash_key { cmdline_fixed_string_t key; }; -#define RSS_HASH_KEY_LENGTH 40 static uint8_t hexa_digit_to_value(char hexa_digit) { @@ -1640,20 +1639,30 @@ cmd_config_rss_hash_key_parsed(void *parsed_result, __attribute__((unused)) void *data) { struct cmd_config_rss_hash_key *res = parsed_result; - uint8_t hash_key[RSS_HASH_KEY_LENGTH]; + uint8_t hash_key[16 * 4]; uint8_t xdgt0; uint8_t xdgt1; int i; + struct rte_eth_dev_info dev_info; + uint8_t hash_key_size; + memset(&dev_info, 0, sizeof(dev_info)); + rte_eth_dev_info_get(res->port_id, &dev_info); + if (dev_info.hash_key_size > 0) { + hash_key_size = dev_info.hash_key_size; + } else { + printf("dev_info did not provide a valid hash key size\n"); + return; + } /* Check the length of the RSS hash key */ - if (strlen(res->key) != (RSS_HASH_KEY_LENGTH * 2)) { + if (strlen(res->key) != (hash_key_size * 2)) { printf("key length: %d invalid - key must be a string of %d" "hexa-decimal numbers\n", (int) strlen(res->key), - RSS_HASH_KEY_LENGTH * 2); + hash_key_size * 2); return; } /* Translate RSS hash key into binary representation */ - for (i = 0; i < RSS_HASH_KEY_LENGTH; i++) { + for (i = 0; i < hash_key_size; i++) { xdgt0 = parse_and_check_key_hexa_digit(res->key, (i * 2)); if (xdgt0 == 0xFF) return; @@ -1663,7 +1672,7 @@ cmd_config_rss_hash_key_parsed(void *parsed_result, hash_key[i] = (uint8_t) ((xdgt0 * 16) + xdgt1); } port_rss_hash_key_update(res->port_id, res->rss_type, hash_key, -RSS_HASH_KEY_LENGTH); + hash_key_size); } cmdline_parse_token_string_t cmd_config_rss_hash_key_port = @@ -1692,7 +1701,8 @@ cmdline_parse_inst_t cmd_config_rss_hash_key = { "port config X rss-hash-key ipv4|ipv4-frag|ipv4-tcp|ipv4-udp|" "ipv4-sctp|ipv4-other|ipv6|ipv6-frag|ipv6-tcp|ipv6-udp|" "ipv6-sctp|ipv6-other|l2-payload|" - "ipv6-ex|ipv6-tcp-ex|ipv6-udp-ex 80 hexa digits\n", + "ipv6-ex|ipv6-tcp-ex|ipv6-udp-ex " + "80 hexa digits (104 hexa digits for fortville)\n", .tokens = { (void *)&cmd_config_rss_hash_key_port, (void *)&cmd_config_rss_hash_key_config, diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index bfcbff9..851408b 100644 --- a/app/test-pmd/config.c +++ b/app/test-pmd/config.c @@ -1012,14 +1012,25 @@ void port_rss_hash_conf_show(portid_t port_id, char rss_info[], int show_rss_key) { struct rte_eth_rss_conf rss_conf; - uint8_t rss_key[10 * 4] = ""; + uint8_t rss_key[16 * 4] = ""; uint64_t rss_hf; uint8_t i; int diag; + struct rte_eth_dev_info dev_info; + uint8_t hash_key_size; if (port_id_is_invalid(port_id, ENABLED_WARN)) return; + memset(&dev_info, 0, sizeof(dev_info)); + rte_eth_dev_info_get(port_id, &dev_info); + if (dev_info.hash_key_size > 0) { + hash_key_size = dev_info.hash_key_size; + } else { + printf("dev_info did not provide a valid hash key size\n"); + return; + } + rss_conf.rss_hf = 0; for (i = 0; i < RTE_DIM(rss_type_table); i++) { if (!strcmp(rss_info, rss_type_table[i].str)) @@ -1028,7 +1039,7 @@ port_rss_hash_conf_show(portid_t port_id, char rss_info[], int show_rss_key) /* Get RSS hash key if asked to display it */ rss_conf.rss_key = (show_rss_key) ? rss_key : NULL; - rss_conf.rss_key_len = sizeof(rss_key); + rss_conf.rss_key_len = hash_key_size; diag = rte_eth_dev_rss_hash_conf_get(port_id, &rss_conf); if (diag != 0) { switch (diag) { @@ -1058,7 +1069,7 @@ port_rss_hash_conf_show(portid_t port_id, char rss_info[], int show_rss_key) if (!show_rss_key) return; printf("RSS key:\n"); - for (i = 0; i < sizeof(rss_key); i++) + for (i = 0; i < hash_key_size; i++) printf("%02X", rss_key[i]); printf("\