i40e: fix handling of HW ATR eviction

PATCH New Jacob Keller 3 participants 4 messages May 4, 2017

Subsystems (2)

Network drivers Intel Ethernet

lore

Share & tools

lore

Jacob Keller

author

2017-05-04 21:16 UTC #0 lore raw

PATCH 1/1 i40e: fix handling of HW ATR eviction

A recent commit to refactor the driver and remove the hw_disabled_flags field accidentally introduced two regressions. First, we overwrote pf->flags which removed various key flags including the MSI-X settings.

Additionally, it was intended that we have now two flags, HW_ATR_EVICT_CAPABLe and HW_ATR_EVICT_ENABLED, but this was not done, and we accidentally were mis-using HW_ATR_EVICT_CAPABLE everywhere.

This patch adds the missing piece, HW_ATR_EVICT_ENABLED, and safely updates pf->flags instead of overwriting it.

Without this patch we will have many problems including disabling MSI-X support, and we'll attempt to use HW ATR eviction on devices which do not support it.

Fixes 47994c119a36 ("i40e: remove hw_disabled_flags in favor of using separate flag bits", 2017-04-19)

Signed-off-by Jacob Keller <[email protected]>

---
Ok. For real, this actually cleans up and adds the missing feature that we should have had. I'm working on the rest, but this should fix the issue you were seeing without introducing other regressions.

 drivers/net/ethernet/intel/i40e/i40e.h         | 1 +
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 4 ++--
 drivers/net/ethernet/intel/i40e/i40e_main.c    | 7 ++++---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c    | 4 ++--
 4 files changed, 9 insertions(+), 7 deletions(-)

drivers/net/ethernet/intel/i40e/i40e.h +1 -0

index 60dc9b2c19ff..395ca94faf80 100644
@@ -399,6 +399,7 @@ struct i40e_pf {
 #define I40E_FLAG_RX_CSUM_ENABLED		BIT_ULL(1)
 #define I40E_FLAG_MSI_ENABLED			BIT_ULL(2)
 #define I40E_FLAG_MSIX_ENABLED			BIT_ULL(3)
+#define I40E_FLAG_HW_ATR_EVICT_ENABLED		BIT_ULL(4)
 #define I40E_FLAG_RSS_ENABLED			BIT_ULL(6)
 #define I40E_FLAG_VMDQ_ENABLED			BIT_ULL(7)
 #define I40E_FLAG_IWARP_ENABLED			BIT_ULL(10)

drivers/net/ethernet/intel/i40e/i40e_ethtool.c +2 -2

index 35a246f05520..3d58762efbc0 100644
@@ -225,7 +225,7 @@ static const struct i40e_priv_flags i40e_gstrings_priv_flags[] = {
 	I40E_PRIV_FLAG("LinkPolling", I40E_FLAG_LINK_POLLING_ENABLED, 0),
 	I40E_PRIV_FLAG("flow-director-atr", I40E_FLAG_FD_ATR_ENABLED, 0),
 	I40E_PRIV_FLAG("veb-stats", I40E_FLAG_VEB_STATS_ENABLED, 0),
-	I40E_PRIV_FLAG("hw-atr-eviction", I40E_FLAG_HW_ATR_EVICT_CAPABLE, 0),
+	I40E_PRIV_FLAG("hw-atr-eviction", I40E_FLAG_HW_ATR_EVICT_ENABLED, 0),
 	I40E_PRIV_FLAG("legacy-rx", I40E_FLAG_LEGACY_RX, 0),
 };

@@ -4093,7 +4093,7 @@ static int i40e_set_priv_flags(struct net_device *dev, u32 flags)
 
 	/* Only allow ATR evict on hardware that is capable of handling it */
 	if (pf->flags & I40E_FLAG_HW_ATR_EVICT_CAPABLE)
-		pf->flags &= ~I40E_FLAG_HW_ATR_EVICT_CAPABLE;
+		pf->flags &= ~I40E_FLAG_HW_ATR_EVICT_ENABLED;
 
 	if (changed_flags & I40E_FLAG_TRUE_PROMISC_SUPPORT) {
 		u16 sw_flags = 0, valid_flags = 0;

drivers/net/ethernet/intel/i40e/i40e_main.c +4 -3

index e4eb97832413..6200a9a55dc2 100644
@@ -8822,11 +8822,12 @@ static int i40e_sw_init(struct i40e_pf *pf)
 		    (pf->hw.aq.api_min_ver > 4))) {
 		/* Supported in FW API version higher than 1.4 */
 		pf->flags |= I40E_FLAG_GENEVE_OFFLOAD_CAPABLE;
-		pf->flags = I40E_FLAG_HW_ATR_EVICT_CAPABLE;
-	} else {
-		pf->flags = I40E_FLAG_HW_ATR_EVICT_CAPABLE;
 	}
 
