[PATCH -next] netfilter: bridge: optionally set indev to vlan

[PATCH -next] netfilter: bridge: optionally set indev to vlan

[Florian Westphal]

if net.bridge.bridge-nf-filter-vlan-tagged sysctl is enabled, bridge
netfilter removes the vlan header temporarily and then feeds the packet
to ip(6)tables.

When the new "bridge-nf-pass-vlan-input-device" sysctl is on
(default off), then bridge netfilter will also change the in/out
interface to the vlan interface; if such an interface exists.

This is needed to make iptables REDIRECT target work with
"vlan-on-top-of-bridge" setups. Also, 'iptables -i' can then be used
to match the vlan device name.

Also update Documentation with current brnf default settings.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 Documentation/networking/ip-sysctl.txt |   13 +++++++++-
 net/bridge/br_netfilter.c              |   39 +++++++++++++++++++++++++------
 2 files changed, 42 insertions(+), 10 deletions(-)

diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index ad3e80e..ee0374a 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -1294,13 +1294,22 @@ bridge-nf-call-ip6tables - BOOLEAN
 bridge-nf-filter-vlan-tagged - BOOLEAN
 	1 : pass bridged vlan-tagged ARP/IP/IPv6 traffic to {arp,ip,ip6}tables.
 	0 : disable this.
-	Default: 1
+	Default: 0
 
 bridge-nf-filter-pppoe-tagged - BOOLEAN
 	1 : pass bridged pppoe-tagged IP/IPv6 traffic to {ip,ip6}tables.
 	0 : disable this.
-	Default: 1
+	Default: 0
 
