The Yukon FE+ chip appears to have a hardware glitch that causes bogus
receive status values to be posted. The data in the packet is good, but
the status value is random garbage.  As a temporary workaround until the
problem is better understood, implement the workaround the vendor driver
used of ignoring the status value on this chip.

Since this means trusting dodgy hardware values; add additional checking
of the receive packet length.

Patch against 2.6.23-rc8, please apply before 2.6.23 release (or it will
need to go to stable).

Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>

--- a/drivers/net/sky2.c	2007-09-26 14:34:34.000000000 -0700
+++ b/drivers/net/sky2.c	2007-09-26 16:57:31.000000000 -0700
@@ -2148,6 +2148,18 @@ static struct sk_buff *sky2_receive(stru
 	sky2->rx_next = (sky2->rx_next + 1) % sky2->rx_pending;
 	prefetch(sky2->rx_ring + sky2->rx_next);
 
+	if (length < ETH_ZLEN || length > sky2->rx_data_size)
+		goto len_error;
+
+	/* This chip has hardware problems that generates bogus status.
+	 * So do only marginal checking and expect higher level protocols
+	 * to handle crap frames.
+	 */
+	if (sky2->hw->chip_id == CHIP_ID_YUKON_FE_P &&
+	    sky2->hw->chip_rev == CHIP_REV_YU_FE2_A0 &&
+	    length != count)
+		goto okay;
+
 	if (status & GMR_FS_ANY_ERR)
 		goto error;
 
@@ -2156,8 +2168,9 @@ static struct sk_buff *sky2_receive(stru
 
 	/* if length reported by DMA does not match PHY, packet was truncated */
 	if (length != count)
-		goto len_mismatch;
+		goto len_error;
 
+okay:
 	if (length < copybreak)
 		skb = receive_copy(sky2, re, length);
 	else
@@ -2167,13 +2180,13 @@ resubmit:
 
 	return skb;
 
-len_mismatch:
+len_error:
 	/* Truncation of overlength packets
 	   causes PHY length to not match MAC length */
 	++sky2->net_stats.rx_length_errors;
 	if (netif_msg_rx_err(sky2) && net_ratelimit())
-		pr_info(PFX "%s: rx length mismatch: length %d status %#x\n",
-			dev->name, length, status);
+		pr_info(PFX "%s: ...
[ message continues ]

Hi Stephen,


Shouldn't the condition be "length == count"?

I hope this helps,
Jochen
--
http://seehuhn.de/


-

[ message continues ]

On Thu, 27 Sep 2007 09:14:11 +0100

No, the code is correct as is.  Basically if length == count, then
the status field is correct, and the driver can go ahead and use it.
If length != count, then the status is bogus but the data is okay.


-- 
Stephen Hemminger <shemminger@linux-foundation.org>
-

[ message continues ]

Hi,


Oh, I see.  Thanks for the explanation.

All the best,
Jochen
--=20
http://seehuhn.de/
[ message continues ]

The FE+ workaround means the driver can no longer trust the status register
to indicate VLAN tagged frames.  The fix for this is to just disable VLAN
acceleration for that chip version. Tested and works fine.

This patch applies to 2.6.23-rc8 after yesterday's patch:
      sky2 FE+ receive status workaround

Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>

--- a/drivers/net/sky2.c	2007-09-27 08:45:13.000000000 -0700
+++ b/drivers/net/sky2.c	2007-09-27 09:43:15.000000000 -0700
@@ -3970,8 +3970,12 @@ static __devinit struct net_device *sky2
 		dev->features |= NETIF_F_HIGHDMA;
 
 #ifdef SKY2_VLAN_TAG_USED
-	dev->features |= NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX;
-	dev->vlan_rx_register = sky2_vlan_rx_register;
+	/* The workaround for FE+ status conflicts with VLAN tag detection. */
+	if (!(sky2->hw->chip_id == CHIP_ID_YUKON_FE_P &&
+	      sky2->hw->chip_rev == CHIP_REV_YU_FE2_A0)) {
+		dev->features |= NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX;
+		dev->vlan_rx_register = sky2_vlan_rx_register;
+	}
 #endif
 
 	/* read the mac address */
-

[ message continues ]

Applied.  Please always put meta-information such as the above quoted 
after the "---" patch description terminator, so that it is not copied 
into the permanent kernel changelog.  This text must be hand-edited out 
of each patch, before application.

See item #14 of Documentation/SubmittingPatches.

-

[ message continues ]

This should fix http://bugzilla.kernel.org/show_bug.cgi?id=8667

After resume, driver has reset the chip so the current state
of transmit checksum offload state machine and DMA state machine
will be undefined.

The fix is to set the state so that first Tx will set MSS and offset
values.

Patch is against 2.6.23-rc8 after last patch:
 sky2: FE+ vlan workaround

(Should also work on older releases with minor fuzz).

Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>

--- a/drivers/net/sky2.c	2007-09-27 08:45:13.000000000 -0700
+++ b/drivers/net/sky2.c	2007-09-27 09:39:49.000000000 -0700
@@ -910,6 +910,20 @@ static inline struct sky2_tx_le *get_tx_
 	return le;
 }
 
+static void tx_init(struct sky2_port *sky2)
+{
+	struct sky2_tx_le *le;
+
+	sky2->tx_prod = sky2->tx_cons = 0;
+	sky2->tx_tcpsum = 0;
+	sky2->tx_last_mss = 0;
+
+	le = get_tx_le(sky2);
+	le->addr = 0;
+	le->opcode = OP_ADDR64 | HW_OWNER;
+	sky2->tx_addr64 = 0;
+}
+
 static inline struct tx_ring_info *tx_le_re(struct sky2_port *sky2,
 					    struct sky2_tx_le *le)
 {
@@ -1320,7 +1334,8 @@ static int sky2_up(struct net_device *de
 				GFP_KERNEL);
 	if (!sky2->tx_ring)
 		goto err_out;
-	sky2->tx_prod = sky2->tx_cons = 0;
+
+	tx_init(sky2);
 
 	sky2->rx_le = pci_alloc_consistent(hw->pdev, RX_LE_BYTES,
 					   &sky2->rx_le_map);
-

[ message continues ]

Applied.  Please always put meta-information such as the above quoted 
after the "---" patch description terminator, so that it is not copied 
into the permanent kernel changelog.  This text must be hand-edited out 
of each patch, before application.

See item #14 of Documentation/SubmittingPatches.


-

[ message continues ]

Applied.  Please always put meta-information such as the above quoted 
after the "---" patch description terminator, so that it is not copied 
into the permanent kernel changelog.  This text must be hand-edited out 
of each patch, before application.

-

[ message continues ]