Skip to contentSkip to conversation

be2net: Ignore spurious UE indication from NIC

PATCH In review Ajit Khaparde 2 participants 4 messages Sep 21, 2012 – Sep 26, 2012

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 be2net: Ignore spurious UE indication from NIC

Ignore spurious UE indication seen on some platforms. Consider the error as un-recoverable only when the bits stay high during second sampling.

---
 drivers/net/ethernet/emulex/benet/be.h      |    2 ++
 drivers/net/ethernet/emulex/benet/be_main.c |   18 +++++++++++++++---
 2 files changed, 17 insertions(+), 3 deletions(-)
drivers/net/ethernet/emulex/benet/be.h +2 -0
index 5b622993..3d4a7bc 100644
@@ -401,6 +401,8 @@ struct be_adapter {
 	bool eeh_error;
 	bool fw_timeout;
 	bool hw_error;
+	u32 ue_lo;
+	u32 ue_hi;
 
 	u32 port_num;
 	bool promiscuous;
drivers/net/ethernet/emulex/benet/be_main.c +15 -3
index 84379f4..e970f77 100644
@@ -2105,6 +2105,7 @@ void be_detect_error(struct be_adapter *adapter)
 	u32 ue_lo = 0, ue_hi = 0, ue_lo_mask = 0, ue_hi_mask = 0;
 	u32 sliport_status = 0, sliport_err1 = 0, sliport_err2 = 0;
 	u32 i;
+	struct device *dev = &adapter->pdev->dev;
 
 	if (be_crit_error(adapter))
 		return;
@@ -2129,13 +2130,22 @@ void be_detect_error(struct be_adapter *adapter)
 
 		ue_lo = (ue_lo & ~ue_lo_mask);
 		ue_hi = (ue_hi & ~ue_hi_mask);
+		if (ue_lo != adapter->ue_lo || ue_hi != adapter->ue_hi) {
+			dev_err(dev, "UE read: 0x%x/0x%x\n", ue_lo, ue_hi);
+			goto done;
+		}
+	}
+
+	if (ue_lo == 0xffffffff || ue_hi == 0xffffffff) {
+		adapter->eeh_error = true;
+		dev_err(dev, "PCI slot disconnected\n");
+		goto done;
 	}
 
 	if (ue_lo || ue_hi ||
 		sliport_status & SLIPORT_STATUS_ERR_MASK) {
 		adapter->hw_error = true;
-		dev_err(&adapter->pdev->dev,
-			"Error detected in the card\n");
+		dev_err(dev, "UE detected\n");
 	}
 
 	if (sliport_status & SLIPORT_STATUS_ERR_MASK) {
@@ -2162,7 +2172,9 @@ void be_detect_error(struct be_adapter *adapter)
 				"UE: %s bit set\n", ue_status_hi_desc[i]);
 		}
 	}
-
+done:
+	adapter->ue_lo = ue_lo;
+	adapter->ue_hi = ue_hi;
 }
 
 static void be_msix_disable(struct be_adapter *adapter)
Signature · 1 line
1.7.9.5
maintainer
#1 lore raw

From: Ajit Khaparde <[email protected]>
Date: Fri, 21 Sep 2012 11:36:20 -0500

Quoted 5 lines

Ignore spurious UE indication seen on some platforms. Consider the error as un-recoverable only when the bits stay high during second sampling.

Treating uncorrectable errors as spurious seems like an invitation for hard to track down data corruption to me.

You'll need to come up with a more sophisticated test for spurious other than "happens more than once" before I'm willing to subject the entire world to this kind of potential problem.

#2 lore raw
Quoted 13 lines

From: David Miller [[email protected]]
Sent: Friday, September 21, 2012 2:04 PM
To: Khaparde, Ajit
Cc: [email protected]
Subject: Re: [PATCH net-next] be2net: Ignore spurious UE indication from NIC

From: Ajit Khaparde <[email protected]>
Date: Fri, 21 Sep 2012 11:36:20 -0500

Quoted 4 lines

Ignore spurious UE indication seen on some platforms. Consider the error as un-recoverable only when the bits stay high during second sampling.

Quoted 8 lines
Quoted 1 line

Treating uncorrectable errors as spurious seems like an invitation for hard to track down data corruption to me.

You'll need to come up with a more sophisticated test for spurious other than "happens more than once" before I'm willing to subject the entire world to this kind of potential problem.

If the UE is real, then the hardware will stop responding to requests. The hardware block goes offline automatically and no traffic will flow. After this the hardware will generate a register dump, which can be retrived using ethtool.

A spurious UE or a data corruption will not generate this dump.

The detection logic is merely to inform the user about the failure and also avoid any further access to the NIC. This will also prevent the driver unload from having to timeout on each access to the hardware which could take a long time to complete.

Please let me know if this is enough to differentiate the scenarios.

Thanks
-Ajit

maintainer
#3 lore raw

From: "Khaparde, Ajit" <[email protected]>
Date: Wed, 26 Sep 2012 22:35:14 +0000

Quoted 27 lines
Quoted 13 lines

From: David Miller [[email protected]]
Sent: Friday, September 21, 2012 2:04 PM
To: Khaparde, Ajit
Cc: [email protected]
Subject: Re: [PATCH net-next] be2net: Ignore spurious UE indication from NIC

From: Ajit Khaparde <[email protected]>
Date: Fri, 21 Sep 2012 11:36:20 -0500

Quoted 4 lines

Ignore spurious UE indication seen on some platforms. Consider the error as un-recoverable only when the bits stay high during second sampling.

Quoted 8 lines
Quoted 1 line

Treating uncorrectable errors as spurious seems like an invitation for hard to track down data corruption to me.

You'll need to come up with a more sophisticated test for spurious other than "happens more than once" before I'm willing to subject the entire world to this kind of potential problem.

If the UE is real, then the hardware will stop responding to requests. The hardware block goes offline automatically and no traffic will flow. After this the hardware will generate a register dump, which can be retrived using ethtool.

Quoted 4 lines

A spurious UE or a data corruption will not generate this dump.

The detection logic is merely to inform the user about the failure and also avoid any further access to the NIC.

So it sounds more like the UE test should be removed completely since you cannot rely upon it, and if a real UE happens the hardware will stop and you will already report the problem when the hardware block goes offline.

Furthermore, it also implies that you can test if the hardware block is offline to validate the UE indication.

All of which says irrefutably that your patch is still the wrong way to do this.