| Message ID | 1295333160-1667-1-git-send-email-jesse@nicira.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
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 ? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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); } }
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(-)