NFQUEUE balancing extension (kernel changes)

NFQUEUE balancing extension (kernel changes)

[Florian Westphal]

Hello netfilter hackers,

here are three patches that i'd like to see in 2.6.31.

This extends the NFQUEUE target to enable a simple load-balancing
feature (packets are distributed among a range of queues, so several
instances of the same userspace-program can process data simultaneously).

I will be sending the userspace change in a separate patch series shortly.

If you have any objections, please let me know and I'll be happy
to fix the patches up.

The 3rd (unrelated) patch removes the rwlock from the MASQUERADE target,
it does not appear to have any use in its current form...

You may also pull these changes from:

git://git.breakpoint.cc/fw/nf-next-2.6.git for-kaber

Thanks for your time,
Florian

[PATCH 1/3] netfilter: NFQUEUE use NFPROTO_UNSPEC

[Florian Westphal]

We can use wildcard matching here, just like
ab4f21e6fb1c09b13c4c3cb8357babe8223471bd ("xtables: use NFPROTO_UNSPEC
in more extensions").

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/netfilter/xt_NFQUEUE.c |   16 +---------------
 1 files changed, 1 insertions(+), 15 deletions(-)

diff --git a/net/netfilter/xt_NFQUEUE.c b/net/netfilter/xt_NFQUEUE.c
index f9977b3..6e0f84d 100644
--- a/net/netfilter/xt_NFQUEUE.c
+++ b/net/netfilter/xt_NFQUEUE.c
@@ -34,21 +34,7 @@ nfqueue_tg(struct sk_buff *skb, const struct xt_target_param *par)
 static struct xt_target nfqueue_tg_reg[] __read_mostly = {
 	{
 		.name		= "NFQUEUE",
-		.family		= NFPROTO_IPV4,
-		.target		= nfqueue_tg,
-		.targetsize	= sizeof(struct xt_NFQ_info),
-		.me		= THIS_MODULE,
-	},
-	{
-		.name		= "NFQUEUE",
-		.family		= NFPROTO_IPV6,
-		.target		= nfqueue_tg,
-		.targetsize	= sizeof(struct xt_NFQ_info),
-		.me		= THIS_MODULE,
-	},
-	{
-		.name		= "NFQUEUE",
-		.family		= NFPROTO_ARP,
+		.family		= NFPROTO_UNSPEC,
 		.target		= nfqueue_tg,
 		.targetsize	= sizeof(struct xt_NFQ_info),
 		.me		= THIS_MODULE,
-- 
1.6.0.6

Re: [PATCH 1/3] netfilter: NFQUEUE use NFPROTO_UNSPEC

[Patrick McHardy]

Florian Westphal wrote:
> We can use wildcard matching here, just like
> ab4f21e6fb1c09b13c4c3cb8357babe8223471bd ("xtables: use NFPROTO_UNSPEC
> in more extensions").

Applied, thanks.

[PATCH 2/3] netfilter: NFQUEUE: queue balancing support

[Florian Westphal]

Adds support for specifying a range of queues instead of a single queue
id.
Flows will be distributed across the given range.

This is useful for multicore systems: Instead of having a single
application read packets from a queue, start multiple
instances on queues x, x+1, .. x+n. Each instance can process
flows independently.

Packets for the same connection are put into the same queue.

Signed-off-by: Holger Eitzenberger <heitzenberger@astaro.com>
Signed-off-by: Florian Westphal <fwestphal@astaro.com>
---
 include/linux/netfilter/xt_NFQUEUE.h |    5 ++
 net/netfilter/xt_NFQUEUE.c           |   93 ++++++++++++++++++++++++++++++++++
 2 files changed, 98 insertions(+), 0 deletions(-)

diff --git a/include/linux/netfilter/xt_NFQUEUE.h b/include/linux/netfilter/xt_NFQUEUE.h
index 982a89f..2584f4a 100644
--- a/include/linux/netfilter/xt_NFQUEUE.h
+++ b/include/linux/netfilter/xt_NFQUEUE.h
@@ -15,4 +15,9 @@ struct xt_NFQ_info {
 	__u16 queuenum;
 };
 
+struct xt_NFQ_info_v1 {
+	__u16 queuenum;
+	__u16 queues_total;
+};
+
 #endif /* _XT_NFQ_TARGET_H */
diff --git a/net/netfilter/xt_NFQUEUE.c b/net/netfilter/xt_NFQUEUE.c
index 6e0f84d..2215b7a 100644
--- a/net/netfilter/xt_NFQUEUE.c
+++ b/net/netfilter/xt_NFQUEUE.c
@@ -11,6 +11,10 @@
 #include <linux/module.h>
 #include <linux/skbuff.h>
 
+#include <linux/ip.h>
+#include <linux/ipv6.h>
+#include <linux/jhash.h>
+
 #include <linux/netfilter.h>
 #include <linux/netfilter_arp.h>
 #include <linux/netfilter/x_tables.h>
@@ -23,6 +27,8 @@ MODULE_ALIAS("ipt_NFQUEUE");
 MODULE_ALIAS("ip6t_NFQUEUE");
 MODULE_ALIAS("arpt_NFQUEUE");
 
+static u32 jhash_initval __read_mostly;
+
 static unsigned int
 nfqueue_tg(struct sk_buff *skb, const struct xt_target_param *par)
 {
@@ -31,6 +37,72 @@ nfqueue_tg(struct sk_buff *skb, const struct xt_target_param *par)
 	return NF_QUEUE_NR(tinfo->queuenum);
 }
 
+static u32 hash_v4(const struct sk_buff *skb)
+{
+	const struct iphdr *iph = ip_hdr(skb);
+	u32 ipaddr;
+
+	/* packets in either direction go into same queue */
+	ipaddr = iph->saddr ^ iph->daddr;
+
+	return jhash_2words(ipaddr, iph->protocol, jhash_initval);
+}
+
+static unsigned int
+nfqueue_tg4_v1(struct sk_buff *skb, const struct xt_target_param *par)
+{
+	const struct xt_NFQ_info_v1 *info = par->targinfo;
+	u32 queue = info->queuenum;
+
+	if (info->queues_total > 1)
+		queue = hash_v4(skb) % info->queues_total + queue;
+	return NF_QUEUE_NR(queue);
+}
+
+#if defined(CONFIG_IP6_NF_IPTABLES) || defined(CONFIG_IP6_NF_IPTABLES_MODULE)
+static u32 hash_v6(const struct sk_buff *skb)
+{
+	const struct ipv6hdr *ip6h = ipv6_hdr(skb);
+	u32 addr[4];
+
+	addr[0] = ip6h->saddr.s6_addr32[0] ^ ip6h->daddr.s6_addr32[0];
+	addr[1] = ip6h->saddr.s6_addr32[1] ^ ip6h->daddr.s6_addr32[1];
+	addr[2] = ip6h->saddr.s6_addr32[2] ^ ip6h->daddr.s6_addr32[2];
+	addr[3] = ip6h->saddr.s6_addr32[3] ^ ip6h->daddr.s6_addr32[3];
+
+	return jhash2(addr, 4, jhash_initval);
+}
+
+static unsigned int
+nfqueue_tg6_v1(struct sk_buff *skb, const struct xt_target_param *par)
+{
+	const struct xt_NFQ_info_v1 *info = par->targinfo;
+	u32 queue = info->queuenum;
+
+	if (info->queues_total > 1)
+		queue = hash_v6(skb) % info->queues_total + queue;
+	return NF_QUEUE_NR(queue);
+}
+#endif
+
+static bool nfqueue_tg_v1_check(const struct xt_tgchk_param *par)
+{
+	const struct xt_NFQ_info_v1 *info = par->targinfo;
+	u32 maxid;
+
+	if (info->queues_total == 0) {
+		printk(KERN_ERR "NFQUEUE: number of total queues is 0\n");
+		return false;
+	}
+	maxid = info->queues_total - 1  + info->queuenum;
+	if (maxid > 0xffff) {
+		printk(KERN_ERR "NFQUEUE: number of queues (%u) out of range (got %u)\n",
+							info->queues_total, maxid);
+		return false;
+	}
+	return true;
+}
+
 static struct xt_target nfqueue_tg_reg[] __read_mostly = {
 	{
 		.name		= "NFQUEUE",
@@ -39,10 +111,31 @@ static struct xt_target nfqueue_tg_reg[] __read_mostly = {
 		.targetsize	= sizeof(struct xt_NFQ_info),
 		.me		= THIS_MODULE,
 	},
+	{
+		.name		= "NFQUEUE",
+		.revision	= 1,
+		.family		= NFPROTO_IPV4,
+		.checkentry	= nfqueue_tg_v1_check,
+		.target		= nfqueue_tg4_v1,
+		.targetsize	= sizeof(struct xt_NFQ_info_v1),
+		.me		= THIS_MODULE,
+	},
+#if defined(CONFIG_IP6_NF_IPTABLES) || defined(CONFIG_IP6_NF_IPTABLES_MODULE)
+	{
+		.name		= "NFQUEUE",
+		.revision	= 1,
+		.family		= NFPROTO_IPV6,
+		.checkentry	= nfqueue_tg_v1_check,
+		.target		= nfqueue_tg6_v1,
+		.targetsize	= sizeof(struct xt_NFQ_info_v1),
+		.me		= THIS_MODULE,
+	},
+#endif
 };
 
 static int __init nfqueue_tg_init(void)
 {
+	get_random_bytes(&jhash_initval, sizeof(jhash_initval));
 	return xt_register_targets(nfqueue_tg_reg, ARRAY_SIZE(nfqueue_tg_reg));
 }
 
-- 
1.6.0.6

Re: [PATCH 2/3] netfilter: NFQUEUE: queue balancing support

[Pablo Neira Ayuso]

Florian Westphal wrote:
> Adds support for specifying a range of queues instead of a single queue
> id.
> Flows will be distributed across the given range.

Interesting :-). One question.

> This is useful for multicore systems: Instead of having a single
> application read packets from a queue, start multiple
> instances on queues x, x+1, .. x+n. Each instance can process
> flows independently.
> 
> Packets for the same connection are put into the same queue.
> 
> Signed-off-by: Holger Eitzenberger <heitzenberger@astaro.com>
> Signed-off-by: Florian Westphal <fwestphal@astaro.com>
> ---
>  include/linux/netfilter/xt_NFQUEUE.h |    5 ++
>  net/netfilter/xt_NFQUEUE.c           |   93 ++++++++++++++++++++++++++++++++++
>  2 files changed, 98 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/netfilter/xt_NFQUEUE.h b/include/linux/netfilter/xt_NFQUEUE.h
> index 982a89f..2584f4a 100644
> --- a/include/linux/netfilter/xt_NFQUEUE.h
> +++ b/include/linux/netfilter/xt_NFQUEUE.h
> @@ -15,4 +15,9 @@ struct xt_NFQ_info {
>  	__u16 queuenum;
>  };
>  
> +struct xt_NFQ_info_v1 {
> +	__u16 queuenum;
> +	__u16 queues_total;
> +};
> +
>  #endif /* _XT_NFQ_TARGET_H */
> diff --git a/net/netfilter/xt_NFQUEUE.c b/net/netfilter/xt_NFQUEUE.c
> index 6e0f84d..2215b7a 100644
> --- a/net/netfilter/xt_NFQUEUE.c
> +++ b/net/netfilter/xt_NFQUEUE.c
> @@ -11,6 +11,10 @@
>  #include <linux/module.h>
>  #include <linux/skbuff.h>
>  
> +#include <linux/ip.h>
> +#include <linux/ipv6.h>
> +#include <linux/jhash.h>
> +
>  #include <linux/netfilter.h>
>  #include <linux/netfilter_arp.h>
>  #include <linux/netfilter/x_tables.h>
> @@ -23,6 +27,8 @@ MODULE_ALIAS("ipt_NFQUEUE");
>  MODULE_ALIAS("ip6t_NFQUEUE");
>  MODULE_ALIAS("arpt_NFQUEUE");
>  
> +static u32 jhash_initval __read_mostly;
> +
>  static unsigned int
>  nfqueue_tg(struct sk_buff *skb, const struct xt_target_param *par)
>  {
> @@ -31,6 +37,72 @@ nfqueue_tg(struct sk_buff *skb, const struct xt_target_param *par)
>  	return NF_QUEUE_NR(tinfo->queuenum);
>  }
>  
> +static u32 hash_v4(const struct sk_buff *skb)
> +{
> +	const struct iphdr *iph = ip_hdr(skb);
> +	u32 ipaddr;
> +
> +	/* packets in either direction go into same queue */
> +	ipaddr = iph->saddr ^ iph->daddr;

Does this guarantee that packets with NAT handlings go to the same queue?

> +
> +	return jhash_2words(ipaddr, iph->protocol, jhash_initval);
> +}

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

Re: [PATCH 2/3] netfilter: NFQUEUE: queue balancing support

[Holger Eitzenberger]

Hi Pablo,

> Does this guarantee that packets with NAT handlings go to the same queue?

Yes, this is guaranteed by the way the hashing is done.  And in fact
it's very much necessary for applications like Snort e. g.

 /holger

Re: [PATCH 2/3] netfilter: NFQUEUE: queue balancing support

[Patrick McHardy]

Florian Westphal wrote:
> Adds support for specifying a range of queues instead of a single queue
> id.
> Flows will be distributed across the given range.
> 
> This is useful for multicore systems: Instead of having a single
> application read packets from a queue, start multiple
> instances on queues x, x+1, .. x+n. Each instance can process
> flows independently.
> 
> Packets for the same connection are put into the same queue.

Nice work, applied. I've made two minor cosmetic changes:

> +#if defined(CONFIG_IP6_NF_IPTABLES) || defined(CONFIG_IP6_NF_IPTABLES_MODULE)
> +static u32 hash_v6(const struct sk_buff *skb)
> +{
> +	const struct ipv6hdr *ip6h = ipv6_hdr(skb);
> +	u32 addr[4];
> +
> +	addr[0] = ip6h->saddr.s6_addr32[0] ^ ip6h->daddr.s6_addr32[0];
> +	addr[1] = ip6h->saddr.s6_addr32[1] ^ ip6h->daddr.s6_addr32[1];
> +	addr[2] = ip6h->saddr.s6_addr32[2] ^ ip6h->daddr.s6_addr32[2];
> +	addr[3] = ip6h->saddr.s6_addr32[3] ^ ip6h->daddr.s6_addr32[3];
> +
> +	return jhash2(addr, 4, jhash_initval);

I changed 4 to ARRAY_SIZE(addr)

> +static bool nfqueue_tg_v1_check(const struct xt_tgchk_param *par)
> +{
> +	const struct xt_NFQ_info_v1 *info = par->targinfo;
> +	u32 maxid;
> +
> +	if (info->queues_total == 0) {
> +		printk(KERN_ERR "NFQUEUE: number of total queues is 0\n");
> +		return false;
> +	}
> +	maxid = info->queues_total - 1  + info->queuenum;
> +	if (maxid > 0xffff) {
> +		printk(KERN_ERR "NFQUEUE: number of queues (%u) out of range (got %u)\n",
> +							info->queues_total, maxid);
> +		return false;
> +	}
> +	return true;
> +}

and these printks to use pr_err to bring the line size down.

Re: [PATCH 2/3] netfilter: NFQUEUE: queue balancing support

[Florian Westphal]

Patrick McHardy <kaber@trash.net> wrote:
> Nice work, applied. I've made two minor cosmetic changes:
[..]
>> +	return jhash2(addr, 4, jhash_initval);
>
> I changed 4 to ARRAY_SIZE(addr)

agreed.

>> +		printk(KERN_ERR "NFQUEUE: number of queues (%u) out of range (got 
>> %u)\n",
>> +							info->queues_total, maxid);
>> +		return false;
> and these printks to use pr_err to bring the line size down.

Indeed, it looks better that way :-)

Thanks a lot Patrick.

Re: [PATCH 2/3] netfilter: NFQUEUE: queue balancing support

[Jan Engelhardt]

On Friday 2009-06-05 03:15, Florian Westphal wrote:
> 
>+static u32 hash_v4(const struct sk_buff *skb)
>+{
>+	const struct iphdr *iph = ip_hdr(skb);
>+	u32 ipaddr;
>+
>+	/* packets in either direction go into same queue */
>+	ipaddr = iph->saddr ^ iph->daddr;
>+
>+	return jhash_2words(ipaddr, iph->protocol, jhash_initval);
>+}
>+
>+static unsigned int
>+nfqueue_tg4_v1(struct sk_buff *skb, const struct xt_target_param *par)
>+{
>+	const struct xt_NFQ_info_v1 *info = par->targinfo;
>+	u32 queue = info->queuenum;
>+
>+	if (info->queues_total > 1)
>+		queue = hash_v4(skb) % info->queues_total + queue;
>+	return NF_QUEUE_NR(queue);
>+}
>+
>+#if defined(CONFIG_IP6_NF_IPTABLES) || defined(CONFIG_IP6_NF_IPTABLES_MODULE)
>+static u32 hash_v6(const struct sk_buff *skb)
>+{
>+	const struct ipv6hdr *ip6h = ipv6_hdr(skb);
>+	u32 addr[4];
>+
>+	addr[0] = ip6h->saddr.s6_addr32[0] ^ ip6h->daddr.s6_addr32[0];
>+	addr[1] = ip6h->saddr.s6_addr32[1] ^ ip6h->daddr.s6_addr32[1];
>+	addr[2] = ip6h->saddr.s6_addr32[2] ^ ip6h->daddr.s6_addr32[2];
>+	addr[3] = ip6h->saddr.s6_addr32[3] ^ ip6h->daddr.s6_addr32[3];
>+
>+	return jhash2(addr, 4, jhash_initval);
>+}
>+
>+static unsigned int
>+nfqueue_tg6_v1(struct sk_buff *skb, const struct xt_target_param *par)
>+{
>+	const struct xt_NFQ_info_v1 *info = par->targinfo;
>+	u32 queue = info->queuenum;
>+
>+	if (info->queues_total > 1)
>+		queue = hash_v6(skb) % info->queues_total + queue;
>+	return NF_QUEUE_NR(queue);
>+}
>+#endif
>+
>+static bool nfqueue_tg_v1_check(const struct xt_tgchk_param *par)
>+{
>+	const struct xt_NFQ_info_v1 *info = par->targinfo;
>+	u32 maxid;
>+
>+	if (info->queues_total == 0) {
>+		printk(KERN_ERR "NFQUEUE: number of total queues is 0\n");
>+		return false;
>+	}
>+	maxid = info->queues_total - 1  + info->queuenum;
>+	if (maxid > 0xffff) {
>+		printk(KERN_ERR "NFQUEUE: number of queues (%u) out of range (got %u)\n",
>+							info->queues_total, maxid);
>+		return false;
>+	}
>+	return true;
>+}
>+
> static struct xt_target nfqueue_tg_reg[] __read_mostly = {
> 	{
> 		.name		= "NFQUEUE",
>@@ -39,10 +111,31 @@ static struct xt_target nfqueue_tg_reg[] __read_mostly = {
> 		.targetsize	= sizeof(struct xt_NFQ_info),
> 		.me		= THIS_MODULE,
> 	},
>+	{
>+		.name		= "NFQUEUE",
>+		.revision	= 1,
>+		.family		= NFPROTO_IPV4,
>+		.checkentry	= nfqueue_tg_v1_check,
>+		.target		= nfqueue_tg4_v1,
>+		.targetsize	= sizeof(struct xt_NFQ_info_v1),
>+		.me		= THIS_MODULE,
>+	},
>+#if defined(CONFIG_IP6_NF_IPTABLES) || defined(CONFIG_IP6_NF_IPTABLES_MODULE)
>+	{
>+		.name		= "NFQUEUE",
>+		.revision	= 1,
>+		.family		= NFPROTO_IPV6,
>+		.checkentry	= nfqueue_tg_v1_check,
>+		.target		= nfqueue_tg6_v1,
>+		.targetsize	= sizeof(struct xt_NFQ_info_v1),
>+		.me		= THIS_MODULE,
>+	},
>+#endif

I'd say this could be done with much less code:

	(tg_reg with NFPROTO_UNSPEC entry) and

static bool nfqueue_v1(skb, tgpar)
{
	const struct xt_NFQ_info_v1 *info = par->targinfo;
	u32 queue = info->queuenum;

	if (info->queues_total > 1) {
		if (tgpar->target->family == NFPROTO_IPV4)
			queue = hash_v4(skb) % info->queues_total + queue;
#if IPV6
		else if (tgpar->target->family == NFPROTO_IPV6)
			queue = hash_v6(skb) % info->queues_total + queue;
#endif
	}
	return NF_QUEUE_NR(queue);
}

Re: [PATCH 2/3] netfilter: NFQUEUE: queue balancing support

[Jan Engelhardt]

On Friday 2009-06-05 15:07, Jan Engelhardt wrote:
>On Friday 2009-06-05 03:15, Florian Westphal wrote:
>> 
>>+static u32 hash_v4(const struct sk_buff *skb)
>>+{
>>+	const struct iphdr *iph = ip_hdr(skb);
>>+	u32 ipaddr;
>>+
>>+	/* packets in either direction go into same queue */
>>+	ipaddr = iph->saddr ^ iph->daddr;
>>+
>>+	return jhash_2words(ipaddr, iph->protocol, jhash_initval);
>>+}
>
>I'd say this could be done with much less code:[...]

Oh well, here's the patch because that's faster.
Compile tested only.


parent 17f2f52be0edb6d1ff5a3675f2bc545aea2dbf76 (v2.6.30-rc6-921-g17f2f52)
commit 6adde1879038923d6d34973af5504d381b4ce6d8
Author: Jan Engelhardt <jengelh@medozas.de>
Date:   Fri Jun 5 15:22:15 2009 +0200

netfilter: xt_NFQUEUE: consolidate v4/v6 targets into one

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 net/netfilter/xt_NFQUEUE.c |   40 ++++++++++-------------------------
 1 files changed, 12 insertions(+), 28 deletions(-)

diff --git a/net/netfilter/xt_NFQUEUE.c b/net/netfilter/xt_NFQUEUE.c
index 498b451..53c98db 100644
--- a/net/netfilter/xt_NFQUEUE.c
+++ b/net/netfilter/xt_NFQUEUE.c
@@ -48,17 +48,6 @@ static u32 hash_v4(const struct sk_buff *skb)
 	return jhash_2words(ipaddr, iph->protocol, jhash_initval);
 }
 
-static unsigned int
-nfqueue_tg4_v1(struct sk_buff *skb, const struct xt_target_param *par)
-{
-	const struct xt_NFQ_info_v1 *info = par->targinfo;
-	u32 queue = info->queuenum;
-
-	if (info->queues_total > 1)
-		queue = hash_v4(skb) % info->queues_total + queue;
-	return NF_QUEUE_NR(queue);
-}
-
 #if defined(CONFIG_IP6_NF_IPTABLES) || defined(CONFIG_IP6_NF_IPTABLES_MODULE)
 static u32 hash_v6(const struct sk_buff *skb)
 {
@@ -72,18 +61,24 @@ static u32 hash_v6(const struct sk_buff *skb)
 
 	return jhash2(addr, ARRAY_SIZE(addr), jhash_initval);
 }
+#endif
 
 static unsigned int
-nfqueue_tg6_v1(struct sk_buff *skb, const struct xt_target_param *par)
+nfqueue_tg_v1(struct sk_buff *skb, const struct xt_target_param *par)
 {
 	const struct xt_NFQ_info_v1 *info = par->targinfo;
 	u32 queue = info->queuenum;
 
-	if (info->queues_total > 1)
-		queue = hash_v6(skb) % info->queues_total + queue;
+	if (info->queues_total > 1) {
+		if (par->target->family == NFPROTO_IPV4)
+			queue = hash_v4(skb) % info->queues_total + queue;
+#if defined(CONFIG_IP6_NF_IPTABLES) || defined(CONFIG_IP6_NF_IPTABLES_MODULE)
+		else if (par->target->family == NFPROTO_IPV6)
+			queue = hash_v6(skb) % info->queues_total + queue;
+#endif
+	}
 	return NF_QUEUE_NR(queue);
 }
-#endif
 
 static bool nfqueue_tg_v1_check(const struct xt_tgchk_param *par)
 {
@@ -114,23 +109,12 @@ static struct xt_target nfqueue_tg_reg[] __read_mostly = {
 	{
 		.name		= "NFQUEUE",
 		.revision	= 1,
-		.family		= NFPROTO_IPV4,
-		.checkentry	= nfqueue_tg_v1_check,
-		.target		= nfqueue_tg4_v1,
-		.targetsize	= sizeof(struct xt_NFQ_info_v1),
-		.me		= THIS_MODULE,
-	},
-#if defined(CONFIG_IP6_NF_IPTABLES) || defined(CONFIG_IP6_NF_IPTABLES_MODULE)
-	{
-		.name		= "NFQUEUE",
-		.revision	= 1,
-		.family		= NFPROTO_IPV6,
+		.family		= NFPROTO_UNSPEC,
 		.checkentry	= nfqueue_tg_v1_check,
-		.target		= nfqueue_tg6_v1,
+		.target		= nfqueue_tg_v1,
 		.targetsize	= sizeof(struct xt_NFQ_info_v1),
 		.me		= THIS_MODULE,
 	},
-#endif
 };
 
 static int __init nfqueue_tg_init(void)
-- 
# Created with git-export-patch

[PATCH 3/3] netfilter: MASQUERADE: remove redundant rwlock

[Florian Westphal]

The lock "protects" an assignment and a comparision of an integer.
When the caller of device_cmp() evaluates the result, nat->masq_index
may already have been changed (regardless if the lock is there or not).

So, the lock either has to be held during nf_ct_iterate_cleanup(),
or can be removed.

This does the latter.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/ipv4/netfilter/ipt_MASQUERADE.c |   12 +-----------
 1 files changed, 1 insertions(+), 11 deletions(-)

diff --git a/net/ipv4/netfilter/ipt_MASQUERADE.c b/net/ipv4/netfilter/ipt_MASQUERADE.c
index f389f60..855505d 100644
--- a/net/ipv4/netfilter/ipt_MASQUERADE.c
+++ b/net/ipv4/netfilter/ipt_MASQUERADE.c
@@ -27,9 +27,6 @@ MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Netfilter Core Team <coreteam@netfilter.org>");
 MODULE_DESCRIPTION("Xtables: automatic-address SNAT");
 
-/* Lock protects masq region inside conntrack */
-static DEFINE_RWLOCK(masq_lock);
-
 /* FIXME: Multiple targets. --RR */
 static bool masquerade_tg_check(const struct xt_tgchk_param *par)
 {
@@ -79,9 +76,7 @@ masquerade_tg(struct sk_buff *skb, const struct xt_target_param *par)
 		return NF_DROP;
 	}
 
-	write_lock_bh(&masq_lock);
 	nat->masq_index = par->out->ifindex;
-	write_unlock_bh(&masq_lock);
 
 	/* Transfer from original range. */
 	newrange = ((struct nf_nat_range)
@@ -97,16 +92,11 @@ static int
 device_cmp(struct nf_conn *i, void *ifindex)
 {
 	const struct nf_conn_nat *nat = nfct_nat(i);
-	int ret;
 
 	if (!nat)
 		return 0;
 
-	read_lock_bh(&masq_lock);
-	ret = (nat->masq_index == (int)(long)ifindex);
-	read_unlock_bh(&masq_lock);
-
-	return ret;
+	return nat->masq_index == (int)(long)ifindex;
 }
 
 static int masq_device_event(struct notifier_block *this,
-- 
1.6.0.6

Re: [PATCH 3/3] netfilter: MASQUERADE: remove redundant rwlock

[Patrick McHardy]

Florian Westphal wrote:
> The lock "protects" an assignment and a comparision of an integer.
> When the caller of device_cmp() evaluates the result, nat->masq_index
> may already have been changed (regardless if the lock is there or not).
> 
> So, the lock either has to be held during nf_ct_iterate_cleanup(),
> or can be removed.
> 
> This does the latter.

Agreed, its useless. Applied, thanks.