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

[Bart De Schuymer <bdschuym@pandora.be>]

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))
The compiler should easily remove any redundant checks that this would 
produce. The same idea goes for the IS_ARP and IS_IPV6 macros you define...
Apart from that the patch looks fine to me.

cheers,

Bart