diff mbox

[RFC,3/3] NFQUEUE: add --queue-cpu-fanout parameter

Message ID 20130319141606.304161536@eitzenberger.org
State RFC
Headers show

Commit Message

holger@eitzenberger.org March 19, 2013, 2:14 p.m. UTC 
Signed-off-by: Holger Eitzenberger <holger@eitzenberger.org>

---
 extensions/libxt_NFQUEUE.c           |   59 +++++++++++++++++++++++++++++++++-
 include/linux/netfilter/xt_NFQUEUE.h |    8 +++++
 2 files changed, 66 insertions(+), 1 deletion(-)


--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Eric Leblond March 19, 2013, 2:34 p.m. UTC | #1 
Hello,

Cool job! This CPU-based setup has proven to be really efficient on
af_packet capture. I hope this will bring a performance boost to NFQ.

If possible, it could be interesting to be able to setup the balance
parameter by using an option in the same way fail-open option:  
       uint32_t flags = NFQA_CFG_F_FAIL_OPEN;
       uint32_t mask = NFQA_CFG_F_FAIL_OPEN;
       int r = nfq_set_queue_flags(qh, mask, flags);
This way, it is possible to tune the system without changing the
ruleset.

What do you think ?

BR,

On Tue, 2013-03-19 at 15:14 +0100, holger@eitzenberger.org wrote:
> plain text document attachment (iptables)
> Signed-off-by: Holger Eitzenberger <holger@eitzenberger.org>
> 
> ---
>  extensions/libxt_NFQUEUE.c           |   59 +++++++++++++++++++++++++++++++++-
>  include/linux/netfilter/xt_NFQUEUE.h |    8 +++++
>  2 files changed, 66 insertions(+), 1 deletion(-)
> 
> diff --git a/extensions/libxt_NFQUEUE.c b/extensions/libxt_NFQUEUE.c
> index 8c2f699..8106425 100644
> --- a/extensions/libxt_NFQUEUE.c
> +++ b/extensions/libxt_NFQUEUE.c
> @@ -13,8 +13,10 @@ enum {
>  	O_QUEUE_NUM = 0,
>  	O_QUEUE_BALANCE,
>  	O_QUEUE_BYPASS,
> +	O_QUEUE_CPU_FANOUT,
>  	F_QUEUE_NUM     = 1 << O_QUEUE_NUM,
>  	F_QUEUE_BALANCE = 1 << O_QUEUE_BALANCE,
> +	F_QUEUE_CPU_FANOUT = 1 << O_QUEUE_CPU_FANOUT,
>  };
>  
>  static void NFQUEUE_help(void)
> @@ -37,7 +39,15 @@ static void NFQUEUE_help_v2(void)
>  {
>  	NFQUEUE_help_v1();
>  	printf(
> -"  --queue-bypass		Bypass Queueing if no queue instance exists.\n");
> +"  --queue-bypass		Bypass Queueing if no queue instance exists.\n"
> +"  --queue-cpu-fanout	Use current CPU (no hashing)\n");
> +}
> +
> +static void NFQUEUE_help_v3(void)
> +{
> +	NFQUEUE_help_v2();
> +	printf(
> +"  --queue-cpu-fanout	Use current CPU (no hashing)\n");
>  }
>  
>  #define s struct xt_NFQ_info
> @@ -48,6 +58,8 @@ static const struct xt_option_entry NFQUEUE_opts[] = {
>  	{.name = "queue-balance", .id = O_QUEUE_BALANCE,
>  	 .type = XTTYPE_UINT16RC, .excl = F_QUEUE_NUM},
>  	{.name = "queue-bypass", .id = O_QUEUE_BYPASS, .type = XTTYPE_NONE},
> +	{.name = "queue-cpu-fanout", .id = O_QUEUE_CPU_FANOUT,
> +	 .type = XTTYPE_NONE, .also = O_QUEUE_BALANCE},
>  	XTOPT_TABLEEND,
>  };
>  #undef s
> @@ -92,6 +104,18 @@ static void NFQUEUE_parse_v2(struct xt_option_call *cb)
>  	}
>  }
>  
> +static void NFQUEUE_parse_v3(struct xt_option_call *cb)
> +{
> +	struct xt_NFQ_info_v3 *info = cb->data;
> +
> +	NFQUEUE_parse_v2(cb);
> +	switch (cb->entry->id) {
> +	case O_QUEUE_CPU_FANOUT:
> +		info->flags |= NFQ_FLAG_CPU_FANOUT;
> +		break;
> +	}
> +}
> +
>  static void NFQUEUE_print(const void *ip,
>                            const struct xt_entry_target *target, int numeric)
>  {
> @@ -124,6 +148,16 @@ static void NFQUEUE_print_v2(const void *ip,
>  		printf(" bypass");
>  }
>  
> +static void NFQUEUE_print_v3(const void *ip,
> +                             const struct xt_entry_target *target, int numeric)
> +{
> +	const struct xt_NFQ_info_v3 *info = (void *)target->data;
> +
> +	NFQUEUE_print_v2(ip, target, numeric);
> +	if (info->flags & NFQ_FLAG_CPU_FANOUT)
> +		printf(" cpu-fanout");
> +}
> +
>  static void NFQUEUE_save(const void *ip, const struct xt_entry_target *target)
>  {
>  	const struct xt_NFQ_info *tinfo =
> @@ -155,6 +189,16 @@ static void NFQUEUE_save_v2(const void *ip, const struct xt_entry_target *target
>  		printf(" --queue-bypass");
>  }
>  
> +static void NFQUEUE_save_v3(const void *ip,
> +			    const struct xt_entry_target *target)
> +{
> +	const struct xt_NFQ_info_v3 *info = (void *)target->data;
> +
> +	NFQUEUE_save_v2(ip, target);
> +	if (info->flags & NFQ_FLAG_CPU_FANOUT)
> +		printf(" --queue-cpu-fanout");
> +}
> +
>  static void NFQUEUE_init_v1(struct xt_entry_target *t)
>  {
>  	struct xt_NFQ_info_v1 *tinfo = (void *)t->data;
> @@ -199,6 +243,19 @@ static struct xtables_target nfqueue_targets[] = {
>  	.save		= NFQUEUE_save_v2,
>  	.x6_parse	= NFQUEUE_parse_v2,
>  	.x6_options	= NFQUEUE_opts,
> +},{
> +	.family		= NFPROTO_UNSPEC,
> +	.revision	= 3,
> +	.name		= "NFQUEUE",
> +	.version	= XTABLES_VERSION,
> +	.size		= XT_ALIGN(sizeof(struct xt_NFQ_info_v3)),
> +	.userspacesize	= XT_ALIGN(sizeof(struct xt_NFQ_info_v3)),
> +	.help		= NFQUEUE_help_v3,
> +	.init		= NFQUEUE_init_v1,
> +	.print		= NFQUEUE_print_v3,
> +	.save		= NFQUEUE_save_v3,
> +	.x6_parse	= NFQUEUE_parse_v3,
> +	.x6_options	= NFQUEUE_opts,
>  }
>  };
>  
> diff --git a/include/linux/netfilter/xt_NFQUEUE.h b/include/linux/netfilter/xt_NFQUEUE.h
> index 9eafdbb..1f24680 100644
> --- a/include/linux/netfilter/xt_NFQUEUE.h
> +++ b/include/linux/netfilter/xt_NFQUEUE.h
> @@ -26,4 +26,12 @@ struct xt_NFQ_info_v2 {
>  	__u16 bypass;
>  };
>  
> +struct xt_NFQ_info_v3 {
> +	__u16 queuenum;
> +	__u16 queues_total;
> +	__u16 bypass;
> +	__u16 flags;
> +#define NFQ_FLAG_CPU_FANOUT		0x01 /* use current CPU (no hashing) */
> +};
> +
>  #endif /* _XT_NFQ_TARGET_H */
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
holger@eitzenberger.org March 19, 2013, 4:07 p.m. UTC | #2 
Hi Eric,

> Cool job! This CPU-based setup has proven to be really efficient on
> af_packet capture. I hope this will bring a performance boost to NFQ.
> 
> If possible, it could be interesting to be able to setup the balance
> parameter by using an option in the same way fail-open option:  
>        uint32_t flags = NFQA_CFG_F_FAIL_OPEN;
>        uint32_t mask = NFQA_CFG_F_FAIL_OPEN;
>        int r = nfq_set_queue_flags(qh, mask, flags);
> This way, it is possible to tune the system without changing the
> ruleset.
> 
> What do you think ?

seems like a good change to me.  I will add that as well!

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
holger@eitzenberger.org March 23, 2013, 7:52 p.m. UTC | #3 
Hi Eric,

> If possible, it could be interesting to be able to setup the balance
> parameter by using an option in the same way fail-open option:  
>        uint32_t flags = NFQA_CFG_F_FAIL_OPEN;
>        uint32_t mask = NFQA_CFG_F_FAIL_OPEN;
>        int r = nfq_set_queue_flags(qh, mask, flags);
> This way, it is possible to tune the system without changing the
> ruleset.

Not sure how the FAIL_OPEN relates to the CPU fanout.  Do you want to
setup the CPU fanout (on, off) per queue?

 /Holger

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Leblond March 23, 2013, 9:53 p.m. UTC | #4 
Hi,

On Sat, 2013-03-23 at 20:52 +0100, Holger Eitzenberger wrote:
> Hi Eric,
> 
> > If possible, it could be interesting to be able to setup the balance
> > parameter by using an option in the same way fail-open option:  
> >        uint32_t flags = NFQA_CFG_F_FAIL_OPEN;
> >        uint32_t mask = NFQA_CFG_F_FAIL_OPEN;
> >        int r = nfq_set_queue_flags(qh, mask, flags);
> > This way, it is possible to tune the system without changing the
> > ruleset.
> 
> Not sure how the FAIL_OPEN relates to the CPU fanout.  Do you want to
> setup the CPU fanout (on, off) per queue?

You are right on a major point. FAIL_OPEN is definitely per-queue and
CPU fanout is related to a fanout group. So we will have to have the
whole mechanism bind to a queue, warn the kernel that it is in a fanout
group...

That could be interesting as we will be able to tune the setting from
the application but this seems to be a lot of work too. And I don't see
any functional benefit omit this tuning from the application.

So, if you don't find any other one, forgot my request for now :)

BR,
diff mbox

Patch

diff --git a/extensions/libxt_NFQUEUE.c b/extensions/libxt_NFQUEUE.c
index 8c2f699..8106425 100644
--- a/extensions/libxt_NFQUEUE.c
+++ b/extensions/libxt_NFQUEUE.c
@@ -13,8 +13,10 @@  enum {
 	O_QUEUE_NUM = 0,
 	O_QUEUE_BALANCE,
 	O_QUEUE_BYPASS,
+	O_QUEUE_CPU_FANOUT,
 	F_QUEUE_NUM     = 1 << O_QUEUE_NUM,
 	F_QUEUE_BALANCE = 1 << O_QUEUE_BALANCE,
+	F_QUEUE_CPU_FANOUT = 1 << O_QUEUE_CPU_FANOUT,
 };
 
 static void NFQUEUE_help(void)
@@ -37,7 +39,15 @@  static void NFQUEUE_help_v2(void)
 {
 	NFQUEUE_help_v1();
 	printf(
-"  --queue-bypass		Bypass Queueing if no queue instance exists.\n");
+"  --queue-bypass		Bypass Queueing if no queue instance exists.\n"
+"  --queue-cpu-fanout	Use current CPU (no hashing)\n");
+}
+
+static void NFQUEUE_help_v3(void)
+{
+	NFQUEUE_help_v2();
+	printf(
+"  --queue-cpu-fanout	Use current CPU (no hashing)\n");
 }
 
 #define s struct xt_NFQ_info
@@ -48,6 +58,8 @@  static const struct xt_option_entry NFQUEUE_opts[] = {
 	{.name = "queue-balance", .id = O_QUEUE_BALANCE,
 	 .type = XTTYPE_UINT16RC, .excl = F_QUEUE_NUM},
 	{.name = "queue-bypass", .id = O_QUEUE_BYPASS, .type = XTTYPE_NONE},
+	{.name = "queue-cpu-fanout", .id = O_QUEUE_CPU_FANOUT,
+	 .type = XTTYPE_NONE, .also = O_QUEUE_BALANCE},
 	XTOPT_TABLEEND,
 };
 #undef s
@@ -92,6 +104,18 @@  static void NFQUEUE_parse_v2(struct xt_option_call *cb)
 	}
 }
 
