[RFC] nftables 0.9.8 -stable backports

[RFC] nftables 0.9.8 -stable backports

[Pablo Neira Ayuso]

[-- Attachment #1: Type: text/plain, Size: 1635 bytes --]

Hi Arturo, Jeremy,

This is a small batch offering fixes for nftables 0.9.8. It only
includes the fixes for the implicit chain regression in recent
kernels.

This is a few dependency patches that are missing in 0.9.8 are
required:

        3542e49cf539 ("evaluate: init cmd pointer for new on-stack context")
        a3ac2527724d ("src: split chain list in table")
        4e718641397c ("cache: rename chain_htable to cache_chain_ht")

a3ac2527724d is fixing an issue with the cache that is required by the
fixes. Then, the backport fixes for the implicit chain regression with
Linux -stable:

        3975430b12d9 ("src: expand table command before evaluation")
        27c753e4a8d4 ("rule: expand standalone chain that contains rules")
        784597a4ed63 ("rule: add helper function to expand chain rules into commands")

I tested with tests/shell at the time of the nftables 0.9.8 release
(*I did not use git HEAD tests/shell as I did for 1.0.6*).

I have kept back the backport of this patch intentionally:

        56c90a2dd2eb ("evaluate: expand sets and maps before evaluation")

this depends on the new src/interval.c code, in 0.9.8 overlap and
automerge come a later stage and cache is not updated incrementally,
I tried the tests coming in this patch and it works fine.

I did run a few more tests with rulesets that I have been collecting
from people that occasionally send them to me for my personal ruleset
repo.

I: results: [OK] 266 [FAILED] 0 [TOTAL] 266

This has been tested with latest Linux kernel 5.10 -stable.

I can still run a few more tests, I will get back to you if I find any
issue.

Let me know, thanks.

[-- Attachment #2: 0001-cache-rename-chain_htable-to-cache_chain_ht.patch --]
[-- Type: text/x-diff, Size: 3105 bytes --]

From 0a39091a75b6255422832126df4cbf73c86845cd Mon Sep 17 00:00:00 2001
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Thu, 1 Apr 2021 22:18:29 +0200
Subject: [PATCH nft 0.9.8 1/7] cache: rename chain_htable to cache_chain_ht

upstream 3542e49cf539ecfcef6ef7c2d4befb7896ade2cd commit.

Rename the hashtable chain that is used for fast cache lookups.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/rule.h | 4 ++--
 src/cache.c    | 6 +++---
 src/rule.c     | 6 +++---
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/include/rule.h b/include/rule.h
index 330a09aa77fa..43872db8947a 100644
--- a/include/rule.h
+++ b/include/rule.h
@@ -154,7 +154,7 @@ struct table {
 	struct handle		handle;
 	struct location		location;
 	struct scope		scope;
-	struct list_head	*chain_htable;
+	struct list_head	*cache_chain_ht;
 	struct list_head	chains;
 	struct list_head	sets;
 	struct list_head	objs;
@@ -220,7 +220,7 @@ struct hook_spec {
  */
 struct chain {
 	struct list_head	list;
-	struct list_head	hlist;
+	struct list_head	cache_hlist;
 	struct handle		handle;
 	struct location		location;
 	unsigned int		refcnt;
diff --git a/src/cache.c b/src/cache.c
index ed2609008e22..7101b74160be 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -194,7 +194,7 @@ static int chain_cache_cb(struct nftnl_chain *nlc, void *arg)
 	if (chain->flags & CHAIN_F_BINDING) {
 		list_add_tail(&chain->list, &ctx->table->chain_bindings);
 	} else {
-		list_add_tail(&chain->hlist, &ctx->table->chain_htable[hash]);
+		list_add_tail(&chain->cache_hlist, &ctx->table->cache_chain_ht[hash]);
 		list_add_tail(&chain->list, &ctx->table->chains);
 	}
 
@@ -238,7 +238,7 @@ void chain_cache_add(struct chain *chain, struct table *table)
 	uint32_t hash;
 
 	hash = djb_hash(chain->handle.chain.name) % NFT_CACHE_HSIZE;
-	list_add_tail(&chain->hlist, &table->chain_htable[hash]);
+	list_add_tail(&chain->cache_hlist, &table->cache_chain_ht[hash]);
 	list_add_tail(&chain->list, &table->chains);
 }
 
@@ -249,7 +249,7 @@ struct chain *chain_cache_find(const struct table *table,
 	uint32_t hash;
 
 	hash = djb_hash(handle->chain.name) % NFT_CACHE_HSIZE;
-	list_for_each_entry(chain, &table->chain_htable[hash], hlist) {
+	list_for_each_entry(chain, &table->cache_chain_ht[hash], cache_hlist) {
 		if (!strcmp(chain->handle.chain.name, handle->chain.name))
 			return chain;
 	}
diff --git a/src/rule.c b/src/rule.c
index e4bb6bae276a..3b445851f657 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -1328,10 +1328,10 @@ struct table *table_alloc(void)
 	init_list_head(&table->scope.symbols);
 	table->refcnt = 1;
 
-	table->chain_htable =
+	table->cache_chain_ht =
 		xmalloc(sizeof(struct list_head) * NFT_CACHE_HSIZE);
 	for (i = 0; i < NFT_CACHE_HSIZE; i++)
-		init_list_head(&table->chain_htable[i]);
+		init_list_head(&table->cache_chain_ht[i]);
 
 	return table;
 }
@@ -1359,7 +1359,7 @@ void table_free(struct table *table)
 		obj_free(obj);
 	handle_free(&table->handle);
 	scope_release(&table->scope);
-	xfree(table->chain_htable);
+	xfree(table->cache_chain_ht);
 	xfree(table);
 }
 
-- 
2.30.2


[-- Attachment #3: 0002-src-split-chain-list-in-table.patch --]
[-- Type: text/x-diff, Size: 6922 bytes --]

From f37e4261130b021edf068e4d5c6ca062ce4e2ac1 Mon Sep 17 00:00:00 2001
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Thu, 1 Apr 2021 22:19:30 +0200
Subject: [PATCH nft 0.9.8 2/7] src: split chain list in table

upstream a3ac2527724dd27628e12caaa55f731b109e4586 commit.

This patch splits table->lists in two:

- Chains that reside in the cache are stored in the new
  tables->cache_chain and tables->cache_chain_ht. The hashtable chain
  cache allows for fast chain lookups.

- Chains that defined via command line / ruleset file reside in
  tables->chains.

Note that chains in the cache (already in the kernel) are not placed in
the table->chains.

By keeping separated lists, chains defined via command line / ruleset
file can be added to cache.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/rule.h |  2 ++
 src/cache.c    |  6 +++---
 src/json.c     |  6 +++---
 src/rule.c     | 18 +++++++++++-------
 4 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/include/rule.h b/include/rule.h
index 43872db8947a..dde32367f48f 100644
--- a/include/rule.h
+++ b/include/rule.h
@@ -155,6 +155,7 @@ struct table {
 	struct location		location;
 	struct scope		scope;
 	struct list_head	*cache_chain_ht;
+	struct list_head	cache_chain;
 	struct list_head	chains;
 	struct list_head	sets;
 	struct list_head	objs;
@@ -221,6 +222,7 @@ struct hook_spec {
 struct chain {
 	struct list_head	list;
 	struct list_head	cache_hlist;
+	struct list_head	cache_list;
 	struct handle		handle;
 	struct location		location;
 	unsigned int		refcnt;
diff --git a/src/cache.c b/src/cache.c
index 7101b74160be..32e6eea4ac5c 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -192,10 +192,10 @@ static int chain_cache_cb(struct nftnl_chain *nlc, void *arg)
 	chain = netlink_delinearize_chain(ctx->nlctx, nlc);
 
 	if (chain->flags & CHAIN_F_BINDING) {
-		list_add_tail(&chain->list, &ctx->table->chain_bindings);
+		list_add_tail(&chain->cache_list, &ctx->table->chain_bindings);
 	} else {
 		list_add_tail(&chain->cache_hlist, &ctx->table->cache_chain_ht[hash]);
-		list_add_tail(&chain->list, &ctx->table->chains);
+		list_add_tail(&chain->cache_list, &ctx->table->cache_chain);
 	}
 
 	nftnl_chain_list_del(nlc);
@@ -239,7 +239,7 @@ void chain_cache_add(struct chain *chain, struct table *table)
 
 	hash = djb_hash(chain->handle.chain.name) % NFT_CACHE_HSIZE;
 	list_add_tail(&chain->cache_hlist, &table->cache_chain_ht[hash]);
-	list_add_tail(&chain->list, &table->chains);
+	list_add_tail(&chain->cache_list, &table->cache_chain);
 }
 
 struct chain *chain_cache_find(const struct table *table,
diff --git a/src/json.c b/src/json.c
index 585d35326ac0..737e73b08b5a 100644
--- a/src/json.c
+++ b/src/json.c
@@ -1594,7 +1594,7 @@ static json_t *table_print_json_full(struct netlink_ctx *ctx,
 		tmp = flowtable_print_json(flowtable);
 		json_array_append_new(root, tmp);
 	}
-	list_for_each_entry(chain, &table->chains, list) {
+	list_for_each_entry(chain, &table->cache_chain, cache_list) {
 		tmp = chain_print_json(chain);
 		json_array_append_new(root, tmp);
 
@@ -1656,7 +1656,7 @@ static json_t *do_list_chain_json(struct netlink_ctx *ctx,
 	struct chain *chain;
 	struct rule *rule;
 
-	list_for_each_entry(chain, &table->chains, list) {
+	list_for_each_entry(chain, &table->cache_chain, cache_list) {
 		if (chain->handle.family != cmd->handle.family ||
 		    strcmp(cmd->handle.chain.name, chain->handle.chain.name))
 			continue;
@@ -1684,7 +1684,7 @@ static json_t *do_list_chains_json(struct netlink_ctx *ctx, struct cmd *cmd)
 		    cmd->handle.family != table->handle.family)
 			continue;
 
-		list_for_each_entry(chain, &table->chains, list) {
+		list_for_each_entry(chain, &table->cache_chain, cache_list) {
 			json_t *tmp = chain_print_json(chain);
 
 			json_array_append_new(root, tmp);
diff --git a/src/rule.c b/src/rule.c
index 3b445851f657..f76c27c9d091 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -953,7 +953,7 @@ struct chain *chain_lookup(const struct table *table, const struct handle *h)
 {
 	struct chain *chain;
 
-	list_for_each_entry(chain, &table->chains, list) {
+	list_for_each_entry(chain, &table->cache_chain, cache_list) {
 		if (!strcmp(chain->handle.chain.name, h->chain.name))
 			return chain;
 	}
@@ -965,7 +965,7 @@ struct chain *chain_binding_lookup(const struct table *table,
 {
 	struct chain *chain;
 
-	list_for_each_entry(chain, &table->chain_bindings, list) {
+	list_for_each_entry(chain, &table->chain_bindings, cache_list) {
 		if (!strcmp(chain->handle.chain.name, chain_name))
 			return chain;
 	}
@@ -983,7 +983,7 @@ struct chain *chain_lookup_fuzzy(const struct handle *h,
 	string_misspell_init(&st);
 
 	list_for_each_entry(table, &cache->list, list) {
-		list_for_each_entry(chain, &table->chains, list) {
+		list_for_each_entry(chain, &table->cache_chain, cache_list) {
 			if (!strcmp(chain->handle.chain.name, h->chain.name)) {
 				*t = table;
 				return chain;
@@ -1321,6 +1321,7 @@ struct table *table_alloc(void)
 
 	table = xzalloc(sizeof(*table));
 	init_list_head(&table->chains);
+	init_list_head(&table->cache_chain);
 	init_list_head(&table->sets);
 	init_list_head(&table->objs);
 	init_list_head(&table->flowtables);
@@ -1349,7 +1350,10 @@ void table_free(struct table *table)
 		xfree(table->comment);
 	list_for_each_entry_safe(chain, next, &table->chains, list)
 		chain_free(chain);
-	list_for_each_entry_safe(chain, next, &table->chain_bindings, list)
+	list_for_each_entry_safe(chain, next, &table->chain_bindings, cache_list)
+		chain_free(chain);
+	/* this is implicitly releasing chains in the hashtable cache */
+	list_for_each_entry_safe(chain, next, &table->cache_chain, cache_list)
 		chain_free(chain);
 	list_for_each_entry_safe(set, nset, &table->sets, list)
 		set_free(set);
@@ -1465,7 +1469,7 @@ static void table_print(const struct table *table, struct output_ctx *octx)
 		flowtable_print(flowtable, octx);
 		delim = "\n";
 	}
-	list_for_each_entry(chain, &table->chains, list) {
+	list_for_each_entry(chain, &table->cache_chain, cache_list) {
 		nft_print(octx, "%s", delim);
 		chain_print(chain, octx);
 		delim = "\n";
@@ -2555,7 +2559,7 @@ static int do_list_chain(struct netlink_ctx *ctx, struct cmd *cmd,
 
 	table_print_declaration(table, &ctx->nft->output);
 
-	list_for_each_entry(chain, &table->chains, list) {
+	list_for_each_entry(chain, &table->cache_chain, cache_list) {
 		if (chain->handle.family != cmd->handle.family ||
 		    strcmp(cmd->handle.chain.name, chain->handle.chain.name) != 0)
 			continue;
@@ -2580,7 +2584,7 @@ static int do_list_chains(struct netlink_ctx *ctx, struct cmd *cmd)
 
 		table_print_declaration(table, &ctx->nft->output);
 
-		list_for_each_entry(chain, &table->chains, list) {
+		list_for_each_entry(chain, &table->cache_chain, cache_list) {
 			chain_print_declaration(chain, &ctx->nft->output);
 			nft_print(&ctx->nft->output, "\t}\n");
 		}
-- 
2.30.2


[-- Attachment #4: 0003-evaluate-init-cmd-pointer-for-new-on-stack-context.patch --]
[-- Type: text/x-diff, Size: 1447 bytes --]

From 5af65a30a12396281c751e635509ab1d9363f4bc Mon Sep 17 00:00:00 2001
From: Florian Westphal <fw@strlen.de>
Date: Fri, 4 Mar 2022 11:30:55 +0100
Subject: [PATCH nft 0.9.8 3/7] evaluate: init cmd pointer for new on-stack
 context

upstream 4e718641397c876315a87db441afc53139863122 commit

else, this will segfault when trying to print the
"table 'x' doesn't exist" error message.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/evaluate.c                                   | 1 +
 tests/shell/testcases/chains/0041chain_binding_0 | 6 ++++++
 2 files changed, 7 insertions(+)

diff --git a/src/evaluate.c b/src/evaluate.c
index c830dcdbd965..f546667131e1 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -3211,6 +3211,7 @@ static int stmt_evaluate_chain(struct eval_ctx *ctx, struct stmt *stmt)
 			struct eval_ctx rule_ctx = {
 				.nft	= ctx->nft,
 				.msgs	= ctx->msgs,
+				.cmd	= ctx->cmd,
 			};
 			struct handle h2 = {};
 
diff --git a/tests/shell/testcases/chains/0041chain_binding_0 b/tests/shell/testcases/chains/0041chain_binding_0
index 59bdbe9f0ba9..4b541bb55c30 100755
--- a/tests/shell/testcases/chains/0041chain_binding_0
+++ b/tests/shell/testcases/chains/0041chain_binding_0
@@ -1,5 +1,11 @@
 #!/bin/bash
 
+# no table x, caused segfault in earlier nft releases
+$NFT insert rule inet x y handle 107 'goto { log prefix "MOO! "; }'
+if [ $? -ne 1 ]; then
+	exit 1
+fi
+
 set -e
 
 EXPECTED="table inet x {
-- 
2.30.2


[-- Attachment #5: 0004-rule-add-helper-function-to-expand-chain-rules-into-.patch --]
[-- Type: text/x-diff, Size: 2485 bytes --]

From 0f559011ee7e805df883be635b88396639fbb87e Mon Sep 17 00:00:00 2001
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Tue, 12 Sep 2023 23:22:46 +0200
Subject: [PATCH nft 0.9.8 4/7] rule: add helper function to expand chain rules
 into commands

upstream 784597a4ed63b9decb10d74fdb49a1b021e22728 commit.

This patch adds a helper function to expand chain rules into commands.
This comes in preparation for the follow up patch.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/rule.c | 39 ++++++++++++++++++++++-----------------
 1 file changed, 22 insertions(+), 17 deletions(-)

diff --git a/src/rule.c b/src/rule.c
index f76c27c9d091..3fbf4271accd 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -1503,6 +1503,25 @@ void cmd_add_loc(struct cmd *cmd, uint16_t offset, const struct location *loc)
 	cmd->num_attrs++;
 }
 
+static void nft_cmd_expand_chain(struct chain *chain, struct list_head *new_cmds)
+{
+	struct rule *rule;
+	struct handle h;
+	struct cmd *new;
+
+	list_for_each_entry(rule, &chain->rules, list) {
+		memset(&h, 0, sizeof(h));
+		handle_merge(&h, &rule->handle);
+		if (chain->flags & CHAIN_F_BINDING) {
+			rule->handle.chain_id = chain->handle.chain_id;
+			rule->handle.chain.location = chain->location;
+		}
+		new = cmd_alloc(CMD_ADD, CMD_OBJ_RULE, &h,
+				&rule->location, rule_get(rule));
+		list_add_tail(&new->list, new_cmds);
+	}
+}
+
 void nft_cmd_expand(struct cmd *cmd)
 {
 	struct list_head new_cmds;
@@ -1510,7 +1529,6 @@ void nft_cmd_expand(struct cmd *cmd)
 	struct flowtable *ft;
 	struct table *table;
 	struct chain *chain;
-	struct rule *rule;
 	struct obj *obj;
 	struct cmd *new;
 	struct handle h;
@@ -1555,22 +1573,9 @@ void nft_cmd_expand(struct cmd *cmd)
 					&ft->location, flowtable_get(ft));
 			list_add_tail(&new->list, &new_cmds);
 		}
-		list_for_each_entry(chain, &table->chains, list) {
-			list_for_each_entry(rule, &chain->rules, list) {
-				memset(&h, 0, sizeof(h));
-				handle_merge(&h, &rule->handle);
-				if (chain->flags & CHAIN_F_BINDING) {
-					rule->handle.chain_id =
-						chain->handle.chain_id;
-					rule->handle.chain.location =
-						chain->location;
-				}
-				new = cmd_alloc(CMD_ADD, CMD_OBJ_RULE, &h,
-						&rule->location,
-						rule_get(rule));
-				list_add_tail(&new->list, &new_cmds);
-			}
-		}
+		list_for_each_entry(chain, &table->chains, list)
+			nft_cmd_expand_chain(chain, &new_cmds);
+
 		list_splice(&new_cmds, &cmd->list);
 		break;
 	case CMD_OBJ_SET:
-- 
2.30.2


[-- Attachment #6: 0005-rule-expand-standalone-chain-that-contains-rules.patch --]
[-- Type: text/x-diff, Size: 3637 bytes --]

From f03bc399d75ef724fcbed184f74fc306ca8ef324 Mon Sep 17 00:00:00 2001
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Mon, 6 Feb 2023 15:28:41 +0100
Subject: [PATCH nft 0.9.8 5/7] rule: expand standalone chain that contains
 rules

upstream 27c753e4a8d4744f479345e3f5e34cafef751602 commit.

Otherwise rules that this chain contains are ignored when expressed
using the following syntax:

chain inet filter input2 {
       type filter hook input priority filter; policy accept;
       ip saddr 1.2.3.4 tcp dport { 22, 443, 123 } drop
}

When expanding the chain, remove the rule so the new CMD_OBJ_CHAIN
case does not expand it again.

Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1655
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/rule.c                                    | 15 +++++++++---
 .../testcases/include/0020include_chain_0     | 23 +++++++++++++++++++
 .../include/dumps/0020include_chain_0.nft     |  6 +++++
 3 files changed, 41 insertions(+), 3 deletions(-)
 create mode 100755 tests/shell/testcases/include/0020include_chain_0
 create mode 100644 tests/shell/testcases/include/dumps/0020include_chain_0.nft

diff --git a/src/rule.c b/src/rule.c
index 3fbf4271accd..9139418e1bf8 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -1505,11 +1505,12 @@ void cmd_add_loc(struct cmd *cmd, uint16_t offset, const struct location *loc)
 
 static void nft_cmd_expand_chain(struct chain *chain, struct list_head *new_cmds)
 {
-	struct rule *rule;
+	struct rule *rule, *next;
 	struct handle h;
 	struct cmd *new;
 
-	list_for_each_entry(rule, &chain->rules, list) {
+	list_for_each_entry_safe(rule, next, &chain->rules, list) {
+		list_del(&rule->list);
 		memset(&h, 0, sizeof(h));
 		handle_merge(&h, &rule->handle);
 		if (chain->flags & CHAIN_F_BINDING) {
@@ -1517,7 +1518,7 @@ static void nft_cmd_expand_chain(struct chain *chain, struct list_head *new_cmds
 			rule->handle.chain.location = chain->location;
 		}
 		new = cmd_alloc(CMD_ADD, CMD_OBJ_RULE, &h,
-				&rule->location, rule_get(rule));
+				&rule->location, rule);
 		list_add_tail(&new->list, new_cmds);
 	}
 }
@@ -1578,6 +1579,14 @@ void nft_cmd_expand(struct cmd *cmd)
 
 		list_splice(&new_cmds, &cmd->list);
 		break;
+	case CMD_OBJ_CHAIN:
+		chain = cmd->chain;
+		if (!chain || list_empty(&chain->rules))
+			break;
+
+		nft_cmd_expand_chain(chain, &new_cmds);
+		list_splice(&new_cmds, &cmd->list);
+		break;
 	case CMD_OBJ_SET:
 	case CMD_OBJ_MAP:
 		set = cmd->set;
diff --git a/tests/shell/testcases/include/0020include_chain_0 b/tests/shell/testcases/include/0020include_chain_0
new file mode 100755
index 000000000000..2ff83c92fda8
--- /dev/null
+++ b/tests/shell/testcases/include/0020include_chain_0
@@ -0,0 +1,23 @@
+#!/bin/bash
+
+set -e
+
+tmpfile1=$(mktemp -p .)
+if [ ! -w $tmpfile1 ] ; then
+	echo "Failed to create tmp file" >&2
+	exit 0
+fi
+
+trap "rm -rf $tmpfile1" EXIT # cleanup if aborted
+
+RULESET="table inet filter { }
+include \"$tmpfile1\""
+
+RULESET2="chain inet filter input2 {
+	type filter hook input priority filter; policy accept;
+	ip saddr 1.2.3.4 tcp dport { 22, 443, 123 } drop
+}"
+
+echo "$RULESET2" > $tmpfile1
+
+$NFT -f - <<< $RULESET
diff --git a/tests/shell/testcases/include/dumps/0020include_chain_0.nft b/tests/shell/testcases/include/dumps/0020include_chain_0.nft
new file mode 100644
index 000000000000..3ad6db14d2f5
--- /dev/null
+++ b/tests/shell/testcases/include/dumps/0020include_chain_0.nft
@@ -0,0 +1,6 @@
+table inet filter {
+	chain input2 {
+		type filter hook input priority filter; policy accept;
+		ip saddr 1.2.3.4 tcp dport { 22, 123, 443 } drop
+	}
+}
-- 
2.30.2


[-- Attachment #7: 0006-src-expand-table-command-before-evaluation.patch --]
[-- Type: text/x-diff, Size: 6794 bytes --]

From 050e0b7a85016b733e1a59285df501d1c05eec0b Mon Sep 17 00:00:00 2001
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Tue, 12 Sep 2023 23:25:27 +0200
Subject: [PATCH nft 0.9.8 6/7] src: expand table command before evaluation

upstream 3975430b12d97c92cdf03753342f2269153d5624 commit.

The nested syntax notation results in one single table command which
includes all other objects. This differs from the flat notation where
there is usually one command per object.

This patch adds a previous step to the evaluation phase to expand the
objects that are contained in the table into independent commands, so
both notations have similar representations.

Remove the code to evaluate the nested representation in the evaluation
phase since commands are independently evaluated after the expansion.

The commands are expanded after the set element collapse step, in case
that there is a long list of singleton element commands to be added to
the set, to shorten the command list iteration.

This approach also avoids interference with the object cache that is
populated in the evaluation, which might refer to objects coming in the
existing command list that is being processed.

There is still a post_expand phase to detach the elements from the set
which could be consolidated by updating the evaluation step to handle
the CMD_OBJ_SETELEMS command type.

This patch fixes 27c753e4a8d4 ("rule: expand standalone chain that
contains rules") which broke rule addition/insertion by index because
the expansion code after the evaluation messes up the cache.

Fixes: 27c753e4a8d4 ("rule: expand standalone chain that contains rules")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/rule.h    |  1 +
 src/evaluate.c    | 39 ---------------------------------------
 src/libnftables.c |  9 ++++++++-
 src/rule.c        | 21 +++++++++++++++++++--
 4 files changed, 28 insertions(+), 42 deletions(-)

diff --git a/include/rule.h b/include/rule.h
index dde32367f48f..f880cfd32bd8 100644
--- a/include/rule.h
+++ b/include/rule.h
@@ -717,6 +717,7 @@ extern struct cmd *cmd_alloc(enum cmd_ops op, enum cmd_obj obj,
 			     const struct handle *h, const struct location *loc,
 			     void *data);
 extern void nft_cmd_expand(struct cmd *cmd);
+extern void nft_cmd_post_expand(struct cmd *cmd);
 extern struct cmd *cmd_alloc_obj_ct(enum cmd_ops op, int type,
 				    const struct handle *h,
 				    const struct location *loc, struct obj *obj);
diff --git a/src/evaluate.c b/src/evaluate.c
index f546667131e1..232ae39020cc 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -4068,7 +4068,6 @@ static uint32_t str2hooknum(uint32_t family, const char *hook)
 static int chain_evaluate(struct eval_ctx *ctx, struct chain *chain)
 {
 	struct table *table;
-	struct rule *rule;
 
 	table = table_lookup_global(ctx);
 	if (table == NULL)
@@ -4122,11 +4121,6 @@ static int chain_evaluate(struct eval_ctx *ctx, struct chain *chain)
 		}
 	}
 
-	list_for_each_entry(rule, &chain->rules, list) {
-		handle_merge(&rule->handle, &chain->handle);
-		if (rule_evaluate(ctx, rule, CMD_INVALID) < 0)
-			return -1;
-	}
 	return 0;
 }
 
@@ -4183,11 +4177,6 @@ static int obj_evaluate(struct eval_ctx *ctx, struct obj *obj)
 
 static int table_evaluate(struct eval_ctx *ctx, struct table *table)
 {
-	struct flowtable *ft;
-	struct chain *chain;
-	struct set *set;
-	struct obj *obj;
-
 	if (table_lookup(&ctx->cmd->handle, &ctx->nft->cache) == NULL) {
 		if (table == NULL) {
 			table = table_alloc();
@@ -4198,34 +4187,6 @@ static int table_evaluate(struct eval_ctx *ctx, struct table *table)
 		}
 	}
 
-	if (ctx->cmd->table == NULL)
-		return 0;
-
-	ctx->table = table;
-	list_for_each_entry(set, &table->sets, list) {
-		expr_set_context(&ctx->ectx, NULL, 0);
-		handle_merge(&set->handle, &table->handle);
-		if (set_evaluate(ctx, set) < 0)
-			return -1;
-	}
-	list_for_each_entry(chain, &table->chains, list) {
-		handle_merge(&chain->handle, &table->handle);
-		ctx->cmd->handle.chain.location = chain->location;
-		if (chain_evaluate(ctx, chain) < 0)
-			return -1;
-	}
-	list_for_each_entry(ft, &table->flowtables, list) {
-		handle_merge(&ft->handle, &table->handle);
-		if (flowtable_evaluate(ctx, ft) < 0)
-			return -1;
-	}
-	list_for_each_entry(obj, &table->objs, list) {
-		handle_merge(&obj->handle, &table->handle);
-		if (obj_evaluate(ctx, obj) < 0)
-			return -1;
-	}
-
-	ctx->table = NULL;
 	return 0;
 }
 
diff --git a/src/libnftables.c b/src/libnftables.c
index 044365914747..b1f57802b90e 100644
--- a/src/libnftables.c
+++ b/src/libnftables.c
@@ -420,6 +420,13 @@ static int nft_evaluate(struct nft_ctx *nft, struct list_head *msgs,
 	if (cache_update(nft, flags, msgs) < 0)
 		return -1;
 
+	list_for_each_entry(cmd, cmds, list) {
+		if (cmd->op != CMD_ADD)
+			continue;
+
+		nft_cmd_expand(cmd);
+	}
+
 	list_for_each_entry(cmd, cmds, list) {
 		struct eval_ctx ectx = {
 			.nft	= nft,
@@ -437,7 +444,7 @@ static int nft_evaluate(struct nft_ctx *nft, struct list_head *msgs,
 		if (cmd->op != CMD_ADD)
 			continue;
 
-		nft_cmd_expand(cmd);
+		nft_cmd_post_expand(cmd);
 	}
 
 	return 0;
diff --git a/src/rule.c b/src/rule.c
index 9139418e1bf8..9c74b89c1fb1 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -1511,8 +1511,9 @@ static void nft_cmd_expand_chain(struct chain *chain, struct list_head *new_cmds
 
 	list_for_each_entry_safe(rule, next, &chain->rules, list) {
 		list_del(&rule->list);
+		handle_merge(&rule->handle, &chain->handle);
 		memset(&h, 0, sizeof(h));
-		handle_merge(&h, &rule->handle);
+		handle_merge(&h, &chain->handle);
 		if (chain->flags & CHAIN_F_BINDING) {
 			rule->handle.chain_id = chain->handle.chain_id;
 			rule->handle.chain.location = chain->location;
@@ -1526,10 +1527,10 @@ static void nft_cmd_expand_chain(struct chain *chain, struct list_head *new_cmds
 void nft_cmd_expand(struct cmd *cmd)
 {
 	struct list_head new_cmds;
-	struct set *set, *newset;
 	struct flowtable *ft;
 	struct table *table;
 	struct chain *chain;
+	struct set *set;
 	struct obj *obj;
 	struct cmd *new;
 	struct handle h;
@@ -1543,6 +1544,7 @@ void nft_cmd_expand(struct cmd *cmd)
 			return;
 
 		list_for_each_entry(chain, &table->chains, list) {
+			handle_merge(&chain->handle, &table->handle);
 			memset(&h, 0, sizeof(h));
 			handle_merge(&h, &chain->handle);
 			h.chain_id = chain->handle.chain_id;
@@ -1587,6 +1589,21 @@ void nft_cmd_expand(struct cmd *cmd)
 		nft_cmd_expand_chain(chain, &new_cmds);
 		list_splice(&new_cmds, &cmd->list);
 		break;
+	default:
+		break;
+	}
+}
+
+void nft_cmd_post_expand(struct cmd *cmd)
+{
+	struct list_head new_cmds;
+	struct set *set, *newset;
+	struct cmd *new;
+	struct handle h;
+
+	init_list_head(&new_cmds);
+
+	switch (cmd->obj) {
 	case CMD_OBJ_SET:
 	case CMD_OBJ_MAP:
 		set = cmd->set;
-- 
2.30.2


[-- Attachment #8: 0007-tests-shell-Stabilize-sets-0043concatenated_ranges_0.patch --]
[-- Type: text/x-diff, Size: 1899 bytes --]

From 69d1ab7c50a6a1dd369b50a5edad769b98779e12 Mon Sep 17 00:00:00 2001
From: Phil Sutter <phil@nwl.cc>
Date: Wed, 23 Aug 2023 18:18:49 +0200
Subject: [PATCH nft 0.9.8 7/7] tests: shell: Stabilize
 sets/0043concatenated_ranges_0 test

upstream c791765cb0d62ba261f0b495e07315437b3ae914 commit.

On a slow system, one of the 'delete element' commands would
occasionally fail. Assuming it can only happen if the 2s timeout passes
"too quickly", work around it by adding elements with a 2m timeout
instead and when wanting to test the element expiry just drop and add
the element again with a short timeout.

Fixes: 6231d3fa4af1e ("tests: shell: Fix for unstable sets/0043concatenated_ranges_0")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 tests/shell/testcases/sets/0043concatenated_ranges_0 | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tests/shell/testcases/sets/0043concatenated_ranges_0 b/tests/shell/testcases/sets/0043concatenated_ranges_0
index 11767373bcd2..8d3dacf6e38a 100755
--- a/tests/shell/testcases/sets/0043concatenated_ranges_0
+++ b/tests/shell/testcases/sets/0043concatenated_ranges_0
@@ -147,7 +147,7 @@ for ta in ${TYPES}; do
 			eval add_b=\$ADD_${tb}
 			eval add_c=\$ADD_${tc}
 			${NFT} add element inet filter test \
-				"{ ${add_a} . ${add_b} . ${add_c} timeout 1s${mapv}}"
+				"{ ${add_a} . ${add_b} . ${add_c} timeout 2m${mapv}}"
 			[ $(${NFT} list ${setmap} inet filter test |	\
 			   grep -c "${add_a} . ${add_b} . ${add_c}") -eq 1 ]
 
@@ -180,6 +180,10 @@ for ta in ${TYPES}; do
 				continue
 			fi
 
+			${NFT} delete element inet filter test \
+				"{ ${add_a} . ${add_b} . ${add_c} ${mapv}}"
+			${NFT} add element inet filter test \
+				"{ ${add_a} . ${add_b} . ${add_c} timeout 1s${mapv}}"
 			sleep 1
 			[ $(${NFT} list ${setmap} inet filter test |		\
 			   grep -c "${add_a} . ${add_b} . ${add_c} ${mapv}") -eq 0 ]
-- 
2.30.2

Re: [RFC] nftables 0.9.8 -stable backports

[Jeremy Sowden]

[-- Attachment #1: Type: text/plain, Size: 30744 bytes --]

On 2023-10-09, at 13:36:23 +0200, Pablo Neira Ayuso wrote:
> Hi Arturo, Jeremy,
> 
> This is a small batch offering fixes for nftables 0.9.8. It only
> includes the fixes for the implicit chain regression in recent
> kernels.
> 
> This is a few dependency patches that are missing in 0.9.8 are
> required:
> 
>         3542e49cf539 ("evaluate: init cmd pointer for new on-stack context")
>         a3ac2527724d ("src: split chain list in table")
>         4e718641397c ("cache: rename chain_htable to cache_chain_ht")
> 
> a3ac2527724d is fixing an issue with the cache that is required by the
> fixes. Then, the backport fixes for the implicit chain regression with
> Linux -stable:
> 
>         3975430b12d9 ("src: expand table command before evaluation")
>         27c753e4a8d4 ("rule: expand standalone chain that contains rules")
>         784597a4ed63 ("rule: add helper function to expand chain rules into commands")
> 
> I tested with tests/shell at the time of the nftables 0.9.8 release
> (*I did not use git HEAD tests/shell as I did for 1.0.6*).
> 
> I have kept back the backport of this patch intentionally:
> 
>         56c90a2dd2eb ("evaluate: expand sets and maps before evaluation")
> 
> this depends on the new src/interval.c code, in 0.9.8 overlap and
> automerge come a later stage and cache is not updated incrementally,
> I tried the tests coming in this patch and it works fine.
> 
> I did run a few more tests with rulesets that I have been collecting
> from people that occasionally send them to me for my personal ruleset
> repo.
> 
> I: results: [OK] 266 [FAILED] 0 [TOTAL] 266
> 
> This has been tested with latest Linux kernel 5.10 -stable.
> 
> I can still run a few more tests, I will get back to you if I find any
> issue.
> 
> Let me know, thanks.

Thanks for this.  I started looking into it, but had not yet identified
the preliminaries needed to get the regression fixes working with 0.9.8,
and I was going to ask you for help.  I'll take a proper look this
evening.

J.

> From 0a39091a75b6255422832126df4cbf73c86845cd Mon Sep 17 00:00:00 2001
> From: Pablo Neira Ayuso <pablo@netfilter.org>
> Date: Thu, 1 Apr 2021 22:18:29 +0200
> Subject: [PATCH nft 0.9.8 1/7] cache: rename chain_htable to cache_chain_ht
> 
> upstream 3542e49cf539ecfcef6ef7c2d4befb7896ade2cd commit.
> 
> Rename the hashtable chain that is used for fast cache lookups.
> 
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
>  include/rule.h | 4 ++--
>  src/cache.c    | 6 +++---
>  src/rule.c     | 6 +++---
>  3 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/include/rule.h b/include/rule.h
> index 330a09aa77fa..43872db8947a 100644
> --- a/include/rule.h
> +++ b/include/rule.h
> @@ -154,7 +154,7 @@ struct table {
>  	struct handle		handle;
>  	struct location		location;
>  	struct scope		scope;
> -	struct list_head	*chain_htable;
> +	struct list_head	*cache_chain_ht;
>  	struct list_head	chains;
>  	struct list_head	sets;
>  	struct list_head	objs;
> @@ -220,7 +220,7 @@ struct hook_spec {
>   */
>  struct chain {
>  	struct list_head	list;
> -	struct list_head	hlist;
> +	struct list_head	cache_hlist;
>  	struct handle		handle;
>  	struct location		location;
>  	unsigned int		refcnt;
> diff --git a/src/cache.c b/src/cache.c
> index ed2609008e22..7101b74160be 100644
> --- a/src/cache.c
> +++ b/src/cache.c
> @@ -194,7 +194,7 @@ static int chain_cache_cb(struct nftnl_chain *nlc, void *arg)
>  	if (chain->flags & CHAIN_F_BINDING) {
>  		list_add_tail(&chain->list, &ctx->table->chain_bindings);
>  	} else {
> -		list_add_tail(&chain->hlist, &ctx->table->chain_htable[hash]);
> +		list_add_tail(&chain->cache_hlist, &ctx->table->cache_chain_ht[hash]);
>  		list_add_tail(&chain->list, &ctx->table->chains);
>  	}
>  
> @@ -238,7 +238,7 @@ void chain_cache_add(struct chain *chain, struct table *table)
>  	uint32_t hash;
>  
>  	hash = djb_hash(chain->handle.chain.name) % NFT_CACHE_HSIZE;
> -	list_add_tail(&chain->hlist, &table->chain_htable[hash]);
> +	list_add_tail(&chain->cache_hlist, &table->cache_chain_ht[hash]);
>  	list_add_tail(&chain->list, &table->chains);
>  }
>  
> @@ -249,7 +249,7 @@ struct chain *chain_cache_find(const struct table *table,
>  	uint32_t hash;
>  
>  	hash = djb_hash(handle->chain.name) % NFT_CACHE_HSIZE;
> -	list_for_each_entry(chain, &table->chain_htable[hash], hlist) {
> +	list_for_each_entry(chain, &table->cache_chain_ht[hash], cache_hlist) {
>  		if (!strcmp(chain->handle.chain.name, handle->chain.name))
>  			return chain;
>  	}
> diff --git a/src/rule.c b/src/rule.c
> index e4bb6bae276a..3b445851f657 100644
> --- a/src/rule.c
> +++ b/src/rule.c
> @@ -1328,10 +1328,10 @@ struct table *table_alloc(void)
>  	init_list_head(&table->scope.symbols);
>  	table->refcnt = 1;
>  
> -	table->chain_htable =
> +	table->cache_chain_ht =
>  		xmalloc(sizeof(struct list_head) * NFT_CACHE_HSIZE);
>  	for (i = 0; i < NFT_CACHE_HSIZE; i++)
> -		init_list_head(&table->chain_htable[i]);
> +		init_list_head(&table->cache_chain_ht[i]);
>  
>  	return table;
>  }
> @@ -1359,7 +1359,7 @@ void table_free(struct table *table)
>  		obj_free(obj);
>  	handle_free(&table->handle);
>  	scope_release(&table->scope);
> -	xfree(table->chain_htable);
> +	xfree(table->cache_chain_ht);
>  	xfree(table);
>  }
>  
> -- 
> 2.30.2
> 

> From f37e4261130b021edf068e4d5c6ca062ce4e2ac1 Mon Sep 17 00:00:00 2001
> From: Pablo Neira Ayuso <pablo@netfilter.org>
> Date: Thu, 1 Apr 2021 22:19:30 +0200
> Subject: [PATCH nft 0.9.8 2/7] src: split chain list in table
> 
> upstream a3ac2527724dd27628e12caaa55f731b109e4586 commit.
> 
> This patch splits table->lists in two:
> 
> - Chains that reside in the cache are stored in the new
>   tables->cache_chain and tables->cache_chain_ht. The hashtable chain
>   cache allows for fast chain lookups.
> 
> - Chains that defined via command line / ruleset file reside in
>   tables->chains.
> 
> Note that chains in the cache (already in the kernel) are not placed in
> the table->chains.
> 
> By keeping separated lists, chains defined via command line / ruleset
> file can be added to cache.
> 
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
>  include/rule.h |  2 ++
>  src/cache.c    |  6 +++---
>  src/json.c     |  6 +++---
>  src/rule.c     | 18 +++++++++++-------
>  4 files changed, 19 insertions(+), 13 deletions(-)
> 
> diff --git a/include/rule.h b/include/rule.h
> index 43872db8947a..dde32367f48f 100644
> --- a/include/rule.h
> +++ b/include/rule.h
> @@ -155,6 +155,7 @@ struct table {
>  	struct location		location;
>  	struct scope		scope;
>  	struct list_head	*cache_chain_ht;
> +	struct list_head	cache_chain;
>  	struct list_head	chains;
>  	struct list_head	sets;
>  	struct list_head	objs;
> @@ -221,6 +222,7 @@ struct hook_spec {
>  struct chain {
>  	struct list_head	list;
>  	struct list_head	cache_hlist;
> +	struct list_head	cache_list;
>  	struct handle		handle;
>  	struct location		location;
>  	unsigned int		refcnt;
> diff --git a/src/cache.c b/src/cache.c
> index 7101b74160be..32e6eea4ac5c 100644
> --- a/src/cache.c
> +++ b/src/cache.c
> @@ -192,10 +192,10 @@ static int chain_cache_cb(struct nftnl_chain *nlc, void *arg)
>  	chain = netlink_delinearize_chain(ctx->nlctx, nlc);
>  
>  	if (chain->flags & CHAIN_F_BINDING) {
> -		list_add_tail(&chain->list, &ctx->table->chain_bindings);
> +		list_add_tail(&chain->cache_list, &ctx->table->chain_bindings);
>  	} else {
>  		list_add_tail(&chain->cache_hlist, &ctx->table->cache_chain_ht[hash]);
> -		list_add_tail(&chain->list, &ctx->table->chains);
> +		list_add_tail(&chain->cache_list, &ctx->table->cache_chain);
>  	}
>  
>  	nftnl_chain_list_del(nlc);
> @@ -239,7 +239,7 @@ void chain_cache_add(struct chain *chain, struct table *table)
>  
>  	hash = djb_hash(chain->handle.chain.name) % NFT_CACHE_HSIZE;
>  	list_add_tail(&chain->cache_hlist, &table->cache_chain_ht[hash]);
> -	list_add_tail(&chain->list, &table->chains);
> +	list_add_tail(&chain->cache_list, &table->cache_chain);
>  }
>  
>  struct chain *chain_cache_find(const struct table *table,
> diff --git a/src/json.c b/src/json.c
> index 585d35326ac0..737e73b08b5a 100644
> --- a/src/json.c
> +++ b/src/json.c
> @@ -1594,7 +1594,7 @@ static json_t *table_print_json_full(struct netlink_ctx *ctx,
>  		tmp = flowtable_print_json(flowtable);
>  		json_array_append_new(root, tmp);
>  	}
> -	list_for_each_entry(chain, &table->chains, list) {
> +	list_for_each_entry(chain, &table->cache_chain, cache_list) {
>  		tmp = chain_print_json(chain);
>  		json_array_append_new(root, tmp);
>  
> @@ -1656,7 +1656,7 @@ static json_t *do_list_chain_json(struct netlink_ctx *ctx,
>  	struct chain *chain;
>  	struct rule *rule;
>  
> -	list_for_each_entry(chain, &table->chains, list) {
> +	list_for_each_entry(chain, &table->cache_chain, cache_list) {
>  		if (chain->handle.family != cmd->handle.family ||
>  		    strcmp(cmd->handle.chain.name, chain->handle.chain.name))
>  			continue;
> @@ -1684,7 +1684,7 @@ static json_t *do_list_chains_json(struct netlink_ctx *ctx, struct cmd *cmd)
>  		    cmd->handle.family != table->handle.family)
>  			continue;
>  
> -		list_for_each_entry(chain, &table->chains, list) {
> +		list_for_each_entry(chain, &table->cache_chain, cache_list) {
>  			json_t *tmp = chain_print_json(chain);
>  
>  			json_array_append_new(root, tmp);
> diff --git a/src/rule.c b/src/rule.c
> index 3b445851f657..f76c27c9d091 100644
> --- a/src/rule.c
> +++ b/src/rule.c
> @@ -953,7 +953,7 @@ struct chain *chain_lookup(const struct table *table, const struct handle *h)
>  {
>  	struct chain *chain;
>  
> -	list_for_each_entry(chain, &table->chains, list) {
> +	list_for_each_entry(chain, &table->cache_chain, cache_list) {
>  		if (!strcmp(chain->handle.chain.name, h->chain.name))
>  			return chain;
>  	}
> @@ -965,7 +965,7 @@ struct chain *chain_binding_lookup(const struct table *table,
>  {
>  	struct chain *chain;
>  
> -	list_for_each_entry(chain, &table->chain_bindings, list) {
> +	list_for_each_entry(chain, &table->chain_bindings, cache_list) {
>  		if (!strcmp(chain->handle.chain.name, chain_name))
>  			return chain;
>  	}
> @@ -983,7 +983,7 @@ struct chain *chain_lookup_fuzzy(const struct handle *h,
>  	string_misspell_init(&st);
>  
>  	list_for_each_entry(table, &cache->list, list) {
> -		list_for_each_entry(chain, &table->chains, list) {
> +		list_for_each_entry(chain, &table->cache_chain, cache_list) {
>  			if (!strcmp(chain->handle.chain.name, h->chain.name)) {
>  				*t = table;
>  				return chain;
> @@ -1321,6 +1321,7 @@ struct table *table_alloc(void)
>  
>  	table = xzalloc(sizeof(*table));
>  	init_list_head(&table->chains);
> +	init_list_head(&table->cache_chain);
>  	init_list_head(&table->sets);
>  	init_list_head(&table->objs);
>  	init_list_head(&table->flowtables);
> @@ -1349,7 +1350,10 @@ void table_free(struct table *table)
>  		xfree(table->comment);
>  	list_for_each_entry_safe(chain, next, &table->chains, list)
>  		chain_free(chain);
> -	list_for_each_entry_safe(chain, next, &table->chain_bindings, list)
> +	list_for_each_entry_safe(chain, next, &table->chain_bindings, cache_list)
> +		chain_free(chain);
> +	/* this is implicitly releasing chains in the hashtable cache */
> +	list_for_each_entry_safe(chain, next, &table->cache_chain, cache_list)
>  		chain_free(chain);
>  	list_for_each_entry_safe(set, nset, &table->sets, list)
>  		set_free(set);
> @@ -1465,7 +1469,7 @@ static void table_print(const struct table *table, struct output_ctx *octx)
>  		flowtable_print(flowtable, octx);
>  		delim = "\n";
>  	}
> -	list_for_each_entry(chain, &table->chains, list) {
> +	list_for_each_entry(chain, &table->cache_chain, cache_list) {
>  		nft_print(octx, "%s", delim);
>  		chain_print(chain, octx);
>  		delim = "\n";
> @@ -2555,7 +2559,7 @@ static int do_list_chain(struct netlink_ctx *ctx, struct cmd *cmd,
>  
>  	table_print_declaration(table, &ctx->nft->output);
>  
> -	list_for_each_entry(chain, &table->chains, list) {
> +	list_for_each_entry(chain, &table->cache_chain, cache_list) {
>  		if (chain->handle.family != cmd->handle.family ||
>  		    strcmp(cmd->handle.chain.name, chain->handle.chain.name) != 0)
>  			continue;
> @@ -2580,7 +2584,7 @@ static int do_list_chains(struct netlink_ctx *ctx, struct cmd *cmd)
>  
>  		table_print_declaration(table, &ctx->nft->output);
>  
> -		list_for_each_entry(chain, &table->chains, list) {
> +		list_for_each_entry(chain, &table->cache_chain, cache_list) {
>  			chain_print_declaration(chain, &ctx->nft->output);
>  			nft_print(&ctx->nft->output, "\t}\n");
>  		}
> -- 
> 2.30.2
> 

> From 5af65a30a12396281c751e635509ab1d9363f4bc Mon Sep 17 00:00:00 2001
> From: Florian Westphal <fw@strlen.de>
> Date: Fri, 4 Mar 2022 11:30:55 +0100
> Subject: [PATCH nft 0.9.8 3/7] evaluate: init cmd pointer for new on-stack
>  context
> 
> upstream 4e718641397c876315a87db441afc53139863122 commit
> 
> else, this will segfault when trying to print the
> "table 'x' doesn't exist" error message.
> 
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>  src/evaluate.c                                   | 1 +
>  tests/shell/testcases/chains/0041chain_binding_0 | 6 ++++++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/src/evaluate.c b/src/evaluate.c
> index c830dcdbd965..f546667131e1 100644
> --- a/src/evaluate.c
> +++ b/src/evaluate.c
> @@ -3211,6 +3211,7 @@ static int stmt_evaluate_chain(struct eval_ctx *ctx, struct stmt *stmt)
>  			struct eval_ctx rule_ctx = {
>  				.nft	= ctx->nft,
>  				.msgs	= ctx->msgs,
> +				.cmd	= ctx->cmd,
>  			};
>  			struct handle h2 = {};
>  
> diff --git a/tests/shell/testcases/chains/0041chain_binding_0 b/tests/shell/testcases/chains/0041chain_binding_0
> index 59bdbe9f0ba9..4b541bb55c30 100755
> --- a/tests/shell/testcases/chains/0041chain_binding_0
> +++ b/tests/shell/testcases/chains/0041chain_binding_0
> @@ -1,5 +1,11 @@
>  #!/bin/bash
>  
> +# no table x, caused segfault in earlier nft releases
> +$NFT insert rule inet x y handle 107 'goto { log prefix "MOO! "; }'
> +if [ $? -ne 1 ]; then
> +	exit 1
> +fi
> +
>  set -e
>  
>  EXPECTED="table inet x {
> -- 
> 2.30.2
> 

> From 0f559011ee7e805df883be635b88396639fbb87e Mon Sep 17 00:00:00 2001
> From: Pablo Neira Ayuso <pablo@netfilter.org>
> Date: Tue, 12 Sep 2023 23:22:46 +0200
> Subject: [PATCH nft 0.9.8 4/7] rule: add helper function to expand chain rules
>  into commands
> 
> upstream 784597a4ed63b9decb10d74fdb49a1b021e22728 commit.
> 
> This patch adds a helper function to expand chain rules into commands.
> This comes in preparation for the follow up patch.
> 
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
>  src/rule.c | 39 ++++++++++++++++++++++-----------------
>  1 file changed, 22 insertions(+), 17 deletions(-)
> 
> diff --git a/src/rule.c b/src/rule.c
> index f76c27c9d091..3fbf4271accd 100644
> --- a/src/rule.c
> +++ b/src/rule.c
> @@ -1503,6 +1503,25 @@ void cmd_add_loc(struct cmd *cmd, uint16_t offset, const struct location *loc)
>  	cmd->num_attrs++;
>  }
>  
> +static void nft_cmd_expand_chain(struct chain *chain, struct list_head *new_cmds)
> +{
> +	struct rule *rule;
> +	struct handle h;
> +	struct cmd *new;
> +
> +	list_for_each_entry(rule, &chain->rules, list) {
> +		memset(&h, 0, sizeof(h));
> +		handle_merge(&h, &rule->handle);
> +		if (chain->flags & CHAIN_F_BINDING) {
> +			rule->handle.chain_id = chain->handle.chain_id;
> +			rule->handle.chain.location = chain->location;
> +		}
> +		new = cmd_alloc(CMD_ADD, CMD_OBJ_RULE, &h,
> +				&rule->location, rule_get(rule));
> +		list_add_tail(&new->list, new_cmds);
> +	}
> +}
> +
>  void nft_cmd_expand(struct cmd *cmd)
>  {
>  	struct list_head new_cmds;
> @@ -1510,7 +1529,6 @@ void nft_cmd_expand(struct cmd *cmd)
>  	struct flowtable *ft;
>  	struct table *table;
>  	struct chain *chain;
> -	struct rule *rule;
>  	struct obj *obj;
>  	struct cmd *new;
>  	struct handle h;
> @@ -1555,22 +1573,9 @@ void nft_cmd_expand(struct cmd *cmd)
>  					&ft->location, flowtable_get(ft));
>  			list_add_tail(&new->list, &new_cmds);
>  		}
> -		list_for_each_entry(chain, &table->chains, list) {
> -			list_for_each_entry(rule, &chain->rules, list) {
> -				memset(&h, 0, sizeof(h));
> -				handle_merge(&h, &rule->handle);
> -				if (chain->flags & CHAIN_F_BINDING) {
> -					rule->handle.chain_id =
> -						chain->handle.chain_id;
> -					rule->handle.chain.location =
> -						chain->location;
> -				}
> -				new = cmd_alloc(CMD_ADD, CMD_OBJ_RULE, &h,
> -						&rule->location,
> -						rule_get(rule));
> -				list_add_tail(&new->list, &new_cmds);
> -			}
> -		}
> +		list_for_each_entry(chain, &table->chains, list)
> +			nft_cmd_expand_chain(chain, &new_cmds);
> +
>  		list_splice(&new_cmds, &cmd->list);
>  		break;
>  	case CMD_OBJ_SET:
> -- 
> 2.30.2
> 

> From f03bc399d75ef724fcbed184f74fc306ca8ef324 Mon Sep 17 00:00:00 2001
> From: Pablo Neira Ayuso <pablo@netfilter.org>
> Date: Mon, 6 Feb 2023 15:28:41 +0100
> Subject: [PATCH nft 0.9.8 5/7] rule: expand standalone chain that contains
>  rules
> 
> upstream 27c753e4a8d4744f479345e3f5e34cafef751602 commit.
> 
> Otherwise rules that this chain contains are ignored when expressed
> using the following syntax:
> 
> chain inet filter input2 {
>        type filter hook input priority filter; policy accept;
>        ip saddr 1.2.3.4 tcp dport { 22, 443, 123 } drop
> }
> 
> When expanding the chain, remove the rule so the new CMD_OBJ_CHAIN
> case does not expand it again.
> 
> Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1655
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
>  src/rule.c                                    | 15 +++++++++---
>  .../testcases/include/0020include_chain_0     | 23 +++++++++++++++++++
>  .../include/dumps/0020include_chain_0.nft     |  6 +++++
>  3 files changed, 41 insertions(+), 3 deletions(-)
>  create mode 100755 tests/shell/testcases/include/0020include_chain_0
>  create mode 100644 tests/shell/testcases/include/dumps/0020include_chain_0.nft
> 
> diff --git a/src/rule.c b/src/rule.c
> index 3fbf4271accd..9139418e1bf8 100644
> --- a/src/rule.c
> +++ b/src/rule.c
> @@ -1505,11 +1505,12 @@ void cmd_add_loc(struct cmd *cmd, uint16_t offset, const struct location *loc)
>  
>  static void nft_cmd_expand_chain(struct chain *chain, struct list_head *new_cmds)
>  {
> -	struct rule *rule;
> +	struct rule *rule, *next;
>  	struct handle h;
>  	struct cmd *new;
>  
> -	list_for_each_entry(rule, &chain->rules, list) {
> +	list_for_each_entry_safe(rule, next, &chain->rules, list) {
> +		list_del(&rule->list);
>  		memset(&h, 0, sizeof(h));
>  		handle_merge(&h, &rule->handle);
>  		if (chain->flags & CHAIN_F_BINDING) {
> @@ -1517,7 +1518,7 @@ static void nft_cmd_expand_chain(struct chain *chain, struct list_head *new_cmds
>  			rule->handle.chain.location = chain->location;
>  		}
>  		new = cmd_alloc(CMD_ADD, CMD_OBJ_RULE, &h,
> -				&rule->location, rule_get(rule));
> +				&rule->location, rule);
>  		list_add_tail(&new->list, new_cmds);
>  	}
>  }
> @@ -1578,6 +1579,14 @@ void nft_cmd_expand(struct cmd *cmd)
>  
>  		list_splice(&new_cmds, &cmd->list);
>  		break;
> +	case CMD_OBJ_CHAIN:
> +		chain = cmd->chain;
> +		if (!chain || list_empty(&chain->rules))
> +			break;
> +
> +		nft_cmd_expand_chain(chain, &new_cmds);
> +		list_splice(&new_cmds, &cmd->list);
> +		break;
>  	case CMD_OBJ_SET:
>  	case CMD_OBJ_MAP:
>  		set = cmd->set;
> diff --git a/tests/shell/testcases/include/0020include_chain_0 b/tests/shell/testcases/include/0020include_chain_0
> new file mode 100755
> index 000000000000..2ff83c92fda8
> --- /dev/null
> +++ b/tests/shell/testcases/include/0020include_chain_0
> @@ -0,0 +1,23 @@
> +#!/bin/bash
> +
> +set -e
> +
> +tmpfile1=$(mktemp -p .)
> +if [ ! -w $tmpfile1 ] ; then
> +	echo "Failed to create tmp file" >&2
> +	exit 0
> +fi
> +
> +trap "rm -rf $tmpfile1" EXIT # cleanup if aborted
> +
> +RULESET="table inet filter { }
> +include \"$tmpfile1\""
> +
> +RULESET2="chain inet filter input2 {
> +	type filter hook input priority filter; policy accept;
> +	ip saddr 1.2.3.4 tcp dport { 22, 443, 123 } drop
> +}"
> +
> +echo "$RULESET2" > $tmpfile1
> +
> +$NFT -f - <<< $RULESET
> diff --git a/tests/shell/testcases/include/dumps/0020include_chain_0.nft b/tests/shell/testcases/include/dumps/0020include_chain_0.nft
> new file mode 100644
> index 000000000000..3ad6db14d2f5
> --- /dev/null
> +++ b/tests/shell/testcases/include/dumps/0020include_chain_0.nft
> @@ -0,0 +1,6 @@
> +table inet filter {
> +	chain input2 {
> +		type filter hook input priority filter; policy accept;
> +		ip saddr 1.2.3.4 tcp dport { 22, 123, 443 } drop
> +	}
> +}
> -- 
> 2.30.2
> 

> From 050e0b7a85016b733e1a59285df501d1c05eec0b Mon Sep 17 00:00:00 2001
> From: Pablo Neira Ayuso <pablo@netfilter.org>
> Date: Tue, 12 Sep 2023 23:25:27 +0200
> Subject: [PATCH nft 0.9.8 6/7] src: expand table command before evaluation
> 
> upstream 3975430b12d97c92cdf03753342f2269153d5624 commit.
> 
> The nested syntax notation results in one single table command which
> includes all other objects. This differs from the flat notation where
> there is usually one command per object.
> 
> This patch adds a previous step to the evaluation phase to expand the
> objects that are contained in the table into independent commands, so
> both notations have similar representations.
> 
> Remove the code to evaluate the nested representation in the evaluation
> phase since commands are independently evaluated after the expansion.
> 
> The commands are expanded after the set element collapse step, in case
> that there is a long list of singleton element commands to be added to
> the set, to shorten the command list iteration.
> 
> This approach also avoids interference with the object cache that is
> populated in the evaluation, which might refer to objects coming in the
> existing command list that is being processed.
> 
> There is still a post_expand phase to detach the elements from the set
> which could be consolidated by updating the evaluation step to handle
> the CMD_OBJ_SETELEMS command type.
> 
> This patch fixes 27c753e4a8d4 ("rule: expand standalone chain that
> contains rules") which broke rule addition/insertion by index because
> the expansion code after the evaluation messes up the cache.
> 
> Fixes: 27c753e4a8d4 ("rule: expand standalone chain that contains rules")
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
>  include/rule.h    |  1 +
>  src/evaluate.c    | 39 ---------------------------------------
>  src/libnftables.c |  9 ++++++++-
>  src/rule.c        | 21 +++++++++++++++++++--
>  4 files changed, 28 insertions(+), 42 deletions(-)
> 
> diff --git a/include/rule.h b/include/rule.h
> index dde32367f48f..f880cfd32bd8 100644
> --- a/include/rule.h
> +++ b/include/rule.h
> @@ -717,6 +717,7 @@ extern struct cmd *cmd_alloc(enum cmd_ops op, enum cmd_obj obj,
>  			     const struct handle *h, const struct location *loc,
>  			     void *data);
>  extern void nft_cmd_expand(struct cmd *cmd);
> +extern void nft_cmd_post_expand(struct cmd *cmd);
>  extern struct cmd *cmd_alloc_obj_ct(enum cmd_ops op, int type,
>  				    const struct handle *h,
>  				    const struct location *loc, struct obj *obj);
> diff --git a/src/evaluate.c b/src/evaluate.c
> index f546667131e1..232ae39020cc 100644
> --- a/src/evaluate.c
> +++ b/src/evaluate.c
> @@ -4068,7 +4068,6 @@ static uint32_t str2hooknum(uint32_t family, const char *hook)
>  static int chain_evaluate(struct eval_ctx *ctx, struct chain *chain)
>  {
>  	struct table *table;
> -	struct rule *rule;
>  
>  	table = table_lookup_global(ctx);
>  	if (table == NULL)
> @@ -4122,11 +4121,6 @@ static int chain_evaluate(struct eval_ctx *ctx, struct chain *chain)
>  		}
>  	}
>  
> -	list_for_each_entry(rule, &chain->rules, list) {
> -		handle_merge(&rule->handle, &chain->handle);
> -		if (rule_evaluate(ctx, rule, CMD_INVALID) < 0)
> -			return -1;
> -	}
>  	return 0;
>  }
>  
> @@ -4183,11 +4177,6 @@ static int obj_evaluate(struct eval_ctx *ctx, struct obj *obj)
>  
>  static int table_evaluate(struct eval_ctx *ctx, struct table *table)
>  {
> -	struct flowtable *ft;
> -	struct chain *chain;
> -	struct set *set;
> -	struct obj *obj;
> -
>  	if (table_lookup(&ctx->cmd->handle, &ctx->nft->cache) == NULL) {
>  		if (table == NULL) {
>  			table = table_alloc();
> @@ -4198,34 +4187,6 @@ static int table_evaluate(struct eval_ctx *ctx, struct table *table)
>  		}
>  	}
>  
> -	if (ctx->cmd->table == NULL)
> -		return 0;
> -
> -	ctx->table = table;
> -	list_for_each_entry(set, &table->sets, list) {
> -		expr_set_context(&ctx->ectx, NULL, 0);
> -		handle_merge(&set->handle, &table->handle);
> -		if (set_evaluate(ctx, set) < 0)
> -			return -1;
> -	}
> -	list_for_each_entry(chain, &table->chains, list) {
> -		handle_merge(&chain->handle, &table->handle);
> -		ctx->cmd->handle.chain.location = chain->location;
> -		if (chain_evaluate(ctx, chain) < 0)
> -			return -1;
> -	}
> -	list_for_each_entry(ft, &table->flowtables, list) {
> -		handle_merge(&ft->handle, &table->handle);
> -		if (flowtable_evaluate(ctx, ft) < 0)
> -			return -1;
> -	}
> -	list_for_each_entry(obj, &table->objs, list) {
> -		handle_merge(&obj->handle, &table->handle);
> -		if (obj_evaluate(ctx, obj) < 0)
> -			return -1;
> -	}
> -
> -	ctx->table = NULL;
>  	return 0;
>  }
>  
> diff --git a/src/libnftables.c b/src/libnftables.c
> index 044365914747..b1f57802b90e 100644
> --- a/src/libnftables.c
> +++ b/src/libnftables.c
> @@ -420,6 +420,13 @@ static int nft_evaluate(struct nft_ctx *nft, struct list_head *msgs,
>  	if (cache_update(nft, flags, msgs) < 0)
>  		return -1;
>  
> +	list_for_each_entry(cmd, cmds, list) {
> +		if (cmd->op != CMD_ADD)
> +			continue;
> +
> +		nft_cmd_expand(cmd);
> +	}
> +
>  	list_for_each_entry(cmd, cmds, list) {
>  		struct eval_ctx ectx = {
>  			.nft	= nft,
> @@ -437,7 +444,7 @@ static int nft_evaluate(struct nft_ctx *nft, struct list_head *msgs,
>  		if (cmd->op != CMD_ADD)
>  			continue;
>  
> -		nft_cmd_expand(cmd);
> +		nft_cmd_post_expand(cmd);
>  	}
>  
>  	return 0;
> diff --git a/src/rule.c b/src/rule.c
> index 9139418e1bf8..9c74b89c1fb1 100644
> --- a/src/rule.c
> +++ b/src/rule.c
> @@ -1511,8 +1511,9 @@ static void nft_cmd_expand_chain(struct chain *chain, struct list_head *new_cmds
>  
>  	list_for_each_entry_safe(rule, next, &chain->rules, list) {
>  		list_del(&rule->list);
> +		handle_merge(&rule->handle, &chain->handle);
>  		memset(&h, 0, sizeof(h));
> -		handle_merge(&h, &rule->handle);
> +		handle_merge(&h, &chain->handle);
>  		if (chain->flags & CHAIN_F_BINDING) {
>  			rule->handle.chain_id = chain->handle.chain_id;
>  			rule->handle.chain.location = chain->location;
> @@ -1526,10 +1527,10 @@ static void nft_cmd_expand_chain(struct chain *chain, struct list_head *new_cmds
>  void nft_cmd_expand(struct cmd *cmd)
>  {
>  	struct list_head new_cmds;
> -	struct set *set, *newset;
>  	struct flowtable *ft;
>  	struct table *table;
>  	struct chain *chain;
> +	struct set *set;
>  	struct obj *obj;
>  	struct cmd *new;
>  	struct handle h;
> @@ -1543,6 +1544,7 @@ void nft_cmd_expand(struct cmd *cmd)
>  			return;
>  
>  		list_for_each_entry(chain, &table->chains, list) {
> +			handle_merge(&chain->handle, &table->handle);
>  			memset(&h, 0, sizeof(h));
>  			handle_merge(&h, &chain->handle);
>  			h.chain_id = chain->handle.chain_id;
> @@ -1587,6 +1589,21 @@ void nft_cmd_expand(struct cmd *cmd)
>  		nft_cmd_expand_chain(chain, &new_cmds);
>  		list_splice(&new_cmds, &cmd->list);
>  		break;
> +	default:
> +		break;
> +	}
> +}
> +
> +void nft_cmd_post_expand(struct cmd *cmd)
> +{
> +	struct list_head new_cmds;
> +	struct set *set, *newset;
> +	struct cmd *new;
> +	struct handle h;
> +
> +	init_list_head(&new_cmds);
> +
> +	switch (cmd->obj) {
>  	case CMD_OBJ_SET:
>  	case CMD_OBJ_MAP:
>  		set = cmd->set;
> -- 
> 2.30.2
> 

> From 69d1ab7c50a6a1dd369b50a5edad769b98779e12 Mon Sep 17 00:00:00 2001
> From: Phil Sutter <phil@nwl.cc>
> Date: Wed, 23 Aug 2023 18:18:49 +0200
> Subject: [PATCH nft 0.9.8 7/7] tests: shell: Stabilize
>  sets/0043concatenated_ranges_0 test
> 
> upstream c791765cb0d62ba261f0b495e07315437b3ae914 commit.
> 
> On a slow system, one of the 'delete element' commands would
> occasionally fail. Assuming it can only happen if the 2s timeout passes
> "too quickly", work around it by adding elements with a 2m timeout
> instead and when wanting to test the element expiry just drop and add
> the element again with a short timeout.
> 
> Fixes: 6231d3fa4af1e ("tests: shell: Fix for unstable sets/0043concatenated_ranges_0")
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
>  tests/shell/testcases/sets/0043concatenated_ranges_0 | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/shell/testcases/sets/0043concatenated_ranges_0 b/tests/shell/testcases/sets/0043concatenated_ranges_0
> index 11767373bcd2..8d3dacf6e38a 100755
> --- a/tests/shell/testcases/sets/0043concatenated_ranges_0
> +++ b/tests/shell/testcases/sets/0043concatenated_ranges_0
> @@ -147,7 +147,7 @@ for ta in ${TYPES}; do
>  			eval add_b=\$ADD_${tb}
>  			eval add_c=\$ADD_${tc}
>  			${NFT} add element inet filter test \
> -				"{ ${add_a} . ${add_b} . ${add_c} timeout 1s${mapv}}"
> +				"{ ${add_a} . ${add_b} . ${add_c} timeout 2m${mapv}}"
>  			[ $(${NFT} list ${setmap} inet filter test |	\
>  			   grep -c "${add_a} . ${add_b} . ${add_c}") -eq 1 ]
>  
> @@ -180,6 +180,10 @@ for ta in ${TYPES}; do
>  				continue
>  			fi
>  
> +			${NFT} delete element inet filter test \
> +				"{ ${add_a} . ${add_b} . ${add_c} ${mapv}}"
> +			${NFT} add element inet filter test \
> +				"{ ${add_a} . ${add_b} . ${add_c} timeout 1s${mapv}}"
>  			sleep 1
>  			[ $(${NFT} list ${setmap} inet filter test |		\
>  			   grep -c "${add_a} . ${add_b} . ${add_c} ${mapv}") -eq 0 ]
> -- 
> 2.30.2
> 


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [RFC] nftables 0.9.8 -stable backports

[Pablo Neira Ayuso]

On Mon, Oct 09, 2023 at 01:36:29PM +0200, Pablo Neira Ayuso wrote:
> Hi Arturo, Jeremy,
> 
> This is a small batch offering fixes for nftables 0.9.8. It only
> includes the fixes for the implicit chain regression in recent
> kernels.
> 
> This is a few dependency patches that are missing in 0.9.8 are
> required:
> 
>         3542e49cf539 ("evaluate: init cmd pointer for new on-stack context")
>         a3ac2527724d ("src: split chain list in table")
>         4e718641397c ("cache: rename chain_htable to cache_chain_ht")
> 
> a3ac2527724d is fixing an issue with the cache that is required by the
> fixes. Then, the backport fixes for the implicit chain regression with
> Linux -stable:
> 
>         3975430b12d9 ("src: expand table command before evaluation")
>         27c753e4a8d4 ("rule: expand standalone chain that contains rules")
>         784597a4ed63 ("rule: add helper function to expand chain rules into commands")
> 
> I tested with tests/shell at the time of the nftables 0.9.8 release
> (*I did not use git HEAD tests/shell as I did for 1.0.6*).
> 
> I have kept back the backport of this patch intentionally:
> 
>         56c90a2dd2eb ("evaluate: expand sets and maps before evaluation")
> 
> this depends on the new src/interval.c code, in 0.9.8 overlap and
> automerge come a later stage and cache is not updated incrementally,
> I tried the tests coming in this patch and it works fine.
> 
> I did run a few more tests with rulesets that I have been collecting
> from people that occasionally send them to me for my personal ruleset
> repo.
> 
> I: results: [OK] 266 [FAILED] 0 [TOTAL] 266
> 
> This has been tested with latest Linux kernel 5.10 -stable.

Amendment:

I: results: [OK] 264 [FAILED] 2 [TOTAL] 266

But this is because stateful expression in sets are not available in 5.10.

W: [FAILED]     ././testcases/sets/0059set_update_multistmt_0
W: [FAILED]     ././testcases/sets/0060set_multistmt_0

and tests/shell in 0.9.8 has not feature detection support.

Re: [RFC] nftables 0.9.8 -stable backports

[Jeremy Sowden]

[-- Attachment #1: Type: text/plain, Size: 2674 bytes --]

On 2023-10-10, at 10:54:51 +0200, Pablo Neira Ayuso wrote:
> On Mon, Oct 09, 2023 at 01:36:29PM +0200, Pablo Neira Ayuso wrote:
> > This is a small batch offering fixes for nftables 0.9.8. It only
> > includes the fixes for the implicit chain regression in recent
> > kernels.
> > 
> > This is a few dependency patches that are missing in 0.9.8 are
> > required:
> > 
> >         3542e49cf539 ("evaluate: init cmd pointer for new on-stack context")
> >         a3ac2527724d ("src: split chain list in table")
> >         4e718641397c ("cache: rename chain_htable to cache_chain_ht")
> > 
> > a3ac2527724d is fixing an issue with the cache that is required by the
> > fixes. Then, the backport fixes for the implicit chain regression with
> > Linux -stable:
> > 
> >         3975430b12d9 ("src: expand table command before evaluation")
> >         27c753e4a8d4 ("rule: expand standalone chain that contains rules")
> >         784597a4ed63 ("rule: add helper function to expand chain rules into commands")
> > 
> > I tested with tests/shell at the time of the nftables 0.9.8 release
> > (*I did not use git HEAD tests/shell as I did for 1.0.6*).
> > 
> > I have kept back the backport of this patch intentionally:
> > 
> >         56c90a2dd2eb ("evaluate: expand sets and maps before evaluation")
> > 
> > this depends on the new src/interval.c code, in 0.9.8 overlap and
> > automerge come a later stage and cache is not updated incrementally,
> > I tried the tests coming in this patch and it works fine.
> > 
> > I did run a few more tests with rulesets that I have been collecting
> > from people that occasionally send them to me for my personal ruleset
> > repo.
> > 
> > I: results: [OK] 266 [FAILED] 0 [TOTAL] 266
> > 
> > This has been tested with latest Linux kernel 5.10 -stable.
> 
> Amendment:
> 
> I: results: [OK] 264 [FAILED] 2 [TOTAL] 266
> 
> But this is because stateful expression in sets are not available in 5.10.
> 
> W: [FAILED]     ././testcases/sets/0059set_update_multistmt_0
> W: [FAILED]     ././testcases/sets/0060set_multistmt_0
>
> and tests/shell in 0.9.8 has not feature detection support.

This is very helpful.  Thanks.

My immediate interest is getting the implicit chain regression fixes
into Debian 11, so for that I'm going to cherry-pick:

  4e718641397c ("cache: rename chain_htable to cache_chain_ht")
  a3ac2527724d ("src: split chain list in table")
  784597a4ed63 ("rule: add helper function to expand chain rules into commands")
  27c753e4a8d4 ("rule: expand standalone chain that contains rules")
  3975430b12d9 ("src: expand table command before evaluation")

J.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [RFC] nftables 0.9.8 -stable backports

[Pablo Neira Ayuso]

On Tue, Oct 10, 2023 at 09:08:38PM +0100, Jeremy Sowden wrote:
> On 2023-10-10, at 10:54:51 +0200, Pablo Neira Ayuso wrote:
> > On Mon, Oct 09, 2023 at 01:36:29PM +0200, Pablo Neira Ayuso wrote:
> > > This is a small batch offering fixes for nftables 0.9.8. It only
> > > includes the fixes for the implicit chain regression in recent
> > > kernels.
> > > 
> > > This is a few dependency patches that are missing in 0.9.8 are
> > > required:
> > > 
> > >         3542e49cf539 ("evaluate: init cmd pointer for new on-stack context")
> > >         a3ac2527724d ("src: split chain list in table")
> > >         4e718641397c ("cache: rename chain_htable to cache_chain_ht")
> > > 
> > > a3ac2527724d is fixing an issue with the cache that is required by the
> > > fixes. Then, the backport fixes for the implicit chain regression with
> > > Linux -stable:
> > > 
> > >         3975430b12d9 ("src: expand table command before evaluation")
> > >         27c753e4a8d4 ("rule: expand standalone chain that contains rules")
> > >         784597a4ed63 ("rule: add helper function to expand chain rules into commands")
> > > 
> > > I tested with tests/shell at the time of the nftables 0.9.8 release
> > > (*I did not use git HEAD tests/shell as I did for 1.0.6*).
> > > 
> > > I have kept back the backport of this patch intentionally:
> > > 
> > >         56c90a2dd2eb ("evaluate: expand sets and maps before evaluation")
> > > 
> > > this depends on the new src/interval.c code, in 0.9.8 overlap and
> > > automerge come a later stage and cache is not updated incrementally,
> > > I tried the tests coming in this patch and it works fine.
> > > 
> > > I did run a few more tests with rulesets that I have been collecting
> > > from people that occasionally send them to me for my personal ruleset
> > > repo.
> > > 
> > > I: results: [OK] 266 [FAILED] 0 [TOTAL] 266
> > > 
> > > This has been tested with latest Linux kernel 5.10 -stable.
> > 
> > Amendment:
> > 
> > I: results: [OK] 264 [FAILED] 2 [TOTAL] 266
> > 
> > But this is because stateful expression in sets are not available in 5.10.
> > 
> > W: [FAILED]     ././testcases/sets/0059set_update_multistmt_0
> > W: [FAILED]     ././testcases/sets/0060set_multistmt_0
> >
> > and tests/shell in 0.9.8 has not feature detection support.
> 
> This is very helpful.  Thanks.
> 
> My immediate interest is getting the implicit chain regression fixes
> into Debian 11, so for that I'm going to cherry-pick:
> 
>   4e718641397c ("cache: rename chain_htable to cache_chain_ht")
>   a3ac2527724d ("src: split chain list in table")
>   784597a4ed63 ("rule: add helper function to expand chain rules into commands")
>   27c753e4a8d4 ("rule: expand standalone chain that contains rules")
>   3975430b12d9 ("src: expand table command before evaluation")

This is also needed:

    3542e49cf539 ("evaluate: init cmd pointer for new on-stack context")

otherwise the test with implicit chain in 0.9.8 crashes, it is a
dependency patch.

Re: [RFC] nftables 0.9.8 -stable backports

[Jeremy Sowden]

[-- Attachment #1: Type: text/plain, Size: 3256 bytes --]

On 2023-10-11, at 00:21:16 +0200, Pablo Neira Ayuso wrote:
> On Tue, Oct 10, 2023 at 09:08:38PM +0100, Jeremy Sowden wrote:
> > On 2023-10-10, at 10:54:51 +0200, Pablo Neira Ayuso wrote:
> > > On Mon, Oct 09, 2023 at 01:36:29PM +0200, Pablo Neira Ayuso wrote:
> > > > This is a small batch offering fixes for nftables 0.9.8. It only
> > > > includes the fixes for the implicit chain regression in recent
> > > > kernels.
> > > > 
> > > > This is a few dependency patches that are missing in 0.9.8 are
> > > > required:
> > > > 
> > > >         3542e49cf539 ("evaluate: init cmd pointer for new on-stack context")
> > > >         a3ac2527724d ("src: split chain list in table")
> > > >         4e718641397c ("cache: rename chain_htable to cache_chain_ht")
> > > > 
> > > > a3ac2527724d is fixing an issue with the cache that is required by the
> > > > fixes. Then, the backport fixes for the implicit chain regression with
> > > > Linux -stable:
> > > > 
> > > >         3975430b12d9 ("src: expand table command before evaluation")
> > > >         27c753e4a8d4 ("rule: expand standalone chain that contains rules")
> > > >         784597a4ed63 ("rule: add helper function to expand chain rules into commands")
> > > > 
> > > > I tested with tests/shell at the time of the nftables 0.9.8 release
> > > > (*I did not use git HEAD tests/shell as I did for 1.0.6*).
> > > > 
> > > > I have kept back the backport of this patch intentionally:
> > > > 
> > > >         56c90a2dd2eb ("evaluate: expand sets and maps before evaluation")
> > > > 
> > > > this depends on the new src/interval.c code, in 0.9.8 overlap and
> > > > automerge come a later stage and cache is not updated incrementally,
> > > > I tried the tests coming in this patch and it works fine.
> > > > 
> > > > I did run a few more tests with rulesets that I have been collecting
> > > > from people that occasionally send them to me for my personal ruleset
> > > > repo.
> > > > 
> > > > I: results: [OK] 266 [FAILED] 0 [TOTAL] 266
> > > > 
> > > > This has been tested with latest Linux kernel 5.10 -stable.
> > > 
> > > Amendment:
> > > 
> > > I: results: [OK] 264 [FAILED] 2 [TOTAL] 266
> > > 
> > > But this is because stateful expression in sets are not available in 5.10.
> > > 
> > > W: [FAILED]     ././testcases/sets/0059set_update_multistmt_0
> > > W: [FAILED]     ././testcases/sets/0060set_multistmt_0
> > >
> > > and tests/shell in 0.9.8 has not feature detection support.
> > 
> > This is very helpful.  Thanks.
> > 
> > My immediate interest is getting the implicit chain regression fixes
> > into Debian 11, so for that I'm going to cherry-pick:
> > 
> >   4e718641397c ("cache: rename chain_htable to cache_chain_ht")
> >   a3ac2527724d ("src: split chain list in table")
> >   784597a4ed63 ("rule: add helper function to expand chain rules into commands")
> >   27c753e4a8d4 ("rule: expand standalone chain that contains rules")
> >   3975430b12d9 ("src: expand table command before evaluation")
> 
> This is also needed:
> 
>     3542e49cf539 ("evaluate: init cmd pointer for new on-stack context")
> 
> otherwise the test with implicit chain in 0.9.8 crashes, it is a
> dependency patch.

Thanks.

J.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [RFC] nftables 0.9.8 -stable backports

[Pablo Neira Ayuso]

On Wed, Oct 11, 2023 at 10:46:13AM +0100, Jeremy Sowden wrote:
> On 2023-10-11, at 00:21:16 +0200, Pablo Neira Ayuso wrote:
> > On Tue, Oct 10, 2023 at 09:08:38PM +0100, Jeremy Sowden wrote:
> > > On 2023-10-10, at 10:54:51 +0200, Pablo Neira Ayuso wrote:
> > > > On Mon, Oct 09, 2023 at 01:36:29PM +0200, Pablo Neira Ayuso wrote:
> > > > > This is a small batch offering fixes for nftables 0.9.8. It only
> > > > > includes the fixes for the implicit chain regression in recent
> > > > > kernels.
> > > > > 
> > > > > This is a few dependency patches that are missing in 0.9.8 are
> > > > > required:
> > > > > 
> > > > >         3542e49cf539 ("evaluate: init cmd pointer for new on-stack context")
> > > > >         a3ac2527724d ("src: split chain list in table")
> > > > >         4e718641397c ("cache: rename chain_htable to cache_chain_ht")
> > > > > 
> > > > > a3ac2527724d is fixing an issue with the cache that is required by the
> > > > > fixes. Then, the backport fixes for the implicit chain regression with
> > > > > Linux -stable:
> > > > > 
> > > > >         3975430b12d9 ("src: expand table command before evaluation")
> > > > >         27c753e4a8d4 ("rule: expand standalone chain that contains rules")
> > > > >         784597a4ed63 ("rule: add helper function to expand chain rules into commands")
> > > > > 
> > > > > I tested with tests/shell at the time of the nftables 0.9.8 release
> > > > > (*I did not use git HEAD tests/shell as I did for 1.0.6*).
> > > > > 
> > > > > I have kept back the backport of this patch intentionally:
> > > > > 
> > > > >         56c90a2dd2eb ("evaluate: expand sets and maps before evaluation")
> > > > > 
> > > > > this depends on the new src/interval.c code, in 0.9.8 overlap and
> > > > > automerge come a later stage and cache is not updated incrementally,
> > > > > I tried the tests coming in this patch and it works fine.
> > > > > 
> > > > > I did run a few more tests with rulesets that I have been collecting
> > > > > from people that occasionally send them to me for my personal ruleset
> > > > > repo.
> > > > > 
> > > > > I: results: [OK] 266 [FAILED] 0 [TOTAL] 266
> > > > > 
> > > > > This has been tested with latest Linux kernel 5.10 -stable.
> > > > 
> > > > Amendment:
> > > > 
> > > > I: results: [OK] 264 [FAILED] 2 [TOTAL] 266
> > > > 
> > > > But this is because stateful expression in sets are not available in 5.10.
> > > > 
> > > > W: [FAILED]     ././testcases/sets/0059set_update_multistmt_0
> > > > W: [FAILED]     ././testcases/sets/0060set_multistmt_0
> > > >
> > > > and tests/shell in 0.9.8 has not feature detection support.
> > > 
> > > This is very helpful.  Thanks.
> > > 
> > > My immediate interest is getting the implicit chain regression fixes
> > > into Debian 11, so for that I'm going to cherry-pick:
> > > 
> > >   4e718641397c ("cache: rename chain_htable to cache_chain_ht")
> > >   a3ac2527724d ("src: split chain list in table")
> > >   784597a4ed63 ("rule: add helper function to expand chain rules into commands")
> > >   27c753e4a8d4 ("rule: expand standalone chain that contains rules")
> > >   3975430b12d9 ("src: expand table command before evaluation")
> > 
> > This is also needed:
> > 
> >     3542e49cf539 ("evaluate: init cmd pointer for new on-stack context")
> > 
> > otherwise the test with implicit chain in 0.9.8 crashes, it is a
> > dependency patch.
> 

Wrong commit id, this:

4e718641397c ("evaluate: init cmd pointer for new on-stack context")

Sorry.

Re: [RFC] nftables 0.9.8 -stable backports

[Jeremy Sowden]

[-- Attachment #1: Type: text/plain, Size: 4044 bytes --]

Hi, Pablo.

On 2023-10-09, at 13:36:23 +0200, Pablo Neira Ayuso wrote:
> This is a small batch offering fixes for nftables 0.9.8. It only
> includes the fixes for the implicit chain regression in recent
> kernels.
> 
> This is a few dependency patches that are missing in 0.9.8 are
> required:
> 
>         3542e49cf539 ("evaluate: init cmd pointer for new on-stack context")
>         a3ac2527724d ("src: split chain list in table")
>         4e718641397c ("cache: rename chain_htable to cache_chain_ht")
> 
> a3ac2527724d is fixing an issue with the cache that is required by the
> fixes. Then, the backport fixes for the implicit chain regression with
> Linux -stable:
> 
>         3975430b12d9 ("src: expand table command before evaluation")
>         27c753e4a8d4 ("rule: expand standalone chain that contains rules")
>         784597a4ed63 ("rule: add helper function to expand chain rules into commands")
> 
> I tested with tests/shell at the time of the nftables 0.9.8 release
> (*I did not use git HEAD tests/shell as I did for 1.0.6*).
> 
> I have kept back the backport of this patch intentionally:
> 
>         56c90a2dd2eb ("evaluate: expand sets and maps before evaluation")
> 
> this depends on the new src/interval.c code, in 0.9.8 overlap and
> automerge come a later stage and cache is not updated incrementally,
> I tried the tests coming in this patch and it works fine.
> 
> I did run a few more tests with rulesets that I have been collecting
> from people that occasionally send them to me for my personal ruleset
> repo.
> 
> I: results: [OK] 266 [FAILED] 0 [TOTAL] 266
> 
> This has been tested with latest Linux kernel 5.10 -stable.
> 
> I can still run a few more tests, I will get back to you if I find any
> issue.
> 
> Let me know, thanks.

A new version of nftables containing these fixes was released as part of
the Debian 11.9 point release, which happened a week ago.  Since then,
we've had a couple of bug-reports:

  https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1063690
  https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1063769

The gist of them is that if nft processes a file containing multiple
table-blocks for the same table, and there is a set definition in one of
the non-initial ones, e.g.:

  table inet t {
  }
  table inet t {
    set s {
      type inet_service
      elements = { 42 }
    }
  }

it crashes with a seg-fault.

The bison parser creates two `CMD_ADD` commands and allocates two
`struct table` objects (which I shall refer to as `t0` and `t1`).  When
it creates the second command, it also allocates a `struct set` object,
`s`, which it adds to `t1->sets`.  After the `CMD_ADD` commands for `t0`
and `t1` have been expanded, when the new `CMD_ADD` command for `s` is
evaluated, `set_evaluate` does this (evaluate.c, ll. 3686ff.):

	table = table_lookup_global(ctx);
	if (table == NULL)
		return table_not_found(ctx);

and later this (evaluate.c, ll. 3762f.):

	if (set_lookup(table, set->handle.set.name) == NULL)
		set_add_hash(set_get(set), table);

The `struct table` object returned by `table_lookup_global` is `t0`,
since this was evaluated first and cached by `table_evaluate`, not `t1`.
Therefore, `set_lookup` returns `NULL`, `set_add_hash` is called, `s` is
added to `t0->sets`, and `t1->sets` is effectively corrupted.  It now
contains two elements which point to each other, and one of them is not
a set at all, but `t0->sets`.  This results in a seg-fault when nft
tries to free `t1`.

I _think_ that the following is all that is needed to fix it:

  @@ -3759,7 +3759,8 @@ static int set_evaluate(struct eval_ctx *ctx, struct set *set)
          }
          ctx->set = NULL;
   
  -       if (set_lookup(table, set->handle.set.name) == NULL)
  +       if (set_lookup(table, set->handle.set.name) == NULL &&
  +           list_empty(&set->list))
                  set_add_hash(set_get(set), table);
   
          return 0;

Does this look good to you?

J.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [RFC] nftables 0.9.8 -stable backports

[Jeremy Sowden]

[-- Attachment #1: Type: text/plain, Size: 7424 bytes --]

On 2024-02-17, at 20:11:42 +0000, Jeremy Sowden wrote:
> On 2023-10-09, at 13:36:23 +0200, Pablo Neira Ayuso wrote:
> > This is a small batch offering fixes for nftables 0.9.8. It only
> > includes the fixes for the implicit chain regression in recent
> > kernels.
> > 
> > This is a few dependency patches that are missing in 0.9.8 are
> > required:
> > 
> >         3542e49cf539 ("evaluate: init cmd pointer for new on-stack context")
> >         a3ac2527724d ("src: split chain list in table")
> >         4e718641397c ("cache: rename chain_htable to cache_chain_ht")
> > 
> > a3ac2527724d is fixing an issue with the cache that is required by the
> > fixes. Then, the backport fixes for the implicit chain regression with
> > Linux -stable:
> > 
> >         3975430b12d9 ("src: expand table command before evaluation")
> >         27c753e4a8d4 ("rule: expand standalone chain that contains rules")
> >         784597a4ed63 ("rule: add helper function to expand chain rules into commands")
> > 
> > I tested with tests/shell at the time of the nftables 0.9.8 release
> > (*I did not use git HEAD tests/shell as I did for 1.0.6*).
> > 
> > I have kept back the backport of this patch intentionally:
> > 
> >         56c90a2dd2eb ("evaluate: expand sets and maps before evaluation")
> > 
> > this depends on the new src/interval.c code, in 0.9.8 overlap and
> > automerge come a later stage and cache is not updated incrementally,
> > I tried the tests coming in this patch and it works fine.
> > 
> > I did run a few more tests with rulesets that I have been collecting
> > from people that occasionally send them to me for my personal ruleset
> > repo.
> > 
> > I: results: [OK] 266 [FAILED] 0 [TOTAL] 266
> > 
> > This has been tested with latest Linux kernel 5.10 -stable.
> > 
> > I can still run a few more tests, I will get back to you if I find any
> > issue.
> > 
> > Let me know, thanks.
> 
> A new version of nftables containing these fixes was released as part of
> the Debian 11.9 point release, which happened a week ago.  Since then,
> we've had a couple of bug-reports:
> 
>   https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1063690
>   https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1063769
> 
> The gist of them is that if nft processes a file containing multiple
> table-blocks for the same table, and there is a set definition in one of
> the non-initial ones, e.g.:
> 
>   table inet t {
>   }
>   table inet t {
>     set s {
>       type inet_service
>       elements = { 42 }
>     }
>   }
> 
> it crashes with a seg-fault.
> 
> The bison parser creates two `CMD_ADD` commands and allocates two
> `struct table` objects (which I shall refer to as `t0` and `t1`).  When
> it creates the second command, it also allocates a `struct set` object,
> `s`, which it adds to `t1->sets`.  After the `CMD_ADD` commands for `t0`
> and `t1` have been expanded, when the new `CMD_ADD` command for `s` is
> evaluated, `set_evaluate` does this (evaluate.c, ll. 3686ff.):
> 
> 	table = table_lookup_global(ctx);
> 	if (table == NULL)
> 		return table_not_found(ctx);
> 
> and later this (evaluate.c, ll. 3762f.):
> 
> 	if (set_lookup(table, set->handle.set.name) == NULL)
> 		set_add_hash(set_get(set), table);
> 
> The `struct table` object returned by `table_lookup_global` is `t0`,
> since this was evaluated first and cached by `table_evaluate`, not `t1`.
> Therefore, `set_lookup` returns `NULL`, `set_add_hash` is called, `s` is
> added to `t0->sets`, and `t1->sets` is effectively corrupted.  It now
> contains two elements which point to each other, and one of them is not
> a set at all, but `t0->sets`.  This results in a seg-fault when nft
> tries to free `t1`.
> 
> I _think_ that the following is all that is needed to fix it:
> 
>   @@ -3759,7 +3759,8 @@ static int set_evaluate(struct eval_ctx *ctx, struct set *set)
>           }
>           ctx->set = NULL;
>    
>   -       if (set_lookup(table, set->handle.set.name) == NULL)
>   +       if (set_lookup(table, set->handle.set.name) == NULL &&
>   +           list_empty(&set->list))
>                   set_add_hash(set_get(set), table);
>    
>           return 0;
> 
> Does this look good to you?

Forgot to run the test-suite.  Doing so revealed that this doesn't quite
work because `set_alloc` doesn't initialize `s->list`.  This, however,
does:

  diff --git a/src/evaluate.c b/src/evaluate.c
  index 232ae39020cc..c58e37e14064 100644
  --- a/src/evaluate.c
  +++ b/src/evaluate.c
  @@ -3759,7 +3759,8 @@ static int set_evaluate(struct eval_ctx *ctx, struct set *set)
          }
          ctx->set = NULL;
   
  -       if (set_lookup(table, set->handle.set.name) == NULL)
  +       if (set_lookup(table, set->handle.set.name) == NULL &&
  +           list_empty(&set->list))
                  set_add_hash(set_get(set), table);
   
          return 0;
  diff --git a/src/rule.c b/src/rule.c
  index c23f87f47ae2..365feec08c32 100644
  --- a/src/rule.c
  +++ b/src/rule.c
  @@ -339,6 +339,7 @@ struct set *set_alloc(const struct location *loc)
          if (loc != NULL)
                  set->location = *loc;
   
  +       init_list_head(&set->list);
          init_list_head(&set->stmt_list);
   
          return set;
  @@ -360,6 +361,7 @@ struct set *set_clone(const struct set *set)
          new_set->policy         = set->policy;
          new_set->automerge      = set->automerge;
          new_set->desc           = set->desc;
  +       init_list_head(&new_set->list);
          init_list_head(&new_set->stmt_list);
   
          return new_set;

Alternatively, we could continue adding the set to the cached table, but
without the seg-fault:

  diff --git a/src/evaluate.c b/src/evaluate.c
  index 232ae39020cc..23ff982b73f0 100644
  --- a/src/evaluate.c
  +++ b/src/evaluate.c
  @@ -3760,7 +3760,7 @@ static int set_evaluate(struct eval_ctx *ctx, struct set *set)
          ctx->set = NULL;
   
          if (set_lookup(table, set->handle.set.name) == NULL)
  -               set_add_hash(set_get(set), table);
  +               set_add_hash(set, table);
   
          return 0;
   }
  diff --git a/src/rule.c b/src/rule.c
  index c23f87f47ae2..0aaefc54c30d 100644
  --- a/src/rule.c
  +++ b/src/rule.c
  @@ -339,6 +339,7 @@ struct set *set_alloc(const struct location *loc)
          if (loc != NULL)
                  set->location = *loc;
   
  +       init_list_head(&set->list);
          init_list_head(&set->stmt_list);
   
          return set;
  @@ -360,6 +361,7 @@ struct set *set_clone(const struct set *set)
          new_set->policy         = set->policy;
          new_set->automerge      = set->automerge;
          new_set->desc           = set->desc;
  +       init_list_head(&new_set->list);
          init_list_head(&new_set->stmt_list);
   
          return new_set;
  @@ -391,7 +393,10 @@ void set_free(struct set *set)
   
   void set_add_hash(struct set *set, struct table *table)
   {
  -       list_add_tail(&set->list, &table->sets);
  +       if (list_empty(&set->list))
  +               list_add_tail(&set_get(set)->list, &table->sets);
  +       else
  +               list_move_tail(&set->list, &table->sets);
   }
   
   struct set *set_lookup(const struct table *table, const char *name)
  
J.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [RFC] nftables 0.9.8 -stable backports

[Pablo Neira Ayuso]

[-- Attachment #1: Type: text/plain, Size: 597 bytes --]

Hi Jeremy,

On Sun, Feb 18, 2024 at 01:56:00PM +0000, Jeremy Sowden wrote:
> On 2024-02-17, at 20:11:42 +0000, Jeremy Sowden wrote:
> > Does this look good to you?
> 
> Forgot to run the test-suite.  Doing so revealed that this doesn't quite
> work because `set_alloc` doesn't initialize `s->list`.

I'd suggest instead a backport of the patch that fixes the set cache
for 0.9.8 instead.

See attached patch, it is partial backport of a upstream patch.

I have run tests/shell (two tests don't pass, because 5.15 does not
support multiple statements) and tests/py for that nftables 0.9.8 version.

[-- Attachment #2: 0001-partial-backport-of-df48e56e987f-cache-add-hashtable.patch --]
[-- Type: text/x-diff, Size: 11490 bytes --]

From 92908c439d1e33f10ee96daf63eae50d1dfcbb52 Mon Sep 17 00:00:00 2001
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Tue, 20 Feb 2024 10:26:22 +0100
Subject: [PATCH] partial backport of df48e56e987f ("cache: add hashtable cache
 for sets")

This patch splits table->sets in two:

- Sets that reside in the cache are stored in the new
  tables->cache_set and tables->cache_set_ht.

- Set that defined via command line / ruleset file reside in
  tables->set.

Sets in the cache (already in the kernel) are not placed in the
table->sets list.

By keeping separated lists, sets defined via command line / ruleset file
can be added to cache.
---
 include/cache.h   |  7 +++++
 include/netlink.h |  1 -
 include/rule.h    |  3 ++-
 src/cache.c       | 69 +++++++++++++++++++++++++++++++++++++++++++++++
 src/evaluate.c    |  2 +-
 src/json.c        |  4 +--
 src/monitor.c     |  2 +-
 src/netlink.c     | 31 ---------------------
 src/rule.c        | 34 ++++++++++++++---------
 9 files changed, 104 insertions(+), 49 deletions(-)

diff --git a/include/cache.h b/include/cache.h
index baa2bb29f1e7..d4abe67611bc 100644
--- a/include/cache.h
+++ b/include/cache.h
@@ -59,4 +59,11 @@ void chain_cache_add(struct chain *chain, struct table *table);
 struct chain *chain_cache_find(const struct table *table,
 			       const struct handle *handle);
 
+struct nftnl_set_list;
+
+struct nftnl_set_list *set_cache_dump(struct netlink_ctx *ctx, int *err);
+int set_cache_init(struct netlink_ctx *ctx, struct table *table,
+		   struct nftnl_set_list *set_list);
+void set_cache_add(struct set *set, struct table *table);
+
 #endif /* _NFT_CACHE_H_ */
diff --git a/include/netlink.h b/include/netlink.h
index cf8aae465324..f93c5322100f 100644
--- a/include/netlink.h
+++ b/include/netlink.h
@@ -139,7 +139,6 @@ extern int netlink_list_tables(struct netlink_ctx *ctx, const struct handle *h);
 extern struct table *netlink_delinearize_table(struct netlink_ctx *ctx,
 					       const struct nftnl_table *nlt);
 
-extern int netlink_list_sets(struct netlink_ctx *ctx, const struct handle *h);
 extern struct set *netlink_delinearize_set(struct netlink_ctx *ctx,
 					   const struct nftnl_set *nls);
 
diff --git a/include/rule.h b/include/rule.h
index f880cfd32bd8..7d1bd75e9d7e 100644
--- a/include/rule.h
+++ b/include/rule.h
@@ -157,6 +157,7 @@ struct table {
 	struct list_head	*cache_chain_ht;
 	struct list_head	cache_chain;
 	struct list_head	chains;
+	struct list_head	cache_set;
 	struct list_head	sets;
 	struct list_head	objs;
 	struct list_head	flowtables;
@@ -323,6 +324,7 @@ void rule_stmt_insert_at(struct rule *rule, struct stmt *nstmt,
  */
 struct set {
 	struct list_head	list;
+	struct list_head	cache_list;
 	struct handle		handle;
 	struct location		location;
 	unsigned int		refcnt;
@@ -351,7 +353,6 @@ extern struct set *set_alloc(const struct location *loc);
 extern struct set *set_get(struct set *set);
 extern void set_free(struct set *set);
 extern struct set *set_clone(const struct set *set);
-extern void set_add_hash(struct set *set, struct table *table);
 extern struct set *set_lookup(const struct table *table, const char *name);
 extern struct set *set_lookup_global(uint32_t family, const char *table,
 				     const char *name, struct nft_cache *cache);
diff --git a/src/cache.c b/src/cache.c
index 32e6eea4ac5c..600e6f02d22e 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -15,6 +15,8 @@
 #include <netlink.h>
 #include <mnl.h>
 #include <libnftnl/chain.h>
+#include <include/linux/netfilter.h>
+#include <linux/netfilter/nf_tables.h>
 
 static unsigned int evaluate_cache_add(struct cmd *cmd, unsigned int flags)
 {
@@ -256,3 +258,70 @@ struct chain *chain_cache_find(const struct table *table,
 
 	return NULL;
 }
+
+struct set_cache_dump_ctx {
+	struct netlink_ctx	*nlctx;
+	struct table		*table;
+};
+
+static int set_cache_cb(struct nftnl_set *nls, void *arg)
+{
+	struct set_cache_dump_ctx *ctx = arg;
+	const char *set_table;
+	uint32_t set_family;
+	struct set *set;
+
+	set_table = nftnl_set_get_str(nls, NFTNL_SET_TABLE);
+	set_family = nftnl_set_get_u32(nls, NFTNL_SET_FAMILY);
+
+	if (set_family != ctx->table->handle.family ||
+	    strcmp(set_table, ctx->table->handle.table.name))
+		return 0;
+
+	set = netlink_delinearize_set(ctx->nlctx, nls);
+	if (!set)
+		return -1;
+
+	list_add_tail(&set->cache_list, &ctx->table->cache_set);
+
+	nftnl_set_list_del(nls);
+	nftnl_set_free(nls);
+	return 0;
+}
+
+int set_cache_init(struct netlink_ctx *ctx, struct table *table,
+		   struct nftnl_set_list *set_list)
+{
+	struct set_cache_dump_ctx dump_ctx = {
+		.nlctx	= ctx,
+		.table	= table,
+	};
+
+	nftnl_set_list_foreach(set_list, set_cache_cb, &dump_ctx);
+
+	return 0;
+}
+
+void set_cache_add(struct set *set, struct table *table)
+{
+	list_add_tail(&set->cache_list, &table->cache_set);
+}
+
+struct nftnl_set_list *set_cache_dump(struct netlink_ctx *ctx, int *err)
+{
+	struct nftnl_set_list *set_list;
+	const char *table = NULL;
+	int family = AF_UNSPEC;
+
+	set_list = mnl_nft_set_dump(ctx, family, table);
+	if (!set_list) {
+		if (errno == EINTR) {
+			*err = -1;
+			return NULL;
+		}
+		*err = 0;
+		return NULL;
+	}
+
+	return set_list;
+}
diff --git a/src/evaluate.c b/src/evaluate.c
index 232ae39020cc..7378174ceb97 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -3760,7 +3760,7 @@ static int set_evaluate(struct eval_ctx *ctx, struct set *set)
 	ctx->set = NULL;
 
 	if (set_lookup(table, set->handle.set.name) == NULL)
-		set_add_hash(set_get(set), table);
+		set_cache_add(set_get(set), table);
 
 	return 0;
 }
diff --git a/src/json.c b/src/json.c
index 737e73b08b5a..13079230af22 100644
--- a/src/json.c
+++ b/src/json.c
@@ -1584,7 +1584,7 @@ static json_t *table_print_json_full(struct netlink_ctx *ctx,
 		tmp = obj_print_json(obj);
 		json_array_append_new(root, tmp);
 	}
-	list_for_each_entry(set, &table->sets, list) {
+	list_for_each_entry(set, &table->cache_set, cache_list) {
 		if (set_is_anonymous(set->flags))
 			continue;
 		tmp = set_print_json(&ctx->nft->output, set);
@@ -1717,7 +1717,7 @@ static json_t *do_list_sets_json(struct netlink_ctx *ctx, struct cmd *cmd)
 		    cmd->handle.family != table->handle.family)
 			continue;
 
-		list_for_each_entry(set, &table->sets, list) {
+		list_for_each_entry(set, &table->cache_set, cache_list) {
 			if (cmd->obj == CMD_OBJ_SETS &&
 			    !set_is_literal(set->flags))
 				continue;
diff --git a/src/monitor.c b/src/monitor.c
index af2998d4272b..946621e28ec0 100644
--- a/src/monitor.c
+++ b/src/monitor.c
@@ -632,7 +632,7 @@ static void netlink_events_cache_addset(struct netlink_mon_handler *monh,
 		goto out;
 	}
 
-	set_add_hash(s, t);
+	set_cache_add(s, t);
 out:
 	nftnl_set_free(nls);
 }
diff --git a/src/netlink.c b/src/netlink.c
index ec2dad29ace1..dcac0ab8f871 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -970,37 +970,6 @@ struct set *netlink_delinearize_set(struct netlink_ctx *ctx,
 	return set;
 }
 
-static int list_set_cb(struct nftnl_set *nls, void *arg)
-{
-	struct netlink_ctx *ctx = arg;
-	struct set *set;
-
-	set = netlink_delinearize_set(ctx, nls);
-	if (set == NULL)
-		return -1;
-	list_add_tail(&set->list, &ctx->list);
-	return 0;
-}
-
-int netlink_list_sets(struct netlink_ctx *ctx, const struct handle *h)
-{
-	struct nftnl_set_list *set_cache;
-	int err;
-
-	set_cache = mnl_nft_set_dump(ctx, h->family, h->table.name);
-	if (set_cache == NULL) {
-		if (errno == EINTR)
-			return -1;
-
-		return 0;
-	}
-
-	ctx->data = h;
-	err = nftnl_set_list_foreach(set_cache, list_set_cb, ctx);
-	nftnl_set_list_free(set_cache);
-	return err;
-}
-
 void alloc_setelem_cache(const struct expr *set, struct nftnl_set *nls)
 {
 	struct nftnl_set_elem *nlse;
diff --git a/src/rule.c b/src/rule.c
index 9c74b89c1fb1..4b2682455253 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -153,6 +153,7 @@ static int cache_init_tables(struct netlink_ctx *ctx, struct handle *h,
 static int cache_init_objects(struct netlink_ctx *ctx, unsigned int flags)
 {
 	struct nftnl_chain_list *chain_list = NULL;
+	struct nftnl_set_list *set_list = NULL;
 	struct rule *rule, *nrule;
 	struct table *table;
 	struct chain *chain;
@@ -164,16 +165,22 @@ static int cache_init_objects(struct netlink_ctx *ctx, unsigned int flags)
 		if (!chain_list)
 			return ret;
 	}
+	if (flags & NFT_CACHE_SET_BIT) {
+		set_list = set_cache_dump(ctx, &ret);
+		if (!set_list) {
+			ret = -1;
+			goto cache_fails;
+		}
+	}
 
 	list_for_each_entry(table, &ctx->nft->cache.list, list) {
 		if (flags & NFT_CACHE_SET_BIT) {
-			ret = netlink_list_sets(ctx, &table->handle);
-			list_splice_tail_init(&ctx->list, &table->sets);
+			ret = set_cache_init(ctx, table, set_list);
 			if (ret < 0)
 				return -1;
 		}
 		if (flags & NFT_CACHE_SETELEM_BIT) {
-			list_for_each_entry(set, &table->sets, list) {
+			list_for_each_entry(set, &table->cache_set, cache_list) {
 				ret = netlink_list_setelems(ctx, &set->handle,
 							    set);
 				if (ret < 0)
@@ -212,6 +219,10 @@ static int cache_init_objects(struct netlink_ctx *ctx, unsigned int flags)
 		}
 	}
 
+cache_fails:
+	if (set_list)
+		nftnl_set_list_free(set_list);
+
 	if (flags & NFT_CACHE_CHAIN_BIT)
 		nftnl_chain_list_free(chain_list);
 
@@ -389,16 +400,11 @@ void set_free(struct set *set)
 	xfree(set);
 }
 
-void set_add_hash(struct set *set, struct table *table)
-{
-	list_add_tail(&set->list, &table->sets);
-}
-
 struct set *set_lookup(const struct table *table, const char *name)
 {
 	struct set *set;
 
-	list_for_each_entry(set, &table->sets, list) {
+	list_for_each_entry(set, &table->cache_set, cache_list) {
 		if (!strcmp(set->handle.set.name, name))
 			return set;
 	}
@@ -416,7 +422,7 @@ struct set *set_lookup_fuzzy(const char *set_name,
 	string_misspell_init(&st);
 
 	list_for_each_entry(table, &cache->list, list) {
-		list_for_each_entry(set, &table->sets, list) {
+		list_for_each_entry(set, &table->cache_set, cache_list) {
 			if (set_is_anonymous(set->flags))
 				continue;
 			if (!strcmp(set->handle.set.name, set_name)) {
@@ -1323,6 +1329,7 @@ struct table *table_alloc(void)
 	init_list_head(&table->chains);
 	init_list_head(&table->cache_chain);
 	init_list_head(&table->sets);
+	init_list_head(&table->cache_set);
 	init_list_head(&table->objs);
 	init_list_head(&table->flowtables);
 	init_list_head(&table->chain_bindings);
@@ -1357,6 +1364,9 @@ void table_free(struct table *table)
 		chain_free(chain);
 	list_for_each_entry_safe(set, nset, &table->sets, list)
 		set_free(set);
+	/* this is implicitly releasing sets in the cache */
+	list_for_each_entry_safe(set, nset, &table->cache_set, cache_list)
+		set_free(set);
 	list_for_each_entry_safe(ft, nft, &table->flowtables, list)
 		flowtable_free(ft);
 	list_for_each_entry_safe(obj, nobj, &table->objs, list)
@@ -1457,7 +1467,7 @@ static void table_print(const struct table *table, struct output_ctx *octx)
 		obj_print(obj, octx);
 		delim = "\n";
 	}
-	list_for_each_entry(set, &table->sets, list) {
+	list_for_each_entry(set, &table->cache_set, cache_list) {
 		if (set_is_anonymous(set->flags))
 			continue;
 		nft_print(octx, "%s", delim);
@@ -1910,7 +1920,7 @@ static int do_list_sets(struct netlink_ctx *ctx, struct cmd *cmd)
 			  family2str(table->handle.family),
 			  table->handle.table.name);
 
-		list_for_each_entry(set, &table->sets, list) {
+		list_for_each_entry(set, &table->cache_set, cache_list) {
 			if (cmd->obj == CMD_OBJ_SETS &&
 			    !set_is_literal(set->flags))
 				continue;
-- 
2.30.2

Re: [RFC] nftables 0.9.8 -stable backports

[Pablo Neira Ayuso]

On Tue, Feb 20, 2024 at 01:27:30PM +0100, Pablo Neira Ayuso wrote:
> Hi Jeremy,
> 
> On Sun, Feb 18, 2024 at 01:56:00PM +0000, Jeremy Sowden wrote:
> > On 2024-02-17, at 20:11:42 +0000, Jeremy Sowden wrote:
> > > Does this look good to you?
> > 
> > Forgot to run the test-suite.  Doing so revealed that this doesn't quite
> > work because `set_alloc` doesn't initialize `s->list`.
> 
> I'd suggest instead a backport of the patch that fixes the set cache
> for 0.9.8 instead.
> 
> See attached patch, it is partial backport of a upstream patch.
> 
> I have run tests/shell (two tests don't pass, because 5.15 does not
> support multiple statements) and tests/py for that nftables 0.9.8 version.

... multiple statements in set elements)

Re: [RFC] nftables 0.9.8 -stable backports

[Jeremy Sowden]

[-- Attachment #1: Type: text/plain, Size: 13364 bytes --]

On 2024-02-20, at 13:27:30 +0100, Pablo Neira Ayuso wrote:
> Hi Jeremy,
> 
> On Sun, Feb 18, 2024 at 01:56:00PM +0000, Jeremy Sowden wrote:
> > On 2024-02-17, at 20:11:42 +0000, Jeremy Sowden wrote:
> > > Does this look good to you?
> > 
> > Forgot to run the test-suite.  Doing so revealed that this doesn't quite
> > work because `set_alloc` doesn't initialize `s->list`.
> 
> I'd suggest instead a backport of the patch that fixes the set cache
> for 0.9.8 instead.
> 
> See attached patch, it is partial backport of a upstream patch.
> 
> I have run tests/shell (two tests don't pass, because 5.15 does not
> support multiple statements) and tests/py for that nftables 0.9.8 version.

Thanks, Pablo.  Looks good.

J.

> From 92908c439d1e33f10ee96daf63eae50d1dfcbb52 Mon Sep 17 00:00:00 2001
> From: Pablo Neira Ayuso <pablo@netfilter.org>
> Date: Tue, 20 Feb 2024 10:26:22 +0100
> Subject: [PATCH] partial backport of df48e56e987f ("cache: add hashtable cache
>  for sets")
> 
> This patch splits table->sets in two:
> 
> - Sets that reside in the cache are stored in the new
>   tables->cache_set and tables->cache_set_ht.
> 
> - Set that defined via command line / ruleset file reside in
>   tables->set.
> 
> Sets in the cache (already in the kernel) are not placed in the
> table->sets list.
> 
> By keeping separated lists, sets defined via command line / ruleset file
> can be added to cache.
> ---
>  include/cache.h   |  7 +++++
>  include/netlink.h |  1 -
>  include/rule.h    |  3 ++-
>  src/cache.c       | 69 +++++++++++++++++++++++++++++++++++++++++++++++
>  src/evaluate.c    |  2 +-
>  src/json.c        |  4 +--
>  src/monitor.c     |  2 +-
>  src/netlink.c     | 31 ---------------------
>  src/rule.c        | 34 ++++++++++++++---------
>  9 files changed, 104 insertions(+), 49 deletions(-)
> 
> diff --git a/include/cache.h b/include/cache.h
> index baa2bb29f1e7..d4abe67611bc 100644
> --- a/include/cache.h
> +++ b/include/cache.h
> @@ -59,4 +59,11 @@ void chain_cache_add(struct chain *chain, struct table *table);
>  struct chain *chain_cache_find(const struct table *table,
>  			       const struct handle *handle);
>  
> +struct nftnl_set_list;
> +
> +struct nftnl_set_list *set_cache_dump(struct netlink_ctx *ctx, int *err);
> +int set_cache_init(struct netlink_ctx *ctx, struct table *table,
> +		   struct nftnl_set_list *set_list);
> +void set_cache_add(struct set *set, struct table *table);
> +
>  #endif /* _NFT_CACHE_H_ */
> diff --git a/include/netlink.h b/include/netlink.h
> index cf8aae465324..f93c5322100f 100644
> --- a/include/netlink.h
> +++ b/include/netlink.h
> @@ -139,7 +139,6 @@ extern int netlink_list_tables(struct netlink_ctx *ctx, const struct handle *h);
>  extern struct table *netlink_delinearize_table(struct netlink_ctx *ctx,
>  					       const struct nftnl_table *nlt);
>  
> -extern int netlink_list_sets(struct netlink_ctx *ctx, const struct handle *h);
>  extern struct set *netlink_delinearize_set(struct netlink_ctx *ctx,
>  					   const struct nftnl_set *nls);
>  
> diff --git a/include/rule.h b/include/rule.h
> index f880cfd32bd8..7d1bd75e9d7e 100644
> --- a/include/rule.h
> +++ b/include/rule.h
> @@ -157,6 +157,7 @@ struct table {
>  	struct list_head	*cache_chain_ht;
>  	struct list_head	cache_chain;
>  	struct list_head	chains;
> +	struct list_head	cache_set;
>  	struct list_head	sets;
>  	struct list_head	objs;
>  	struct list_head	flowtables;
> @@ -323,6 +324,7 @@ void rule_stmt_insert_at(struct rule *rule, struct stmt *nstmt,
>   */
>  struct set {
>  	struct list_head	list;
> +	struct list_head	cache_list;
>  	struct handle		handle;
>  	struct location		location;
>  	unsigned int		refcnt;
> @@ -351,7 +353,6 @@ extern struct set *set_alloc(const struct location *loc);
>  extern struct set *set_get(struct set *set);
>  extern void set_free(struct set *set);
>  extern struct set *set_clone(const struct set *set);
> -extern void set_add_hash(struct set *set, struct table *table);
>  extern struct set *set_lookup(const struct table *table, const char *name);
>  extern struct set *set_lookup_global(uint32_t family, const char *table,
>  				     const char *name, struct nft_cache *cache);
> diff --git a/src/cache.c b/src/cache.c
> index 32e6eea4ac5c..600e6f02d22e 100644
> --- a/src/cache.c
> +++ b/src/cache.c
> @@ -15,6 +15,8 @@
>  #include <netlink.h>
>  #include <mnl.h>
>  #include <libnftnl/chain.h>
> +#include <include/linux/netfilter.h>
> +#include <linux/netfilter/nf_tables.h>
>  
>  static unsigned int evaluate_cache_add(struct cmd *cmd, unsigned int flags)
>  {
> @@ -256,3 +258,70 @@ struct chain *chain_cache_find(const struct table *table,
>  
>  	return NULL;
>  }
> +
> +struct set_cache_dump_ctx {
> +	struct netlink_ctx	*nlctx;
> +	struct table		*table;
> +};
> +
> +static int set_cache_cb(struct nftnl_set *nls, void *arg)
> +{
> +	struct set_cache_dump_ctx *ctx = arg;
> +	const char *set_table;
> +	uint32_t set_family;
> +	struct set *set;
> +
> +	set_table = nftnl_set_get_str(nls, NFTNL_SET_TABLE);
> +	set_family = nftnl_set_get_u32(nls, NFTNL_SET_FAMILY);
> +
> +	if (set_family != ctx->table->handle.family ||
> +	    strcmp(set_table, ctx->table->handle.table.name))
> +		return 0;
> +
> +	set = netlink_delinearize_set(ctx->nlctx, nls);
> +	if (!set)
> +		return -1;
> +
> +	list_add_tail(&set->cache_list, &ctx->table->cache_set);
> +
> +	nftnl_set_list_del(nls);
> +	nftnl_set_free(nls);
> +	return 0;
> +}
> +
> +int set_cache_init(struct netlink_ctx *ctx, struct table *table,
> +		   struct nftnl_set_list *set_list)
> +{
> +	struct set_cache_dump_ctx dump_ctx = {
> +		.nlctx	= ctx,
> +		.table	= table,
> +	};
> +
> +	nftnl_set_list_foreach(set_list, set_cache_cb, &dump_ctx);
> +
> +	return 0;
> +}
> +
> +void set_cache_add(struct set *set, struct table *table)
> +{
> +	list_add_tail(&set->cache_list, &table->cache_set);
> +}
> +
> +struct nftnl_set_list *set_cache_dump(struct netlink_ctx *ctx, int *err)
> +{
> +	struct nftnl_set_list *set_list;
> +	const char *table = NULL;
> +	int family = AF_UNSPEC;
> +
> +	set_list = mnl_nft_set_dump(ctx, family, table);
> +	if (!set_list) {
> +		if (errno == EINTR) {
> +			*err = -1;
> +			return NULL;
> +		}
> +		*err = 0;
> +		return NULL;
> +	}
> +
> +	return set_list;
> +}
> diff --git a/src/evaluate.c b/src/evaluate.c
> index 232ae39020cc..7378174ceb97 100644
> --- a/src/evaluate.c
> +++ b/src/evaluate.c
> @@ -3760,7 +3760,7 @@ static int set_evaluate(struct eval_ctx *ctx, struct set *set)
>  	ctx->set = NULL;
>  
>  	if (set_lookup(table, set->handle.set.name) == NULL)
> -		set_add_hash(set_get(set), table);
> +		set_cache_add(set_get(set), table);
>  
>  	return 0;
>  }
> diff --git a/src/json.c b/src/json.c
> index 737e73b08b5a..13079230af22 100644
> --- a/src/json.c
> +++ b/src/json.c
> @@ -1584,7 +1584,7 @@ static json_t *table_print_json_full(struct netlink_ctx *ctx,
>  		tmp = obj_print_json(obj);
>  		json_array_append_new(root, tmp);
>  	}
> -	list_for_each_entry(set, &table->sets, list) {
> +	list_for_each_entry(set, &table->cache_set, cache_list) {
>  		if (set_is_anonymous(set->flags))
>  			continue;
>  		tmp = set_print_json(&ctx->nft->output, set);
> @@ -1717,7 +1717,7 @@ static json_t *do_list_sets_json(struct netlink_ctx *ctx, struct cmd *cmd)
>  		    cmd->handle.family != table->handle.family)
>  			continue;
>  
> -		list_for_each_entry(set, &table->sets, list) {
> +		list_for_each_entry(set, &table->cache_set, cache_list) {
>  			if (cmd->obj == CMD_OBJ_SETS &&
>  			    !set_is_literal(set->flags))
>  				continue;
> diff --git a/src/monitor.c b/src/monitor.c
> index af2998d4272b..946621e28ec0 100644
> --- a/src/monitor.c
> +++ b/src/monitor.c
> @@ -632,7 +632,7 @@ static void netlink_events_cache_addset(struct netlink_mon_handler *monh,
>  		goto out;
>  	}
>  
> -	set_add_hash(s, t);
> +	set_cache_add(s, t);
>  out:
>  	nftnl_set_free(nls);
>  }
> diff --git a/src/netlink.c b/src/netlink.c
> index ec2dad29ace1..dcac0ab8f871 100644
> --- a/src/netlink.c
> +++ b/src/netlink.c
> @@ -970,37 +970,6 @@ struct set *netlink_delinearize_set(struct netlink_ctx *ctx,
>  	return set;
>  }
>  
> -static int list_set_cb(struct nftnl_set *nls, void *arg)
> -{
> -	struct netlink_ctx *ctx = arg;
> -	struct set *set;
> -
> -	set = netlink_delinearize_set(ctx, nls);
> -	if (set == NULL)
> -		return -1;
> -	list_add_tail(&set->list, &ctx->list);
> -	return 0;
> -}
> -
> -int netlink_list_sets(struct netlink_ctx *ctx, const struct handle *h)
> -{
> -	struct nftnl_set_list *set_cache;
> -	int err;
> -
> -	set_cache = mnl_nft_set_dump(ctx, h->family, h->table.name);
> -	if (set_cache == NULL) {
> -		if (errno == EINTR)
> -			return -1;
> -
> -		return 0;
> -	}
> -
> -	ctx->data = h;
> -	err = nftnl_set_list_foreach(set_cache, list_set_cb, ctx);
> -	nftnl_set_list_free(set_cache);
> -	return err;
> -}
> -
>  void alloc_setelem_cache(const struct expr *set, struct nftnl_set *nls)
>  {
>  	struct nftnl_set_elem *nlse;
> diff --git a/src/rule.c b/src/rule.c
> index 9c74b89c1fb1..4b2682455253 100644
> --- a/src/rule.c
> +++ b/src/rule.c
> @@ -153,6 +153,7 @@ static int cache_init_tables(struct netlink_ctx *ctx, struct handle *h,
>  static int cache_init_objects(struct netlink_ctx *ctx, unsigned int flags)
>  {
>  	struct nftnl_chain_list *chain_list = NULL;
> +	struct nftnl_set_list *set_list = NULL;
>  	struct rule *rule, *nrule;
>  	struct table *table;
>  	struct chain *chain;
> @@ -164,16 +165,22 @@ static int cache_init_objects(struct netlink_ctx *ctx, unsigned int flags)
>  		if (!chain_list)
>  			return ret;
>  	}
> +	if (flags & NFT_CACHE_SET_BIT) {
> +		set_list = set_cache_dump(ctx, &ret);
> +		if (!set_list) {
> +			ret = -1;
> +			goto cache_fails;
> +		}
> +	}
>  
>  	list_for_each_entry(table, &ctx->nft->cache.list, list) {
>  		if (flags & NFT_CACHE_SET_BIT) {
> -			ret = netlink_list_sets(ctx, &table->handle);
> -			list_splice_tail_init(&ctx->list, &table->sets);
> +			ret = set_cache_init(ctx, table, set_list);
>  			if (ret < 0)
>  				return -1;
>  		}
>  		if (flags & NFT_CACHE_SETELEM_BIT) {
> -			list_for_each_entry(set, &table->sets, list) {
> +			list_for_each_entry(set, &table->cache_set, cache_list) {
>  				ret = netlink_list_setelems(ctx, &set->handle,
>  							    set);
>  				if (ret < 0)
> @@ -212,6 +219,10 @@ static int cache_init_objects(struct netlink_ctx *ctx, unsigned int flags)
>  		}
>  	}
>  
> +cache_fails:
> +	if (set_list)
> +		nftnl_set_list_free(set_list);
> +
>  	if (flags & NFT_CACHE_CHAIN_BIT)
>  		nftnl_chain_list_free(chain_list);
>  
> @@ -389,16 +400,11 @@ void set_free(struct set *set)
>  	xfree(set);
>  }
>  
> -void set_add_hash(struct set *set, struct table *table)
> -{
> -	list_add_tail(&set->list, &table->sets);
> -}
> -
>  struct set *set_lookup(const struct table *table, const char *name)
>  {
>  	struct set *set;
>  
> -	list_for_each_entry(set, &table->sets, list) {
> +	list_for_each_entry(set, &table->cache_set, cache_list) {
>  		if (!strcmp(set->handle.set.name, name))
>  			return set;
>  	}
> @@ -416,7 +422,7 @@ struct set *set_lookup_fuzzy(const char *set_name,
>  	string_misspell_init(&st);
>  
>  	list_for_each_entry(table, &cache->list, list) {
> -		list_for_each_entry(set, &table->sets, list) {
> +		list_for_each_entry(set, &table->cache_set, cache_list) {
>  			if (set_is_anonymous(set->flags))
>  				continue;
>  			if (!strcmp(set->handle.set.name, set_name)) {
> @@ -1323,6 +1329,7 @@ struct table *table_alloc(void)
>  	init_list_head(&table->chains);
>  	init_list_head(&table->cache_chain);
>  	init_list_head(&table->sets);
> +	init_list_head(&table->cache_set);
>  	init_list_head(&table->objs);
>  	init_list_head(&table->flowtables);
>  	init_list_head(&table->chain_bindings);
> @@ -1357,6 +1364,9 @@ void table_free(struct table *table)
>  		chain_free(chain);
>  	list_for_each_entry_safe(set, nset, &table->sets, list)
>  		set_free(set);
> +	/* this is implicitly releasing sets in the cache */
> +	list_for_each_entry_safe(set, nset, &table->cache_set, cache_list)
> +		set_free(set);
>  	list_for_each_entry_safe(ft, nft, &table->flowtables, list)
>  		flowtable_free(ft);
>  	list_for_each_entry_safe(obj, nobj, &table->objs, list)
> @@ -1457,7 +1467,7 @@ static void table_print(const struct table *table, struct output_ctx *octx)
>  		obj_print(obj, octx);
>  		delim = "\n";
>  	}
> -	list_for_each_entry(set, &table->sets, list) {
> +	list_for_each_entry(set, &table->cache_set, cache_list) {
>  		if (set_is_anonymous(set->flags))
>  			continue;
>  		nft_print(octx, "%s", delim);
> @@ -1910,7 +1920,7 @@ static int do_list_sets(struct netlink_ctx *ctx, struct cmd *cmd)
>  			  family2str(table->handle.family),
>  			  table->handle.table.name);
>  
> -		list_for_each_entry(set, &table->sets, list) {
> +		list_for_each_entry(set, &table->cache_set, cache_list) {
>  			if (cmd->obj == CMD_OBJ_SETS &&
>  			    !set_is_literal(set->flags))
>  				continue;
> -- 
> 2.30.2
> 


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]