[dpdk-dev] [PATCH v3 3/8] i40e: support of setting hash lookup table size

[Thomas Monjalon]

2014-10-27 13:21, Matthew Hall:
> On Mon, Oct 27, 2014 at 03:13:39PM +0100, Thomas Monjalon wrote:
> > You didn't answer to my previous comment on this.
> > I think these definitions are useless. 64 is 64.
> 
> Putting labels on the constants gives meaning to them as well as a numeric 
> value. Not doing so is an antipattern referred to as "magic numbers" 
> antipattern.

Are you kidding Matthew?
I'm referring to these constants:
> +#define ETH_RSS_RETA_SIZE_64  64
> +#define ETH_RSS_RETA_SIZE_128 128
> +#define ETH_RSS_RETA_SIZE_512 512

It's not RETA_SIZE which would have a meaning but
RETA_SIZE_64. We could also define RETA_SIZE_32 or RETA_SIZE_33...

> A maintainence programmer or community member will have a difficult time 
> figuring out lost context when grepping through the code.
> 
> Matthew.

[dpdk-dev] [PATCH v3 3/8] i40e: support of setting hash lookup table size

[Thomas Monjalon]

2014-10-22 19:53, Helin Zhang:
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -430,6 +430,9 @@ struct rte_eth_rss_conf {
>  /* Definitions used for redirection table entry size */
>  #define ETH_RSS_RETA_NUM_ENTRIES 128
>  #define ETH_RSS_RETA_MAX_QUEUE   16
> +#define ETH_RSS_RETA_SIZE_64  64
> +#define ETH_RSS_RETA_SIZE_128 128
> +#define ETH_RSS_RETA_SIZE_512 512

You didn't answer to my previous comment on this.
I think these definitions are useless. 64 is 64.

-- 
Thomas

[dpdk-dev] [PATCH v3 3/8] i40e: support of setting hash lookup table size

[Matthew Hall]

On Mon, Oct 27, 2014 at 03:13:39PM +0100, Thomas Monjalon wrote:
> You didn't answer to my previous comment on this.
> I think these definitions are useless. 64 is 64.

Putting labels on the constants gives meaning to them as well as a numeric 
value. Not doing so is an antipattern referred to as "magic numbers" 
antipattern.

A maintainence programmer or community member will have a difficult time 
figuring out lost context when grepping through the code.

Matthew.

[dpdk-dev] [PATCH v3 3/8] i40e: support of setting hash lookup table size

[Helin Zhang]

Add support of setting hash lookup table size according to
the hardawre capability.

Signed-off-by: Helin Zhang 
---
 lib/librte_ether/rte_ethdev.h |  3 +++
 lib/librte_pmd_i40e/i40e_ethdev.c | 14 +-
 lib/librte_pmd_i40e/i40e_ethdev.h |  1 +
 3 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index b69a6af..7db08c2 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -430,6 +430,9 @@ struct rte_eth_rss_conf {
 /* Definitions used for redirection table entry size */
 #define ETH_RSS_RETA_NUM_ENTRIES 128
 #define ETH_RSS_RETA_MAX_QUEUE   16
+#define ETH_RSS_RETA_SIZE_64  64
+#define ETH_RSS_RETA_SIZE_128 128
+#define ETH_RSS_RETA_SIZE_512 512

 /* Definitions used for VMDQ and DCB functionality */
 #define ETH_VMDQ_MAX_VLAN_FILTERS   64 /**< Maximum nb. of VMDQ vlan filters. 
*/
diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c 
b/lib/librte_pmd_i40e/i40e_ethdev.c
index 3b75f0f..ef24175 100644
--- a/lib/librte_pmd_i40e/i40e_ethdev.c
+++ b/lib/librte_pmd_i40e/i40e_ethdev.c
@@ -2943,7 +2943,19 @@ i40e_pf_setup(struct i40e_pf *pf)

/* Configure filter control */
memset(&settings, 0, sizeof(settings));
-   settings.hash_lut_size = I40E_HASH_LUT_SIZE_128;
+   if (hw->func_caps.rss_table_size == ETH_RSS_RETA_SIZE_128)
+   settings.hash_lut_size = I40E_HASH_LUT_SIZE_128;
+   else if (hw->func_caps.rss_table_size == ETH_RSS_RETA_SIZE_512)
+   settings.hash_lut_size = I40E_HASH_LUT_SIZE_512;
+   else {
+   PMD_DRV_LOG(ERR, "Hash lookup table size (%u) not supported\n",
+   hw->func_caps.rss_table_size);
+   return I40E_ERR_PARAM;
+   }
+   PMD_DRV_LOG(INFO, "Hardware capability of hash lookup table "
+   "size: %u\n", hw->func_caps.rss_table_size);
+   pf->hash_lut_size = hw->func_caps.rss_table_size;
+
/* Enable ethtype and macvlan filters */
settings.enable_ethtype = TRUE;
settings.enable_macvlan = TRUE;
diff --git a/lib/librte_pmd_i40e/i40e_ethdev.h 
b/lib/librte_pmd_i40e/i40e_ethdev.h
index 1d42cd2..22b693f 100644
--- a/lib/librte_pmd_i40e/i40e_ethdev.h
+++ b/lib/librte_pmd_i40e/i40e_ethdev.h
@@ -246,6 +246,7 @@ struct i40e_pf {
uint16_t vmdq_nb_qps; /* The number of queue pairs of VMDq */
uint16_t vf_nb_qps; /* The number of queue pairs of VF */
uint16_t fdir_nb_qps; /* The number of queue pairs of Flow Director */
+   uint16_t hash_lut_size; /* The size of hash lookup table */
 };

 enum pending_msg {
-- 
1.8.1.4