+static void NFQUEUE_parse_v3(struct xt_option_call *cb)
+{
+	struct xt_NFQ_info_v3 *info = cb->data;
+
+	NFQUEUE_parse_v2(cb);
+	switch (cb->entry->id) {
+	case O_QUEUE_CPU_FANOUT:
+		info->flags |= NFQ_FLAG_CPU_FANOUT;
+		break;
+	}
+}
+
 static void NFQUEUE_print(const void *ip,
                           const struct xt_entry_target *target, int numeric)
 {
@@ -124,6 +148,16 @@  static void NFQUEUE_print_v2(const void *ip,
 		printf(" bypass");
 }
 
+static void NFQUEUE_print_v3(const void *ip,
+                             const struct xt_entry_target *target, int numeric)
+{
+	const struct xt_NFQ_info_v3 *info = (void *)target->data;
+
+	NFQUEUE_print_v2(ip, target, numeric);
+	if (info->flags & NFQ_FLAG_CPU_FANOUT)
+		printf(" cpu-fanout");
+}
+
 static void NFQUEUE_save(const void *ip, const struct xt_entry_target *target)
 {
 	const struct xt_NFQ_info *tinfo =
@@ -155,6 +189,16 @@  static void NFQUEUE_save_v2(const void *ip, const struct xt_entry_target *target
 		printf(" --queue-bypass");
 }
 
+static void NFQUEUE_save_v3(const void *ip,
+			    const struct xt_entry_target *target)
+{
+	const struct xt_NFQ_info_v3 *info = (void *)target->data;
+
+	NFQUEUE_save_v2(ip, target);
+	if (info->flags & NFQ_FLAG_CPU_FANOUT)
+		printf(" --queue-cpu-fanout");
+}
+
 static void NFQUEUE_init_v1(struct xt_entry_target *t)
 {
 	struct xt_NFQ_info_v1 *tinfo = (void *)t->data;
@@ -199,6 +243,19 @@  static struct xtables_target nfqueue_targets[] = {
 	.save		= NFQUEUE_save_v2,
 	.x6_parse	= NFQUEUE_parse_v2,
 	.x6_options	= NFQUEUE_opts,
+},{
+	.family		= NFPROTO_UNSPEC,
+	.revision	= 3,
+	.name		= "NFQUEUE",
+	.version	= XTABLES_VERSION,
+	.size		= XT_ALIGN(sizeof(struct xt_NFQ_info_v3)),
+	.userspacesize	= XT_ALIGN(sizeof(struct xt_NFQ_info_v3)),
+	.help		= NFQUEUE_help_v3,
+	.init		= NFQUEUE_init_v1,
+	.print		= NFQUEUE_print_v3,
+	.save		= NFQUEUE_save_v3,
+	.x6_parse	= NFQUEUE_parse_v3,
+	.x6_options	= NFQUEUE_opts,
 }
 };
 
diff --git a/include/linux/netfilter/xt_NFQUEUE.h b/include/linux/netfilter/xt_NFQUEUE.h
index 9eafdbb..1f24680 100644
--- a/include/linux/netfilter/xt_NFQUEUE.h
+++ b/include/linux/netfilter/xt_NFQUEUE.h
@@ -26,4 +26,12 @@  struct xt_NFQ_info_v2 {
 	__u16 bypass;
 };
 
+struct xt_NFQ_info_v3 {
+	__u16 queuenum;
+	__u16 queues_total;
+	__u16 bypass;
+	__u16 flags;
+#define NFQ_FLAG_CPU_FANOUT		0x01 /* use current CPU (no hashing) */
+};
+
 #endif /* _XT_NFQ_TARGET_H */