[dpdk-dev] [PATCH v3 7/8] ethdev: support of multiple sizes of redirection table

2014-10-31 Thread Thomas Monjalon 
2014-10-31 01:39, Zhang, Helin:
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > 2014-10-28 00:37, Zhang, Helin:
> > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > > 2014-10-22 19:53, Helin Zhang:
> > > > > +#define RTE_BIT_WIDTH_64 (CHAR_BIT * sizeof(uint64_t))
> > > >
> > > > How can it be different of 64?
> > > > Using 64 would be simpler to understand than RTE_BIT_WIDTH_64.
> > > >
> > > > > + uint8_t reta[RTE_BIT_WIDTH_64]; /**< 64 redirection table 
> > > > > entries. */
> > > We always try to use macro in code to replace numeric, this can get the 
> > > numeric
> > more understandable.
> > 
> > How bit width 64 is more understandable than 64?
> > Especially when you count a number of entries, not a bit width.
> > RETA_ENTRIES_MAX would be more understandable.
> 
> Renaming the macro is needed. I plan to rename it to RTE_RETA_GROUP_SIZE,
> as it is a group of 64 reta entries, but not the maximum number of valid 
> entries.

OK, good.

-- 
Thomas

[dpdk-dev] [PATCH v3 7/8] ethdev: support of multiple sizes of redirection table

2014-10-31 Thread Zhang, Helin 


> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Tuesday, October 28, 2014 6:05 PM
> To: Zhang, Helin
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3 7/8] ethdev: support of multiple sizes of
> redirection table
> 
> 2014-10-28 00:37, Zhang, Helin:
> > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > 2014-10-22 19:53, Helin Zhang:
> > > > +#define RTE_BIT_WIDTH_64 (CHAR_BIT * sizeof(uint64_t))
> > >
> > > How can it be different of 64?
> > > Using 64 would be simpler to understand than RTE_BIT_WIDTH_64.
> > >
> > > > +   uint8_t reta[RTE_BIT_WIDTH_64]; /**< 64 redirection table 
> > > > entries. */
> > We always try to use macro in code to replace numeric, this can get the 
> > numeric
> more understandable.
> 
> How bit width 64 is more understandable than 64?
> Especially when you count a number of entries, not a bit width.
> RETA_ENTRIES_MAX would be more understandable.
Renaming the macro is needed. I plan to rename it to RTE_RETA_GROUP_SIZE,
as it is a group of 64 reta entries, but not the maximum number of valid 
entries.

> 
> --
> Thomas

Regards,
Helin

[dpdk-dev] [PATCH v3 7/8] ethdev: support of multiple sizes of redirection table

2014-10-28 Thread Thomas Monjalon 
2014-10-28 00:37, Zhang, Helin:
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > 2014-10-22 19:53, Helin Zhang:
> > > +#define RTE_BIT_WIDTH_64 (CHAR_BIT * sizeof(uint64_t))
> > 
> > How can it be different of 64?
> > Using 64 would be simpler to understand than RTE_BIT_WIDTH_64.
> > 
> > > + uint8_t reta[RTE_BIT_WIDTH_64]; /**< 64 redirection table entries. */
> We always try to use macro in code to replace numeric, this can get the 
> numeric more understandable.

How bit width 64 is more understandable than 64?
Especially when you count a number of entries, not a bit width.
RETA_ENTRIES_MAX would be more understandable.

-- 
Thomas

[dpdk-dev] [PATCH v3 7/8] ethdev: support of multiple sizes of redirection table

2014-10-28 Thread Zhang, Helin 
Hi Thomas

See my answers inlined.

> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Monday, October 27, 2014 10:24 PM
> To: Zhang, Helin
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3 7/8] ethdev: support of multiple sizes of
> redirection table
> 
> 2014-10-22 19:53, Helin Zhang:
> > +#define RTE_BIT_WIDTH_64 (CHAR_BIT * sizeof(uint64_t))
> 
> How can it be different of 64?
> Using 64 would be simpler to understand than RTE_BIT_WIDTH_64.
> 
> > +   uint8_t reta[RTE_BIT_WIDTH_64]; /**< 64 redirection table entries. */
We always try to use macro in code to replace numeric, this can get the numeric 
more understandable.

> 
> Even your comment is saying that it's 64.
> 
> --
> Thomas

Regards,
Helin

[dpdk-dev] [PATCH v3 7/8] ethdev: support of multiple sizes of redirection table

2014-10-27 Thread Thomas Monjalon 
2014-10-22 19:53, Helin Zhang:
> +#define RTE_BIT_WIDTH_64 (CHAR_BIT * sizeof(uint64_t))

How can it be different of 64?
Using 64 would be simpler to understand than RTE_BIT_WIDTH_64.

> + uint8_t reta[RTE_BIT_WIDTH_64]; /**< 64 redirection table entries. */

Even your comment is saying that it's 64.

-- 
Thomas

