Re: [Bridge] [PATCH] Add vlan id to bridge forward database

[Stephen Hemminger <shemminger@linux-foundation.org>]

On Mon, 28 Jan 2008 16:39:14 +0100
Jaime Medrano <jaime.medrano@gmail.com> wrote:

> I'm resending this mail since I got no answer.
> 
> Is there any major problem in this?
> 
> Stephen Hemminger wrote:
> >
> > What about the nested vlan case?
> >
> 
> Below is a new patch that handles the double-tagging case. I'm not sure
> if it is worth a more generic case. ¿Are triple-tagging and so really used?
> 
> > This is a user/kernel ABI change. Does it break old tools?
> 
> New patch gets rid of the unused field but it still doesn't break old tools.
> 
> Anyway, the user part is not really needed. I just think it could be useful.
> 
> Regards,
> Jaime.
> 


Minor stuff:
1. Please use shorter variable names, rather than:
              unsigned short vlan_first_id;
      I would choose:
              u16 vlan1;

2. You probably can use skb->protocol rather than having to look at the packet
    contents to check for 8021Q.

3. Don't use __constant_htons(), just use htons().
   The macro is smart enough to handle the
   constant case, and it reads better, without the __constant_prefix.


Major stuff:
1. This won't work with hardware accel VLAN receive. The tag is not put in
   the skb?