| Message ID | 1395865711-3347-1-git-send-email-vyasevic@redhat.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Vlad Yasevich <vyasevic@redhat.com> Date: Wed, 26 Mar 2014 16:28:31 -0400 > Some drivers incorrectly assign vlan acceleration features to > vlan_features thus causing issues for Q-in-Q vlan configurations. > Prevent this once and for all by masking off acceleration features > for vlan devices until such time as we support stacked acceleration. > > Signed-off-by: Vlad Yasevich <vyasevic@redhat.com> So what if we do have chips that can offload Q-in-Q? Will we use a new flag? -- 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 Wed, Mar 26, 2014 at 05:10:38PM -0400, David Miller wrote: > From: Vlad Yasevich <vyasevic@redhat.com> > Date: Wed, 26 Mar 2014 16:28:31 -0400 > > > Some drivers incorrectly assign vlan acceleration features to > > vlan_features thus causing issues for Q-in-Q vlan configurations. > > Prevent this once and for all by masking off acceleration features > > for vlan devices until such time as we support stacked acceleration. > > > > Signed-off-by: Vlad Yasevich <vyasevic@redhat.com> > > So what if we do have chips that can offload Q-in-Q? Will we use > a new flag? For 802.1ad that was the intention, using the NETIF_F_HW_VLAN_STAG_* flags. -- 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 03/26/2014 05:15 PM, Patrick McHardy wrote: > On Wed, Mar 26, 2014 at 05:10:38PM -0400, David Miller wrote: >> From: Vlad Yasevich <vyasevic@redhat.com> >> Date: Wed, 26 Mar 2014 16:28:31 -0400 >> >>> Some drivers incorrectly assign vlan acceleration features to >>> vlan_features thus causing issues for Q-in-Q vlan configurations. >>> Prevent this once and for all by masking off acceleration features >>> for vlan devices until such time as we support stacked acceleration. >>> >>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com> >> >> So what if we do have chips that can offload Q-in-Q? Will we use >> a new flag? > > For 802.1ad that was the intention, using the NETIF_F_HW_VLAN_STAG_* flags. > But we can't really do that as we don't carry multiple vlan_tci in the skb. If the card advertises VLAN_STAG_*, then we'll offload that, but CTAG will have to be computed by software. More infrastructure is needed to support nested vlan offloads. -vlad -- 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 03/26/2014 05:10 PM, David Miller wrote: > From: Vlad Yasevich <vyasevic@redhat.com> > Date: Wed, 26 Mar 2014 16:28:31 -0400 > >> Some drivers incorrectly assign vlan acceleration features to >> vlan_features thus causing issues for Q-in-Q vlan configurations. >> Prevent this once and for all by masking off acceleration features >> for vlan devices until such time as we support stacked acceleration. >> >> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com> > > So what if we do have chips that can offload Q-in-Q? Will we use > a new flag? > If there are chips that can take both the inner and outer tags an put both vlan headers on the packet, then we'll need a lot more then just a new flag to support that since the skbs will need to carry 2 vlan_tci values in that case. With this patch you can still offload the outer tag, whether STAG and or CTAG (old Q-in-Q), but the inner tag can't be offloaded. -vlad -- 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 Wed, Mar 26, 2014 at 08:53:12PM -0400, Vlad Yasevich wrote: > On 03/26/2014 05:15 PM, Patrick McHardy wrote: > > On Wed, Mar 26, 2014 at 05:10:38PM -0400, David Miller wrote: > >> From: Vlad Yasevich <vyasevic@redhat.com> > >> Date: Wed, 26 Mar 2014 16:28:31 -0400 > >> > >>> Some drivers incorrectly assign vlan acceleration features to > >>> vlan_features thus causing issues for Q-in-Q vlan configurations. > >>> Prevent this once and for all by masking off acceleration features > >>> for vlan devices until such time as we support stacked acceleration. > >>> > >>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com> > >> > >> So what if we do have chips that can offload Q-in-Q? Will we use > >> a new flag? > > > > For 802.1ad that was the intention, using the NETIF_F_HW_VLAN_STAG_* flags. > > > > But we can't really do that as we don't carry multiple vlan_tci in the > skb. If the card advertises VLAN_STAG_*, then we'll offload that, but > CTAG will have to be computed by software. More infrastructure is > needed to support nested vlan offloads. True, we'd need more room in the skb to store both tags for Q-in-Q. -- 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: Vlad Yasevich <vyasevic@redhat.com> Date: Wed, 26 Mar 2014 16:28:31 -0400 > Some drivers incorrectly assign vlan acceleration features to > vlan_features thus causing issues for Q-in-Q vlan configurations. > Prevent this once and for all by masking off acceleration features > for vlan devices until such time as we support stacked acceleration. > > Signed-off-by: Vlad Yasevich <vyasevic@redhat.com> Vlad, I've thought more about this, I'd rather we emit a warning so we can fix the drivers. If they are setting the flags wrong, we probably want to go take a look to see if they are doing anything else related wrong too. -- 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 03/27/2014 03:12 PM, David Miller wrote: > From: Vlad Yasevich <vyasevic@redhat.com> > Date: Wed, 26 Mar 2014 16:28:31 -0400 > >> Some drivers incorrectly assign vlan acceleration features to >> vlan_features thus causing issues for Q-in-Q vlan configurations. >> Prevent this once and for all by masking off acceleration features >> for vlan devices until such time as we support stacked acceleration. >> >> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com> > > Vlad, I've thought more about this, I'd rather we emit a warning so > we can fix the drivers. > > If they are setting the flags wrong, we probably want to go take a > look to see if they are doing anything else related wrong too. > OK. I'll rework the patch to emit a warning. I took a quick glance through all the devices that set vlan_features and there are 3 more the have vlan_features wrong. I'll include those when I re-submit. Thanks -vlad -- 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/include/linux/netdev_features.h b/include/linux/netdev_features.h index 1005ebf..5a09a48 100644 --- a/include/linux/netdev_features.h +++ b/include/linux/netdev_features.h @@ -163,4 +163,11 @@ enum { /* changeable features with no special hardware requirements */ #define NETIF_F_SOFT_FEATURES (NETIF_F_GSO | NETIF_F_GRO) +#define NETIF_F_VLAN_FEATURES (NETIF_F_HW_VLAN_CTAG_FILTER | \ + NETIF_F_HW_VLAN_CTAG_RX | \ + NETIF_F_HW_VLAN_CTAG_TX | \ + NETIF_F_HW_VLAN_STAG_FILTER | \ + NETIF_F_HW_VLAN_STAG_RX | \ + NETIF_F_HW_VLAN_STAG_TX) + #endif /* _LINUX_NETDEV_FEATURES_H */ diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c index a9591ff..d99e8df 100644 --- a/net/8021q/vlan_dev.c +++ b/net/8021q/vlan_dev.c @@ -576,7 +576,8 @@ static int vlan_dev_init(struct net_device *dev) NETIF_F_HIGHDMA | NETIF_F_SCTP_CSUM | NETIF_F_ALL_FCOE; - dev->features |= real_dev->vlan_features | NETIF_F_LLTX; + dev->features |= (real_dev->vlan_features & ~NETIF_F_VLAN_FEATURES) | + NETIF_F_LLTX; dev->gso_max_size = real_dev->gso_max_size; /* ipv6 shared card related stuff */ @@ -651,6 +652,7 @@ static netdev_features_t vlan_dev_fix_features(struct net_device *dev, features &= real_dev->features; features |= old_features & NETIF_F_SOFT_FEATURES; + features &= ~NETIF_F_VLAN_FEATURES; features |= NETIF_F_LLTX; return features;
Some drivers incorrectly assign vlan acceleration features to vlan_features thus causing issues for Q-in-Q vlan configurations. Prevent this once and for all by masking off acceleration features for vlan devices until such time as we support stacked acceleration. Signed-off-by: Vlad Yasevich <vyasevic@redhat.com> --- include/linux/netdev_features.h | 7 +++++++ net/8021q/vlan_dev.c | 4 +++- 2 files changed, 10 insertions(+), 1 deletion(-)