From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 7E7DD2C37 for ; Thu, 4 Aug 2016 10:53:54 +0200 (CEST) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga101.jf.intel.com with ESMTP; 04 Aug 2016 01:53:53 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.28,469,1464678000"; d="scan'208";a="859530233" Received: from irsmsx103.ger.corp.intel.com ([163.33.3.157]) by orsmga003.jf.intel.com with ESMTP; 04 Aug 2016 01:53:52 -0700 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.102]) by IRSMSX103.ger.corp.intel.com ([169.254.3.204]) with mapi id 14.03.0248.002; Thu, 4 Aug 2016 09:53:51 +0100 From: "Ananyev, Konstantin" To: "Awal, Mohammad Abdul" , "De Lara Guarch, Pablo" CC: "dev@dpdk.org" , "Awal, Mohammad Abdul" Thread-Topic: [dpdk-dev] [PATCH] app/testpmd: fix RSS-hash-key size Thread-Index: AQHR7aNJtKxNubVCSkmes3WcrTLSL6A3cajw Date: Thu, 4 Aug 2016 08:53:50 +0000 Message-ID: <2601191342CEEE43887BDE71AB97725836B8E80A@irsmsx105.ger.corp.intel.com> References: <1470241265-6242-1-git-send-email-mohammad.abdul.awal@intel.com> In-Reply-To: <1470241265-6242-1-git-send-email-mohammad.abdul.awal@intel.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.180] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix RSS-hash-key size X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 04 Aug 2016 08:53:55 -0000 Hi Awal, As I said in the offline discussion, here are few nits that I think need to be addressed. See below.=20 Konstantin >=20 > RSS hash-key-size is retrieved from device configuration instead of using= a fixed size of 40 bytes. >=20 > Fixes: f79959ea1504 ("app/testpmd: allow to configure RSS hash key") >=20 > Signed-off-by: Mohammad Abdul Awal > --- > app/test-pmd/cmdline.c | 24 +++++++++++++++++------- app/test-pmd/confi= g.c | 17 ++++++++++++++--- > 2 files changed, 31 insertions(+), 10 deletions(-) >=20 > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index f90bef= c..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; > }; >=20 > -#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_resul= t, > __attribute__((unused)) void *data) { > struct cmd_config_rss_hash_key *res =3D 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 b= e even bigger) value. > uint8_t xdgt0; > uint8_t xdgt1; > int i; > + struct rte_eth_dev_info dev_info; > + uint8_t hash_key_size; >=20 > + 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 <=3D sizeof(hash_key) { > + hash_key_size =3D 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) !=3D (RSS_HASH_KEY_LENGTH * 2)) { > + if (strlen(res->key) !=3D (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 =3D 0; i < RSS_HASH_KEY_LENGTH; i++) { > + for (i =3D 0; i < hash_key_size; i++) { > xdgt0 =3D parse_and_check_key_hexa_digit(res->key, (i * 2)); > if (xdgt0 =3D=3D 0xFF) > return; > @@ -1663,7 +1672,7 @@ cmd_config_rss_hash_key_parsed(void *parsed_result, > hash_key[i] =3D (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); > } >=20 > cmdline_parse_token_string_t cmd_config_rss_hash_key_port =3D @@ -1692,7= +1701,8 @@ cmdline_parse_inst_t > cmd_config_rss_hash_key =3D { > "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 =3D { > (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] =3D ""; > + uint8_t rss_key[16 * 4] =3D ""; 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; >=20 > if (port_id_is_invalid(port_id, ENABLED_WARN)) > return; >=20 > + 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 =3D dev_info.hash_key_size; > + } else { > + printf("dev_info did not provide a valid hash key size\n"); > + return; > + } > + > rss_conf.rss_hf =3D 0; > for (i =3D 0; i < RTE_DIM(rss_type_table); i++) { > if (!strcmp(rss_info, rss_type_table[i].str)) @@ -1028,7 +1039,7 @@ po= rt_rss_hash_conf_show(portid_t port_id, char > rss_info[], int show_rss_key) >=20 > /* Get RSS hash key if asked to display it */ > rss_conf.rss_key =3D (show_rss_key) ? rss_key : NULL; > - rss_conf.rss_key_len =3D sizeof(rss_key); > + rss_conf.rss_key_len =3D hash_key_size; > diag =3D rte_eth_dev_rss_hash_conf_get(port_id, &rss_conf); > if (diag !=3D 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 =3D 0; i < sizeof(rss_key); i++) > + for (i =3D 0; i < hash_key_size; i++) > printf("%02X", rss_key[i]); > printf("\n"); > } > -- > 2.7.4