[dpdk-dev] [PATCH v3 7/8] ethdev: support of multiple sizes of redirection table

2014-10-22 Thread Helin Zhang 
As 40G NIC supports different sizes (128/512/64 entries) of
redirection table from that (128 entries) of 1G and 10G NICs,
support of multiple sizes of redirection table is needed.
It includes,
* Redefine 'struct rte_eth_rss_reta' in ethdev.
  - To 'struct rte_eth_rss_reta_entry64' which contains 64
entries and 64 bits mask.
  - Array of above new structure can be used for any number
of redirection table entries, as long as the number is
multiple of 64. This is quite flexible for the future
expanding of redirection table.
* Redefinition of relevant interfaces in ethdev.
  - Interface of reta update has been redefined with new
parameters.
  - Interface of reta query has been redefined with new
parameters.
* Rework of 1G PMD in igb.
  - reta update has been reworked.
  - reta query has been reworked.
* Rework of 10G PMD in ixgbe.
  - reta update has been reworked.
  - reta query has been reworked.
* Rework of 40G PMD (PF only) in i40e.
  - reta update has been reworked.
  - reta query has been reworked.
* Implement relevant commands in testpmd.

Signed-off-by: Helin Zhang 
---
 app/test-pmd/cmdline.c  | 152 ++--
 app/test-pmd/config.c   |  37 +
 app/test-pmd/testpmd.h  |   4 +-
 lib/librte_ether/rte_ethdev.c   | 116 ---
 lib/librte_ether/rte_ethdev.h   |  38 +
 lib/librte_pmd_e1000/igb_ethdev.c   | 109 +-
 lib/librte_pmd_i40e/i40e_ethdev.c   |  93 --
 lib/librte_pmd_i40e/i40e_ethdev.h   |  12 +++
 lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 108 +
 9 files changed, 405 insertions(+), 264 deletions(-)

v2 changes:
* Put rework of updating/querying igb reta to a single patch.
* Put rework of updating/querying ixgbe reta to a single patch.
* Put rework of updating/querying i40e reta to a single patch.

v3 changes:
* Put all redefinitions of structures and interfaces into a
  single patch.
* Put all reworks of igb/igbe/i40e of supporting multiple sizes
  of reta into the same patch.
* Put all relevant testpmd reworks of supporting multiple sizes
  of reta into the same patch.

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 9de574d..8a55fb5 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -59,6 +59,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -186,6 +187,11 @@ static void cmd_help_long_parsed(void *parsed_result,
"show port (info|stats|xstats|fdir|stat_qmap) 
(port_id|all)\n"
"Display information for port_id, or all.\n\n"

+   "show port X rss reta (size) (mask0,mask1,...)\n"
+   "Display the rss redirection table entry indicated"
+   " by masks on port X. size is used to indicate the"
+   " hardware supported reta size\n\n"
+
"show port rss-hash [key]\n"
"Display the RSS hash functions and RSS hash key"
" of port X\n\n"
@@ -1539,11 +1545,13 @@ struct cmd_config_rss_reta {
 };

 static int
-parse_reta_config(const char *str, struct rte_eth_rss_reta *reta_conf)
+parse_reta_config(const char *str,
+ struct rte_eth_rss_reta_entry64 *reta_conf,
+ uint16_t nb_entries)
 {
int i;
unsigned size;
-   uint8_t hash_index;
+   uint16_t hash_index, idx, shift;
uint8_t nb_queue;
char s[256];
const char *p, *p0 = str;
@@ -1571,24 +1579,23 @@ parse_reta_config(const char *str, struct 
rte_eth_rss_reta *reta_conf)
for (i = 0; i < _NUM_FLD; i++) {
errno = 0;
int_fld[i] = strtoul(str_fld[i], , 0);
-   if (errno != 0 || end == str_fld[i] || int_fld[i] > 255)
+   if (errno != 0 || end == str_fld[i] ||
+   int_fld[i] > 65535)
return -1;
}

-   hash_index = (uint8_t)int_fld[FLD_HASH_INDEX];
+   hash_index = (uint16_t)int_fld[FLD_HASH_INDEX];
nb_queue = (uint8_t)int_fld[FLD_QUEUE];

-   if (hash_index >= ETH_RSS_RETA_NUM_ENTRIES) {
-   printf("Invalid RETA hash index=%d", hash_index);
+   if (hash_index >= nb_entries) {
+   printf("Invalid RETA hash index=%d\n", hash_index);
return -1;
}

-   if (hash_index < ETH_RSS_RETA_NUM_ENTRIES/2)
-   reta_conf->mask_lo |= (1ULL << hash_index);
-   else
-   reta_conf->mask_hi |= (1ULL << (hash_index - 
ETH_RSS_RETA_NUM_ENTRIES/2));
-
-   reta_conf->reta[hash_index] = nb_queue;
+   idx = hash_index / RTE_BIT_WIDTH_64;
+