Skip to contentSkip to conversation

: VLAN TCI truncation fixes

PATCH Applied Patrick McHardy 4 participants 5 messages Jul 6, 2008 – Jul 7, 2008

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

cover : VLAN TCI truncation fixes

These patches fix all drivers supporting VLAN header stripping to pass the complete VLAN TCI to vlan_hwaccel_{rx,receive_skb} instead of just the VID. The upper three bits contain the priority and are used for ingress priority mappings.

The e1000 patch is runtime tested, the others only compile tested.

drivers/net/e1000/e1000_main.c |   12 ++++--------
drivers/net/e1000e/netdev.c    |    3 +--
drivers/net/igb/igb_main.c     |    3 +--
drivers/net/ixgb/ixgb_main.c   |    6 ++----
drivers/net/starfire.c         |    2 +-
drivers/net/tehuti.c           |    2 +-
drivers/net/tehuti.h           |    1 +
7 files changed, 11 insertions(+), 18 deletions(-)

Patrick McHardy (6):
e1000: don't truncate VLAN TCI with VLAN stripping
e1000e: don't truncate VLAN TCI with VLAN stripping
igb: don't truncate VLAN TCI with VLAN stripping
ixgb: don't truncate VLAN TCI with VLAN stripping
starfire: don't truncate VLAN TCI with VLAN stripping
tehuti: don't truncate VLAN TCI with VLAN stripping

Acked-by on the v1 series
#1 lore raw
Quoted 17 lines · Patrick McHardy

These patches fix all drivers supporting VLAN header stripping to pass the complete VLAN TCI to vlan_hwaccel_{rx,receive_skb} instead of just the VID. The upper three bits contain the priority and are used for ingress priority mappings.

The e1000 patch is runtime tested, the others only compile tested.

drivers/net/e1000/e1000_main.c |   12 ++++--------
drivers/net/e1000e/netdev.c    |    3 +--
drivers/net/igb/igb_main.c     |    3 +--
drivers/net/ixgb/ixgb_main.c   |    6 ++----
drivers/net/starfire.c         |    2 +-
drivers/net/tehuti.c           |    2 +-
drivers/net/tehuti.h           |    1 +
7 files changed, 11 insertions(+), 18 deletions(-)
Quoted 7 lines

Patrick McHardy (6):
e1000: don't truncate VLAN TCI with VLAN stripping
e1000e: don't truncate VLAN TCI with VLAN stripping
igb: don't truncate VLAN TCI with VLAN stripping
ixgb: don't truncate VLAN TCI with VLAN stripping
starfire: don't truncate VLAN TCI with VLAN stripping
tehuti: don't truncate VLAN TCI with VLAN stripping

Though I would suggest combining all these patches into a single patch, since they constitute a single logical change, and each change is self-contained and small.

Jeff

#2 lore raw
Quoted 9 lines · Jeff Garzik
Quoted 6 lines · Patrick McHardy

e1000: don't truncate VLAN TCI with VLAN stripping
e1000e: don't truncate VLAN TCI with VLAN stripping
igb: don't truncate VLAN TCI with VLAN stripping
ixgb: don't truncate VLAN TCI with VLAN stripping
starfire: don't truncate VLAN TCI with VLAN stripping
tehuti: don't truncate VLAN TCI with VLAN stripping

Thanks.

Quoted 2 lines

Though I would suggest combining all these patches into a single patch, since they constitute a single logical change, and each change is self-contained and small.

Attached is a combined patch with these 6 patches.

commit c9825e6591be8de5f93e43f115c08ed0ab80685e

Author: Patrick McHardy <[email protected]>
Date:   Mon Jul 7 04:12:12 2008 +0200

netdrv: don't truncate VLAN TCI with VLAN stripping

The vlan_hwaccel_{rx,receive_skb} functions expect the full TCI field for priority mappings, don't truncate the upper 4 bits.

drivers/net/e1000/e1000_main.c +4 -8
index 311ca26..7aa458f 100644
@@ -4277,8 +4277,7 @@ e1000_clean_rx_irq(struct e1000_adapter *adapter,
 		if (unlikely(adapter->vlgrp &&
 			    (status & E1000_RXD_STAT_VP))) {
 			vlan_hwaccel_receive_skb(skb, adapter->vlgrp,
-						 le16_to_cpu(rx_desc->special) &
-						 E1000_RXD_SPC_VLAN_MASK);
+						 le16_to_cpu(rx_desc->special));
 		} else {
 			netif_receive_skb(skb);
 		}
