All of lore.kernel.org
 help / color / mirror / Atom feed
From: Florian Westphal <fw@strlen.de>
To: <netfilter-devel@vger.kernel.org>
Cc: Florian Westphal <fw@strlen.de>, Yi Chen <yiche@redhat.com>,
	Pablo Neira Ayuso <pablo@netfilter.org>
Subject: [PATCH nft] evaluate: attempt to set_eval flag if dynamic updates requested
Date: Tue, 11 Jan 2022 12:35:31 +0100	[thread overview]
Message-ID: <20220111113531.4849-1-fw@strlen.de> (raw)

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


             reply	other threads:[~2022-01-11 11:35 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-11 11:35 Florian Westphal [this message]
2022-01-11 11:52 ` [PATCH nft] evaluate: attempt to set_eval flag if dynamic updates requested Pablo Neira Ayuso

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220111113531.4849-1-fw@strlen.de \
    --to=fw@strlen.de \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=pablo@netfilter.org \
    --cc=yiche@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.