+bridge-nf-pass-vlan-input-dev - BOOLEAN
+	1: if bridge-nf-filter-vlan-tagged is enabled, try to find a vlan
+	interface on the bridge and set the netfilter input device to the vlan.
+	This allows use of e.g. "iptables -i br0.1" and makes the REDIRECT
+	target work with vlan-on-top-of-bridge interfaces.  When no matching
+	vlan interface is found, or this switch is off, the input device is
+	set to the bridge interface.
+	0: disable bridge netfilter vlan interface lookup.
+	Default: 0
 
 proc/sys/net/sctp/* Variables:
 
diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
index dec4f38..1d69feb 100644
--- a/net/bridge/br_netfilter.c
+++ b/net/bridge/br_netfilter.c
@@ -53,6 +53,7 @@ static int brnf_call_iptables __read_mostly = 1;
 static int brnf_call_ip6tables __read_mostly = 1;
 static int brnf_call_arptables __read_mostly = 1;
 static int brnf_filter_vlan_tagged __read_mostly = 0;
+static int brnf_pass_vlan_indev __read_mostly = 0;
 static int brnf_filter_pppoe_tagged __read_mostly = 0;
 #else
 #define brnf_call_iptables 1
@@ -503,6 +504,19 @@ bridged_dnat:
 	return 0;
 }
 
+static struct net_device *brnf_get_logical_dev(struct sk_buff *skb, const struct net_device *dev)
+{
+	struct net_device *vlan, *br;
+
+	br = bridge_parent(dev);
+	if (brnf_pass_vlan_indev == 0 || !vlan_tx_tag_present(skb))
+		return br;
+
+	vlan = __vlan_find_dev_deep(br, vlan_tx_tag_get(skb) & VLAN_VID_MASK);
+
+	return vlan ? vlan : br;
+}
+
 /* Some common code for IPv4/IPv6 */
 static struct net_device *setup_pre_routing(struct sk_buff *skb)
 {
@@ -515,7 +529,7 @@ static struct net_device *setup_pre_routing(struct sk_buff *skb)
 
 	nf_bridge->mask |= BRNF_NF_BRIDGE_PREROUTING;
 	nf_bridge->physindev = skb->dev;
-	skb->dev = bridge_parent(skb->dev);
+	skb->dev = brnf_get_logical_dev(skb, skb->dev);
 	if (skb->protocol == htons(ETH_P_8021Q))
 		nf_bridge->mask |= BRNF_8021Q;
 	else if (skb->protocol == htons(ETH_P_PPP_SES))
@@ -737,7 +751,7 @@ static unsigned int br_nf_forward_ip(unsigned int hook, struct sk_buff *skb,
 				     int (*okfn)(struct sk_buff *))
 {
 	struct nf_bridge_info *nf_bridge;
-	struct net_device *parent;
+	struct net_device *parent, *indev;
 	u_int8_t pf;
 
 	if (!skb->nf_bridge)
@@ -748,10 +762,6 @@ static unsigned int br_nf_forward_ip(unsigned int hook, struct sk_buff *skb,
 	if (!nf_bridge_unshare(skb))
 		return NF_DROP;
 
-	parent = bridge_parent(out);
-	if (!parent)
-		return NF_DROP;
-
 	if (IS_IP(skb) || IS_VLAN_IP(skb) || IS_PPPOE_IP(skb))
 		pf = PF_INET;
 	else if (IS_IPV6(skb) || IS_VLAN_IPV6(skb) || IS_PPPOE_IPV6(skb))
@@ -759,6 +769,10 @@ static unsigned int br_nf_forward_ip(unsigned int hook, struct sk_buff *skb,
 	else
 		return NF_ACCEPT;
 
+	parent = brnf_get_logical_dev(skb, out);
+	if (!parent)
+		return NF_DROP;
+
 	nf_bridge_pull_encap_header(skb);
 
 	nf_bridge = skb->nf_bridge;
@@ -778,7 +792,9 @@ static unsigned int br_nf_forward_ip(unsigned int hook, struct sk_buff *skb,
 	else
 		skb->protocol = htons(ETH_P_IPV6);
 
-	NF_HOOK(pf, NF_INET_FORWARD, skb, bridge_parent(in), parent,
+	indev = brnf_get_logical_dev(skb, in);
+
+	NF_HOOK(pf, NF_INET_FORWARD, skb, indev, parent,
 		br_nf_forward_finish);
 
 	return NF_STOLEN;
@@ -850,7 +866,7 @@ static unsigned int br_nf_post_routing(unsigned int hook, struct sk_buff *skb,
 				       int (*okfn)(struct sk_buff *))
 {
 	struct nf_bridge_info *nf_bridge = skb->nf_bridge;
-	struct net_device *realoutdev = bridge_parent(skb->dev);
+	struct net_device *realoutdev = brnf_get_logical_dev(skb, out);
 	u_int8_t pf;
 
 	if (!nf_bridge || !(nf_bridge->mask & BRNF_BRIDGED))
@@ -1006,6 +1022,13 @@ static ctl_table brnf_table[] = {
 		.mode		= 0644,
 		.proc_handler	= brnf_sysctl_call_tables,
 	},
+	{
+		.procname	= "bridge-nf-pass-vlan-input-dev",
+		.data		= &brnf_pass_vlan_indev,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= brnf_sysctl_call_tables,
+	},
 	{ }
 };
 
-- 
1.7.3.4

Re: [PATCH -next] netfilter: bridge: optionally set indev to vlan

[Pablo Neira Ayuso]

On Wed, Apr 04, 2012 at 09:57:29PM +0200, Florian Westphal wrote:
> if net.bridge.bridge-nf-filter-vlan-tagged sysctl is enabled, bridge
> netfilter removes the vlan header temporarily and then feeds the packet
> to ip(6)tables.
> 
> When the new "bridge-nf-pass-vlan-input-device" sysctl is on
> (default off), then bridge netfilter will also change the in/out
> interface to the vlan interface; if such an interface exists.
> 
> This is needed to make iptables REDIRECT target work with
> "vlan-on-top-of-bridge" setups. Also, 'iptables -i' can then be used
> to match the vlan device name.
> 
> Also update Documentation with current brnf default settings.

Better to implement a vlan device match for iptables?

Re: [PATCH -next] netfilter: bridge: optionally set indev to vlan

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Wed, Apr 04, 2012 at 09:57:29PM +0200, Florian Westphal wrote:
> > if net.bridge.bridge-nf-filter-vlan-tagged sysctl is enabled, bridge
> > netfilter removes the vlan header temporarily and then feeds the packet
> > to ip(6)tables.
> > 
> > When the new "bridge-nf-pass-vlan-input-device" sysctl is on
> > (default off), then bridge netfilter will also change the in/out
> > interface to the vlan interface; if such an interface exists.
> > 
> > This is needed to make iptables REDIRECT target work with
> > "vlan-on-top-of-bridge" setups. Also, 'iptables -i' can then be used
> > to match the vlan device name.
> > 
> > Also update Documentation with current brnf default settings.
> 
> Better to implement a vlan device match for iptables?

That would mean that REDIRECT target is unsupported in vlan-on-bridge
case.
If so, would you accept a man page update that documents this?

Thanks,
Florian

Re: [PATCH -next] netfilter: bridge: optionally set indev to vlan

[Pablo Neira Ayuso]

On Thu, Apr 05, 2012 at 09:30:59AM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > On Wed, Apr 04, 2012 at 09:57:29PM +0200, Florian Westphal wrote:
> > > if net.bridge.bridge-nf-filter-vlan-tagged sysctl is enabled, bridge
> > > netfilter removes the vlan header temporarily and then feeds the packet
> > > to ip(6)tables.
> > > 
> > > When the new "bridge-nf-pass-vlan-input-device" sysctl is on
> > > (default off), then bridge netfilter will also change the in/out
> > > interface to the vlan interface; if such an interface exists.
> > > 
> > > This is needed to make iptables REDIRECT target work with
> > > "vlan-on-top-of-bridge" setups. Also, 'iptables -i' can then be used
> > > to match the vlan device name.
> > > 
> > > Also update Documentation with current brnf default settings.
> > 
> > Better to implement a vlan device match for iptables?
> 
> That would mean that REDIRECT target is unsupported in vlan-on-bridge
> case.

I see, it would not support any other targets that relies on skb->dev
either.

Let me think if I can come with a better solution. If not, I'll take
your patch.

Thanks Florian.

Re: [PATCH -next] netfilter: bridge: optionally set indev to vlan

[Bart De Schuymer]

Op 4/04/2012 21:57, Florian Westphal schreef:
> if net.bridge.bridge-nf-filter-vlan-tagged sysctl is enabled, bridge
> netfilter removes the vlan header temporarily and then feeds the packet
> to ip(6)tables.
>
> When the new "bridge-nf-pass-vlan-input-device" sysctl is on
> (default off), then bridge netfilter will also change the in/out
> interface to the vlan interface; if such an interface exists.
>
> This is needed to make iptables REDIRECT target work with
> "vlan-on-top-of-bridge" setups. Also, 'iptables -i' can then be used
> to match the vlan device name.
>
> Also update Documentation with current brnf default settings.
>
>   proc/sys/net/sctp/* Variables:
>
> diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
> index dec4f38..1d69feb 100644
> --- a/net/bridge/br_netfilter.c
> +++ b/net/bridge/br_netfilter.c
> @@ -53,6 +53,7 @@ static int brnf_call_iptables __read_mostly = 1;
>   static int brnf_call_ip6tables __read_mostly = 1;
>   static int brnf_call_arptables __read_mostly = 1;
>   static int brnf_filter_vlan_tagged __read_mostly = 0;
> +static int brnf_pass_vlan_indev __read_mostly = 0;
>   static int brnf_filter_pppoe_tagged __read_mostly = 0;
>   #else
>   #define brnf_call_iptables 1
> @@ -503,6 +504,19 @@ bridged_dnat:
>   	return 0;
>   }

You should also provide a macro in case the proc file system isn't 
enabled: currently it won't compile with your patch.

>   /* Some common code for IPv4/IPv6 */
>   static struct net_device *setup_pre_routing(struct sk_buff *skb)
>   {
> @@ -515,7 +529,7 @@ static struct net_device *setup_pre_routing(struct sk_buff *skb)
>
>   	nf_bridge->mask |= BRNF_NF_BRIDGE_PREROUTING;
>   	nf_bridge->physindev = skb->dev;
> -	skb->dev = bridge_parent(skb->dev);
> +	skb->dev = brnf_get_logical_dev(skb, skb->dev);
>   	if (skb->protocol == htons(ETH_P_8021Q))
>   		nf_bridge->mask |= BRNF_8021Q;
>   	else if (skb->protocol == htons(ETH_P_PPP_SES))
> @@ -737,7 +751,7 @@ static unsigned int br_nf_forward_ip(unsigned int hook, struct sk_buff *skb,
>   				     int (*okfn)(struct sk_buff *))
>   {
>   	struct nf_bridge_info *nf_bridge;
> -	struct net_device *parent;
> +	struct net_device *parent, *indev;
>   	u_int8_t pf;
>
>   	if (!skb->nf_bridge)
> @@ -748,10 +762,6 @@ static unsigned int br_nf_forward_ip(unsigned int hook, struct sk_buff *skb,
>   	if (!nf_bridge_unshare(skb))
>   		return NF_DROP;
>
> -	parent = bridge_parent(out);
> -	if (!parent)
> -		return NF_DROP;
> -
>   	if (IS_IP(skb) || IS_VLAN_IP(skb) || IS_PPPOE_IP(skb))
>   		pf = PF_INET;
>   	else if (IS_IPV6(skb) || IS_VLAN_IPV6(skb) || IS_PPPOE_IPV6(skb))
> @@ -759,6 +769,10 @@ static unsigned int br_nf_forward_ip(unsigned int hook, struct sk_buff *skb,
>   	else
>   		return NF_ACCEPT;
>
> +	parent = brnf_get_logical_dev(skb, out);
> +	if (!parent)
> +		return NF_DROP;
> +
>   	nf_bridge_pull_encap_header(skb);
>
>   	nf_bridge = skb->nf_bridge;
> @@ -778,7 +792,9 @@ static unsigned int br_nf_forward_ip(unsigned int hook, struct sk_buff *skb,
>   	else
>   		skb->protocol = htons(ETH_P_IPV6);
>
> -	NF_HOOK(pf, NF_INET_FORWARD, skb, bridge_parent(in), parent,
> +	indev = brnf_get_logical_dev(skb, in);
> +
> +	NF_HOOK(pf, NF_INET_FORWARD, skb, indev, parent,
>   		br_nf_forward_finish);
>
>   	return NF_STOLEN;
> @@ -850,7 +866,7 @@ static unsigned int br_nf_post_routing(unsigned int hook, struct sk_buff *skb,
>   				       int (*okfn)(struct sk_buff *))
>   {
>   	struct nf_bridge_info *nf_bridge = skb->nf_bridge;
> -	struct net_device *realoutdev = bridge_parent(skb->dev);
> +	struct net_device *realoutdev = brnf_get_logical_dev(skb, out);
I think it's best to keep the bridge_parent as output device in the 
FOWARD and POSTROUTING chain: only change the input device. Do you have 
a reason for changing the output device?

cheers,
Bart


-- 
Bart De Schuymer
www.artinalgorithms.be

Re: [PATCH -next] netfilter: bridge: optionally set indev to vlan

[Florian Westphal]

Bart De Schuymer <bdschuym@pandora.be> wrote:
> > +static int brnf_pass_vlan_indev __read_mostly = 0;
> >   static int brnf_filter_pppoe_tagged __read_mostly = 0;
> >   #else
> >   #define brnf_call_iptables 1
> > @@ -503,6 +504,19 @@ bridged_dnat:
> >   	return 0;
> >   }
> 
> You should also provide a macro in case the proc file system isn't 
> enabled: currently it won't compile with your patch.

You're right, of course.
Thanks for spotting this, I'll fix it.

> >   	struct nf_bridge_info *nf_bridge = skb->nf_bridge;
> > -	struct net_device *realoutdev = bridge_parent(skb->dev);
> > +	struct net_device *realoutdev = brnf_get_logical_dev(skb, out);
> I think it's best to keep the bridge_parent as output device in the 
> FOWARD and POSTROUTING chain: only change the input device. Do you have 
> a reason for changing the output device?

Consistency.  (locally generated packet is '-o br0.X, forwarded br0).
OTOH, we could call it a feature ;-)

I have no special requirements for the outdev; we can keep it as-is.

Regards,
Florian