@@ -4286,8 +4285,7 @@ e1000_clean_rx_irq(struct e1000_adapter *adapter,
 		if (unlikely(adapter->vlgrp &&
 			    (status & E1000_RXD_STAT_VP))) {
 			vlan_hwaccel_rx(skb, adapter->vlgrp,
-					le16_to_cpu(rx_desc->special) &
-					E1000_RXD_SPC_VLAN_MASK);
+					le16_to_cpu(rx_desc->special));
 		} else {
 			netif_rx(skb);
 		}
@@ -4464,16 +4462,14 @@ copydone:
 #ifdef CONFIG_E1000_NAPI
 		if (unlikely(adapter->vlgrp && (staterr & E1000_RXD_STAT_VP))) {
 			vlan_hwaccel_receive_skb(skb, adapter->vlgrp,
-				le16_to_cpu(rx_desc->wb.middle.vlan) &
-				E1000_RXD_SPC_VLAN_MASK);
+				le16_to_cpu(rx_desc->wb.middle.vlan));
 		} else {
 			netif_receive_skb(skb);
 		}
 #else /* CONFIG_E1000_NAPI */
 		if (unlikely(adapter->vlgrp && (staterr & E1000_RXD_STAT_VP))) {
 			vlan_hwaccel_rx(skb, adapter->vlgrp,
-				le16_to_cpu(rx_desc->wb.middle.vlan) &
-				E1000_RXD_SPC_VLAN_MASK);
+				le16_to_cpu(rx_desc->wb.middle.vlan));
 		} else {
 			netif_rx(skb);
 		}
drivers/net/e1000e/netdev.c +1 -2
index acdd616..80592bd 100644
@@ -98,8 +98,7 @@ static void e1000_receive_skb(struct e1000_adapter *adapter,
 
 	if (adapter->vlgrp && (status & E1000_RXD_STAT_VP))
 		vlan_hwaccel_receive_skb(skb, adapter->vlgrp,
-					 le16_to_cpu(vlan) &
-					 E1000_RXD_SPC_VLAN_MASK);
+					 le16_to_cpu(vlan));
 	else
 		netif_receive_skb(skb);
 
drivers/net/igb/igb_main.c +1 -2
index 7bc6fae..afd4ce3 100644
@@ -3391,8 +3391,7 @@ static void igb_receive_skb(struct igb_adapter *adapter, u8 status, __le16 vlan,
 {
 	if (adapter->vlgrp && (status & E1000_RXD_STAT_VP))
 		vlan_hwaccel_receive_skb(skb, adapter->vlgrp,
-					 le16_to_cpu(vlan) &
-					 E1000_RXD_SPC_VLAN_MASK);
+					 le16_to_cpu(vlan));
 	else
 		netif_receive_skb(skb);
 }
drivers/net/ixgb/ixgb_main.c +2 -4
index cb8dadd..41f3adf 100644
@@ -2045,16 +2045,14 @@ ixgb_clean_rx_irq(struct ixgb_adapter *adapter)
 #ifdef CONFIG_IXGB_NAPI
 		if(adapter->vlgrp && (status & IXGB_RX_DESC_STATUS_VP)) {
 			vlan_hwaccel_receive_skb(skb, adapter->vlgrp,
-				le16_to_cpu(rx_desc->special) &
-					IXGB_RX_DESC_SPECIAL_VLAN_MASK);
+				le16_to_cpu(rx_desc->special));
 		} else {
 			netif_receive_skb(skb);
 		}
 #else /* CONFIG_IXGB_NAPI */
 		if(adapter->vlgrp && (status & IXGB_RX_DESC_STATUS_VP)) {
 			vlan_hwaccel_rx(skb, adapter->vlgrp,
-				le16_to_cpu(rx_desc->special) &
-					IXGB_RX_DESC_SPECIAL_VLAN_MASK);
+				le16_to_cpu(rx_desc->special));
 		} else {
 			netif_rx(skb);
 		}
