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

Message ID 1413978810-24610-4-git-send-email-helin.zhang@intel.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Zhang, Helin Oct. 22, 2014, 11:53 a.m. UTC
  Add support of setting hash lookup table size according to
the hardawre capability.

Signed-off-by: Helin Zhang <helin.zhang@intel.com>
---
 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(-)
  

Comments

Thomas Monjalon Oct. 27, 2014, 2:13 p.m. UTC | #1
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.
  
Matthew Hall Oct. 27, 2014, 8:21 p.m. UTC | #2
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.
  
Thomas Monjalon Oct. 27, 2014, 9:41 p.m. UTC | #3
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.
  

Patch

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 {