[dpdk-dev] net/i40e: fix the hash filter invalid calculation in X722

Message ID 1476931738-44140-1-git-send-email-jia.guo@intel.com
State Superseded, archived
Delegated to: Bruce Richardson
Headers show

Commit Message

Jeff Guo Oct. 20, 2016, 2:48 a.m.
As X722 extracts IPv4 header to Field Vector different with XL710/X710,
need to corresponding to modify the fields of IPv4 header in input set
to map different default Field Vector Table of different NICs.

Signed-off-by: Jeff Guo <jia.guo@intel.com>
---
v3:
remove the x722 macro
v2:
fix compile error when x722 macro is not defined and simplify
the code to avoid duplication.
---
 drivers/net/i40e/i40e_ethdev.c | 60 +++++++++++++++++++++++++++++++++---------
 1 file changed, 47 insertions(+), 13 deletions(-)

Comments

Wu, Jingjing Oct. 24, 2016, 9:10 a.m. | #1
> -----Original Message-----
> From: Guo, Jia
> Sent: Thursday, October 20, 2016 10:49 AM
> To: Zhang, Helin <helin.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>
> Cc: dev@dpdk.org; Guo, Jia <jia.guo@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>
> Subject: [PATCH] net/i40e: fix the hash filter invalid calculation in X722
> 
> As X722 extracts IPv4 header to Field Vector different with XL710/X710,
> need to corresponding to modify the fields of IPv4 header in input set
> to map different default Field Vector Table of different NICs.
> Signed-off-by: Jeff Guo <jia.guo@intel.com>
> ---
> v3:
> remove the x722 macro
> v2:
> fix compile error when x722 macro is not defined and simplify
> the code to avoid duplication.
> ---
>  drivers/net/i40e/i40e_ethdev.c | 60 +++++++++++++++++++++++++++++++++---------
>  1 file changed, 47 insertions(+), 13 deletions(-)
> 

How about change the commit log it like:

When verifying the Hash filtering on X722, we found the behavior was not expected. For example, the hash value in descriptor is incorrect.
That was because X722 uses different way of hash key word selection comparing with X710/XL710.
This patch fixes it by setting X722 specific key selection.

And few minor comments:

If this is not the first patch, please use [PATCH v3] instead of [PATCH].
And the fixes line is missed.

Thanks
Jingijng
Jeff Guo Oct. 25, 2016, 2:11 a.m. | #2
I will refine the commit log. Other hand,  since the issue is not directly related with prior patch, it just because some nic type adding request some special behavior handle. So it is reported issue fix, but may be have not corresponding fix line. Thanks jingjing's review. 

Best regards,
Jeff Guo


-----Original Message-----
From: Wu, Jingjing 
Sent: Monday, October 24, 2016 5:10 PM
To: Guo, Jia <jia.guo@intel.com>; Zhang, Helin <helin.zhang@intel.com>
Cc: dev@dpdk.org; Yigit, Ferruh <ferruh.yigit@intel.com>
Subject: RE: [PATCH] net/i40e: fix the hash filter invalid calculation in X722


> -----Original Message-----
> From: Guo, Jia
> Sent: Thursday, October 20, 2016 10:49 AM
> To: Zhang, Helin <helin.zhang@intel.com>; Wu, Jingjing 
> <jingjing.wu@intel.com>
> Cc: dev@dpdk.org; Guo, Jia <jia.guo@intel.com>; Yigit, Ferruh 
> <ferruh.yigit@intel.com>
> Subject: [PATCH] net/i40e: fix the hash filter invalid calculation in 
> X722
> 
> As X722 extracts IPv4 header to Field Vector different with 
> XL710/X710, need to corresponding to modify the fields of IPv4 header 
> in input set to map different default Field Vector Table of different NICs.
> Signed-off-by: Jeff Guo <jia.guo@intel.com>
> ---
> v3:
> remove the x722 macro
> v2:
> fix compile error when x722 macro is not defined and simplify the code 
> to avoid duplication.
> ---
>  drivers/net/i40e/i40e_ethdev.c | 60 
> +++++++++++++++++++++++++++++++++---------
>  1 file changed, 47 insertions(+), 13 deletions(-)
> 

How about change the commit log it like:

When verifying the Hash filtering on X722, we found the behavior was not expected. For example, the hash value in descriptor is incorrect.
That was because X722 uses different way of hash key word selection comparing with X710/XL710.
This patch fixes it by setting X722 specific key selection.

And few minor comments:

If this is not the first patch, please use [PATCH v3] instead of [PATCH].
And the fixes line is missed.

Thanks
Jingijng

Patch

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 5af0e43..6ef5ec6 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -211,6 +211,14 @@ 
 #define I40E_REG_INSET_L3_SRC_IP4                0x0001800000000000ULL
 /* Destination IPv4 address */
 #define I40E_REG_INSET_L3_DST_IP4                0x0000001800000000ULL
+/* Source IPv4 address for X722 */
+#define I40E_X722_REG_INSET_L3_SRC_IP4           0x0006000000000000ULL
+/* Destination IPv4 address for X722 */
+#define I40E_X722_REG_INSET_L3_DST_IP4           0x0000060000000000ULL
+/* IPv4 Protocol for X722 */
+#define I40E_X722_REG_INSET_L3_IP4_PROTO         0x0010000000000000ULL
+/* IPv4 Time to Live for X722 */
+#define I40E_X722_REG_INSET_L3_IP4_TTL           0x0010000000000000ULL
 /* IPv4 Type of Service (TOS) */
 #define I40E_REG_INSET_L3_IP4_TOS                0x0040000000000000ULL
 /* IPv4 Protocol */
