[PATCH nft,v2 1/3] src: fix reset element support for interval set type

[PATCH nft,v2 1/3] src: fix reset element support for interval set type

[Pablo Neira Ayuso]

From: Florian Westphal <fw@strlen.de>

Running reset command yields on an interval (rbtree) set yields:
nft reset element inet filter rbtreeset {1.2.3.4}
BUG: unhandled op 8

This is easy to fix, CMD_RESET doesn't add or remove so it should be
treated like CMD_GET.

Unfortunately, this still doesn't work properly:

nft get element inet filter rbset {1.2.3.4}
returns:
 ... elements = { 1.2.3.4 }

but its expected that "get" and "reset" also return stateful objects
associated with the element.  This works for other set types, but for
rbtree, the list of statements gets lost during segtree processing.

After fix, get/reset returns:
  elements = { 1.2.3.4 counter packets 10 ...

A follow up patch will add a test case.

Fixes: 83e0f4402fb7 ("Implement 'reset {set,map,element}' commands")
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
v2: while at this, fix maps too.

@Florian: This is a follow-up on top of your series.

 src/evaluate.c |  1 +
 src/segtree.c  | 36 ++++++++++++++++++++++++++++++------
 2 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/src/evaluate.c b/src/evaluate.c
index c9c56588cee4..e27d08ce7ef8 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1953,6 +1953,7 @@ static int interval_set_eval(struct eval_ctx *ctx, struct set *set,
 				 ctx->nft->debug_mask);
 		break;
 	case CMD_GET:
+	case CMD_RESET:
 		break;
 	default:
 		BUG("unhandled op %d\n", ctx->cmd->op);
diff --git a/src/segtree.c b/src/segtree.c
index 2e32a3291979..bce38eef293c 100644
--- a/src/segtree.c
+++ b/src/segtree.c
@@ -206,6 +206,27 @@ static struct expr *expr_to_set_elem(struct expr *e)
 	return __expr_to_set_elem(e, expr);
 }
 
+static void set_compound_expr_add(struct expr *compound, struct expr *expr, struct expr *orig)
+{
+	struct expr *elem;
+
+	switch (expr->etype) {
+	case EXPR_SET_ELEM:
+		list_splice_init(&orig->stmt_list, &expr->stmt_list);
+		compound_expr_add(compound, expr);
+		break;
+	case EXPR_MAPPING:
+		list_splice_init(&orig->left->stmt_list, &expr->left->stmt_list);
+		compound_expr_add(compound, expr);
+		break;
+	default:
+		elem = set_elem_expr_alloc(&orig->location, expr);
+		list_splice_init(&orig->stmt_list, &elem->stmt_list);
+		compound_expr_add(compound, elem);
+		break;
+	}
+}
+
 int get_set_decompose(struct set *cache_set, struct set *set)
 {
 	struct expr *i, *next, *range;
@@ -227,20 +248,23 @@ int get_set_decompose(struct set *cache_set, struct set *set)
 				errno = ENOENT;
 				return -1;
 			}
+
+			set_compound_expr_add(new_init, range, left);
+
 			expr_free(left);
 			expr_free(i);
 
-			compound_expr_add(new_init, range);
 			left = NULL;
 		} else {
 			if (left) {
 				range = get_set_interval_find(cache_set,
 							      left, NULL);
+
 				if (range)
-					compound_expr_add(new_init, range);
+					set_compound_expr_add(new_init, range, left);
 				else
-					compound_expr_add(new_init,
-							  expr_to_set_elem(left));
+					set_compound_expr_add(new_init,
+							      expr_to_set_elem(left), left);
 			}
 			left = i;
 		}
@@ -248,9 +272,9 @@ int get_set_decompose(struct set *cache_set, struct set *set)
 	if (left) {
 		range = get_set_interval_find(cache_set, left, NULL);
 		if (range)
-			compound_expr_add(new_init, range);
+			set_compound_expr_add(new_init, range, left);
 		else
-			compound_expr_add(new_init, expr_to_set_elem(left));
+			set_compound_expr_add(new_init, expr_to_set_elem(left), left);
 	}
 
 	expr_free(set->init);
-- 
2.30.2

[PATCH nft,v2 2/3] segtree: incomplete output in get element command with maps

[Pablo Neira Ayuso]

