| Message ID | 20101026215933.2339.45454.stgit@jf-dev1-dcblab |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On Tue, Oct 26, 2010 at 2:59 PM, John Fastabend <john.r.fastabend@intel.com> wrote: > Toggling the vlan tx|rx hw offloads needs to set the hard_header_len > as well otherwise we end up using LL_RESERVED_SPACE incorrectly. > This results in pskb_expand_head() being used unnecessarily. > > This add a check in vlan_transfer_features to catch the ETH_FLAG_TXVLAN > flag and set the header length. This requires drivers to add the > ETH_FLAG_TXVLAN to vlan_features. > > Signed-off-by: John Fastabend <john.r.fastabend@intel.com> I think this addresses all of the original problems. However, I don't think that we want to have drivers claim to support vlan offloading as a feature for vlan packets. That implies some type of QinQ functionality to me. In addition, if the vlan device claims to support offloading and a second vlan device is stacked on top of it, then the two will clobber skb->vlan_tci. It's probably simpler to just keep track of whether vlan offloading is currently enabled so we can find out whether it changed. > --- > > net/8021q/vlan.c | 10 ++++++++++ > 1 files changed, 10 insertions(+), 0 deletions(-) > > diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c > index 05b867e..825011b 100644 > --- a/net/8021q/vlan.c > +++ b/net/8021q/vlan.c > @@ -334,6 +334,16 @@ static void vlan_transfer_features(struct net_device *dev, > vlandev->features &= ~dev->vlan_features; > vlandev->features |= dev->features & dev->vlan_features; > vlandev->gso_max_size = dev->gso_max_size; > + > + /* is ETH_FLAGS_TXVLAN being toggled */ > + if ((vlandev->features & ETH_FLAG_TXVLAN) ^ > + (old_features & ETH_FLAG_TXVLAN)) { > + if (vlandev->features & ETH_FLAG_TXVLAN) > + vlandev->hard_header_len -= VLAN_HLEN; > + else > + vlandev->hard_header_len += VLAN_HLEN; > + } The correct flag for dev->features is NETIF_F_HW_VLAN_TX. ETH_FLAGS_TXVLAN is an ethtool construct (that happens to have the same value). 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/8021q/vlan.c b/net/8021q/vlan.c index 05b867e..825011b 100644 --- a/net/8021q/vlan.c +++ b/net/8021q/vlan.c @@ -334,6 +334,16 @@ static void vlan_transfer_features(struct net_device *dev, vlandev->features &= ~dev->vlan_features; vlandev->features |= dev->features & dev->vlan_features; vlandev->gso_max_size = dev->gso_max_size; + + /* is ETH_FLAGS_TXVLAN being toggled */ + if ((vlandev->features & ETH_FLAG_TXVLAN) ^ + (old_features & ETH_FLAG_TXVLAN)) { + if (vlandev->features & ETH_FLAG_TXVLAN) + vlandev->hard_header_len -= VLAN_HLEN; + else + vlandev->hard_header_len += VLAN_HLEN; + } + #if defined(CONFIG_FCOE) || defined(CONFIG_FCOE_MODULE) vlandev->fcoe_ddp_xid = dev->fcoe_ddp_xid; #endif
Toggling the vlan tx|rx hw offloads needs to set the hard_header_len as well otherwise we end up using LL_RESERVED_SPACE incorrectly. This results in pskb_expand_head() being used unnecessarily. This add a check in vlan_transfer_features to catch the ETH_FLAG_TXVLAN flag and set the header length. This requires drivers to add the ETH_FLAG_TXVLAN to vlan_features. Signed-off-by: John Fastabend <john.r.fastabend@intel.com> --- net/8021q/vlan.c | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-) -- 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