* [PATCH nft] evaluate: attempt to set_eval flag if dynamic updates requested
@ 2022-01-11 11:35 Florian Westphal
2022-01-11 11:52 ` Pablo Neira Ayuso
0 siblings, 1 reply; 2+ messages in thread
From: Florian Westphal @ 2022-01-11 11:35 UTC (permalink / raw)
To: netfilter-devel; +Cc: Florian Westphal, Yi Chen, Pablo Neira Ayuso
When passing no upper size limit, the dynset expression forces
an internal 64k upperlimit.
In some cases, this can result in 'nft -f' to restore the ruleset.
Avoid this by always setting the EVAL flag on a set definition when
we encounter packet-path update attempt in the batch.
Reported-by: Yi Chen <yiche@redhat.com>
Suggested-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
src/evaluate.c | 10 ++++++
.../testcases/sets/dumps/dynset_missing.nft | 12 +++++++
tests/shell/testcases/sets/dynset_missing | 32 +++++++++++++++++++
3 files changed, 54 insertions(+)
create mode 100644 tests/shell/testcases/sets/dumps/dynset_missing.nft
create mode 100755 tests/shell/testcases/sets/dynset_missing
diff --git a/src/evaluate.c b/src/evaluate.c
index 8edefbd1be21..437eacb8209f 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -3621,6 +3621,7 @@ 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);
@@ -3650,6 +3651,15 @@ 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
new file mode 100644
index 000000000000..6c8ed323bdc9
--- /dev/null
+++ b/tests/shell/testcases/sets/dumps/dynset_missing.nft
@@ -0,0 +1,12 @@
+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
new file mode 100755
index 000000000000..fdf5f49edb9c
--- /dev/null
+++ b/tests/shell/testcases/sets/dynset_missing
@@ -0,0 +1,32 @@
+#!/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.34.1
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH nft] evaluate: attempt to set_eval flag if dynamic updates requested
2022-01-11 11:35 [PATCH nft] evaluate: attempt to set_eval flag if dynamic updates requested Florian Westphal
@ 2022-01-11 11:52 ` Pablo Neira Ayuso
0 siblings, 0 replies; 2+ messages in thread
From: Pablo Neira Ayuso @ 2022-01-11 11:52 UTC (permalink / raw)
To: Florian Westphal; +Cc: netfilter-devel, Yi Chen
On Tue, Jan 11, 2022 at 12:35:31PM +0100, Florian Westphal wrote:
> When passing no upper size limit, the dynset expression forces
> an internal 64k upperlimit.
>
> In some cases, this can result in 'nft -f' to restore the ruleset.
> Avoid this by always setting the EVAL flag on a set definition when
> we encounter packet-path update attempt in the batch.
LGTM.
> Reported-by: Yi Chen <yiche@redhat.com>
> Suggested-by: Pablo Neira Ayuso <pablo@netfilter.org>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
> src/evaluate.c | 10 ++++++
> .../testcases/sets/dumps/dynset_missing.nft | 12 +++++++
> tests/shell/testcases/sets/dynset_missing | 32 +++++++++++++++++++
> 3 files changed, 54 insertions(+)
> create mode 100644 tests/shell/testcases/sets/dumps/dynset_missing.nft
> create mode 100755 tests/shell/testcases/sets/dynset_missing
>
> diff --git a/src/evaluate.c b/src/evaluate.c
> index 8edefbd1be21..437eacb8209f 100644
> --- a/src/evaluate.c
> +++ b/src/evaluate.c
> @@ -3621,6 +3621,7 @@ 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);
> @@ -3650,6 +3651,15 @@ 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
> new file mode 100644
> index 000000000000..6c8ed323bdc9
> --- /dev/null
> +++ b/tests/shell/testcases/sets/dumps/dynset_missing.nft
> @@ -0,0 +1,12 @@
> +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
> new file mode 100755
> index 000000000000..fdf5f49edb9c
> --- /dev/null
> +++ b/tests/shell/testcases/sets/dynset_missing
> @@ -0,0 +1,32 @@
> +#!/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.34.1
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-01-11 11:52 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-01-11 11:35 [PATCH nft] evaluate: attempt to set_eval flag if dynamic updates requested Florian Westphal
2022-01-11 11:52 ` Pablo Neira Ayuso
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).