Re: [PATCH] bridge: netfilter: don't call iptables on vlan packets if sysctl is off

[Florian Westphal <fw@strlen.de>]

Bart De Schuymer <bdschuym@pandora.be> wrote:
> Op 1/03/2012 22:42, Florian Westphal schreef:
> > When net.bridge.bridge-nf-filter-vlan-tagged is 0 (default), vlan packets
> > arriving should not be sent to ip(6)tables by bridge netfilter.
> >
> > However, it turns out that we currently always send VLAN packets to
> > netfilter, if ..
> > a), CONFIG_VLAN_8021Q is enabled ; or
> > b), CONFIG_VLAN_8021Q is not set but rx vlan offload is enabled
> >     on the bridge port.
> >
> > This is because bridge netfilter treats skb with
> > skb->protocol == ETH_P_IP{V6} as "non-vlan packet".
> 
> > -	else if (skb->protocol == htons(ETH_P_IPV6) || IS_VLAN_IPV6(skb) ||
> > -		 IS_PPPOE_IPV6(skb))
> > +	else if (IS_IPV6(skb) || IS_PPPOE_IPV6(skb))
> 
> > -	if (skb->protocol != htons(ETH_P_ARP)) {
> > -		if (!IS_VLAN_ARP(skb))
> > -			return NF_ACCEPT;
> > -		nf_bridge_pull_encap_header(skb);
> > -	}
> > +	if (!IS_ARP(skb))
> > +		return NF_ACCEPT;
> > +	nf_bridge_pull_encap_header(skb);
> 
> > -	if (skb->protocol == htons(ETH_P_IP) || IS_VLAN_IP(skb) ||
> > -	    IS_PPPOE_IP(skb))
> > +	if (IS_IP(skb) || IS_PPPOE_IP(skb))
> 
> I think the above is quite unclear to read. I would prefer something 
> like this:
> if (IS_IP(skb) || IS_VLAN_IP(skb) || IS_PPPOE_IP(skb))

I disagree.  But fair enough, I'll change it & respin on monday, if...

> The compiler should easily remove any redundant checks that this would 
> produce.

... gcc removes the redundant checks (I expect it to).

> Apart from that the patch looks fine to me.

Thanks for reviewing!

Regards,
Florian