All of lore.kernel.org
 help / color / mirror / Atom feed
* NFQUEUE balancing extension (kernel changes)
@ 2009-06-05  1:15 Florian Westphal
  2009-06-05  1:15 ` [PATCH 1/3] netfilter: NFQUEUE use NFPROTO_UNSPEC Florian Westphal
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Florian Westphal @ 2009-06-05  1:15 UTC (permalink / raw)
  To: netfilter-devel

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/3] netfilter: NFQUEUE use NFPROTO_UNSPEC
  2009-06-05  1:15 NFQUEUE balancing extension (kernel changes) Florian Westphal
@ 2009-06-05  1:15 ` Florian Westphal
  2009-06-05 11:19   ` Patrick McHardy
  2009-06-05  1:15 ` [PATCH 2/3] netfilter: NFQUEUE: queue balancing support Florian Westphal
  2009-06-05  1:15 ` [PATCH 3/3] netfilter: MASQUERADE: remove redundant rwlock Florian Westphal
  2 siblings, 1 reply; 12+ messages in thread
From: Florian Westphal @ 2009-06-05  1:15 UTC (permalink / raw)
  To: netfilter-devel; +Cc: 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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/3] netfilter: NFQUEUE: queue balancing support
  2009-06-05  1:15 NFQUEUE balancing extension (kernel changes) Florian Westphal
  2009-06-05  1:15 ` [PATCH 1/3] netfilter: NFQUEUE use NFPROTO_UNSPEC Florian Westphal
@ 2009-06-05  1:15 ` Florian Westphal
  2009-06-05  9:50   ` Pablo Neira Ayuso
                     ` (2 more replies)
  2009-06-05  1:15 ` [PATCH 3/3] netfilter: MASQUERADE: remove redundant rwlock Florian Westphal
  2 siblings, 3 replies; 12+ messages in thread
From: Florian Westphal @ 2009-06-05  1:15 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal, Holger Eitzenberger, 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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/3] netfilter: MASQUERADE: remove redundant rwlock
  2009-06-05  1:15 NFQUEUE balancing extension (kernel changes) Florian Westphal
  2009-06-05  1:15 ` [PATCH 1/3] netfilter: NFQUEUE use NFPROTO_UNSPEC Florian Westphal
  2009-06-05  1:15 ` [PATCH 2/3] netfilter: NFQUEUE: queue balancing support Florian Westphal
@ 2009-06-05  1:15 ` Florian Westphal
  2009-06-05 11:27   ` Patrick McHardy
  2 siblings, 1 reply; 12+ messages in thread
From: Florian Westphal @ 2009-06-05  1:15 UTC (permalink / raw)
  To: netfilter-devel; +Cc: 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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] netfilter: NFQUEUE: queue balancing support
  2009-06-05  1:15 ` [PATCH 2/3] netfilter: NFQUEUE: queue balancing support Florian Westphal
@ 2009-06-05  9:50   ` Pablo Neira Ayuso
  2009-06-05 10:43     ` Holger Eitzenberger
  2009-06-05 11:26   ` Patrick McHardy
  2009-06-05 13:07   ` Jan Engelhardt
  2 siblings, 1 reply; 12+ messages in thread
From: Pablo Neira Ayuso @ 2009-06-05  9:50 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel, Holger Eitzenberger, Florian Westphal

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] netfilter: NFQUEUE: queue balancing support
  2009-06-05  9:50   ` Pablo Neira Ayuso
@ 2009-06-05 10:43     ` Holger Eitzenberger
  0 siblings, 0 replies; 12+ messages in thread
From: Holger Eitzenberger @ 2009-06-05 10:43 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: Florian Westphal, netfilter-devel, Holger Eitzenberger, Florian Westphal

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] netfilter: NFQUEUE use NFPROTO_UNSPEC
  2009-06-05  1:15 ` [PATCH 1/3] netfilter: NFQUEUE use NFPROTO_UNSPEC Florian Westphal
@ 2009-06-05 11:19   ` Patrick McHardy
  0 siblings, 0 replies; 12+ messages in thread
From: Patrick McHardy @ 2009-06-05 11:19 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel

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

Applied, thanks.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] netfilter: NFQUEUE: queue balancing support
  2009-06-05  1:15 ` [PATCH 2/3] netfilter: NFQUEUE: queue balancing support Florian Westphal
  2009-06-05  9:50   ` Pablo Neira Ayuso
@ 2009-06-05 11:26   ` Patrick McHardy
  2009-06-05 13:35     ` Florian Westphal
  2009-06-05 13:07   ` Jan Engelhardt
  2 siblings, 1 reply; 12+ messages in thread
From: Patrick McHardy @ 2009-06-05 11:26 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel, Holger Eitzenberger, Florian Westphal

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.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] netfilter: MASQUERADE: remove redundant rwlock
  2009-06-05  1:15 ` [PATCH 3/3] netfilter: MASQUERADE: remove redundant rwlock Florian Westphal
@ 2009-06-05 11:27   ` Patrick McHardy
  0 siblings, 0 replies; 12+ messages in thread
From: Patrick McHardy @ 2009-06-05 11:27 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel

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.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] netfilter: NFQUEUE: queue balancing support
  2009-06-05  1:15 ` [PATCH 2/3] netfilter: NFQUEUE: queue balancing support Florian Westphal
  2009-06-05  9:50   ` Pablo Neira Ayuso
  2009-06-05 11:26   ` Patrick McHardy
@ 2009-06-05 13:07   ` Jan Engelhardt
  2009-06-05 13:23     ` Jan Engelhardt
  2 siblings, 1 reply; 12+ messages in thread
From: Jan Engelhardt @ 2009-06-05 13:07 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel, Holger Eitzenberger, Florian Westphal


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);
}


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] netfilter: NFQUEUE: queue balancing support
  2009-06-05 13:07   ` Jan Engelhardt
@ 2009-06-05 13:23     ` Jan Engelhardt
  0 siblings, 0 replies; 12+ messages in thread
From: Jan Engelhardt @ 2009-06-05 13:23 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel, Holger Eitzenberger, Florian Westphal


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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] netfilter: NFQUEUE: queue balancing support
  2009-06-05 11:26   ` Patrick McHardy
@ 2009-06-05 13:35     ` Florian Westphal
  0 siblings, 0 replies; 12+ messages in thread
From: Florian Westphal @ 2009-06-05 13:35 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: Florian Westphal, netfilter-devel, Holger Eitzenberger

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.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2009-06-05 13:35 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-06-05  1:15 NFQUEUE balancing extension (kernel changes) Florian Westphal
2009-06-05  1:15 ` [PATCH 1/3] netfilter: NFQUEUE use NFPROTO_UNSPEC Florian Westphal
2009-06-05 11:19   ` Patrick McHardy
2009-06-05  1:15 ` [PATCH 2/3] netfilter: NFQUEUE: queue balancing support Florian Westphal
2009-06-05  9:50   ` Pablo Neira Ayuso
2009-06-05 10:43     ` Holger Eitzenberger
2009-06-05 11:26   ` Patrick McHardy
2009-06-05 13:35     ` Florian Westphal
2009-06-05 13:07   ` Jan Engelhardt
2009-06-05 13:23     ` Jan Engelhardt
2009-06-05  1:15 ` [PATCH 3/3] netfilter: MASQUERADE: remove redundant rwlock Florian Westphal
2009-06-05 11:27   ` Patrick McHardy

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.