This is a revert of commit 8d443adfcc8c1 ("evaluate: attempt to set_eval
flag if dynamic updates requested"), implementing the alternative
mentioned in the comment it added.
Reason is the inconsistent behaviour when applying the same ruleset
twice: In the first call, the set lacking 'dynamic' flag does not exist
and is therefore added to the cache. Consequently, both the 'add set'
command and the set statement point at the same set object. In the
second call, a set with same name exists already, so the object created
for 'add set' command is not added to cache and consequently not updated
with the missing flag. The kernel thus rejects the NEWSET request as the
existing set differs from the new one.
Fixes: 8d443adfcc8c1 ("evaluate: attempt to set_eval flag if dynamic updates requested")
Signed-off-by: Phil Sutter <phil@xxxxxx>
---
*Note*: This is the best I could come up with. If you see a better way
to fix this (i.e., fix up cmd->set from stmt_evaluate_set()), please
speak up. :)
---
src/evaluate.c | 13 ++------
.../testcases/sets/dumps/dynset_missing.nft | 12 -------
tests/shell/testcases/sets/dynset_missing | 32 -------------------
3 files changed, 3 insertions(+), 54 deletions(-)
delete mode 100644 tests/shell/testcases/sets/dumps/dynset_missing.nft
delete mode 100755 tests/shell/testcases/sets/dynset_missing
diff --git a/src/evaluate.c b/src/evaluate.c
index fe15d7ace5dde..9fb768cbd4c9f 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -4139,7 +4139,6 @@ static int stmt_evaluate_log(struct eval_ctx *ctx, struct stmt *stmt)
static int stmt_evaluate_set(struct eval_ctx *ctx, struct stmt *stmt)
{
- struct set *this_set;
struct stmt *this;
expr_set_context(&ctx->ectx, NULL, 0);
@@ -4148,6 +4147,9 @@ static int stmt_evaluate_set(struct eval_ctx *ctx, struct stmt *stmt)
if (stmt->set.set->etype != EXPR_SET_REF)
return expr_error(ctx->msgs, stmt->set.set,
"Expression does not refer to a set");
+ if (!(stmt->set.set->set->flags & NFT_SET_EVAL))
+ return expr_error(ctx->msgs, stmt->set.set,
+ "Referenced set lacks \"dynamic\" flag");
if (stmt_evaluate_key(ctx, stmt,
stmt->set.set->set->key->dtype,
@@ -4169,15 +4171,6 @@ static int stmt_evaluate_set(struct eval_ctx *ctx, struct stmt *stmt)
"statement must be stateful");
}
- this_set = stmt->set.set->set;
-
- /* Make sure EVAL flag is set on set definition so that kernel
- * picks a set that allows updates from the packet path.
- *
- * Alternatively we could error out in case 'flags dynamic' was
- * not given, but we can repair this here.
- */
- this_set->flags |= NFT_SET_EVAL;
return 0;
}
diff --git a/tests/shell/testcases/sets/dumps/dynset_missing.nft b/tests/shell/testcases/sets/dumps/dynset_missing.nft
deleted file mode 100644
index 6c8ed323bdc94..0000000000000
--- a/tests/shell/testcases/sets/dumps/dynset_missing.nft
+++ /dev/null
@@ -1,12 +0,0 @@
-table ip test {
- set dlist {
- type ipv4_addr
- size 65535
- flags dynamic
- }
-
- chain output {
- type filter hook output priority filter; policy accept;
- udp dport 1234 update @dlist { ip daddr } counter packets 0 bytes 0
- }
-}
diff --git a/tests/shell/testcases/sets/dynset_missing b/tests/shell/testcases/sets/dynset_missing
deleted file mode 100755
index fdf5f49edb9c6..0000000000000
--- a/tests/shell/testcases/sets/dynset_missing
+++ /dev/null
@@ -1,32 +0,0 @@
-#!/bin/bash
-
-set -e
-
-$NFT -f /dev/stdin <<EOF
-table ip test {
- chain output { type filter hook output priority 0;
- }
-}
-EOF
-
-# misses 'flags dynamic'
-$NFT 'add set ip test dlist {type ipv4_addr; }'
-
-# picks rhash backend because 'size' was also missing.
-$NFT 'add rule ip test output udp dport 1234 update @dlist { ip daddr } counter'
-
-tmpfile=$(mktemp)
-
-trap "rm -rf $tmpfile" EXIT
-
-# kernel has forced an 64k upper size, i.e. this restore file
-# has 'size 65536' but no 'flags dynamic'.
-$NFT list ruleset > $tmpfile
-
-# this restore works, because set is still the rhash backend.
-$NFT -f $tmpfile # success
-$NFT flush ruleset
-
-# fails without commit 'attempt to set_eval flag if dynamic updates requested',
-# because set in $tmpfile has 'size x' but no 'flags dynamic'.
-$NFT -f $tmpfile
--
2.40.0