@@ -7560,25 +7568,23 @@  i40e_parse_input_set(uint64_t *inset,
  * and vice versa
  */
 static uint64_t
-i40e_translate_input_set_reg(uint64_t input)
+i40e_translate_input_set_reg(enum i40e_mac_type type, uint64_t input)
 {
 	uint64_t val = 0;
 	uint16_t i;
 
-	static const struct {
+	struct inset_map {
 		uint64_t inset;
 		uint64_t inset_reg;
-	} inset_map[] = {
+	};
+
+	static const struct inset_map inset_map_common[] = {
 		{I40E_INSET_DMAC, I40E_REG_INSET_L2_DMAC},
 		{I40E_INSET_SMAC, I40E_REG_INSET_L2_SMAC},
 		{I40E_INSET_VLAN_OUTER, I40E_REG_INSET_L2_OUTER_VLAN},
 		{I40E_INSET_VLAN_INNER, I40E_REG_INSET_L2_INNER_VLAN},
 		{I40E_INSET_LAST_ETHER_TYPE, I40E_REG_INSET_LAST_ETHER_TYPE},
-		{I40E_INSET_IPV4_SRC, I40E_REG_INSET_L3_SRC_IP4},
-		{I40E_INSET_IPV4_DST, I40E_REG_INSET_L3_DST_IP4},
 		{I40E_INSET_IPV4_TOS, I40E_REG_INSET_L3_IP4_TOS},
-		{I40E_INSET_IPV4_PROTO, I40E_REG_INSET_L3_IP4_PROTO},
-		{I40E_INSET_IPV4_TTL, I40E_REG_INSET_L3_IP4_TTL},
 		{I40E_INSET_IPV6_SRC, I40E_REG_INSET_L3_SRC_IP6},
 		{I40E_INSET_IPV6_DST, I40E_REG_INSET_L3_DST_IP6},
 		{I40E_INSET_IPV6_TC, I40E_REG_INSET_L3_IP6_TC},
@@ -7607,13 +7613,40 @@  i40e_translate_input_set_reg(uint64_t input)
 		{I40E_INSET_FLEX_PAYLOAD_W8, I40E_REG_INSET_FLEX_PAYLOAD_WORD8},
 	};
 
+    /* some different registers map in x722*/
+	static const struct inset_map inset_map_diff_x722[] = {
+		{I40E_INSET_IPV4_SRC, I40E_X722_REG_INSET_L3_SRC_IP4},
+		{I40E_INSET_IPV4_DST, I40E_X722_REG_INSET_L3_DST_IP4},
+		{I40E_INSET_IPV4_PROTO, I40E_X722_REG_INSET_L3_IP4_PROTO},
+		{I40E_INSET_IPV4_TTL, I40E_X722_REG_INSET_L3_IP4_TTL},
+	};
+
+	static const struct inset_map inset_map_diff_not_x722[] = {
+		{I40E_INSET_IPV4_SRC, I40E_REG_INSET_L3_SRC_IP4},
+		{I40E_INSET_IPV4_DST, I40E_REG_INSET_L3_DST_IP4},
+		{I40E_INSET_IPV4_PROTO, I40E_REG_INSET_L3_IP4_PROTO},
+		{I40E_INSET_IPV4_TTL, I40E_REG_INSET_L3_IP4_TTL},
+	};
+
 	if (input == 0)
 		return val;
 
 	/* Translate input set to register aware inset */
-	for (i = 0; i < RTE_DIM(inset_map); i++) {
-		if (input & inset_map[i].inset)
-			val |= inset_map[i].inset_reg;
+	if (type == I40E_MAC_X722) {
+		for (i = 0; i < RTE_DIM(inset_map_diff_x722); i++) {
+			if (input & inset_map_diff_x722[i].inset)
+				val |= inset_map_diff_x722[i].inset_reg;
+		}
+	} else {
+		for (i = 0; i < RTE_DIM(inset_map_diff_not_x722); i++) {
+			if (input & inset_map_diff_not_x722[i].inset)
+				val |= inset_map_diff_not_x722[i].inset_reg;
+		}
+	}
+
+	for (i = 0; i < RTE_DIM(inset_map_common); i++) {
+		if (input & inset_map_common[i].inset)
+			val |= inset_map_common[i].inset_reg;
 	}
 
 	return val;
@@ -7698,7 +7731,8 @@  i40e_filter_input_set_init(struct i40e_pf *pf)
 						   I40E_INSET_MASK_NUM_REG);
 		if (num < 0)
 			return;
-		inset_reg = i40e_translate_input_set_reg(input_set);
+		inset_reg = i40e_translate_input_set_reg(hw->mac.type,
+					input_set);
 
 		i40e_check_write_reg(hw, I40E_PRTQF_FD_INSET(pctype, 0),
 				      (uint32_t)(inset_reg & UINT32_MAX));
@@ -7789,7 +7823,7 @@  i40e_hash_filter_inset_select(struct i40e_hw *hw,
 	if (num < 0)
 		return -EINVAL;
 
-	inset_reg |= i40e_translate_input_set_reg(input_set);
+	inset_reg |= i40e_translate_input_set_reg(hw->mac.type, input_set);
 
 	i40e_check_write_reg(hw, I40E_GLQF_HASH_INSET(0, pctype),
 			      (uint32_t)(inset_reg & UINT32_MAX));
@@ -7867,7 +7901,7 @@  i40e_fdir_filter_inset_select(struct i40e_pf *pf,
 	if (num < 0)
 		return -EINVAL;
 
-	inset_reg |= i40e_translate_input_set_reg(input_set);
+	inset_reg |= i40e_translate_input_set_reg(hw->mac.type, input_set);
 
 	i40e_check_write_reg(hw, I40E_PRTQF_FD_INSET(pctype, 0),
 			      (uint32_t)(inset_reg & UINT32_MAX));