+	/* Enable HW ATR eviction if possible */
+	if (pf->flags & I40E_FLAG_HW_ATR_EVICT_CAPABLE)
+		pf->flags |= I40E_FLAG_HW_ATR_EVICT_ENABLED;
+
 	pf->eeprom_version = 0xDEAD;
 	pf->lan_veb = I40E_NO_VEB;
 	pf->lan_vsi = I40E_NO_VSI;

drivers/net/ethernet/intel/i40e/i40e_txrx.c +2 -2

index c2e9013d05eb..3ef82f237f9e 100644
@@ -2340,7 +2340,7 @@ static void i40e_atr(struct i40e_ring *tx_ring, struct sk_buff *skb,
 	/* Due to lack of space, no more new filters can be programmed */
 	if (th->syn && (pf->flags & I40E_FLAG_FD_ATR_AUTO_DISABLED))
 		return;
-	if (pf->flags & I40E_FLAG_HW_ATR_EVICT_CAPABLE) {
+	if (pf->flags & I40E_FLAG_HW_ATR_EVICT_ENABLED) {
 		/* HW ATR eviction will take care of removing filters on FIN
 		 * and RST packets.
 		 */

@@ -2402,7 +2402,7 @@ static void i40e_atr(struct i40e_ring *tx_ring, struct sk_buff *skb,
 			I40E_TXD_FLTR_QW1_CNTINDEX_SHIFT) &
 			I40E_TXD_FLTR_QW1_CNTINDEX_MASK;
 
-	if (pf->flags & I40E_FLAG_HW_ATR_EVICT_CAPABLE)
+	if (pf->flags & I40E_FLAG_HW_ATR_EVICT_ENABLED)
 		dtype_cmd |= I40E_TXD_FLTR_QW1_ATR_MASK;
 
 	fdir_desc->qindex_flex_ptype_vsi = cpu_to_le32(flex_ptype);

Signature · 1 line

2.13.0.rc0.317.gcc792a6cad5a

Jeff Kirsher

2017-05-04 21:53 UTC #1 lore raw

PATCH 1/1 i40e: fix handling of HW ATR eviction

Quoted 27 lines · Jacob Keller

A recent commit to refactor the driver and remove the hw_disabled_flags
field accidentally introduced two regressions. First, we overwrote pf->flags which removed various key flags including the MSI-X settings.
Additionally, it was intended that we have now two flags, HW_ATR_EVICT_CAPABLe and HW_ATR_EVICT_ENABLED, but this was not done, and we accidentally were mis-using HW_ATR_EVICT_CAPABLE everywhere.
This patch adds the missing piece, HW_ATR_EVICT_ENABLED, and safely updates pf->flags instead of overwriting it.
Without this patch we will have many problems including disabling MSI-X
support, and we'll attempt to use HW ATR eviction on devices which do not support it.
Fixes 47994c119a36 ("i40e: remove hw_disabled_flags in favor of
using separate flag bits", 2017-04-19)
Signed-off-by Jacob Keller <[email protected]>
---
Ok. For real, this actually cleans up and adds the missing feature that
we should have had. I'm working on the rest, but this should fix the issue you were seeing without introducing other regressions.

Quoted 5 lines

?drivers/net/ethernet/intel/i40e/i40e.h???????? | 1 +
?drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 4 ++--
?drivers/net/ethernet/intel/i40e/i40e_main.c??? | 7 ++++---
?drivers/net/ethernet/intel/i40e/i40e_txrx.c??? | 4 ++--

?4 files changed, 9 insertions(+), 7 deletions(-)

Ok is this the "final" version of the fix???
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20170504/743c07a7/attachment.asc>

Keller

2017-05-04 23:29 UTC #2 lore raw

PATCH 1/1 i40e: fix handling of HW ATR eviction

Quoted 40 lines

-----Original Message-----
From: Kirsher, Jeffrey T
Sent: Thursday, May 04, 2017 2:53 PM
To: Keller, Jacob E <[email protected]>; Intel Wired LAN <intel-wired- lan at lists.osuosl.org>
Cc: Duyck, Alexander H <[email protected]>; Singhai, Anjali <[email protected]>
Subject: Re: [PATCH] i40e: fix handling of HW ATR eviction
Quoted 8 lines · Jacob Keller
A recent commit to refactor the driver and remove the hw_disabled_flags
field accidentally introduced two regressions. First, we overwrote pf->flags which removed various key flags including the MSI-X settings.
Additionally, it was intended that we have now two flags, HW_ATR_EVICT_CAPABLe and HW_ATR_EVICT_ENABLED, but this was not
done,
Quoted 19 lines
and we accidentally were mis-using HW_ATR_EVICT_CAPABLE everywhere.
This patch adds the missing piece, HW_ATR_EVICT_ENABLED, and safely updates pf->flags instead of overwriting it.
Without this patch we will have many problems including disabling MSI-X
support, and we'll attempt to use HW ATR eviction on devices which do not support it.
Fixes 47994c119a36 ("i40e: remove hw_disabled_flags in favor of
using separate flag bits", 2017-04-19)
Signed-off-by Jacob Keller <[email protected]>
---
Ok. For real, this actually cleans up and adds the missing feature that
we should have had. I'm working on the rest, but this should fix the issue you were seeing without introducing other regressions.

Quoted 7 lines

Quoted 5 lines
?drivers/net/ethernet/intel/i40e/i40e.h???????? | 1 +
?drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 4 ++--
?drivers/net/ethernet/intel/i40e/i40e_main.c??? | 7 ++++---
?drivers/net/ethernet/intel/i40e/i40e_txrx.c??? | 4 ++--
?4 files changed, 9 insertions(+), 7 deletions(-)
Ok is this the "final" version of the fix???

Yes thanks.

Regards,
Jake

Bowers

2017-05-04 22:40 UTC #3 lore raw

PATCH 1/1 i40e: fix handling of HW ATR eviction

In reply to Jacob Keller

Quoted 31 lines

-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On Behalf Of Jacob Keller
Sent: Thursday, May 4, 2017 2:17 PM
To: Intel Wired LAN <[email protected]>
Subject: [Intel-wired-lan] [PATCH] i40e: fix handling of HW ATR eviction
A recent commit to refactor the driver and remove the hw_disabled_flags field accidentally introduced two regressions. First, we overwrote pf->flags which removed various key flags including the MSI-X settings.
Additionally, it was intended that we have now two flags, HW_ATR_EVICT_CAPABLe and HW_ATR_EVICT_ENABLED, but this was not done, and we accidentally were mis-using HW_ATR_EVICT_CAPABLE everywhere.
This patch adds the missing piece, HW_ATR_EVICT_ENABLED, and safely updates pf->flags instead of overwriting it.
Without this patch we will have many problems including disabling MSI-X support, and we'll attempt to use HW ATR eviction on devices which do not support it.
Fixes 47994c119a36 ("i40e: remove hw_disabled_flags in favor of using
separate flag bits", 2017-04-19)
Signed-off-by Jacob Keller <[email protected]>
---
Ok. For real, this actually cleans up and adds the missing feature that we should have had. I'm working on the rest, but this should fix the issue you were seeing without introducing other regressions.

Quoted 5 lines

 drivers/net/ethernet/intel/i40e/i40e.h         | 1 +
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 4 ++--
 drivers/net/ethernet/intel/i40e/i40e_main.c    | 7 ++++---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c    | 4 ++--
 4 files changed, 9 insertions(+), 7 deletions(-)

Tested-by Andrew Bowers <[email protected]>