[PATCH] net offloading: Do not mask out NETIF_F_HW_VLAN_TX for vlan.

In netif_skb_features() we return only the features that are valid for vlans
if we have a vlan packet.  However, we should not mask out NETIF_F_HW_VLAN_TX
since it enables transmission of vlan tags and is obviously valid.

Reported-by: Eric Dumazet <redacted>
Signed-off-by: Jesse Gross <redacted>
---
 net/core/dev.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 83507c2..4c58d11 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c

@@ -2023,13 +2023,13 @@ int netif_skb_features(struct sk_buff *skb)
 		return harmonize_features(skb, protocol, features);
 	}
 
-	features &= skb->dev->vlan_features;
+	features &= (skb->dev->vlan_features | NETIF_F_HW_VLAN_TX);
 
 	if (protocol != htons(ETH_P_8021Q)) {
 		return harmonize_features(skb, protocol, features);
 	} else {
 		features &= NETIF_F_SG | NETIF_F_HIGHDMA | NETIF_F_FRAGLIST |
-				NETIF_F_GEN_CSUM;
+				NETIF_F_GEN_CSUM | NETIF_F_HW_VLAN_TX;
 		return harmonize_features(skb, protocol, features);
 	}
 }

-- 
1.7.1

Le lundi 17 janvier 2011 à 22:46 -0800, Jesse Gross a écrit :

In netif_skb_features() we return only the features that are valid for vlans
if we have a vlan packet.  However, we should not mask out NETIF_F_HW_VLAN_TX
since it enables transmission of vlan tags and is obviously valid.

Reported-by: Eric Dumazet <redacted>
Signed-off-by: Jesse Gross <redacted>

Thanks Jesse

Acked-by: Eric Dumazet <redacted>

Now back to the "ethtool -K eth0 txvlan off" problem on bnx2

Is it a driver/software problem or hardware/firmware one ?

On Tue, Jan 18, 2011 at 1:55 AM, Eric Dumazet [off-list ref] wrote:

Le lundi 17 janvier 2011 à 22:46 -0800, Jesse Gross a écrit :

quoted

In netif_skb_features() we return only the features that are valid for vlans
if we have a vlan packet.  However, we should not mask out NETIF_F_HW_VLAN_TX
since it enables transmission of vlan tags and is obviously valid.

Reported-by: Eric Dumazet <redacted>
Signed-off-by: Jesse Gross <redacted>

Thanks Jesse

Acked-by: Eric Dumazet <redacted>

Now back to the "ethtool -K eth0 txvlan off" problem on bnx2

Is it a driver/software problem or hardware/firmware one ?

CC'ing Michael Chan

It looks like bnx2 is storing the offsets of various headers so the
hardware can find them for TSO.  The parsing logic doesn't do anything
for vlan tags, so the hardware gets confused if one is present in the
packet itself.

Quick fix is to simply disallow disabling TX vlan offload or disable
TSO at the same time or some other Ethtool game.  However, if the
hardware supports it then it would be nicer to fix up the TSO setup
logic.  Maybe we can just add the size of the vlan tag to the offset
but I am not certain.  Michael, do you know if this is possible?

On Mon, 2011-01-17 at 23:12 -0800, Jesse Gross wrote:

On Tue, Jan 18, 2011 at 1:55 AM, Eric Dumazet [off-list ref] wrote:

quoted

Le lundi 17 janvier 2011 à 22:46 -0800, Jesse Gross a écrit :

quoted

In netif_skb_features() we return only the features that are valid for vlans
if we have a vlan packet.  However, we should not mask out NETIF_F_HW_VLAN_TX
since it enables transmission of vlan tags and is obviously valid.

Reported-by: Eric Dumazet <redacted>
Signed-off-by: Jesse Gross <redacted>

Thanks Jesse

Acked-by: Eric Dumazet <redacted>

Now back to the "ethtool -K eth0 txvlan off" problem on bnx2

Is it a driver/software problem or hardware/firmware one ?

CC'ing Michael Chan

It looks like bnx2 is storing the offsets of various headers so the
hardware can find them for TSO.  The parsing logic doesn't do anything
for vlan tags, so the hardware gets confused if one is present in the
packet itself.

Yeah, I don't think the hardware/firmware can replicate the vlan tag +
headers properly for TSO if there is a VLAN tag in the packet.  Even
simple tx checksum offload may have problem, but I'll need to check.

Quick fix is to simply disallow disabling TX vlan offload or disable
TSO at the same time or some other Ethtool game.  However, if the
hardware supports it then it would be nicer to fix up the TSO setup
logic.  Maybe we can just add the size of the vlan tag to the offset
but I am not certain.  Michael, do you know if this is possible?

I doubt it, as the VLAN tag needs to be replicated for each transmitted
packet.  I'll check with the firmware/hardware guys.

Thanks.

From: Eric Dumazet <redacted>
Date: Tue, 18 Jan 2011 07:55:14 +0100

Le lundi 17 janvier 2011 à 22:46 -0800, Jesse Gross a écrit :

quoted

In netif_skb_features() we return only the features that are valid for vlans
if we have a vlan packet.  However, we should not mask out NETIF_F_HW_VLAN_TX
since it enables transmission of vlan tags and is obviously valid.

Reported-by: Eric Dumazet <redacted>
Signed-off-by: Jesse Gross <redacted>

Thanks Jesse

Acked-by: Eric Dumazet <redacted>

Applied, thanks.