[PATCH nft 1/3] evaluate: allow stateful statements with anonymous verdict maps

[PATCH nft 1/3] evaluate: allow stateful statements with anonymous verdict maps

[Pablo Neira Ayuso]

Evaluation fails to accept stateful statements in verdict maps, relax
the following check for anonymous sets:

test.nft:4:29-35: Error: missing statement in map declaration
                ip saddr vmap { 127.0.0.1 counter : drop, * : accep
                                          ^^^^^^^

The existing code generates correctly the counter in the anonymous
verdict map.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/evaluate.c                                  | 3 ++-
 tests/shell/testcases/maps/0009vmap_0           | 2 +-
 tests/shell/testcases/maps/dumps/0009vmap_0.nft | 2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/evaluate.c b/src/evaluate.c
index a1c3895cfb02..bc8f437ee7ea 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1604,7 +1604,8 @@ static int __expr_evaluate_set_elem(struct eval_ctx *ctx, struct expr *elem)
 					  "but element has %d", num_set_exprs,
 					  num_elem_exprs);
 		} else if (num_set_exprs == 0) {
-			if (!(set->flags & NFT_SET_EVAL)) {
+			if (!(set->flags & NFT_SET_ANONYMOUS) &&
+			    !(set->flags & NFT_SET_EVAL)) {
 				elem_stmt = list_first_entry(&elem->stmt_list, struct stmt, list);
 				return stmt_error(ctx, elem_stmt,
 						  "missing statement in %s declaration",
diff --git a/tests/shell/testcases/maps/0009vmap_0 b/tests/shell/testcases/maps/0009vmap_0
index 7627c81d99e0..d31e1608f792 100755
--- a/tests/shell/testcases/maps/0009vmap_0
+++ b/tests/shell/testcases/maps/0009vmap_0
@@ -12,7 +12,7 @@ EXPECTED="table inet filter {
 
         chain prerouting {
                 type filter hook prerouting priority -300; policy accept;
-                iif vmap { "lo" : jump wan_input }
+                iif vmap { "lo" counter : jump wan_input }
         }
 }"
 
diff --git a/tests/shell/testcases/maps/dumps/0009vmap_0.nft b/tests/shell/testcases/maps/dumps/0009vmap_0.nft
index c556feceb1aa..c37574ad5fad 100644
--- a/tests/shell/testcases/maps/dumps/0009vmap_0.nft
+++ b/tests/shell/testcases/maps/dumps/0009vmap_0.nft
@@ -8,6 +8,6 @@ table inet filter {
 
 	chain prerouting {
 		type filter hook prerouting priority raw; policy accept;
-		iif vmap { "lo" : jump wan_input }
+		iif vmap { "lo" counter packets 0 bytes 0 : jump wan_input }
 	}
 }
-- 
2.30.2

[PATCH nft 2/3] evaluate: skip optimization if anonymous set uses stateful statement

[Pablo Neira Ayuso]

fee6bda06403 ("evaluate: remove anon sets with exactly one element")
introduces an optimization to remove use of sets with single element.
Skip this optimization if set element contains stateful statements.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/evaluate.c                                                 | 2 +-
 tests/shell/testcases/optimizations/dumps/single_anon_set.nft  | 1 +
 .../testcases/optimizations/dumps/single_anon_set.nft.input    | 3 +++
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/evaluate.c b/src/evaluate.c
index bc8f437ee7ea..08243220f159 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1802,7 +1802,7 @@ static int expr_evaluate_set(struct eval_ctx *ctx, struct expr **expr)
 			set->set_flags |= NFT_SET_CONCAT;
 	} else if (set->size == 1) {
 		i = list_first_entry(&set->expressions, struct expr, list);
-		if (i->etype == EXPR_SET_ELEM) {
+		if (i->etype == EXPR_SET_ELEM && list_empty(&i->stmt_list)) {
 			switch (i->key->etype) {
 			case EXPR_PREFIX:
 			case EXPR_RANGE:
diff --git a/tests/shell/testcases/optimizations/dumps/single_anon_set.nft b/tests/shell/testcases/optimizations/dumps/single_anon_set.nft
index 35e3f36e1a54..3f703034d80f 100644
--- a/tests/shell/testcases/optimizations/dumps/single_anon_set.nft
+++ b/tests/shell/testcases/optimizations/dumps/single_anon_set.nft
@@ -11,5 +11,6 @@ table ip test {
 		ip daddr . tcp dport { 192.168.0.1 . 22 } accept
 		meta mark set ip daddr map { 192.168.0.1 : 0x00000001 }
 		ct state { established, related } accept
+		meta mark { 0x0000000a counter packets 0 bytes 0 }
 	}
 }
diff --git a/tests/shell/testcases/optimizations/dumps/single_anon_set.nft.input b/tests/shell/testcases/optimizations/dumps/single_anon_set.nft.input
index 35b93832420f..ecc5691ba581 100644
--- a/tests/shell/testcases/optimizations/dumps/single_anon_set.nft.input
+++ b/tests/shell/testcases/optimizations/dumps/single_anon_set.nft.input
@@ -31,5 +31,8 @@ table ip test {
 		# ct state cannot be both established and related
 		# at the same time, but this needs extra work.
 		ct state { established, related } accept
+
+		# with stateful statement
+		meta mark { 0x0000000a counter }
 	}
 }
-- 
2.30.2

[PATCH nft 3/3] optimize: do not remove counter in verdict maps

[Pablo Neira Ayuso]

Add counter to set element instead of dropping it:

 # nft -c -o -f test.nft
 Merging:
 test.nft:6:3-50:              ip saddr 1.1.1.1 ip daddr 2.2.2.2 counter accept
 test.nft:7:3-48:              ip saddr 1.1.1.2 ip daddr 3.3.3.3 counter drop
 into:
       ip daddr . ip saddr vmap { 2.2.2.2 . 1.1.1.1 counter : accept, 3.3.3.3 . 1.1.1.2 counter : drop }

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/optimize.c                                | 50 ++++++++++++++++---
 .../optimizations/dumps/merge_stmts_vmap.nft  |  4 ++
 .../testcases/optimizations/merge_stmts_vmap  |  4 ++
 3 files changed, 51 insertions(+), 7 deletions(-)

diff --git a/src/optimize.c b/src/optimize.c
index 22dfbcd92e5e..7ca57ce73873 100644
--- a/src/optimize.c
+++ b/src/optimize.c
@@ -689,7 +689,8 @@ static void merge_concat_stmts(const struct optimize_ctx *ctx,
 	}
 }
 
-static void build_verdict_map(struct expr *expr, struct stmt *verdict, struct expr *set)
+static void build_verdict_map(struct expr *expr, struct stmt *verdict,
+			      struct expr *set, struct stmt *counter)
 {
 	struct expr *item, *elem, *mapping;
 
@@ -697,6 +698,9 @@ static void build_verdict_map(struct expr *expr, struct stmt *verdict, struct ex
 	case EXPR_LIST:
 		list_for_each_entry(item, &expr->expressions, list) {
 			elem = set_elem_expr_alloc(&internal_location, expr_get(item));
+			if (counter)
+				list_add_tail(&counter->list, &elem->stmt_list);
+
 			mapping = mapping_expr_alloc(&internal_location, elem,
 						     expr_get(verdict->expr));
 			compound_expr_add(set, mapping);
@@ -705,6 +709,9 @@ static void build_verdict_map(struct expr *expr, struct stmt *verdict, struct ex
 	case EXPR_SET:
 		list_for_each_entry(item, &expr->expressions, list) {
 			elem = set_elem_expr_alloc(&internal_location, expr_get(item->key));
+			if (counter)
+				list_add_tail(&counter->list, &elem->stmt_list);
+
 			mapping = mapping_expr_alloc(&internal_location, elem,
 						     expr_get(verdict->expr));
 			compound_expr_add(set, mapping);
@@ -716,6 +723,9 @@ static void build_verdict_map(struct expr *expr, struct stmt *verdict, struct ex
 	case EXPR_SYMBOL:
 	case EXPR_CONCAT:
 		elem = set_elem_expr_alloc(&internal_location, expr_get(expr));
+		if (counter)
+			list_add_tail(&counter->list, &elem->stmt_list);
+
 		mapping = mapping_expr_alloc(&internal_location, elem,
 					     expr_get(verdict->expr));
 		compound_expr_add(set, mapping);
@@ -744,6 +754,26 @@ static void remove_counter(const struct optimize_ctx *ctx, uint32_t from)
 	}
 }
 
+static struct stmt *zap_counter(const struct optimize_ctx *ctx, uint32_t from)
+{
+	struct stmt *stmt;
+	uint32_t i;
+
+	/* remove counter statement */
+	for (i = 0; i < ctx->num_stmts; i++) {
+		stmt = ctx->stmt_matrix[from][i];
+		if (!stmt)
+			continue;
+
+		if (stmt->ops->type == STMT_COUNTER) {
+			list_del(&stmt->list);
+			return stmt;
+		}
+	}
+
+	return NULL;
+}
+
 static void merge_stmts_vmap(const struct optimize_ctx *ctx,
 			     uint32_t from, uint32_t to,
 			     const struct merge *merge)
@@ -751,31 +781,33 @@ static void merge_stmts_vmap(const struct optimize_ctx *ctx,
 	struct stmt *stmt_a = ctx->stmt_matrix[from][merge->stmt[0]];
 	struct stmt *stmt_b, *verdict_a, *verdict_b, *stmt;
 	struct expr *expr_a, *expr_b, *expr, *left, *set;
+	struct stmt *counter;
 	uint32_t i;
 	int k;
 
 	k = stmt_verdict_find(ctx);
 	assert(k >= 0);
 
-	verdict_a = ctx->stmt_matrix[from][k];
 	set = set_expr_alloc(&internal_location, NULL);
 	set->set_flags |= NFT_SET_ANONYMOUS;
 
 	expr_a = stmt_a->expr->right;
-	build_verdict_map(expr_a, verdict_a, set);
+	verdict_a = ctx->stmt_matrix[from][k];
+	counter = zap_counter(ctx, from);
+	build_verdict_map(expr_a, verdict_a, set, counter);
+
 	for (i = from + 1; i <= to; i++) {
 		stmt_b = ctx->stmt_matrix[i][merge->stmt[0]];
 		expr_b = stmt_b->expr->right;
 		verdict_b = ctx->stmt_matrix[i][k];
-
-		build_verdict_map(expr_b, verdict_b, set);
+		counter = zap_counter(ctx, i);
+		build_verdict_map(expr_b, verdict_b, set, counter);
 	}
 
 	left = expr_get(stmt_a->expr->left);
 	expr = map_expr_alloc(&internal_location, left, set);
 	stmt = verdict_stmt_alloc(&internal_location, expr);
 
-	remove_counter(ctx, from);
 	list_add(&stmt->list, &stmt_a->list);
 	list_del(&stmt_a->list);
 	stmt_free(stmt_a);
@@ -789,12 +821,17 @@ static void __merge_concat_stmts_vmap(const struct optimize_ctx *ctx,
 {
 	struct expr *concat, *next, *elem, *mapping;
 	LIST_HEAD(concat_list);
+	struct stmt *counter;
 
+	counter = zap_counter(ctx, i);
 	__merge_concat(ctx, i, merge, &concat_list);
 
 	list_for_each_entry_safe(concat, next, &concat_list, list) {
 		list_del(&concat->list);
 		elem = set_elem_expr_alloc(&internal_location, concat);
+		if (counter)
+			list_add_tail(&counter->list, &elem->stmt_list);
+
 		mapping = mapping_expr_alloc(&internal_location, elem,
 					     expr_get(verdict->expr));
 		compound_expr_add(set, mapping);
@@ -833,7 +870,6 @@ static void merge_concat_stmts_vmap(const struct optimize_ctx *ctx,
 	expr = map_expr_alloc(&internal_location, concat_a, set);
 	stmt = verdict_stmt_alloc(&internal_location, expr);
 
-	remove_counter(ctx, from);
 	list_add(&stmt->list, &orig_stmt->list);
 	list_del(&orig_stmt->list);
 	stmt_free(orig_stmt);
diff --git a/tests/shell/testcases/optimizations/dumps/merge_stmts_vmap.nft b/tests/shell/testcases/optimizations/dumps/merge_stmts_vmap.nft
index 5a9b3006743b..8ecbd9276fd9 100644
--- a/tests/shell/testcases/optimizations/dumps/merge_stmts_vmap.nft
+++ b/tests/shell/testcases/optimizations/dumps/merge_stmts_vmap.nft
@@ -6,4 +6,8 @@ table ip x {
 	chain z {
 		tcp dport vmap { 1 : accept, 2-3 : drop, 4 : accept }
 	}
+
+	chain w {
+		ip saddr vmap { 1.1.1.1 counter packets 0 bytes 0 : accept, 1.1.1.2 counter packets 0 bytes 0 : drop }
+	}
 }
diff --git a/tests/shell/testcases/optimizations/merge_stmts_vmap b/tests/shell/testcases/optimizations/merge_stmts_vmap
index 79350076d6c6..6e0f0762b7bb 100755
--- a/tests/shell/testcases/optimizations/merge_stmts_vmap
+++ b/tests/shell/testcases/optimizations/merge_stmts_vmap
@@ -12,6 +12,10 @@ RULESET="table ip x {
 		tcp dport 2-3 drop
 		tcp dport 4 accept
 	}
+	chain w {
+		ip saddr 1.1.1.1 counter accept
+		ip saddr 1.1.1.2 counter drop
+	}
 }"
 
 $NFT -o -f - <<< $RULESET
-- 
2.30.2