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

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

[Jesse Gross]

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 <eric.dumazet@gmail.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>
---
 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

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

[Eric Dumazet]

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 <eric.dumazet@gmail.com>
> Signed-off-by: Jesse Gross <jesse@nicira.com>

Thanks Jesse

Acked-by: Eric Dumazet <eric.dumazet@gmail.com>

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

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

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

[Jesse Gross]

On Tue, Jan 18, 2011 at 1:55 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> 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 <eric.dumazet@gmail.com>
>> Signed-off-by: Jesse Gross <jesse@nicira.com>
>
> Thanks Jesse
>
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
>
> 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?

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

[Michael Chan]

On Mon, 2011-01-17 at 23:12 -0800, Jesse Gross wrote:
> On Tue, Jan 18, 2011 at 1:55 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > 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 <eric.dumazet@gmail.com>
> >> Signed-off-by: Jesse Gross <jesse@nicira.com>
> >
> > Thanks Jesse
> >
> > Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
> >
> > 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.

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

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 18 Jan 2011 07:55:14 +0100

> 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 <eric.dumazet@gmail.com>
>> Signed-off-by: Jesse Gross <jesse@nicira.com>
> 
> Thanks Jesse
> 
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied, thanks.