All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Hemminger <shemminger@vyatta.com>
To: "Niccolò Belli" <darkbasic@linuxsystems.it>
Cc: netdev@vger.kernel.org
Subject: [PATCH] sky2:  fix receive length error in mixed non-VLAN/VLAN traffic
Date: Fri, 27 Apr 2012 20:03:10 -0700	[thread overview]
Message-ID: <20120427200310.60325fb1@nehalam.linuxnetplumber.net> (raw)
In-Reply-To: <4F9B2641.9070209@linuxsystems.it>

Bug: The VLAN bit of the MAC RX Status Word is unreliable in several older
supported chips. Sometimes the VLAN bit is not set for valid VLAN packets
and also sometimes the VLAN bit is set for non-VLAN packets that came after
a VLAN packet. This results in a receive length error when VLAN hardware
tagging is enabled.

Fix: The driver uses only VLAN information included in VLAN status list elements,
that signals that the VLAN tag field is valid. It must ignore the VLAN bit in the
MAC RX Status Word. An additional variable set when evaluating the VLAN opcodes
is used to indicate that the received packet is a VLAN packet and a packet length
correction (subtraction of VLAN header length) must be done.

This is an alternative to Mirko's patch that relies on the new method of
encoding VLAN tag, and therefore means extra code can be removed.

Testing: Tested with regular and VLAN traffic on 88E8056, but this chip
does not have the bug, just proves that the alternative method of handling
VLAN tag will work.

Reported-by: Mirko Lindner <mlindner@marvell.com>
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

---
This version is built and tested against net-next (3.4-rc4)

--- a/drivers/net/ethernet/marvell/sky2.c	2012-04-27 19:37:31.891137630 -0700
+++ b/drivers/net/ethernet/marvell/sky2.c	2012-04-27 19:42:54.242757283 -0700
@@ -2568,6 +2568,14 @@ nobuf:
 	return NULL;
 }
 
+/* Apply vlan tag to the current Rx skb */
+static inline void sky2_rx_tag(struct sky2_port *sky2, u16 tag)
+{
+	struct rx_ring_info *re = sky2->rx_ring + sky2->rx_next;
+
+	__vlan_hwaccel_put_tag(re->skb, be16_to_cpu(tag));
+}
+
 /*
  * Receive one packet.
  * For larger packets, get new buffer.
@@ -2580,7 +2588,7 @@ static struct sk_buff *sky2_receive(stru
 	struct sk_buff *skb = NULL;
 	u16 count = (status & GMR_FS_LEN) >> 16;
 
-	if (status & GMR_FS_VLAN)
+	if (vlan_tx_tag_present(re->skb))
 		count -= VLAN_HLEN;	/* Account for vlan tag */
 
 	netif_printk(sky2, rx_status, KERN_DEBUG, dev,
@@ -2646,12 +2654,9 @@ static inline void sky2_tx_done(struct n
 	}
 }
 
-static inline void sky2_skb_rx(const struct sky2_port *sky2,
-			       u32 status, struct sk_buff *skb)
+static inline void sky2_skb_rx(struct sky2_port *sky2,
+			       struct sk_buff *skb)
 {
-	if (status & GMR_FS_VLAN)
-		__vlan_hwaccel_put_tag(skb, be16_to_cpu(sky2->rx_tag));
-
 	if (skb->ip_summed == CHECKSUM_NONE)
 		netif_receive_skb(skb);
 	else
@@ -2764,7 +2769,7 @@ static int sky2_status_intr(struct sky2_
 
 			skb->protocol = eth_type_trans(skb, dev);
 
-			sky2_skb_rx(sky2, status, skb);
+			sky2_skb_rx(sky2, skb);
 
 			/* Stop after net poll weight */
 			if (++work_done >= to_do)
@@ -2772,11 +2777,11 @@ static int sky2_status_intr(struct sky2_
 			break;
 
 		case OP_RXVLAN:
-			sky2->rx_tag = length;
+			sky2_rx_tag(sky2, length);
 			break;
 
 		case OP_RXCHKSVLAN:
-			sky2->rx_tag = length;
+			sky2_rx_tag(sky2, length);
 			/* fall through */
 		case OP_RXCHKS:
 			if (likely(dev->features & NETIF_F_RXCSUM))
--- a/drivers/net/ethernet/marvell/sky2.h	2012-04-27 19:37:31.891137630 -0700
+++ b/drivers/net/ethernet/marvell/sky2.h	2012-04-27 19:41:01.857495110 -0700
@@ -2241,7 +2241,6 @@ struct sky2_port {
 	u16		     rx_pending;
 	u16		     rx_data_size;
 	u16		     rx_nfrags;
-	u16		     rx_tag;
 
 	struct {
 		unsigned long last;

  parent reply	other threads:[~2012-04-28  3:03 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-27 20:15 sky2 still badly broken Niccolò Belli
2012-04-27 20:41 ` [RFT] sky2: fix status length check on older chips Stephen Hemminger
2012-04-27 20:48   ` David Miller
2012-04-27 20:55   ` Stephen Hemminger
2012-04-27 23:05     ` Niccolò Belli
2012-04-28  2:46       ` Stephen Hemminger
2012-04-28 10:25         ` Niccolò Belli
2012-04-30 16:19           ` Stephen Hemminger
2012-04-30 18:23             ` Niccolò Belli
2012-04-28  3:03       ` Stephen Hemminger [this message]
2012-04-28  3:05         ` [PATCH] sky2: fix receive length error in mixed non-VLAN/VLAN traffic Stephen Hemminger
2012-09-17 14:20     ` [RFT] sky2: fix status length check on older chips Ivan Minev
2012-04-30 19:25 ` sky2 still badly broken Stephen Hemminger
2012-04-30 23:50   ` Niccolò Belli
2012-05-02 15:12   ` Niccolò Belli
2012-05-02 18:56     ` Stephen Hemminger
2012-05-03  9:40       ` Niccolò Belli
2012-05-03 15:23         ` Stephen Hemminger
2012-05-03 18:06           ` Niccolò Belli
2012-05-03 18:15             ` Stephen Hemminger
2012-05-03 19:26               ` Niccolò Belli
2012-05-03 19:36           ` Niccolò Belli
2012-05-03 21:25             ` Stephen Hemminger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120427200310.60325fb1@nehalam.linuxnetplumber.net \
    --to=shemminger@vyatta.com \
    --cc=darkbasic@linuxsystems.it \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.