diff mbox

[nft] src: evaluate: Show error for fanout without balance

Message ID 20160407093640.GA5034@shivani
State Changes Requested
Delegated to: Pablo Neira
Headers show

Commit Message

Shivani Bhardwaj April 7, 2016, 9:36 a.m. UTC
The idea of fanout option is to improve the performance by indexing CPU
ID to map packets to the queues. This is used for load balancing.
Fanout option is not required when there is a single queue specified.

According to iptables, queue balance should be specified in order to use
fanout, following that, throw an error in nftables if the range of
queues for load balancing is not specified with the fanout option.

Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
---
 src/evaluate.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Pablo Neira Ayuso April 7, 2016, 5:13 p.m. UTC | #1
On Thu, Apr 07, 2016 at 03:06:40PM +0530, Shivani Bhardwaj wrote:
> The idea of fanout option is to improve the performance by indexing CPU
> ID to map packets to the queues. This is used for load balancing.
> Fanout option is not required when there is a single queue specified.
> 
> According to iptables, queue balance should be specified in order to use
> fanout, following that, throw an error in nftables if the range of
> queues for load balancing is not specified with the fanout option.

Curious, how does iptables behave when you pass fanout and a single
queue?

Could you also include how the nft error output looks like after your
patch in your description?

Thanks.
--
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
Shivani Bhardwaj April 7, 2016, 5:18 p.m. UTC | #2
On Thu, Apr 7, 2016 at 10:43 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Thu, Apr 07, 2016 at 03:06:40PM +0530, Shivani Bhardwaj wrote:
>> The idea of fanout option is to improve the performance by indexing CPU
>> ID to map packets to the queues. This is used for load balancing.
>> Fanout option is not required when there is a single queue specified.
>>
>> According to iptables, queue balance should be specified in order to use
>> fanout, following that, throw an error in nftables if the range of
>> queues for load balancing is not specified with the fanout option.
>
> Curious, how does iptables behave when you pass fanout and a single
> queue?
>

It throws an error:

$ sudo iptables -A FORWARD -j NFQUEUE --queue-num 0 --queue-cpu-fanout
iptables v1.6.0: NFQUEUE: option "--queue-cpu-fanout" also requires
"--queue-balance".

Try `iptables -h' or 'iptables --help' for more information.

Since, queue-balance is done as queue num with a range in nftables, I
thought it should follow the same routine as iptables.

> Could you also include how the nft error output looks like after your
> patch in your description?
>

Yes I'll do that.
Thanks.

> Thanks.
--
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
diff mbox

Patch

diff --git a/src/evaluate.c b/src/evaluate.c
index 473f014..f3fe13d 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -2000,6 +2000,11 @@  static int stmt_evaluate_queue(struct eval_ctx *ctx, struct stmt *stmt)
 		if (!expr_is_constant(stmt->queue.queue))
 			return expr_error(ctx->msgs, stmt->queue.queue,
 					  "queue number is not constant");
+		if (stmt->queue.queue->ops->type != EXPR_RANGE &&
+		    (stmt->queue.flags & NFT_QUEUE_FLAG_CPU_FANOUT))
+			return expr_error(ctx->msgs, stmt->queue.queue,
+					  "fanout requires queue num range"
+					  " to be specified");
 	}
 	return 0;
 }