Patchwork vlan_dev: VLAN 0 should be treated as "no vlan tag" (802.1p packet)

login
register
mail settings
Submitter Eric Dumazet
Date June 16, 2010, 6:58 p.m.
Message ID <1276714691.2970.9.camel@edumazet-laptop>
Download mbox | patch
Permalink /patch/55921/
State RFC
Delegated to: David Miller
Headers show

Comments

Eric Dumazet - June 16, 2010, 6:58 p.m.
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)





--
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
Vladislav Zolotarov - June 17, 2010, 8:56 a.m.
> -----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
Eric Dumazet - June 17, 2010, 10:28 a.m.
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

Patch

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);