Re: [PATCH nf-next 3/3] netfilter: bridge: copy back VLAN header for bridge packet queued to userspace

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Fri, Jan 15, 2016 at 03:04:12PM +0100, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > For the specific case of nfnetlink_queue, I would expose the vlan
> > information through a new netlink attribute NFQA_VLAN (similar to what
> > we do for NFQA_HWADDR for the layer 3).
> 
> If we do this I think it does make sense to consider putting
> the entire L2 mac header under its own attr too.
> 
> This is especially good if we'd later add support for NETDEV
> family.  Since drivers already pull the L2 header userspace
> would not need to handle arbirary L2 protocols.

Good point, fine with me.

> > > +				payload += VLAN_HLEN;
> > > +				payload_len -= VLAN_HLEN;
> > > +			} else {
> > > +				entry->skb->vlan_tci &= ~VLAN_TAG_PRESENT;
> > > +				entry->skb->protocol = veth->h_vlan_proto;
> > > +			}
> > > +		}
> > 
> > I'm awar it's more work, but it would be good to reduce ifdef pollution
> > by placing all this bridge netfilter code wrapped into functions under
> > one single ifdef in this file to improve maintainability.
> 
> Right, but for anything family specifiy it would be even better to push
> it into nf afinfo. In case thats too much work or too cumbersome (e.g.
> because you'd need 12 function arguments ...) then the ifdef-wrapped
> helper is fine of course.

I'm fine with pushing this info afinfo if it fits fine there.