drivers/net/starfire.c +1 -1
index 7b7b171..2038f38 100644
@@ -1510,7 +1510,7 @@ static int __netdev_rx(struct net_device *dev, int *quota)
 			if (debug > 4)
 				printk(KERN_DEBUG "  netdev_rx() vlanid = %d\n", le16_to_cpu(desc->vlanid));
 			/* vlan_netdev_receive_skb() expects a packet with the VLAN tag stripped out */
-			vlan_netdev_receive_skb(skb, np->vlgrp, le16_to_cpu(desc->vlanid) & VLAN_VID_MASK);
+			vlan_netdev_receive_skb(skb, np->vlgrp, le16_to_cpu(desc->vlanid));
 		} else
 #endif /* VLAN_SUPPORT */
 			netdev_receive_skb(skb);
drivers/net/tehuti.c +1 -1
index 432e837..91f9054 100644
@@ -1165,7 +1165,7 @@ NETIF_RX_MUX(struct bdx_priv *priv, u32 rxd_val1, u16 rxd_vlan,
 					  GET_RXD_VLAN_ID(rxd_vlan))->name);
 		/* NAPI variant of receive functions */
 		vlan_hwaccel_receive_skb(skb, priv->vlgrp,
-					 GET_RXD_VLAN_ID(rxd_vlan));
+					 GET_RXD_VLAN_TCI(rxd_vlan));
 	} else {
 		netif_receive_skb(skb);
 	}
drivers/net/tehuti.h +1 -0
index efd170f..c66dfc9 100644
@@ -309,6 +309,7 @@ struct rxf_desc {
 #define GET_RXD_PKT_ID(x)		GET_BITS_SHIFT((x), 3, 28)
 #define GET_RXD_VTAG(x)			GET_BITS_SHIFT((x), 1, 31)
 #define GET_RXD_VLAN_ID(x)		GET_BITS_SHIFT((x), 12, 0)
+#define GET_RXD_VLAN_TCI(x)		GET_BITS_SHIFT((x), 16, 0)
 #define GET_RXD_CFI(x)			GET_BITS_SHIFT((x), 1, 12)
 #define GET_RXD_PRIO(x)			GET_BITS_SHIFT((x), 3, 13)
 
maintainer Applied
#3 lore raw

From: Patrick McHardy <[email protected]>
Date: Mon, 07 Jul 2008 04:11:21 +0200

Quoted 9 lines
Quoted 8 lines · Jeff Garzik
Quoted 6 lines · Patrick McHardy

e1000: don't truncate VLAN TCI with VLAN stripping
e1000e: don't truncate VLAN TCI with VLAN stripping
igb: don't truncate VLAN TCI with VLAN stripping
ixgb: don't truncate VLAN TCI with VLAN stripping
starfire: don't truncate VLAN TCI with VLAN stripping
tehuti: don't truncate VLAN TCI with VLAN stripping

Quoted 8 lines
Quoted 1 line

Thanks.

Quoted 4 lines

Though I would suggest combining all these patches into a single patch, since they constitute a single logical change, and each change is self-contained and small.

Attached is a combined patch with these 6 patches.

Applied, thanks everyone.

Acked-by
#4 lore raw
Quoted 15 lines · David Miller

From: Patrick McHardy <[email protected]>
Date: Mon, 07 Jul 2008 04:11:21 +0200

Quoted 12 lines
Quoted 9 lines · Jeff Garzik
Quoted 6 lines · Patrick McHardy

e1000: don't truncate VLAN TCI with VLAN stripping
e1000e: don't truncate VLAN TCI with VLAN stripping
igb: don't truncate VLAN TCI with VLAN stripping
ixgb: don't truncate VLAN TCI with VLAN stripping
starfire: don't truncate VLAN TCI with VLAN stripping
tehuti: don't truncate VLAN TCI with VLAN stripping

Thanks.

Quoted 1 line
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines

Though I would suggest combining all these patches into a single patch, since they constitute a single logical change, and each change is self-contained and small.

Attached is a combined patch with these 6 patches.

Applied, thanks everyone.

Although it appears to be to late, since Dave has already pulled these in.

Signature · 2 lines
Cheers,
Jeff