| Message ID | 1276714691.2970.9.camel@edumazet-laptop |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
> -----Original Message----- > From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On > Behalf Of Eric Dumazet > Sent: Wednesday, June 16, 2010 9:58 PM > To: Arnd Bergmann > Cc: Patrick McHardy; Pedro Garcia; netdev@vger.kernel.org; Ben Hutchings > Subject: Re: [PATCH] vlan_dev: VLAN 0 should be treated as "no vlan tag" > (802.1p packet) > > Le mercredi 16 juin 2010 à 20:26 +0200, Arnd Bergmann a écrit : > > On Wednesday 16 June 2010 17:28:23 Patrick McHardy wrote: > > > > > Since we don't have any special VLAN handling in the bridging code, I > > > guess it comes down to optionally using a different ethertype value > > > (0x88a8) in the VLAN code. We probably also need some indication from > > > device drivers whether they are able to add these headers to avoid > > > trying to offload tagging in case they're not. > > > > It's probably a little more than just supporting the new ethertype, but not > > much. The outer tag can be handled like our current VLAN module does, > > but the standard does not allow a regular frame to be encapsulated > directly, > > but rather requires one of > > > > 1. In 802.1ad: an 802.1Q VLAN tag (ethertype 0x8100) followed by the frame > > 2. In 802.1ah: A service tag (ethertype 0x88e7) followed by the 802.1Q VLAN > tag > > and then the frame. > > > > Maybe what we can do is extend the vlan code to understand all three frame > > formats (q, ad and ah) or at least the first two so we configure both the > > provider VID and the Customer VID for the interface in case of 802.1ad but > > only the regular VID in 802.1Q. > > > > Device drivers can then flag whether they support both formats or just > > the regular Q tag. > > > > Arnd > > Speaking of device drivers, I see bnx2 (hardware accelerated) is able to > insert a 8021q tag in case no vlgrp is defined (the 8201q tag that was > removed by NIC)... interesting ping pong games, since our 8021q stack > will remove it again, eventually. > > So VLAN 0 'problem' on bnx2 could be solved with following patch > (avoiding this insert if vtag==0) > > > > diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c > index 522de9f..b5d4d05 100644 > --- a/drivers/net/bnx2.c > +++ b/drivers/net/bnx2.c > @@ -3192,7 +3192,7 @@ bnx2_rx_int(struct bnx2 *bp, struct bnx2_napi *bnapi, > int budget) > hw_vlan = 1; > else > #endif > - { > + if (vtag) { > struct vlan_ethhdr *ve = (struct vlan_ethhdr *) > __skb_push(skb, 4); > > > > -- This way u will loose all the priority information that was on the VLAN header. > 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
Le jeudi 17 juin 2010 à 01:56 -0700, Vladislav Zolotarov a écrit : > > > -----Original Message----- > > From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On > > Behalf Of Eric Dumazet > > Sent: Wednesday, June 16, 2010 9:58 PM > > To: Arnd Bergmann > > Cc: Patrick McHardy; Pedro Garcia; netdev@vger.kernel.org; Ben Hutchings > > Subject: Re: [PATCH] vlan_dev: VLAN 0 should be treated as "no vlan tag" > > (802.1p packet) > > > > Le mercredi 16 juin 2010 à 20:26 +0200, Arnd Bergmann a écrit : > > > On Wednesday 16 June 2010 17:28:23 Patrick McHardy wrote: > > > > > > > Since we don't have any special VLAN handling in the bridging code, I > > > > guess it comes down to optionally using a different ethertype value > > > > (0x88a8) in the VLAN code. We probably also need some indication from > > > > device drivers whether they are able to add these headers to avoid > > > > trying to offload tagging in case they're not. > > > > > > It's probably a little more than just supporting the new ethertype, but not > > > much. The outer tag can be handled like our current VLAN module does, > > > but the standard does not allow a regular frame to be encapsulated > > directly, > > > but rather requires one of > > > > > > 1. In 802.1ad: an 802.1Q VLAN tag (ethertype 0x8100) followed by the frame > > > 2. In 802.1ah: A service tag (ethertype 0x88e7) followed by the 802.1Q VLAN > > tag > > > and then the frame. > > > > > > Maybe what we can do is extend the vlan code to understand all three frame > > > formats (q, ad and ah) or at least the first two so we configure both the > > > provider VID and the Customer VID for the interface in case of 802.1ad but > > > only the regular VID in 802.1Q. > > > > > > Device drivers can then flag whether they support both formats or just > > > the regular Q tag. > > > > > > Arnd > > > > Speaking of device drivers, I see bnx2 (hardware accelerated) is able to > > insert a 8021q tag in case no vlgrp is defined (the 8201q tag that was > > removed by NIC)... interesting ping pong games, since our 8021q stack > > will remove it again, eventually. > > > > So VLAN 0 'problem' on bnx2 could be solved with following patch > > (avoiding this insert if vtag==0) > > > > > > > > diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c > > index 522de9f..b5d4d05 100644 > > --- a/drivers/net/bnx2.c > > +++ b/drivers/net/bnx2.c > > @@ -3192,7 +3192,7 @@ bnx2_rx_int(struct bnx2 *bp, struct bnx2_napi *bnapi, > > int budget) > > hw_vlan = 1; > > else > > #endif > > - { > > + if (vtag) { > > struct vlan_ethhdr *ve = (struct vlan_ethhdr *) > > __skb_push(skb, 4); > > > > > > > > -- > > This way u will loose all the priority information that was on the VLAN header. 16bits vtag = 0 : there is no priority information. -- 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/drivers/net/bnx2.c b/drivers/net/bnx2.c index 522de9f..b5d4d05 100644 --- a/drivers/net/bnx2.c +++ b/drivers/net/bnx2.c @@ -3192,7 +3192,7 @@ bnx2_rx_int(struct bnx2 *bp, struct bnx2_napi *bnapi, int budget) hw_vlan = 1; else #endif - { + if (vtag) { struct vlan_ethhdr *ve = (struct vlan_ethhdr *) __skb_push(skb, 4);