[PATCH nft,v3 1/3] optimize: more robust statement merge with vmap

[PATCH nft,v3 1/3] optimize: more robust statement merge with vmap

[Pablo Neira Ayuso]

Check expressions that are expected on the rhs rather than using a
catch-all default case.

Actually, lists and sets need to be their own routine, because this
needs the set element key expression to be merged.

This is a follow up to 99eb46969f3d ("optimize: fix vmap with anonymous
sets").

Fixes: 1542082e259b ("optimize: merge same selector with different verdict into verdict map")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
v3: - EXPR_VALUE is possible, parser might allocate this expression before eval.
    - EXPR_CONCAT is also supported.

 src/optimize.c                                 | 18 ++++++++++++++++--
 .../optimizations/dumps/merge_stmts_vmap.nft   |  2 +-
 .../testcases/optimizations/merge_stmts_vmap   |  1 +
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/src/optimize.c b/src/optimize.c
index 64c0a4dbe764..af075da437f9 100644
--- a/src/optimize.c
+++ b/src/optimize.c
@@ -437,7 +437,6 @@ static void build_verdict_map(struct expr *expr, struct stmt *verdict, struct ex
 
 	switch (expr->etype) {
 	case EXPR_LIST:
-	case EXPR_SET:
 		list_for_each_entry(item, &expr->expressions, list) {
 			elem = set_elem_expr_alloc(&internal_location, expr_get(item));
 			mapping = mapping_expr_alloc(&internal_location, elem,
@@ -445,12 +444,27 @@ static void build_verdict_map(struct expr *expr, struct stmt *verdict, struct ex
 			compound_expr_add(set, mapping);
 		}
 		break;
-	default:
+	case EXPR_SET:
+		list_for_each_entry(item, &expr->expressions, list) {
+			elem = set_elem_expr_alloc(&internal_location, expr_get(item->key));
+			mapping = mapping_expr_alloc(&internal_location, elem,
+						     expr_get(verdict->expr));
+			compound_expr_add(set, mapping);
+		}
+		break;
+	case EXPR_PREFIX:
+	case EXPR_RANGE:
+	case EXPR_VALUE:
+	case EXPR_SYMBOL:
+	case EXPR_CONCAT:
 		elem = set_elem_expr_alloc(&internal_location, expr_get(expr));
 		mapping = mapping_expr_alloc(&internal_location, elem,
 					     expr_get(verdict->expr));
 		compound_expr_add(set, mapping);
 		break;
+	default:
+		assert(0);
+		break;
 	}
 }
 
diff --git a/tests/shell/testcases/optimizations/dumps/merge_stmts_vmap.nft b/tests/shell/testcases/optimizations/dumps/merge_stmts_vmap.nft
index 427572954a18..5a9b3006743b 100644
--- a/tests/shell/testcases/optimizations/dumps/merge_stmts_vmap.nft
+++ b/tests/shell/testcases/optimizations/dumps/merge_stmts_vmap.nft
@@ -4,6 +4,6 @@ table ip x {
 	}
 
 	chain z {
-		tcp dport vmap { 1 : accept, 2-3 : drop }
+		tcp dport vmap { 1 : accept, 2-3 : drop, 4 : accept }
 	}
 }
diff --git a/tests/shell/testcases/optimizations/merge_stmts_vmap b/tests/shell/testcases/optimizations/merge_stmts_vmap
index 6511c7b20cb6..79350076d6c6 100755
--- a/tests/shell/testcases/optimizations/merge_stmts_vmap
+++ b/tests/shell/testcases/optimizations/merge_stmts_vmap
@@ -10,6 +10,7 @@ RULESET="table ip x {
 	chain z {
 		tcp dport { 1 } accept
 		tcp dport 2-3 drop
+		tcp dport 4 accept
 	}
 }"
 
-- 
2.30.2

[PATCH nft,v3 2/3] optimize: incorrect assert() for unexpected expression type

[Pablo Neira Ayuso]

assert(1) is noop, this should be assert(0) instead.

Fixes: 561aa3cfa8da ("optimize: merge verdict maps with same lookup key")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
v3: no changes

 src/optimize.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/optimize.c b/src/optimize.c
index af075da437f9..6ba7e5a10cf8 100644
--- a/src/optimize.c
+++ b/src/optimize.c
@@ -362,11 +362,11 @@ static void merge_verdict_stmts(const struct optimize_ctx *ctx,
 				merge_vmap(ctx, stmt_a, stmt_b);
 				break;
 			default:
-				assert(1);
+				assert(0);
 			}
 			break;
 		default:
-			assert(1);
+			assert(0);
 			break;
 		}
 	}
@@ -385,7 +385,7 @@ static void merge_stmts(const struct optimize_ctx *ctx,
 		merge_verdict_stmts(ctx, from, to, merge, stmt_a);
 		break;
 	default:
-		assert(1);
+		assert(0);
 	}
 }
 
-- 
2.30.2

[PATCH nft,v3 3/3] optimize: do not merge unsupported statement expressions

[Pablo Neira Ayuso]

Only value, range, prefix, set and list are supported at this stage.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
v3: EXPR_VALUE is possible, keep it

 src/optimize.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/src/optimize.c b/src/optimize.c
index 6ba7e5a10cf8..f8dd7f8d159f 100644
--- a/src/optimize.c
+++ b/src/optimize.c
@@ -91,6 +91,23 @@ static bool __expr_cmp(const struct expr *expr_a, const struct expr *expr_b)
 	return true;
 }
 
+static bool stmt_expr_supported(const struct expr *expr)
+{
+	switch (expr->right->etype) {
+	case EXPR_SYMBOL:
+	case EXPR_RANGE:
+	case EXPR_PREFIX:
+	case EXPR_SET:
+	case EXPR_LIST:
+	case EXPR_VALUE:
+		return true;
+	default:
+		break;
+	}
+
+	return false;
+}
+
 static bool __stmt_type_eq(const struct stmt *stmt_a, const struct stmt *stmt_b)
 {
 	struct expr *expr_a, *expr_b;
@@ -103,6 +120,10 @@ static bool __stmt_type_eq(const struct stmt *stmt_a, const struct stmt *stmt_b)
 		expr_a = stmt_a->expr;
 		expr_b = stmt_b->expr;
 
+		if (!stmt_expr_supported(expr_a) ||
+		    !stmt_expr_supported(expr_b))
+			return false;
+
 		return __expr_cmp(expr_a->left, expr_b->left);
 	case STMT_COUNTER:
 	case STMT_NOTRACK:
-- 
2.30.2