get element command displays an incomplete range:

 # nft get element x y { 1.1.1.2 }

Before:

 table ip x {
        map y {
                type ipv4_addr : mark
                flags interval,timeout
                elements = { 1.1.1.1 counter packets 0 bytes 0 timeout 10m expires 1m24s160ms : 0x00000014 }
        }
 }

Note that it displays 1.1.1.1, instead of 1.1.1.1-1.1.1.10.

After this fix:

 table ip x {
        map y {
                type ipv4_addr : mark
                flags interval,timeout
                elements = { 1.1.1.1-1.1.1.10 counter packets 0 bytes 0 timeout 10m expires 1m24s160ms : 0x00000014 }
        }
 }

Fixes: a43cc8d53096 ("src: support for get element command")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
v2: new in this series

 src/segtree.c | 39 ++++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/src/segtree.c b/src/segtree.c
index bce38eef293c..e785fc25b5a2 100644
--- a/src/segtree.c
+++ b/src/segtree.c
@@ -110,19 +110,34 @@ struct expr *get_set_intervals(const struct set *set, const struct expr *init)
 	return new_init;
 }
 
+static struct expr *expr_value(struct expr *expr)
+{
+	switch (expr->etype) {
+	case EXPR_MAPPING:
+		return expr->left->key;
+	case EXPR_SET_ELEM:
+		return expr->key;
+	case EXPR_VALUE:
+		return expr;
+	default:
+		BUG("invalid expression type %s\n", expr_name(expr));
+	}
+}
+
 static struct expr *get_set_interval_find(const struct set *cache_set,
 					  struct expr *left,
 					  struct expr *right)
 {
 	const struct set *set = cache_set;
 	struct expr *range = NULL;
-	struct expr *i;
+	struct expr *i, *key;
 	mpz_t val;
 
 	mpz_init2(val, set->key->len);
 
 	list_for_each_entry(i, &set->init->expressions, list) {
-		switch (i->key->etype) {
+		key = expr_value(i);
+		switch (key->etype) {
 		case EXPR_VALUE:
 			if (expr_basetype(i->key)->type != TYPE_STRING)
 				break;
@@ -131,14 +146,14 @@ static struct expr *get_set_interval_find(const struct set *cache_set,
 		case EXPR_PREFIX:
 		case EXPR_RANGE:
 			range_expr_value_low(val, i);
-			if (left && mpz_cmp(left->key->value, val))
+			if (left && mpz_cmp(expr_value(left)->value, val))
 				break;
 
 			range_expr_value_high(val, i);
-			if (right && mpz_cmp(right->key->value, val))
+			if (right && mpz_cmp(expr_value(right)->value, val))
 				break;
 
-			range = expr_clone(i->key);
+			range = expr_clone(i);
 			goto out;
 		default:
 			break;
@@ -150,20 +165,6 @@ out:
 	return range;
 }
 
-static struct expr *expr_value(struct expr *expr)
-{
-	switch (expr->etype) {
-	case EXPR_MAPPING:
-		return expr->left->key;
-	case EXPR_SET_ELEM:
-		return expr->key;
-	case EXPR_VALUE:
-		return expr;
-	default:
-		BUG("invalid expression type %s\n", expr_name(expr));
-	}
-}
-
 static struct expr *__expr_to_set_elem(struct expr *low, struct expr *expr)
 {
 	struct expr *elem = set_elem_expr_alloc(&low->location, expr);
-- 
2.30.2

[PATCH nft,v2 3/3] tests: extend reset test case to cover interval set and map type

[Pablo Neira Ayuso]

From: Florian Westphal <fw@strlen.de>

Make sure segtree processing doesn't drop associated stateful elements.

Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
v2: add map tests.

 tests/shell/testcases/sets/reset_command_0 | 70 ++++++++++++++++++----
 1 file changed, 60 insertions(+), 10 deletions(-)

diff --git a/tests/shell/testcases/sets/reset_command_0 b/tests/shell/testcases/sets/reset_command_0
index d38ddb3ffeeb..c59cc56d20b8 100755
--- a/tests/shell/testcases/sets/reset_command_0
+++ b/tests/shell/testcases/sets/reset_command_0
@@ -17,6 +17,19 @@ RULESET="table t {
 			2.0.0.2 . tcp . 22 counter packets 10 bytes 100 timeout 15m expires 10m
 		}
 	}
+
+	set s2 {
+		type ipv4_addr
+		flags interval, timeout
+		counter
+		timeout 30m
+		elements = {
+			1.0.0.1 counter packets 5 bytes 30 expires 20m,
+			1.0.1.1-1.0.1.10 counter packets 5 bytes 30 expires 20m,
+			2.0.0.2 counter packets 10 bytes 100 timeout 15m expires 10m
+		}
+	}
+
 	map m {
 		type ipv4_addr : ipv4_addr
 		quota 50 bytes
@@ -25,6 +38,27 @@ RULESET="table t {
 			5.6.7.8 quota 100 bytes used 50 bytes : 50.6.7.8
 		}
 	}
+
+	map m1 {
+		type ipv4_addr : ipv4_addr
+		counter
+		timeout 30m
+		elements = {
+			1.2.3.4 counter packets 5 bytes 30 expires 20m : 10.2.3.4,
+			5.6.7.8 counter packets 10 bytes 100 timeout 15m expires 10m : 50.6.7.8
+		}
+	}
+
+	map m2 {
+		type ipv4_addr : ipv4_addr
+		flags interval, timeout
+		counter
+		timeout 30m
+		elements = {
+			1.2.3.4-1.2.3.10 counter packets 5 bytes 30 expires 20m : 10.2.3.4,
+			5.6.7.8-5.6.7.10 counter packets 10 bytes 100 timeout 15m expires 10m : 50.6.7.8
+		}
+	}
 }"
 
 echo -n "applying test ruleset: "
@@ -38,17 +72,33 @@ expires_minutes() {
 	sed -n 's/.*expires \([0-9]*\)m.*/\1/p'
 }
 
-echo -n "get set elem matches reset set elem: "
-elem='element t s { 1.0.0.1 . udp . 53 }'
-[[ $($NFT "get $elem ; reset $elem" | \
-	grep 'elements = ' | drop_seconds | uniq | wc -l) == 1 ]]
-echo OK
+get_and_reset()
+{
+	local setname="$1"
+	local key="$2"
 
-echo -n "counters are reset, expiry left alone: "
-NEW=$($NFT "get $elem")
-grep -q 'counter packets 0 bytes 0' <<< "$NEW"
-[[ $(expires_minutes <<< "$NEW") -lt 20 ]]
-echo OK
+	echo -n "get set elem matches reset set elem in set $setname: "
+
+	elem="element t $setname { $key }"
+	echo $NFT get $elem
+	$NFT get $elem
+	[[ $($NFT "get $elem ; reset $elem" | \
+		grep 'elements = ' | drop_seconds | uniq | wc -l) == 1 ]]
+	echo OK
+
+	echo -n "counters are reset, expiry left alone in set $setname: "
+	NEW=$($NFT "get $elem")
+	echo NEW $NEW
+	grep -q 'counter packets 0 bytes 0' <<< "$NEW"
+	[[ $(expires_minutes <<< "$NEW") -lt 20 ]]
+	echo OK
+}
+
+get_and_reset "s" "1.0.0.1 . udp . 53"
+get_and_reset "s2" "1.0.0.1"
+get_and_reset "s2" "1.0.1.1-1.0.1.10"
+get_and_reset "m1" "1.2.3.4"
+get_and_reset "m2" "1.2.3.4-1.2.3.10"
 
 echo -n "get map elem matches reset map elem: "
 elem='element t m { 1.2.3.4 }'
-- 
2.30.2

Re: [PATCH nft,v2 1/3] src: fix reset element support for interval set type

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> From: Florian Westphal <fw@strlen.de>
> 
> Running reset command yields on an interval (rbtree) set yields:
> nft reset element inet filter rbtreeset {1.2.3.4}
> BUG: unhandled op 8

Thanks, please, push it out.

Re: [PATCH nft,v2 1/3] src: fix reset element support for interval set type

[Pablo Neira Ayuso]

On Fri, Mar 07, 2025 at 12:16:52PM +0100, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > From: Florian Westphal <fw@strlen.de>
> > 
> > Running reset command yields on an interval (rbtree) set yields:
> > nft reset element inet filter rbtreeset {1.2.3.4}
> > BUG: unhandled op 8
> 
> Thanks, please, push it out.

Done.