Re: [E1000-devel] BUG? strange behavior with vlans and virtual functions on i350 -- possible solution

[Chris F. <chr...@ge...>]

There seems to be a bug in the igbvf sourceforge driver. In igbvf_receive_skb()
we're ANDing the big-endian vlan ID with the little-endian mask.

Also, I think the criteria for setting IGBVF_FLAG_RX_LB_VLAN_BSWAP is incorrect.
The 82576 datasheet also indicates that the VLAN tag is big endian.  Since both
currently-supported devices are big-endian, maybe this should be the default?

Accordingly, I think the following patch may be in order:


diff --git a/src/netdev.c b/src/netdev.c
index a18f000..3834c69 100644
--- a/src/netdev.c
+++ b/src/netdev.c
@@ -127,9 +127,9 @@ static void igbvf_receive_skb(struct igbvf_adapter *adapter,
 	if (status & E1000_RXD_STAT_VP) {
 		if ((adapter->flags & IGBVF_FLAG_RX_LB_VLAN_BSWAP) &&
 		    (status & E1000_RXDEXT_STATERR_LB))
-			vid = be16_to_cpu(vlan & E1000_RXD_SPC_VLAN_MASK);
+			vid = be16_to_cpu(vlan) & E1000_RXD_SPC_VLAN_MASK;
 		else
-			vid = le16_to_cpu(vlan & E1000_RXD_SPC_VLAN_MASK);
+			vid = le16_to_cpu(vlan) & E1000_RXD_SPC_VLAN_MASK;
 	}
 	/*
 	 * On some adapters, trunk VLANs are incorrectly indicated in the
@@ -2944,9 +2944,8 @@ static int __devinit igbvf_probe(struct pci_dev *pdev,
 	/* reset the hardware with the new settings */
 	igbvf_reset(adapter);
 
-	/* set hardware-specific flags */
-	if (adapter->hw.mac.type == e1000_vfadapt_i350)
-		adapter->flags |= IGBVF_FLAG_RX_LB_VLAN_BSWAP;
+	/* both 82576 and i350 store vlan ID in network order */
+	adapter->flags |= IGBVF_FLAG_RX_LB_VLAN_BSWAP;
 
 	/* tell the stack to leave us alone until igbvf_open() is called */
 	netif_carrier_off(netdev);


Incidentally, the igbvf driver in the current mainline sources is missing support
for IGBVF_FLAG_RX_LB_VLAN_BSWAP so it will break when using VLAN in a VF.  I'll
be sending in a patch.

Chris