[iptables PATCH v2 00/18] iptables: introduce cache evaluation phase

[iptables PATCH v2 00/18] iptables: introduce cache evaluation phase

[Phil Sutter]

Hi Pablo,

As promised, here's a revised version of your cache rework series from
January. It restores performance according to my tests (which are yet to
be published somewhere) and passes the testsuites.

Patches 1-3 are code simplifications which are not directly related
to the actual caching changes.

Patch 4 enhances set fetching by adding support for passing a table name
to kernel but no set name. Not a big deal for iptables but it aligns the
code with chain fetching.

Patch 5 is a respin of a patch submitted a few weeks ago, namely adding
implicit commits to arptables- and ebtables-restore tools which don't
support explicit COMMIT lines in input. Big benefit here is that we
won't see consecutive commands for different tables anymore, so
selective cache fetching doesn't have to deal with too many odd cases.

Patches 6-10 are yours, I rebased and revisited them. Any changes are
recorded in per-patch changelogs.

Patch 11 simplifies fetch_set_cache() and fetch_rule_cache() functions
as they no longer have to be aware of previous invocations.

Patch 12 improves NFT_CL_FAKE integration considerably, easily possible
now that there is nft_cache_level_set() function.

Patch 13 introduces an embedded struct into struct nft_handle which
holds cache requirements collected from parsed commands. At first there
is just the desired cache level, further patches extend it.

Patches 14-16 re-establish per table/chain cache.

Patch 17 reduces cache requirements for flush command by making
nft_xt_builtin_init() cache-aware.

Patch 18 fixes the segfault reported in nfbz#1407.

Pablo Neira Ayuso (5):
  nft: split parsing from netlink commands
  nft: calculate cache requirements from list of commands
  nft: restore among support
  nft: remove cache build calls
  nft: missing nft_fini() call in bridge family

Phil Sutter (13):
  ebtables-restore: Drop custom table flush routine
  nft: cache: Eliminate init_chain_cache()
  nft: cache: Init per table set list along with chain list
  nft: cache: Fetch sets per table
  ebtables-restore: Table line to trigger implicit commit
  nft: cache: Simplify rule and set fetchers
  nft: cache: Improve fake cache integration
  nft: cache: Introduce struct nft_cache_req
  nft-cache: Fetch cache per table
  nft-cache: Introduce __fetch_chain_cache()
  nft: cache: Fetch cache for specific chains
  nft: cache: Optimize caching for flush command
  nft: Fix for '-F' in iptables dumps

 iptables/Makefile.am                          |   2 +-
 iptables/nft-arp.c                            |   5 +-
 iptables/nft-bridge.c                         |  18 +-
 iptables/nft-cache.c                          | 318 +++++++-------
 iptables/nft-cache.h                          |   6 +-
 iptables/nft-cmd.c                            | 387 ++++++++++++++++++
 iptables/nft-cmd.h                            |  79 ++++
 iptables/nft-shared.c                         |   6 +-
 iptables/nft-shared.h                         |   4 +-
 iptables/nft.c                                | 369 ++++++++++++-----
 iptables/nft.h                                |  62 ++-
 .../testcases/ip6tables/0004-return-codes_0   |   1 +
 .../testcases/iptables/0004-return-codes_0    |   6 +
 .../testcases/nft-only/0006-policy-override_0 |  29 ++
 iptables/xtables-arp.c                        |  26 +-
 iptables/xtables-eb-standalone.c              |   2 +
 iptables/xtables-eb.c                         |  26 +-
 iptables/xtables-restore.c                    | 126 +-----
 iptables/xtables-save.c                       |   3 +
 iptables/xtables.c                            |  57 ++-
 20 files changed, 1100 insertions(+), 432 deletions(-)
 create mode 100644 iptables/nft-cmd.c
 create mode 100644 iptables/nft-cmd.h
 create mode 100755 iptables/tests/shell/testcases/nft-only/0006-policy-override_0

-- 
2.25.1

[iptables PATCH v2 01/18] ebtables-restore: Drop custom table flush routine

[Phil Sutter]

At least since flushing xtables-restore doesn't fetch chains from kernel
anymore, problems with pending policy rule delete jobs can't happen
anymore.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft.c             | 21 ---------------------
 iptables/nft.h             |  1 -
 iptables/xtables-restore.c |  9 +--------
 3 files changed, 1 insertion(+), 30 deletions(-)

diff --git a/iptables/nft.c b/iptables/nft.c
index cf3ab9fe239aa..468c703a1d09f 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -2985,27 +2985,6 @@ int nft_abort(struct nft_handle *h)
 	return nft_action(h, NFT_COMPAT_ABORT);
 }
 
-int nft_abort_policy_rule(struct nft_handle *h, const char *table)
-{
-	struct obj_update *n, *tmp;
-
-	list_for_each_entry_safe(n, tmp, &h->obj_list, head) {
-		if (n->type != NFT_COMPAT_RULE_APPEND &&
-		    n->type != NFT_COMPAT_RULE_DELETE)
-			continue;
-
-		if (strcmp(table,
-			   nftnl_rule_get_str(n->rule, NFTNL_RULE_TABLE)))
-			continue;
-
-		if (!nft_rule_is_policy_rule(n->rule))
-			continue;
-
-		batch_obj_del(h, n);
-	}
-	return 0;
-}
-
 int nft_compatible_revision(const char *name, uint8_t rev, int opt)
 {
 	struct mnl_socket *nl;
diff --git a/iptables/nft.h b/iptables/nft.h
index 2094b01455194..ebb4044d1a453 100644
--- a/iptables/nft.h
+++ b/iptables/nft.h
@@ -160,7 +160,6 @@ uint32_t nft_invflags2cmp(uint32_t invflags, uint32_t flag);
 int nft_commit(struct nft_handle *h);
 int nft_bridge_commit(struct nft_handle *h);
 int nft_abort(struct nft_handle *h);
-int nft_abort_policy_rule(struct nft_handle *h, const char *table);
 
 /*
  * revision compatibility.
diff --git a/iptables/xtables-restore.c b/iptables/xtables-restore.c
index c472ac9bf651b..fe7148c9fcb3f 100644
--- a/iptables/xtables-restore.c
+++ b/iptables/xtables-restore.c
@@ -484,17 +484,10 @@ int xtables_ip6_restore_main(int argc, char *argv[])
 				    argc, argv);
 }
 
-static int ebt_table_flush(struct nft_handle *h, const char *table)
-{
-	/* drop any pending policy rule add/removal jobs */
-	nft_abort_policy_rule(h, table);
-	return nft_table_flush(h, table);
-}
-
 static const struct nft_xt_restore_cb ebt_restore_cb = {
 	.commit		= nft_bridge_commit,
 	.table_new	= nft_table_new,
-	.table_flush	= ebt_table_flush,
+	.table_flush	= nft_table_flush,
 	.do_command	= do_commandeb,
 	.chain_set	= nft_chain_set,
 	.chain_restore  = nft_chain_restore,
-- 
2.25.1

Re: [iptables PATCH v2 01/18] ebtables-restore: Drop custom table flush routine

[Florian Westphal]

Phil Sutter <phil@nwl.cc> wrote:
> At least since flushing xtables-restore doesn't fetch chains from kernel
> anymore, problems with pending policy rule delete jobs can't happen
> anymore.

Acked-by: Florian Westphal <fw@strlen.de>

[iptables PATCH v2 02/18] nft: cache: Eliminate init_chain_cache()

[Phil Sutter]

The function is always called immediately after fetch_table_cache(), so
merge it into the latter.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft-cache.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/iptables/nft-cache.c b/iptables/nft-cache.c
index a0c76705c848e..369692fe44fc7 100644
--- a/iptables/nft-cache.c
+++ b/iptables/nft-cache.c
@@ -86,7 +86,7 @@ static int fetch_table_cache(struct nft_handle *h)
 	char buf[16536];
 	struct nlmsghdr *nlh;
 	struct nftnl_table_list *list;
-	int ret;
+	int i, ret;
 
 	if (h->cache->tables)
 		return 0;
@@ -104,13 +104,6 @@ static int fetch_table_cache(struct nft_handle *h)
 
 	h->cache->tables = list;
 
-	return 1;
-}
-
-static int init_chain_cache(struct nft_handle *h)
-{
-	int i;
-
 	for (i = 0; i < NFT_TABLE_MAX; i++) {
 		enum nft_table_type type = h->tables[i].type;
 
@@ -119,9 +112,10 @@ static int init_chain_cache(struct nft_handle *h)
 
 		h->cache->table[type].chains = nftnl_chain_list_alloc();
 		if (!h->cache->table[type].chains)
-			return -1;
+			return 0;
 	}
-	return 0;
+
+	return 1;
 }
 
 struct nftnl_chain_list_cb_data {
@@ -458,7 +452,6 @@ __nft_build_cache(struct nft_handle *h, enum nft_cache_level level,
 	switch (h->cache_level) {
 	case NFT_CL_NONE:
 		fetch_table_cache(h);
-		init_chain_cache(h);
 		if (level == NFT_CL_TABLES)
 			break;
 		/* fall through */
@@ -505,7 +498,6 @@ void nft_build_cache(struct nft_handle *h, struct nftnl_chain *c)
 void nft_fake_cache(struct nft_handle *h)
 {
 	fetch_table_cache(h);
-	init_chain_cache(h);
 
 	h->cache_level = NFT_CL_FAKE;
 	mnl_genid_get(h, &h->nft_genid);
-- 
2.25.1

Re: [iptables PATCH v2 02/18] nft: cache: Eliminate init_chain_cache()

[Florian Westphal]

Phil Sutter <phil@nwl.cc> wrote:
> The function is always called immediately after fetch_table_cache(), so
> merge it into the latter.

Yes, function is also small enough, so:

Acked-by: Florian Westphal <fw@strlen.de>

[iptables PATCH v2 03/18] nft: cache: Init per table set list along with chain list

[Phil Sutter]

This simplifies code a bit and also aligns set and chain lists handling
in cache.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft-cache.c | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/iptables/nft-cache.c b/iptables/nft-cache.c
index 369692fe44fc7..e042bd83bebf5 100644
--- a/iptables/nft-cache.c
+++ b/iptables/nft-cache.c
@@ -113,6 +113,10 @@ static int fetch_table_cache(struct nft_handle *h)
 		h->cache->table[type].chains = nftnl_chain_list_alloc();
 		if (!h->cache->table[type].chains)
 			return 0;
+
+		h->cache->table[type].sets = nftnl_set_list_alloc();
+		if (!h->cache->table[type].sets)
+			return 0;
 	}
 
 	return 1;
@@ -254,21 +258,6 @@ static int fetch_set_cache(struct nft_handle *h,
 	char buf[16536];
 	int i, ret;
 
-	if (!t) {
-		for (i = 0; i < NFT_TABLE_MAX; i++) {
-			enum nft_table_type type = h->tables[i].type;
-
-			if (!h->tables[i].name)
-				continue;
-
-			h->cache->table[type].sets = nftnl_set_list_alloc();
-			if (!h->cache->table[type].sets)
-				return -1;
-		}
-	} else if (!h->cache->table[t->type].sets) {
-		h->cache->table[t->type].sets = nftnl_set_list_alloc();
-	}
-
 	if (t && set) {
 		struct nftnl_set *s = nftnl_set_alloc();
 
-- 
2.25.1

Re: [iptables PATCH v2 03/18] nft: cache: Init per table set list along with chain list

[Florian Westphal]

Phil Sutter <phil@nwl.cc> wrote:
> This simplifies code a bit and also aligns set and chain lists handling
> in cache.

Acked-by: Florian Westphal <fw@strlen.de>

[iptables PATCH v2 04/18] nft: cache: Fetch sets per table

[Phil Sutter]

Kernel accepts a table name when dumping sets, so make use of that in
case a table was passed to fetch_set_cache() but no set name.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft-cache.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/iptables/nft-cache.c b/iptables/nft-cache.c
index e042bd83bebf5..51b371c51c3f4 100644
--- a/iptables/nft-cache.c
+++ b/iptables/nft-cache.c
@@ -254,25 +254,31 @@ static int fetch_set_cache(struct nft_handle *h,
 		.h = h,
 		.t = t,
 	};
+	uint16_t flags = NLM_F_DUMP;
+	struct nftnl_set *s = NULL;
 	struct nlmsghdr *nlh;
 	char buf[16536];
 	int i, ret;
 
-	if (t && set) {
-		struct nftnl_set *s = nftnl_set_alloc();
-
+	if (t) {
+		s = nftnl_set_alloc();
 		if (!s)
 			return -1;
 
-		nlh = nftnl_set_nlmsg_build_hdr(buf, NFT_MSG_GETSET, h->family,
-						NLM_F_ACK, h->seq);
 		nftnl_set_set_str(s, NFTNL_SET_TABLE, t->name);
-		nftnl_set_set_str(s, NFTNL_SET_NAME, set);
+
+		if (set) {
+			nftnl_set_set_str(s, NFTNL_SET_NAME, set);
+			flags = NLM_F_ACK;
+		}
+	}
+
+	nlh = nftnl_set_nlmsg_build_hdr(buf, NFT_MSG_GETSET,
+					h->family, flags, h->seq);
+
+	if (s) {
 		nftnl_set_nlmsg_build_payload(nlh, s);
 		nftnl_set_free(s);
-	} else {
-		nlh = nftnl_set_nlmsg_build_hdr(buf, NFT_MSG_GETSET, h->family,
-						NLM_F_DUMP, h->seq);
 	}
 
 	ret = mnl_talk(h, nlh, nftnl_set_list_cb, &d);
@@ -282,8 +288,6 @@ static int fetch_set_cache(struct nft_handle *h,
 	}
 
 	if (t && set) {
-		struct nftnl_set *s;
-
 		s = nftnl_set_list_lookup_byname(h->cache->table[t->type].sets,
 						 set);
 		set_fetch_elem_cb(s, h);
-- 
2.25.1

Re: [iptables PATCH v2 04/18] nft: cache: Fetch sets per table

[Florian Westphal]

Phil Sutter <phil@nwl.cc> wrote:
> Kernel accepts a table name when dumping sets, so make use of that in
> case a table was passed to fetch_set_cache() but no set name.

Acked-by: Florian Westphal <fw@strlen.de>

[iptables PATCH v2 05/18] ebtables-restore: Table line to trigger implicit commit

[Phil Sutter]

Cache code is suited for holding multiple tables' data at once. The only
users of that are xtables-save and ebtables-restore with its support for
multiple tables and lack of explicit COMMIT lines.

Remove the second user by introducing implicit commits upon table line
parsing. This would allow to make cache single table only, but then
xtables-save would fetch cache multiple times (once for each table) and
therefore lose atomicity with regards to the acquired kernel ruleset
image.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v1:
- Drop the custom table_new callback, committing from there is too late
  since table_flush happens before. Instead explicitly call commit()
  from parser.
---
 iptables/xtables-restore.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/iptables/xtables-restore.c b/iptables/xtables-restore.c
index fe7148c9fcb3f..53a0d3738eb74 100644
--- a/iptables/xtables-restore.c
+++ b/iptables/xtables-restore.c
@@ -126,6 +126,10 @@ static void xtables_restore_parse_line(struct nft_handle *h,
 		if (p->tablename && (strcmp(p->tablename, table) != 0))
 			return;
 
+		/* implicit commit if no explicit COMMIT supported */
+		if (!p->commit)
+			cb->commit(h);
+
 		if (h->noflush == 0) {
 			DEBUGP("Cleaning all chains of table '%s'\n", table);
 			if (cb->table_flush)
-- 
2.25.1

[iptables PATCH v2 06/18] nft: split parsing from netlink commands

[Phil Sutter]

From: Pablo Neira Ayuso <pablo@netfilter.org>

This patch updates the parser to generate a list of command objects.
This list of commands is then transformed to a list of netlink jobs.
This new command object stores the rule using the nftnl representation
via nft_rule_new().

To reduce the number of updates in this patch, the nft_*_rule_find()
functions have been updated to restore the native representation to
skip the update of the rule comparison code.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v1:
- Store rules' target chain name and check existence after fetching
  kernel ruleset to maintain compatibility with legacy iptables in error
  case. Consequently fold previous patch "nft: do not check for existing
  chain from parser" into this one.
- Drop unused struct iptables_command_state field from struct nft_cmd.
---
 iptables/Makefile.am                          |   2 +-
 iptables/nft-arp.c                            |   5 +-
 iptables/nft-bridge.c                         |   5 +-
 iptables/nft-cmd.c                            | 319 ++++++++++++++++++
 iptables/nft-cmd.h                            |  78 +++++
 iptables/nft-shared.c                         |   6 +-
 iptables/nft-shared.h                         |   4 +-
 iptables/nft.c                                | 258 ++++++++++----
 iptables/nft.h                                |  39 ++-
 .../testcases/ip6tables/0004-return-codes_0   |   1 +
 .../testcases/iptables/0004-return-codes_0    |   6 +
 iptables/xtables-arp.c                        |  26 +-
 iptables/xtables-eb.c                         |  26 +-
 iptables/xtables-restore.c                    |  26 +-
 iptables/xtables.c                            |  57 ++--
 15 files changed, 715 insertions(+), 143 deletions(-)
 create mode 100644 iptables/nft-cmd.c
 create mode 100644 iptables/nft-cmd.h

diff --git a/iptables/Makefile.am b/iptables/Makefile.am
index 71b1b1d48d4b6..dc66b3cc09c08 100644
--- a/iptables/Makefile.am
+++ b/iptables/Makefile.am
@@ -38,7 +38,7 @@ xtables_nft_multi_SOURCES += xtables-save.c xtables-restore.c \
 				nft-shared.c nft-ipv4.c nft-ipv6.c nft-arp.c \
 				xtables-monitor.c nft-cache.c \
 				xtables-arp-standalone.c xtables-arp.c \
-				nft-bridge.c \
+				nft-bridge.c nft-cmd.c \
 				xtables-eb-standalone.c xtables-eb.c \
 				xtables-eb-translate.c \
 				xtables-translate.c
diff --git a/iptables/nft-arp.c b/iptables/nft-arp.c
index d4a86610ec217..748784bc49048 100644
--- a/iptables/nft-arp.c
+++ b/iptables/nft-arp.c
@@ -634,14 +634,15 @@ static bool nft_arp_is_same(const void *data_a,
 }
 
 static bool nft_arp_rule_find(struct nft_handle *h, struct nftnl_rule *r,
-			      void *data)
+			      struct nftnl_rule *rule)
 {
-	const struct iptables_command_state *cs = data;
+	struct iptables_command_state _cs = {}, *cs = &_cs;
 	struct iptables_command_state this = {};
 	bool ret = false;
 
 	/* Delete by matching rule case */
 	nft_rule_to_iptables_command_state(h, r, &this);
+	nft_rule_to_iptables_command_state(h, rule, cs);
 
 	if (!nft_arp_is_same(&cs->arp, &this.arp))
 		goto out;
diff --git a/iptables/nft-bridge.c b/iptables/nft-bridge.c
index 3f85cbbf5e4cf..a5aaa3f87187e 100644
--- a/iptables/nft-bridge.c
+++ b/iptables/nft-bridge.c
@@ -748,13 +748,14 @@ static bool nft_bridge_is_same(const void *data_a, const void *data_b)
 }
 
 static bool nft_bridge_rule_find(struct nft_handle *h, struct nftnl_rule *r,
-				 void *data)
+				 struct nftnl_rule *rule)
 {
-	struct iptables_command_state *cs = data;
+	struct iptables_command_state _cs = {}, *cs = &_cs;
 	struct iptables_command_state this = {};
 	bool ret = false;
 
 	nft_rule_to_ebtables_command_state(h, r, &this);
+	nft_rule_to_ebtables_command_state(h, rule, cs);
 
 	DEBUGP("comparing with... ");
 
diff --git a/iptables/nft-cmd.c b/iptables/nft-cmd.c
new file mode 100644
index 0000000000000..1d9834c0f94d7
--- /dev/null
+++ b/iptables/nft-cmd.c
@@ -0,0 +1,319 @@
+/*
+ * (C) 2012 by Pablo Neira Ayuso <pablo@netfilter.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published
+ * by the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This code has been sponsored by Sophos Astaro <http://www.sophos.com>
+ */
+
+#include <stdlib.h>
+#include <string.h>
+#include "nft.h"
+#include "nft-cmd.h"
+
+struct nft_cmd *nft_cmd_new(struct nft_handle *h, int command,
+			    const char *table, const char *chain,
+			    struct iptables_command_state *state,
+			    int rulenum, bool verbose)
+{
+	struct nftnl_rule *rule;
+	struct nft_cmd *cmd;
+
+	cmd = calloc(1, sizeof(struct nft_cmd));
+	if (!cmd)
+		return NULL;
+
+	cmd->command = command;
+	cmd->table = strdup(table);
+	if (chain)
+		cmd->chain = strdup(chain);
+	cmd->rulenum = rulenum;
+	cmd->verbose = verbose;
+
+	if (state) {
+		rule = nft_rule_new(h, chain, table, state);
+		if (!rule)
+			return NULL;
+
+		cmd->obj.rule = rule;
+
+		if (!state->target && strlen(state->jumpto) > 0)
+			cmd->jumpto = strdup(state->jumpto);
+	}
+
+	list_add_tail(&cmd->head, &h->cmd_list);
+
+	return cmd;
+}
+
+void nft_cmd_free(struct nft_cmd *cmd)
+{
+	free((void *)cmd->table);
+	free((void *)cmd->chain);
+	free((void *)cmd->policy);
+	free((void *)cmd->rename);
+	free((void *)cmd->jumpto);
+
+	/* cmd->obj.rule not released here. */
+
+	list_del(&cmd->head);
+	free(cmd);
+}
+
+int nft_cmd_rule_append(struct nft_handle *h, const char *chain,
+			const char *table, struct iptables_command_state *state,
+			void *ref, bool verbose)
+{
+	struct nft_cmd *cmd;
+
+	cmd = nft_cmd_new(h, NFT_COMPAT_RULE_APPEND, table, chain, state, -1,
+			  verbose);
+	if (!cmd)
+		return 0;
+
+	return 1;
+}
+
+int nft_cmd_rule_insert(struct nft_handle *h, const char *chain,
+			const char *table, struct iptables_command_state *state,
+			int rulenum, bool verbose)
+{
+	struct nft_cmd *cmd;
+
+	cmd = nft_cmd_new(h, NFT_COMPAT_RULE_INSERT, table, chain, state,
+			  rulenum, verbose);
+	if (!cmd)
+		return 0;
+
+	return 1;
+}
+
+int nft_cmd_rule_delete(struct nft_handle *h, const char *chain,
+			const char *table, struct iptables_command_state *state,
+			bool verbose)
+{
+	struct nft_cmd *cmd;
+
+	cmd = nft_cmd_new(h, NFT_COMPAT_RULE_DELETE, table, chain, state,
+			  -1, verbose);
+	if (!cmd)
+		return 0;
+
+	return 1;
+}
+
+int nft_cmd_rule_delete_num(struct nft_handle *h, const char *chain,
+			    const char *table, int rulenum, bool verbose)
+{
+	struct nft_cmd *cmd;
+
+	cmd = nft_cmd_new(h, NFT_COMPAT_RULE_DELETE, table, chain, NULL,
+			  rulenum, verbose);
+	if (!cmd)
+		return 0;
+
+	return 1;
+}
+
+int nft_cmd_rule_flush(struct nft_handle *h, const char *chain,
+		       const char *table, bool verbose)
+{
+	struct nft_cmd *cmd;
+
+	cmd = nft_cmd_new(h, NFT_COMPAT_RULE_FLUSH, table, chain, NULL, -1,
+			  verbose);
+	if (!cmd)
+		return 0;
+
+	return 1;
+}
+
+int nft_cmd_chain_zero_counters(struct nft_handle *h, const char *chain,
+				const char *table, bool verbose)
+{
+	struct nft_cmd *cmd;
+
+	cmd = nft_cmd_new(h, NFT_COMPAT_CHAIN_ZERO, table, chain, NULL, -1,
+			  verbose);
+	if (!cmd)
+		return 0;
+
+	return 1;
+}
+
+int nft_cmd_chain_user_add(struct nft_handle *h, const char *chain,
+			   const char *table)
+{
+	struct nft_cmd *cmd;
+
+	cmd = nft_cmd_new(h, NFT_COMPAT_CHAIN_USER_ADD, table, chain, NULL, -1,
+			  false);
+	if (!cmd)
+		return 0;
+
+	return 1;
+}
+
+int nft_cmd_chain_user_del(struct nft_handle *h, const char *chain,
+			   const char *table, bool verbose)
+{
+	struct nft_cmd *cmd;
+
+	cmd = nft_cmd_new(h, NFT_COMPAT_CHAIN_USER_DEL, table, chain, NULL, -1,
+			  verbose);
+	if (!cmd)
+		return 0;
+
+	return 1;
+}
+
+int nft_cmd_chain_user_rename(struct nft_handle *h,const char *chain,
+			      const char *table, const char *newname)
+{
+	struct nft_cmd *cmd;
+
+	cmd = nft_cmd_new(h, NFT_COMPAT_CHAIN_RENAME, table, chain, NULL, -1,
+			  false);
+	if (!cmd)
+		return 0;
+
+	cmd->rename = strdup(newname);
+
+	return 1;
+}
+
+int nft_cmd_rule_list(struct nft_handle *h, const char *chain,
+		      const char *table, int rulenum, unsigned int format)
+{
+	struct nft_cmd *cmd;
+
+	cmd = nft_cmd_new(h, NFT_COMPAT_RULE_LIST, table, chain, NULL, rulenum,
+			  false);
+	if (!cmd)
+		return 0;
+
+	cmd->format = format;
+
+	return 1;
+}
+
+int nft_cmd_rule_replace(struct nft_handle *h, const char *chain,
+			 const char *table, void *data, int rulenum,
+			 bool verbose)
+{
+	struct nft_cmd *cmd;
+
+	cmd = nft_cmd_new(h, NFT_COMPAT_RULE_REPLACE, table, chain, data,
+			  rulenum, verbose);
+	if (!cmd)
+		return 0;
+
+	return 1;
+}
+
+int nft_cmd_rule_check(struct nft_handle *h, const char *chain,
+		       const char *table, void *data, bool verbose)
+{
+	struct nft_cmd *cmd;
+
+	cmd = nft_cmd_new(h, NFT_COMPAT_RULE_CHECK, table, chain, data, -1,
+			  verbose);
+	if (!cmd)
+		return 0;
+
+	return 1;
+}
+
+int nft_cmd_chain_set(struct nft_handle *h, const char *table,
+		      const char *chain, const char *policy,
+		      const struct xt_counters *counters)
+{
+	struct nft_cmd *cmd;
+
+	cmd = nft_cmd_new(h, NFT_COMPAT_CHAIN_UPDATE, table, chain, NULL, -1,
+			  false);
+	if (!cmd)
+		return 0;
+
+	cmd->policy = strdup(policy);
+	if (counters)
+		cmd->counters = *counters;
+
+	return 1;
+}
+
+int nft_cmd_table_flush(struct nft_handle *h, const char *table)
+{
+	struct nft_cmd *cmd;
+
+	cmd = nft_cmd_new(h, NFT_COMPAT_TABLE_FLUSH, table, NULL, NULL, -1,
+			  false);
+	if (!cmd)
+		return 0;
+
+	return 1;
+}
+
+int nft_cmd_chain_restore(struct nft_handle *h, const char *chain,
+			  const char *table)
+{
+	struct nft_cmd *cmd;
+
+	cmd = nft_cmd_new(h, NFT_COMPAT_CHAIN_RESTORE, table, chain, NULL, -1,
+			  false);
+	if (!cmd)
+		return 0;
+
+	return 1;
+}
+
+int nft_cmd_rule_zero_counters(struct nft_handle *h, const char *chain,
+			       const char *table, int rulenum)
+{
+	struct nft_cmd *cmd;
+
+	cmd = nft_cmd_new(h, NFT_COMPAT_RULE_ZERO, table, chain, NULL, rulenum,
+			  false);
+	if (!cmd)
+		return 0;
+
+	return 1;
+}
+
+int nft_cmd_rule_list_save(struct nft_handle *h, const char *chain,
+			   const char *table, int rulenum, int counters)
+{
+	struct nft_cmd *cmd;
+
+	cmd = nft_cmd_new(h, NFT_COMPAT_RULE_SAVE, table, chain, NULL, rulenum,
+			  false);
+	if (!cmd)
+		return 0;
+
+	cmd->counters_save = counters;
+
+	return 1;
+}
+
+int ebt_cmd_user_chain_policy(struct nft_handle *h, const char *table,
+                              const char *chain, const char *policy)
+{
+	struct nft_cmd *cmd;
+
+	cmd = nft_cmd_new(h, NFT_COMPAT_BRIDGE_USER_CHAIN_UPDATE, table, chain,
+			  NULL, -1, false);
+	if (!cmd)
+		return 0;
+
+	cmd->policy = strdup(policy);
+
+	return 1;
+}
+
+void nft_cmd_table_new(struct nft_handle *h, const char *table)
+{
+	nft_cmd_new(h, NFT_COMPAT_TABLE_NEW, table, NULL, NULL, -1, false);
+}
diff --git a/iptables/nft-cmd.h b/iptables/nft-cmd.h
new file mode 100644
index 0000000000000..33ee766ae823f
--- /dev/null
+++ b/iptables/nft-cmd.h
@@ -0,0 +1,78 @@
+#ifndef _NFT_CMD_H_
+#define _NFT_CMD_H_
+
+#include <libiptc/linux_list.h>
+#include <stdbool.h>
+#include "nft.h"
+
+struct nftnl_rule;
+
+struct nft_cmd {
+	struct list_head		head;
+	int				command;
+	const char			*table;
+	const char			*chain;
+	const char			*jumpto;
+	int				rulenum;
+	bool				verbose;
+	unsigned int			format;
+	struct {
+		struct nftnl_rule	*rule;
+	} obj;
+	const char			*policy;
+	struct xt_counters		counters;
+	const char			*rename;
+	int				counters_save;
+};
+
+struct nft_cmd *nft_cmd_new(struct nft_handle *h, int command,
+			    const char *table, const char *chain,
+			    struct iptables_command_state *state,
+			    int rulenum, bool verbose);
+void nft_cmd_free(struct nft_cmd *cmd);
+
+int nft_cmd_rule_append(struct nft_handle *h, const char *chain,
+			const char *table, struct iptables_command_state *state,
+                        void *ref, bool verbose);
+int nft_cmd_rule_insert(struct nft_handle *h, const char *chain,
+			const char *table, struct iptables_command_state *state,
+			int rulenum, bool verbose);
+int nft_cmd_rule_delete(struct nft_handle *h, const char *chain,
+                        const char *table, struct iptables_command_state *state,
+			bool verbose);
+int nft_cmd_rule_delete_num(struct nft_handle *h, const char *chain,
+			    const char *table, int rulenum, bool verbose);
+int nft_cmd_rule_flush(struct nft_handle *h, const char *chain,
+		       const char *table, bool verbose);
+int nft_cmd_zero_counters(struct nft_handle *h, const char *chain,
+			  const char *table, bool verbose);
+int nft_cmd_chain_user_add(struct nft_handle *h, const char *chain,
+			   const char *table);
+int nft_cmd_chain_user_del(struct nft_handle *h, const char *chain,
+			   const char *table, bool verbose);
+int nft_cmd_chain_zero_counters(struct nft_handle *h, const char *chain,
+				const char *table, bool verbose);
+int nft_cmd_rule_list(struct nft_handle *h, const char *chain,
+		      const char *table, int rulenum, unsigned int format);
+int nft_cmd_rule_check(struct nft_handle *h, const char *chain,
+                       const char *table, void *data, bool verbose);
+int nft_cmd_chain_set(struct nft_handle *h, const char *table,
+		      const char *chain, const char *policy,
+		      const struct xt_counters *counters);
+int nft_cmd_chain_user_rename(struct nft_handle *h,const char *chain,
+			      const char *table, const char *newname);
+int nft_cmd_rule_replace(struct nft_handle *h, const char *chain,
+			 const char *table, void *data, int rulenum,
+			 bool verbose);
+int nft_cmd_table_flush(struct nft_handle *h, const char *table);
+int nft_cmd_chain_restore(struct nft_handle *h, const char *chain,
+			  const char *table);
+int nft_cmd_rule_zero_counters(struct nft_handle *h, const char *chain,
+			       const char *table, int rulenum);
+int nft_cmd_rule_list_save(struct nft_handle *h, const char *chain,
+			   const char *table, int rulenum, int counters);
+int ebt_cmd_user_chain_policy(struct nft_handle *h, const char *table,
+			      const char *chain, const char *policy);
+void nft_cmd_table_new(struct nft_handle *h, const char *table);
+
+#endif /* _NFT_CMD_H_ */
diff --git a/iptables/nft-shared.c b/iptables/nft-shared.c
index 5192e36358b7c..6b425f8525d3a 100644
--- a/iptables/nft-shared.c
+++ b/iptables/nft-shared.c
@@ -989,12 +989,14 @@ void nft_ipv46_parse_target(struct xtables_target *t, void *data)
 	cs->target = t;
 }
 
-bool nft_ipv46_rule_find(struct nft_handle *h, struct nftnl_rule *r, void *data)
+bool nft_ipv46_rule_find(struct nft_handle *h, struct nftnl_rule *r,
+			 struct nftnl_rule *rule)
 {
-	struct iptables_command_state *cs = data, this = {};
+	struct iptables_command_state _cs = {}, this = {}, *cs = &_cs;
 	bool ret = false;
 
 	nft_rule_to_iptables_command_state(h, r, &this);
+	nft_rule_to_iptables_command_state(h, rule, cs);
 
 	DEBUGP("comparing with... ");
 #ifdef DEBUG_DEL
diff --git a/iptables/nft-shared.h b/iptables/nft-shared.h
index bee99a7dd0c93..89e9d0b9be335 100644
--- a/iptables/nft-shared.h
+++ b/iptables/nft-shared.h
@@ -110,7 +110,7 @@ struct nft_family_ops {
 			   struct iptables_command_state *cs);
 	void (*clear_cs)(struct iptables_command_state *cs);
 	bool (*rule_find)(struct nft_handle *h, struct nftnl_rule *r,
-			  void *data);
+			  struct nftnl_rule *rule);
 	int (*xlate)(const void *data, struct xt_xlate *xl);
 };
 
@@ -172,7 +172,7 @@ struct nft_family_ops *nft_family_ops_lookup(int family);
 
 void nft_ipv46_parse_target(struct xtables_target *t, void *data);
 bool nft_ipv46_rule_find(struct nft_handle *h, struct nftnl_rule *r,
-			 void *data);
+			 struct nftnl_rule *rule);
 
 bool compare_matches(struct xtables_rule_match *mt1, struct xtables_rule_match *mt2);
 bool compare_targets(struct xtables_target *tg1, struct xtables_target *tg2);
diff --git a/iptables/nft.c b/iptables/nft.c
index 468c703a1d09f..bbbf7c6166ac6 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -256,24 +256,6 @@ static int mnl_batch_talk(struct nft_handle *h, int numcmds)
 	return err;
 }
 
-enum obj_update_type {
-	NFT_COMPAT_TABLE_ADD,
-	NFT_COMPAT_TABLE_FLUSH,
-	NFT_COMPAT_CHAIN_ADD,
-	NFT_COMPAT_CHAIN_USER_ADD,
-	NFT_COMPAT_CHAIN_USER_DEL,
-	NFT_COMPAT_CHAIN_USER_FLUSH,
-	NFT_COMPAT_CHAIN_UPDATE,
-	NFT_COMPAT_CHAIN_RENAME,
-	NFT_COMPAT_CHAIN_ZERO,
-	NFT_COMPAT_RULE_APPEND,
-	NFT_COMPAT_RULE_INSERT,
-	NFT_COMPAT_RULE_REPLACE,
-	NFT_COMPAT_RULE_DELETE,
-	NFT_COMPAT_RULE_FLUSH,
-	NFT_COMPAT_SET_ADD,
-};
-
 enum obj_action {
 	NFT_COMPAT_COMMIT,
 	NFT_COMPAT_ABORT,
@@ -362,6 +344,15 @@ static int mnl_append_error(const struct nft_handle *h,
 		snprintf(tcr, sizeof(tcr), "set %s",
 			 nftnl_set_get_str(o->set, NFTNL_SET_NAME));
 		break;
+	case NFT_COMPAT_RULE_LIST:
+	case NFT_COMPAT_RULE_CHECK:
+	case NFT_COMPAT_CHAIN_RESTORE:
+	case NFT_COMPAT_RULE_SAVE:
+	case NFT_COMPAT_RULE_ZERO:
+	case NFT_COMPAT_BRIDGE_USER_CHAIN_UPDATE:
+	case NFT_COMPAT_TABLE_NEW:
+		assert(0);
+		break;
 	}
 
 	return snprintf(buf, len, "%s: %s", errmsg, tcr);
@@ -813,12 +804,18 @@ int nft_init(struct nft_handle *h, int family, const struct builtin_table *t)
 
 	INIT_LIST_HEAD(&h->obj_list);
 	INIT_LIST_HEAD(&h->err_list);
+	INIT_LIST_HEAD(&h->cmd_list);
 
 	return 0;
 }
 
 void nft_fini(struct nft_handle *h)
 {
+	struct nft_cmd *cmd, *next;
+
+	list_for_each_entry_safe(cmd, next, &h->cmd_list, head)
+		nft_cmd_free(cmd);
+
 	flush_chain_cache(h, NULL);
 	mnl_socket_close(h->nl);
 }
@@ -1327,7 +1324,7 @@ void add_compat(struct nftnl_rule *r, uint32_t proto, bool inv)
 			      inv ? NFT_RULE_COMPAT_F_INV : 0);
 }
 
-static struct nftnl_rule *
+struct nftnl_rule *
 nft_rule_new(struct nft_handle *h, const char *chain, const char *table,
 	     void *data)
 {
@@ -1355,10 +1352,9 @@ nft_chain_find(struct nft_handle *h, const char *table, const char *chain);
 
 int
 nft_rule_append(struct nft_handle *h, const char *chain, const char *table,
-		void *data, struct nftnl_rule *ref, bool verbose)
+		struct nftnl_rule *r, struct nftnl_rule *ref, bool verbose)
 {
 	struct nftnl_chain *c;
-	struct nftnl_rule *r;
 	int type;
 
 	nft_xt_builtin_init(h, table);
@@ -1373,10 +1369,6 @@ nft_rule_append(struct nft_handle *h, const char *chain, const char *table,
 
 	nft_fn = nft_rule_append;
 
-	r = nft_rule_new(h, chain, table, data);
-	if (r == NULL)
-		return 0;
-
 	if (ref) {
 		nftnl_rule_set_u64(r, NFTNL_RULE_HANDLE,
 				   nftnl_rule_get_u64(ref, NFTNL_RULE_HANDLE));
@@ -1384,10 +1376,8 @@ nft_rule_append(struct nft_handle *h, const char *chain, const char *table,
 	} else
 		type = NFT_COMPAT_RULE_APPEND;
 
-	if (batch_rule_add(h, type, r) == NULL) {
-		nftnl_rule_free(r);
+	if (batch_rule_add(h, type, r) == NULL)
 		return 0;
-	}
 
 	if (verbose)
 		h->ops->print_rule(h, r, 0, FMT_PRINT_RULE);
@@ -1747,7 +1737,7 @@ int nft_chain_restore(struct nft_handle *h, const char *chain, const char *table
 	} else {
 		c = nftnl_chain_alloc();
 		if (!c)
-			return -1;
+			return 0;
 
 		nftnl_chain_set_str(c, NFTNL_CHAIN_TABLE, table);
 		nftnl_chain_set_str(c, NFTNL_CHAIN_NAME, chain);
@@ -1758,7 +1748,7 @@ int nft_chain_restore(struct nft_handle *h, const char *chain, const char *table
 		nftnl_chain_set_u32(c, NFTNL_CHAIN_POLICY, NF_ACCEPT);
 
 	if (!created)
-		return 0;
+		return 1;
 
 	ret = batch_chain_add(h, NFT_COMPAT_CHAIN_USER_ADD, c);
 
@@ -1766,7 +1756,8 @@ int nft_chain_restore(struct nft_handle *h, const char *chain, const char *table
 	if (list)
 		nftnl_chain_list_add(c, list);
 
-	return ret;
+	/* the core expects 1 for success and 0 for error */
+	return ret == 0 ? 1 : 0;
 }
 
 /* From linux/netlink.h */
@@ -2073,7 +2064,8 @@ static int __nft_rule_del(struct nft_handle *h, struct nftnl_rule *r)
 }
 
 static struct nftnl_rule *
-nft_rule_find(struct nft_handle *h, struct nftnl_chain *c, void *data, int rulenum)
+nft_rule_find(struct nft_handle *h, struct nftnl_chain *c,
+	      struct nftnl_rule *rule, int rulenum)
 {
 	struct nftnl_rule *r;
 	struct nftnl_rule_iter *iter;
@@ -2091,7 +2083,7 @@ nft_rule_find(struct nft_handle *h, struct nftnl_chain *c, void *data, int rulen
 
 	r = nftnl_rule_iter_next(iter);
 	while (r != NULL) {
-		found = h->ops->rule_find(h, r, data);
+		found = h->ops->rule_find(h, r, rule);
 		if (found)
 			break;
 		r = nftnl_rule_iter_next(iter);
@@ -2103,7 +2095,7 @@ nft_rule_find(struct nft_handle *h, struct nftnl_chain *c, void *data, int rulen
 }
 
 int nft_rule_check(struct nft_handle *h, const char *chain,
-		   const char *table, void *data, bool verbose)
+		   const char *table, struct nftnl_rule *rule, bool verbose)
 {
 	struct nftnl_chain *c;
 	struct nftnl_rule *r;
@@ -2114,7 +2106,7 @@ int nft_rule_check(struct nft_handle *h, const char *chain,
 	if (!c)
 		goto fail_enoent;
 
-	r = nft_rule_find(h, c, data, -1);
+	r = nft_rule_find(h, c, rule, -1);
 	if (r == NULL)
 		goto fail_enoent;
 
@@ -2128,7 +2120,7 @@ fail_enoent:
 }
 
 int nft_rule_delete(struct nft_handle *h, const char *chain,
-		    const char *table, void *data, bool verbose)
+		    const char *table, struct nftnl_rule *rule, bool verbose)
 {
 	int ret = 0;
 	struct nftnl_chain *c;
@@ -2142,7 +2134,7 @@ int nft_rule_delete(struct nft_handle *h, const char *chain,
 		return 0;
 	}
 
-	r = nft_rule_find(h, c, data, -1);
+	r = nft_rule_find(h, c, rule, -1);
 	if (r != NULL) {
 		ret =__nft_rule_del(h, r);
 		if (ret < 0)
@@ -2157,16 +2149,11 @@ int nft_rule_delete(struct nft_handle *h, const char *chain,
 
 static struct nftnl_rule *
 nft_rule_add(struct nft_handle *h, const char *chain,
-	     const char *table, struct iptables_command_state *cs,
+	     const char *table, struct nftnl_rule *r,
 	     struct nftnl_rule *ref, bool verbose)
 {
-	struct nftnl_rule *r;
 	uint64_t ref_id;
 
-	r = nft_rule_new(h, chain, table, cs);
-	if (r == NULL)
-		return NULL;
-
 	if (ref) {
 		ref_id = nftnl_rule_get_u64(ref, NFTNL_RULE_HANDLE);
 		if (ref_id > 0) {
@@ -2183,10 +2170,8 @@ nft_rule_add(struct nft_handle *h, const char *chain,
 		}
 	}
 
-	if (!batch_rule_add(h, NFT_COMPAT_RULE_INSERT, r)) {
-		nftnl_rule_free(r);
+	if (!batch_rule_add(h, NFT_COMPAT_RULE_INSERT, r))
 		return NULL;
-	}
 
 	if (verbose)
 		h->ops->print_rule(h, r, 0, FMT_PRINT_RULE);
@@ -2195,9 +2180,10 @@ nft_rule_add(struct nft_handle *h, const char *chain,
 }
 
 int nft_rule_insert(struct nft_handle *h, const char *chain,
-		    const char *table, void *data, int rulenum, bool verbose)
+		    const char *table, struct nftnl_rule *new_rule, int rulenum,
+		    bool verbose)
 {
-	struct nftnl_rule *r = NULL, *new_rule;
+	struct nftnl_rule *r = NULL;
 	struct nftnl_chain *c;
 
 	nft_xt_builtin_init(h, table);
@@ -2211,22 +2197,22 @@ int nft_rule_insert(struct nft_handle *h, const char *chain,
 	}
 
 	if (rulenum > 0) {
-		r = nft_rule_find(h, c, data, rulenum);
+		r = nft_rule_find(h, c, new_rule, rulenum);
 		if (r == NULL) {
 			/* special case: iptables allows to insert into
 			 * rule_count + 1 position.
 			 */
-			r = nft_rule_find(h, c, data, rulenum - 1);
+			r = nft_rule_find(h, c, new_rule, rulenum - 1);
 			if (r != NULL)
-				return nft_rule_append(h, chain, table, data,
-						       NULL, verbose);
+				return nft_rule_append(h, chain, table,
+						       new_rule, NULL, verbose);
 
 			errno = E2BIG;
 			goto err;
 		}
 	}
 
-	new_rule = nft_rule_add(h, chain, table, data, r, verbose);
+	new_rule = nft_rule_add(h, chain, table, new_rule, r, verbose);
 	if (!new_rule)
 		goto err;
 
@@ -2268,7 +2254,8 @@ int nft_rule_delete_num(struct nft_handle *h, const char *chain,
 }
 
 int nft_rule_replace(struct nft_handle *h, const char *chain,
-		     const char *table, void *data, int rulenum, bool verbose)
+		     const char *table, struct nftnl_rule *rule,
+		     int rulenum, bool verbose)
 {
 	int ret = 0;
 	struct nftnl_chain *c;
@@ -2282,13 +2269,13 @@ int nft_rule_replace(struct nft_handle *h, const char *chain,
 		return 0;
 	}
 
-	r = nft_rule_find(h, c, data, rulenum);
+	r = nft_rule_find(h, c, rule, rulenum);
 	if (r != NULL) {
 		DEBUGP("replacing rule with handle=%llu\n",
 			(unsigned long long)
 			nftnl_rule_get_u64(r, NFTNL_RULE_HANDLE));
 
-		ret = nft_rule_append(h, chain, table, data, r, verbose);
+		ret = nft_rule_append(h, chain, table, rule, r, verbose);
 	} else
 		errno = E2BIG;
 
@@ -2521,8 +2508,8 @@ int nft_rule_zero_counters(struct nft_handle *h, const char *chain,
 			   const char *table, int rulenum)
 {
 	struct iptables_command_state cs = {};
+	struct nftnl_rule *r, *new_rule;
 	struct nftnl_chain *c;
-	struct nftnl_rule *r;
 	int ret = 0;
 
 	nft_fn = nft_rule_delete;
@@ -2541,8 +2528,11 @@ int nft_rule_zero_counters(struct nft_handle *h, const char *chain,
 	nft_rule_to_iptables_command_state(h, r, &cs);
 
 	cs.counters.pcnt = cs.counters.bcnt = 0;
+	new_rule = nft_rule_new(h, chain, table, &cs);
+	if (!new_rule)
+		return 1;
 
-	ret =  nft_rule_append(h, chain, table, &cs, r, false);
+	ret = nft_rule_append(h, chain, table, new_rule, r, false);
 
 error:
 	return ret;
@@ -2644,6 +2634,15 @@ static void batch_obj_del(struct nft_handle *h, struct obj_update *o)
 	case NFT_COMPAT_SET_ADD:
 		nftnl_set_free(o->set);
 		break;
+	case NFT_COMPAT_RULE_LIST:
+	case NFT_COMPAT_RULE_CHECK:
+	case NFT_COMPAT_CHAIN_RESTORE:
+	case NFT_COMPAT_RULE_SAVE:
+	case NFT_COMPAT_RULE_ZERO:
+	case NFT_COMPAT_BRIDGE_USER_CHAIN_UPDATE:
+	case NFT_COMPAT_TABLE_NEW:
+		assert(0);
+		break;
 	}
 	h->obj_list_num--;
 	list_del(&o->head);
@@ -2711,6 +2710,13 @@ static void nft_refresh_transaction(struct nft_handle *h)
 		case NFT_COMPAT_RULE_DELETE:
 		case NFT_COMPAT_RULE_FLUSH:
 		case NFT_COMPAT_SET_ADD:
+		case NFT_COMPAT_RULE_LIST:
+		case NFT_COMPAT_RULE_CHECK:
+		case NFT_COMPAT_CHAIN_RESTORE:
+		case NFT_COMPAT_RULE_SAVE:
+		case NFT_COMPAT_RULE_ZERO:
+		case NFT_COMPAT_BRIDGE_USER_CHAIN_UPDATE:
+		case NFT_COMPAT_TABLE_NEW:
 			break;
 		}
 	}
@@ -2808,6 +2814,14 @@ retry:
 						     NLM_F_CREATE, &n->seq, n->set);
 			seq = n->seq;
 			break;
+		case NFT_COMPAT_RULE_LIST:
+		case NFT_COMPAT_RULE_CHECK:
+		case NFT_COMPAT_CHAIN_RESTORE:
+		case NFT_COMPAT_RULE_SAVE:
+		case NFT_COMPAT_RULE_ZERO:
+		case NFT_COMPAT_BRIDGE_USER_CHAIN_UPDATE:
+		case NFT_COMPAT_TABLE_NEW:
+			assert(0);
 		}
 
 		mnl_nft_batch_continue(h->batch);
@@ -2969,19 +2983,145 @@ static void nft_bridge_commit_prepare(struct nft_handle *h)
 	}
 }
 
+static void assert_chain_exists(struct nft_handle *h,
+				const char *table, const char *chain)
+{
+	if (chain && !nft_chain_exists(h, table, chain))
+		xtables_error(PARAMETER_PROBLEM,
+			      "Chain '%s' does not exist", chain);
+}
+
+static int nft_prepare(struct nft_handle *h)
+{
+	struct nft_cmd *cmd, *next;
+	int ret = 1;
+
+	list_for_each_entry_safe(cmd, next, &h->cmd_list, head) {
+		switch (cmd->command) {
+		case NFT_COMPAT_TABLE_FLUSH:
+			ret = nft_table_flush(h, cmd->table);
+			break;
+		case NFT_COMPAT_CHAIN_USER_ADD:
+			ret = nft_chain_user_add(h, cmd->chain, cmd->table);
+			break;
+		case NFT_COMPAT_CHAIN_USER_DEL:
+			ret = nft_chain_user_del(h, cmd->chain, cmd->table,
+						 cmd->verbose);
+			break;
+		case NFT_COMPAT_CHAIN_RESTORE:
+			ret = nft_chain_restore(h, cmd->chain, cmd->table);
+			break;
+		case NFT_COMPAT_CHAIN_UPDATE:
+			ret = nft_chain_set(h, cmd->table, cmd->chain,
+					    cmd->policy, &cmd->counters);
+			break;
+		case NFT_COMPAT_CHAIN_RENAME:
+			ret = nft_chain_user_rename(h, cmd->chain, cmd->table,
+						    cmd->rename);
+			break;
+		case NFT_COMPAT_CHAIN_ZERO:
+			ret = nft_chain_zero_counters(h, cmd->chain, cmd->table,
+						      cmd->verbose);
+			break;
+		case NFT_COMPAT_RULE_APPEND:
+			assert_chain_exists(h, cmd->table, cmd->jumpto);
+			ret = nft_rule_append(h, cmd->chain, cmd->table,
+					      cmd->obj.rule, NULL, cmd->verbose);
+			break;
+		case NFT_COMPAT_RULE_INSERT:
+			assert_chain_exists(h, cmd->table, cmd->jumpto);
+			ret = nft_rule_insert(h, cmd->chain, cmd->table,
+					      cmd->obj.rule, cmd->rulenum,
+					      cmd->verbose);
+			break;
+		case NFT_COMPAT_RULE_REPLACE:
+			assert_chain_exists(h, cmd->table, cmd->jumpto);
+			ret = nft_rule_replace(h, cmd->chain, cmd->table,
+					      cmd->obj.rule, cmd->rulenum,
+					      cmd->verbose);
+			break;
+		case NFT_COMPAT_RULE_DELETE:
+			assert_chain_exists(h, cmd->table, cmd->jumpto);
+			if (cmd->rulenum >= 0)
+				ret = nft_rule_delete_num(h, cmd->chain,
+							  cmd->table,
+							  cmd->rulenum,
+							  cmd->verbose);
+			else
+				ret = nft_rule_delete(h, cmd->chain, cmd->table,
+						      cmd->obj.rule, cmd->verbose);
+			break;
+		case NFT_COMPAT_RULE_FLUSH:
+			ret = nft_rule_flush(h, cmd->chain, cmd->table,
+					     cmd->verbose);
+			break;
+		case NFT_COMPAT_RULE_LIST:
+			ret = nft_rule_list(h, cmd->chain, cmd->table,
+					    cmd->rulenum, cmd->format);
+			break;
+		case NFT_COMPAT_RULE_CHECK:
+			assert_chain_exists(h, cmd->table, cmd->jumpto);
+			ret = nft_rule_check(h, cmd->chain, cmd->table,
+					     cmd->obj.rule, cmd->rulenum);
+			break;
+		case NFT_COMPAT_RULE_ZERO:
+			ret = nft_rule_zero_counters(h, cmd->chain, cmd->table,
+                                                     cmd->rulenum);
+			break;
+		case NFT_COMPAT_RULE_SAVE:
+			ret = nft_rule_list_save(h, cmd->chain, cmd->table,
+						 cmd->rulenum,
+						 cmd->counters_save);
+			break;
+		case NFT_COMPAT_BRIDGE_USER_CHAIN_UPDATE:
+			ret = ebt_set_user_chain_policy(h, cmd->table,
+							cmd->chain, cmd->policy);
+			break;
+		case NFT_COMPAT_TABLE_NEW:
+			nft_xt_builtin_init(h, cmd->table);
+			ret = 1;
+			break;
+		case NFT_COMPAT_TABLE_ADD:
+		case NFT_COMPAT_CHAIN_ADD:
+		case NFT_COMPAT_SET_ADD:
+			assert(0);
+			break;
+		}
+
+		nft_cmd_free(cmd);
+
+		if (ret == 0)
+			return 0;
+	}
+
+	return 1;
+}
+
 int nft_commit(struct nft_handle *h)
 {
+	if (!nft_prepare(h))
+		return 0;
+
 	return nft_action(h, NFT_COMPAT_COMMIT);
 }
 
 int nft_bridge_commit(struct nft_handle *h)
 {
+	if (!nft_prepare(h))
+		return 0;
+
 	nft_bridge_commit_prepare(h);
-	return nft_commit(h);
+
+	return nft_action(h, NFT_COMPAT_COMMIT);
 }
 
 int nft_abort(struct nft_handle *h)
 {
+	struct nft_cmd *cmd, *next;
+
+	list_for_each_entry_safe(cmd, next, &h->cmd_list, head)
+		nft_cmd_free(cmd);
+
 	return nft_action(h, NFT_COMPAT_ABORT);
 }
 
diff --git a/iptables/nft.h b/iptables/nft.h
index ebb4044d1a453..7ddc3a8bbb042 100644
--- a/iptables/nft.h
+++ b/iptables/nft.h
@@ -3,6 +3,8 @@
 
 #include "xshared.h"
 #include "nft-shared.h"
+#include "nft-cache.h"
+#include "nft-cmd.h"
 #include <libiptc/linux_list.h>
 
 enum nft_table_type {
@@ -45,6 +47,31 @@ struct nft_cache {
 	} table[NFT_TABLE_MAX];
 };
 
+enum obj_update_type {
+	NFT_COMPAT_TABLE_ADD,
+	NFT_COMPAT_TABLE_FLUSH,
+	NFT_COMPAT_CHAIN_ADD,
+	NFT_COMPAT_CHAIN_USER_ADD,
+	NFT_COMPAT_CHAIN_USER_DEL,
+	NFT_COMPAT_CHAIN_USER_FLUSH,
+	NFT_COMPAT_CHAIN_UPDATE,
+	NFT_COMPAT_CHAIN_RENAME,
+	NFT_COMPAT_CHAIN_ZERO,
+	NFT_COMPAT_RULE_APPEND,
+	NFT_COMPAT_RULE_INSERT,
+	NFT_COMPAT_RULE_REPLACE,
+	NFT_COMPAT_RULE_DELETE,
+	NFT_COMPAT_RULE_FLUSH,
+	NFT_COMPAT_SET_ADD,
+	NFT_COMPAT_RULE_LIST,
+	NFT_COMPAT_RULE_CHECK,
+	NFT_COMPAT_CHAIN_RESTORE,
+	NFT_COMPAT_RULE_SAVE,
+	NFT_COMPAT_RULE_ZERO,
+	NFT_COMPAT_BRIDGE_USER_CHAIN_UPDATE,
+	NFT_COMPAT_TABLE_NEW,
+};
+
 struct nft_handle {
 	int			family;
 	struct mnl_socket	*nl;
@@ -67,6 +94,7 @@ struct nft_handle {
 	bool			restore;
 	bool			noflush;
 	int8_t			config_done;
+	struct list_head	cmd_list;
 
 	/* meta data, for error reporting */
 	struct {
@@ -121,12 +149,13 @@ void nft_bridge_chain_postprocess(struct nft_handle *h,
  */
 struct nftnl_rule;
 
-int nft_rule_append(struct nft_handle *h, const char *chain, const char *table, void *data, struct nftnl_rule *ref, bool verbose);
-int nft_rule_insert(struct nft_handle *h, const char *chain, const char *table, void *data, int rulenum, bool verbose);
-int nft_rule_check(struct nft_handle *h, const char *chain, const char *table, void *data, bool verbose);
-int nft_rule_delete(struct nft_handle *h, const char *chain, const char *table, void *data, bool verbose);
+struct nftnl_rule *nft_rule_new(struct nft_handle *h, const char *chain, const char *table, void *data);
+int nft_rule_append(struct nft_handle *h, const char *chain, const char *table, struct nftnl_rule *r, struct nftnl_rule *ref, bool verbose);
+int nft_rule_insert(struct nft_handle *h, const char *chain, const char *table, struct nftnl_rule *r, int rulenum, bool verbose);
+int nft_rule_check(struct nft_handle *h, const char *chain, const char *table, struct nftnl_rule *r, bool verbose);
+int nft_rule_delete(struct nft_handle *h, const char *chain, const char *table, struct nftnl_rule *r, bool verbose);
 int nft_rule_delete_num(struct nft_handle *h, const char *chain, const char *table, int rulenum, bool verbose);
-int nft_rule_replace(struct nft_handle *h, const char *chain, const char *table, void *data, int rulenum, bool verbose);
+int nft_rule_replace(struct nft_handle *h, const char *chain, const char *table, struct nftnl_rule *r, int rulenum, bool verbose);
 int nft_rule_list(struct nft_handle *h, const char *chain, const char *table, int rulenum, unsigned int format);
 int nft_rule_list_save(struct nft_handle *h, const char *chain, const char *table, int rulenum, int counters);
 int nft_rule_save(struct nft_handle *h, const char *table, unsigned int format);
diff --git a/iptables/tests/shell/testcases/ip6tables/0004-return-codes_0 b/iptables/tests/shell/testcases/ip6tables/0004-return-codes_0
index f023b7915498e..c583b0ebd97c3 100755
--- a/iptables/tests/shell/testcases/ip6tables/0004-return-codes_0
+++ b/iptables/tests/shell/testcases/ip6tables/0004-return-codes_0
@@ -26,6 +26,7 @@ cmd 1 ip6tables -N foo
 # test rule adding
 cmd 0 ip6tables -A INPUT -j ACCEPT
 cmd 1 ip6tables -A noexist -j ACCEPT
+cmd 2 ip6tables -I INPUT -j foobar
 
 # test rule checking
 cmd 0 ip6tables -C INPUT -j ACCEPT
diff --git a/iptables/tests/shell/testcases/iptables/0004-return-codes_0 b/iptables/tests/shell/testcases/iptables/0004-return-codes_0
index ce02e0bcb128b..f730bede1f612 100755
--- a/iptables/tests/shell/testcases/iptables/0004-return-codes_0
+++ b/iptables/tests/shell/testcases/iptables/0004-return-codes_0
@@ -54,10 +54,16 @@ cmd 1 "$ENOENT" iptables -Z bar
 # test chain rename
 cmd 0 iptables -E foo bar
 cmd 1 "$EEXIST_F" iptables -E foo bar
+cmd 1 "$ENOENT" iptables -E foo bar2
+cmd 0 iptables -N foo2
+cmd 1 "$EEXIST_F" iptables -E foo2 bar
 
 # test rule adding
 cmd 0 iptables -A INPUT -j ACCEPT
 cmd 1 "$ENOENT" iptables -A noexist -j ACCEPT
+cmd 2 "" iptables -I INPUT -j foobar
+cmd 2 "" iptables -R INPUT 1 -j foobar
+cmd 2 "" iptables -D INPUT -j foobar
 
 # test rulenum commands
 cmd 1 "$E2BIG_I" iptables -I INPUT 23 -j ACCEPT
diff --git a/iptables/xtables-arp.c b/iptables/xtables-arp.c
index c8196f08baa59..a0136059bb710 100644
--- a/iptables/xtables-arp.c
+++ b/iptables/xtables-arp.c
@@ -400,7 +400,7 @@ list_entries(struct nft_handle *h, const char *chain, const char *table,
 	if (linenumbers)
 		format |= FMT_LINENUMBERS;
 
-	return nft_rule_list(h, chain, table, rulenum, format);
+	return nft_cmd_rule_list(h, chain, table, rulenum, format);
 }
 
 static int
@@ -427,10 +427,10 @@ append_entry(struct nft_handle *h,
 			cs->arp.arp.tgt.s_addr = daddrs[j].s_addr;
 			cs->arp.arp.tmsk.s_addr = dmasks[j].s_addr;
 			if (append) {
-				ret = nft_rule_append(h, chain, table, cs, NULL,
+				ret = nft_cmd_rule_append(h, chain, table, cs, NULL,
 						      verbose);
 			} else {
-				ret = nft_rule_insert(h, chain, table, cs,
+				ret = nft_cmd_rule_insert(h, chain, table, cs,
 						      rulenum, verbose);
 			}
 		}
@@ -455,7 +455,7 @@ replace_entry(const char *chain,
 	cs->arp.arp.smsk.s_addr = smask->s_addr;
 	cs->arp.arp.tmsk.s_addr = dmask->s_addr;
 
-	return nft_rule_replace(h, chain, table, cs, rulenum, verbose);
+	return nft_cmd_rule_replace(h, chain, table, cs, rulenum, verbose);
 }
 
 static int
@@ -479,7 +479,7 @@ delete_entry(const char *chain,
 		for (j = 0; j < ndaddrs; j++) {
 			cs->arp.arp.tgt.s_addr = daddrs[j].s_addr;
 			cs->arp.arp.tmsk.s_addr = dmasks[j].s_addr;
-			ret = nft_rule_delete(h, chain, table, cs, verbose);
+			ret = nft_cmd_rule_delete(h, chain, table, cs, verbose);
 		}
 	}
 
@@ -955,7 +955,7 @@ int do_commandarp(struct nft_handle *h, int argc, char *argv[], char **table,
 				   options&OPT_VERBOSE, h);
 		break;
 	case CMD_DELETE_NUM:
-		ret = nft_rule_delete_num(h, chain, *table, rulenum - 1, verbose);
+		ret = nft_cmd_rule_delete_num(h, chain, *table, rulenum - 1, verbose);
 		break;
 	case CMD_REPLACE:
 		ret = replace_entry(chain, *table, &cs, rulenum - 1,
@@ -977,10 +977,10 @@ int do_commandarp(struct nft_handle *h, int argc, char *argv[], char **table,
 				   options&OPT_LINENUMBERS);
 		break;
 	case CMD_FLUSH:
-		ret = nft_rule_flush(h, chain, *table, options & OPT_VERBOSE);
+		ret = nft_cmd_rule_flush(h, chain, *table, options & OPT_VERBOSE);
 		break;
 	case CMD_ZERO:
-		ret = nft_chain_zero_counters(h, chain, *table,
+		ret = nft_cmd_chain_zero_counters(h, chain, *table,
 					      options & OPT_VERBOSE);
 		break;
 	case CMD_LIST|CMD_ZERO:
@@ -990,21 +990,21 @@ int do_commandarp(struct nft_handle *h, int argc, char *argv[], char **table,
 				   /*options&OPT_EXPANDED*/0,
 				   options&OPT_LINENUMBERS);
 		if (ret)
-			ret = nft_chain_zero_counters(h, chain, *table,
+			ret = nft_cmd_chain_zero_counters(h, chain, *table,
 						      options & OPT_VERBOSE);
 		break;
 	case CMD_NEW_CHAIN:
-		ret = nft_chain_user_add(h, chain, *table);
+		ret = nft_cmd_chain_user_add(h, chain, *table);
 		break;
 	case CMD_DELETE_CHAIN:
-		ret = nft_chain_user_del(h, chain, *table,
+		ret = nft_cmd_chain_user_del(h, chain, *table,
 					 options & OPT_VERBOSE);
 		break;
 	case CMD_RENAME_CHAIN:
-		ret = nft_chain_user_rename(h, chain, *table, newname);
+		ret = nft_cmd_chain_user_rename(h, chain, *table, newname);
 		break;
 	case CMD_SET_POLICY:
-		ret = nft_chain_set(h, *table, chain, policy, NULL);
+		ret = nft_cmd_chain_set(h, *table, chain, policy, NULL);
 		if (ret < 0)
 			xtables_error(PARAMETER_PROBLEM, "Wrong policy `%s'\n",
 				      policy);
diff --git a/iptables/xtables-eb.c b/iptables/xtables-eb.c
index c006bc95ac681..07ed651370913 100644
--- a/iptables/xtables-eb.c
+++ b/iptables/xtables-eb.c
@@ -150,9 +150,9 @@ append_entry(struct nft_handle *h,
 	int ret = 1;
 
 	if (append)
-		ret = nft_rule_append(h, chain, table, cs, NULL, verbose);
+		ret = nft_cmd_rule_append(h, chain, table, cs, NULL, verbose);
 	else
-		ret = nft_rule_insert(h, chain, table, cs, rule_nr, verbose);
+		ret = nft_cmd_rule_insert(h, chain, table, cs, rule_nr, verbose);
 
 	return ret;
 }
@@ -169,10 +169,10 @@ delete_entry(struct nft_handle *h,
 	int ret = 1;
 
 	if (rule_nr == -1)
-		ret = nft_rule_delete(h, chain, table, cs, verbose);
+		ret = nft_cmd_rule_delete(h, chain, table, cs, verbose);
 	else {
 		do {
-			ret = nft_rule_delete_num(h, chain, table,
+			ret = nft_cmd_rule_delete_num(h, chain, table,
 						  rule_nr, verbose);
 			rule_nr++;
 		} while (rule_nr < rule_nr_end);
@@ -427,7 +427,7 @@ static int list_rules(struct nft_handle *h, const char *chain, const char *table
 	if (!counters)
 		format |= FMT_NOCOUNTS;
 
-	return nft_rule_list(h, chain, table, rule_nr, format);
+	return nft_cmd_rule_list(h, chain, table, rule_nr, format);
 }
 
 static int parse_rule_range(const char *argv, int *rule_nr, int *rule_nr_end)
@@ -813,7 +813,7 @@ int do_commandeb(struct nft_handle *h, int argc, char *argv[], char **table,
 			flags |= OPT_COMMAND;
 
 			if (c == 'N') {
-				ret = nft_chain_user_add(h, chain, *table);
+				ret = nft_cmd_chain_user_add(h, chain, *table);
 				break;
 			} else if (c == 'X') {
 				/* X arg is optional, optarg is NULL */
@@ -821,7 +821,7 @@ int do_commandeb(struct nft_handle *h, int argc, char *argv[], char **table,
 					chain = argv[optind];
 					optind++;
 				}
-				ret = nft_chain_user_del(h, chain, *table, 0);
+				ret = nft_cmd_chain_user_del(h, chain, *table, 0);
 				break;
 			}
 
@@ -835,7 +835,7 @@ int do_commandeb(struct nft_handle *h, int argc, char *argv[], char **table,
 				else if (strchr(argv[optind], ' ') != NULL)
 					xtables_error(PARAMETER_PROBLEM, "Use of ' ' not allowed in chain names");
 
-				ret = nft_chain_user_rename(h, chain, *table,
+				ret = nft_cmd_chain_user_rename(h, chain, *table,
 							    argv[optind]);
 				if (ret != 0 && errno == ENOENT)
 					xtables_error(PARAMETER_PROBLEM, "Chain '%s' doesn't exists", chain);
@@ -1137,7 +1137,7 @@ print_zero:
 		/*case 7 :*/ /* atomic-init */
 		/*case 10:*/ /* atomic-save */
 		case 11: /* init-table */
-			nft_table_flush(h, *table);
+			nft_cmd_table_flush(h, *table);
 			return 1;
 		/*
 			replace->command = c;
@@ -1225,13 +1225,13 @@ print_zero:
 
 	if (command == 'P') {
 		if (selected_chain >= NF_BR_NUMHOOKS) {
-			ret = ebt_set_user_chain_policy(h, *table, chain, policy);
+			ret = ebt_cmd_user_chain_policy(h, *table, chain, policy);
 		} else {
 			if (strcmp(policy, "RETURN") == 0) {
 				xtables_error(PARAMETER_PROBLEM,
 					      "Policy RETURN only allowed for user defined chains");
 			}
-			ret = nft_chain_set(h, *table, chain, policy, NULL);
+			ret = nft_cmd_chain_set(h, *table, chain, policy, NULL);
 			if (ret < 0)
 				xtables_error(PARAMETER_PROBLEM, "Wrong policy");
 		}
@@ -1244,9 +1244,9 @@ print_zero:
 				 flags&LIST_C);
 	}
 	if (flags & OPT_ZERO) {
-		ret = nft_chain_zero_counters(h, chain, *table, 0);
+		ret = nft_cmd_chain_zero_counters(h, chain, *table, 0);
 	} else if (command == 'F') {
-		ret = nft_rule_flush(h, chain, *table, 0);
+		ret = nft_cmd_rule_flush(h, chain, *table, 0);
 	} else if (command == 'A') {
 		ret = append_entry(h, chain, *table, &cs, 0, 0, true);
 	} else if (command == 'I') {
diff --git a/iptables/xtables-restore.c b/iptables/xtables-restore.c
index 53a0d3738eb74..c00ccb558784c 100644
--- a/iptables/xtables-restore.c
+++ b/iptables/xtables-restore.c
@@ -61,11 +61,11 @@ static void print_usage(const char *name, const char *version)
 static const struct nft_xt_restore_cb restore_cb = {
 	.commit		= nft_commit,
 	.abort		= nft_abort,
-	.table_new	= nft_table_new,
-	.table_flush	= nft_table_flush,
+	.table_new	= nft_cmd_table_new,
+	.table_flush	= nft_cmd_table_flush,
 	.do_command	= do_commandx,
-	.chain_set	= nft_chain_set,
-	.chain_restore  = nft_chain_restore,
+	.chain_set	= nft_cmd_chain_set,
+	.chain_restore  = nft_cmd_chain_restore,
 };
 
 struct nft_xt_restore_state {
@@ -193,7 +193,7 @@ static void xtables_restore_parse_line(struct nft_handle *h,
 				      "cannot create chain '%s' (%s)\n",
 				      chain, strerror(errno));
 		} else if (h->family == NFPROTO_BRIDGE &&
-			   !ebt_set_user_chain_policy(h, state->curtable->name,
+			   !ebt_cmd_user_chain_policy(h, state->curtable->name,
 						      chain, policy)) {
 			xtables_error(OTHER_PROBLEM,
 				      "Can't set policy `%s' on `%s' line %u: %s\n",
@@ -490,11 +490,11 @@ int xtables_ip6_restore_main(int argc, char *argv[])
 
 static const struct nft_xt_restore_cb ebt_restore_cb = {
 	.commit		= nft_bridge_commit,
-	.table_new	= nft_table_new,
-	.table_flush	= nft_table_flush,
+	.table_new	= nft_cmd_table_new,
+	.table_flush	= nft_cmd_table_flush,
 	.do_command	= do_commandeb,
-	.chain_set	= nft_chain_set,
-	.chain_restore  = nft_chain_restore,
+	.chain_set	= nft_cmd_chain_set,
+	.chain_restore  = nft_cmd_chain_restore,
 };
 
 static const struct option ebt_restore_options[] = {
@@ -536,11 +536,11 @@ int xtables_eb_restore_main(int argc, char *argv[])
 
 static const struct nft_xt_restore_cb arp_restore_cb = {
 	.commit		= nft_commit,
-	.table_new	= nft_table_new,
-	.table_flush	= nft_table_flush,
+	.table_new	= nft_cmd_table_new,
+	.table_flush	= nft_cmd_table_flush,
 	.do_command	= do_commandarp,
-	.chain_set	= nft_chain_set,
-	.chain_restore  = nft_chain_restore,
+	.chain_set	= nft_cmd_chain_set,
+	.chain_restore  = nft_cmd_chain_restore,
 };
 
 int xtables_arp_restore_main(int argc, char *argv[])
diff --git a/iptables/xtables.c b/iptables/xtables.c
index 8c2d21d42b7d2..c180af13975f8 100644
--- a/iptables/xtables.c
+++ b/iptables/xtables.c
@@ -361,11 +361,11 @@ add_entry(const char *chain,
 				cs->fw.ip.dmsk.s_addr = d.mask.v4[j].s_addr;
 
 				if (append) {
-					ret = nft_rule_append(h, chain, table,
+					ret = nft_cmd_rule_append(h, chain, table,
 							      cs, NULL,
 							      verbose);
 				} else {
-					ret = nft_rule_insert(h, chain, table,
+					ret = nft_cmd_rule_insert(h, chain, table,
 							      cs, rulenum,
 							      verbose);
 				}
@@ -381,11 +381,11 @@ add_entry(const char *chain,
 				memcpy(&cs->fw6.ipv6.dmsk,
 				       &d.mask.v6[j], sizeof(struct in6_addr));
 				if (append) {
-					ret = nft_rule_append(h, chain, table,
+					ret = nft_cmd_rule_append(h, chain, table,
 							      cs, NULL,
 							      verbose);
 				} else {
-					ret = nft_rule_insert(h, chain, table,
+					ret = nft_cmd_rule_insert(h, chain, table,
 							      cs, rulenum,
 							      verbose);
 				}
@@ -418,7 +418,7 @@ replace_entry(const char *chain, const char *table,
 	} else
 		return 1;
 
-	return nft_rule_replace(h, chain, table, cs, rulenum, verbose);
+	return nft_cmd_rule_replace(h, chain, table, cs, rulenum, verbose);
 }
 
 static int
@@ -440,7 +440,7 @@ delete_entry(const char *chain, const char *table,
 			for (j = 0; j < d.naddrs; j++) {
 				cs->fw.ip.dst.s_addr = d.addr.v4[j].s_addr;
 				cs->fw.ip.dmsk.s_addr = d.mask.v4[j].s_addr;
-				ret = nft_rule_delete(h, chain,
+				ret = nft_cmd_rule_delete(h, chain,
 						      table, cs, verbose);
 			}
 		} else if (family == AF_INET6) {
@@ -453,7 +453,7 @@ delete_entry(const char *chain, const char *table,
 				       &d.addr.v6[j], sizeof(struct in6_addr));
 				memcpy(&cs->fw6.ipv6.dmsk,
 				       &d.mask.v6[j], sizeof(struct in6_addr));
-				ret = nft_rule_delete(h, chain,
+				ret = nft_cmd_rule_delete(h, chain,
 						      table, cs, verbose);
 			}
 		}
@@ -480,7 +480,7 @@ check_entry(const char *chain, const char *table,
 			for (j = 0; j < d.naddrs; j++) {
 				cs->fw.ip.dst.s_addr = d.addr.v4[j].s_addr;
 				cs->fw.ip.dmsk.s_addr = d.mask.v4[j].s_addr;
-				ret = nft_rule_check(h, chain,
+				ret = nft_cmd_rule_check(h, chain,
 						     table, cs, verbose);
 			}
 		} else if (family == AF_INET6) {
@@ -493,7 +493,7 @@ check_entry(const char *chain, const char *table,
 				       &d.addr.v6[j], sizeof(struct in6_addr));
 				memcpy(&cs->fw6.ipv6.dmsk,
 				       &d.mask.v6[j], sizeof(struct in6_addr));
-				ret = nft_rule_check(h, chain,
+				ret = nft_cmd_rule_check(h, chain,
 						     table, cs, verbose);
 			}
 		}
@@ -524,7 +524,7 @@ list_entries(struct nft_handle *h, const char *chain, const char *table,
 	if (linenumbers)
 		format |= FMT_LINENUMBERS;
 
-	return nft_rule_list(h, chain, table, rulenum, format);
+	return nft_cmd_rule_list(h, chain, table, rulenum, format);
 }
 
 static int
@@ -534,7 +534,7 @@ list_rules(struct nft_handle *h, const char *chain, const char *table,
 	if (counters)
 	    counters = -1;		/* iptables -c format */
 
-	return nft_rule_list_save(h, chain, table, rulenum, counters);
+	return nft_cmd_rule_list_save(h, chain, table, rulenum, counters);
 }
 
 void do_parse(struct nft_handle *h, int argc, char *argv[],
@@ -1022,11 +1022,6 @@ void do_parse(struct nft_handle *h, int argc, char *argv[],
 					   opt2char(OPT_VIANAMEIN),
 					   p->chain);
 		}
-
-		if (!p->xlate && !cs->target && strlen(cs->jumpto) > 0 &&
-		    !nft_chain_exists(h, p->table, cs->jumpto))
-			xtables_error(PARAMETER_PROBLEM,
-				      "Chain '%s' does not exist", cs->jumpto);
 	}
 }
 
@@ -1057,8 +1052,8 @@ int do_commandx(struct nft_handle *h, int argc, char *argv[], char **table,
 				   cs.options & OPT_VERBOSE, h);
 		break;
 	case CMD_DELETE_NUM:
-		ret = nft_rule_delete_num(h, p.chain, p.table,
-					  p.rulenum - 1, p.verbose);
+		ret = nft_cmd_rule_delete_num(h, p.chain, p.table,
+					      p.rulenum - 1, p.verbose);
 		break;
 	case CMD_CHECK:
 		ret = check_entry(p.chain, p.table, &cs, h->family,
@@ -1076,15 +1071,15 @@ int do_commandx(struct nft_handle *h, int argc, char *argv[], char **table,
 				cs.options&OPT_VERBOSE, h, false);
 		break;
 	case CMD_FLUSH:
-		ret = nft_rule_flush(h, p.chain, p.table,
-				     cs.options & OPT_VERBOSE);
+		ret = nft_cmd_rule_flush(h, p.chain, p.table,
+					 cs.options & OPT_VERBOSE);
 		break;
 	case CMD_ZERO:
-		ret = nft_chain_zero_counters(h, p.chain, p.table,
-					      cs.options & OPT_VERBOSE);
+		ret = nft_cmd_chain_zero_counters(h, p.chain, p.table,
+						  cs.options & OPT_VERBOSE);
 		break;
 	case CMD_ZERO_NUM:
-		ret = nft_rule_zero_counters(h, p.chain, p.table,
+		ret = nft_cmd_rule_zero_counters(h, p.chain, p.table,
 					     p.rulenum - 1);
 		break;
 	case CMD_LIST:
@@ -1096,11 +1091,11 @@ int do_commandx(struct nft_handle *h, int argc, char *argv[], char **table,
 				   cs.options & OPT_EXPANDED,
 				   cs.options & OPT_LINENUMBERS);
 		if (ret && (p.command & CMD_ZERO)) {
-			ret = nft_chain_zero_counters(h, p.chain, p.table,
+			ret = nft_cmd_chain_zero_counters(h, p.chain, p.table,
 						      cs.options & OPT_VERBOSE);
 		}
 		if (ret && (p.command & CMD_ZERO_NUM)) {
-			ret = nft_rule_zero_counters(h, p.chain, p.table,
+			ret = nft_cmd_rule_zero_counters(h, p.chain, p.table,
 						     p.rulenum - 1);
 		}
 		nft_check_xt_legacy(h->family, false);
@@ -1111,27 +1106,27 @@ int do_commandx(struct nft_handle *h, int argc, char *argv[], char **table,
 		ret = list_rules(h, p.chain, p.table, p.rulenum,
 				 cs.options & OPT_VERBOSE);
 		if (ret && (p.command & CMD_ZERO)) {
-			ret = nft_chain_zero_counters(h, p.chain, p.table,
+			ret = nft_cmd_chain_zero_counters(h, p.chain, p.table,
 						      cs.options & OPT_VERBOSE);
 		}
 		if (ret && (p.command & CMD_ZERO_NUM)) {
-			ret = nft_rule_zero_counters(h, p.chain, p.table,
+			ret = nft_cmd_rule_zero_counters(h, p.chain, p.table,
 						     p.rulenum - 1);
 		}
 		nft_check_xt_legacy(h->family, false);
 		break;
 	case CMD_NEW_CHAIN:
-		ret = nft_chain_user_add(h, p.chain, p.table);
+		ret = nft_cmd_chain_user_add(h, p.chain, p.table);
 		break;
 	case CMD_DELETE_CHAIN:
-		ret = nft_chain_user_del(h, p.chain, p.table,
+		ret = nft_cmd_chain_user_del(h, p.chain, p.table,
 					 cs.options & OPT_VERBOSE);
 		break;
 	case CMD_RENAME_CHAIN:
-		ret = nft_chain_user_rename(h, p.chain, p.table, p.newname);
+		ret = nft_cmd_chain_user_rename(h, p.chain, p.table, p.newname);
 		break;
 	case CMD_SET_POLICY:
-		ret = nft_chain_set(h, p.table, p.chain, p.policy, NULL);
+		ret = nft_cmd_chain_set(h, p.table, p.chain, p.policy, NULL);
 		break;
 	case CMD_NONE:
 	/* do_parse ignored the line (eg: -4 with ip6tables-restore) */
-- 
2.25.1

[iptables PATCH v2 07/18] nft: calculate cache requirements from list of commands

[Phil Sutter]

From: Pablo Neira Ayuso <pablo@netfilter.org>

This patch uses the new list of commands to calculate the cache
requirements, the rationale after this updates is the following:

 #1 Parsing, that builds the list of commands and it also calculates
    cache level requirements.
 #2 Cache building.
 #3 Translate commands to jobs
 #4 Translate jobs to netlink

This patch removes the pre-parsing code in xtables-restore.c to
calculate the cache.

After this patch, cache is calculated only once, there is no need
to cancel and refetch for an in-transit transaction.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v1:
- Rebased and resolved conflicts.
- Adjust for dropped init_chain_cache().
- Adjust for nft_rebuild_cache() respecting fake cache.
- Use if-clauses in __nft_build_cache() instead of switch() as that
  better fits the staged caching.
- Set cache_level to NFT_CL_TABLES in nft_cmd_table_flush(), not
  NFT_CL_CHAINS: When flushing the whole table, chain list is not
  relevant.
- Move nft_cache_level_set() into nft-cache.c as it belongs there, make
  it public and have xtables_save_main() call it instead of manipulating
  h->cache_level directly.
- Actually drop input buffering in xtables-restore.c, it is not needed
  anymore.
---
 iptables/nft-cache.c       | 68 +++++++++++++----------------
 iptables/nft-cache.h       |  2 +
 iptables/nft-cmd.c         | 66 +++++++++++++++++++++++++++++
 iptables/nft-cmd.h         |  1 +
 iptables/nft.c             | 17 +++++++-
 iptables/nft.h             |  2 +-
 iptables/xtables-restore.c | 87 +-------------------------------------
 iptables/xtables-save.c    |  3 ++
 8 files changed, 119 insertions(+), 127 deletions(-)

diff --git a/iptables/nft-cache.c b/iptables/nft-cache.c
index 51b371c51c3f4..38e353bd7231f 100644
--- a/iptables/nft-cache.c
+++ b/iptables/nft-cache.c
@@ -24,6 +24,14 @@
 #include "nft.h"
 #include "nft-cache.h"
 
+void nft_cache_level_set(struct nft_handle *h, int level)
+{
+	if (level <= h->cache_level)
+		return;
+
+	h->cache_level = level;
+}
+
 static int genid_cb(const struct nlmsghdr *nlh, void *data)
 {
 	uint32_t *genid = data;
@@ -436,42 +444,20 @@ __nft_build_cache(struct nft_handle *h, enum nft_cache_level level,
 		  const struct builtin_table *t, const char *set,
 		  const char *chain)
 {
-	if (level <= h->cache_level)
+	if (h->cache_init)
 		return;
 
-	if (!h->nft_genid)
-		mnl_genid_get(h, &h->nft_genid);
+	h->cache_init = true;
+	mnl_genid_get(h, &h->nft_genid);
 
-	switch (h->cache_level) {
-	case NFT_CL_NONE:
+	if (h->cache_level >= NFT_CL_TABLES)
 		fetch_table_cache(h);
-		if (level == NFT_CL_TABLES)
-			break;
-		/* fall through */
-	case NFT_CL_TABLES:
+	if (h->cache_level >= NFT_CL_CHAINS)
 		fetch_chain_cache(h, t, chain);
-		if (level == NFT_CL_CHAINS)
-			break;
-		/* fall through */
-	case NFT_CL_CHAINS:
+	if (h->cache_level >= NFT_CL_SETS)
 		fetch_set_cache(h, t, set);
-		if (level == NFT_CL_SETS)
-			break;
-		/* fall through */
-	case NFT_CL_SETS:
+	if (h->cache_level >= NFT_CL_RULES)
 		fetch_rule_cache(h, t, chain);
-		if (level == NFT_CL_RULES)
-			break;
-		/* fall through */
-	case NFT_CL_RULES:
-	case NFT_CL_FAKE:
-		break;
-	}
-
-	if (!t && !chain)
-		h->cache_level = level;
-	else if (h->cache_level < NFT_CL_TABLES)
-		h->cache_level = NFT_CL_TABLES;
 }
 
 void nft_build_cache(struct nft_handle *h, struct nftnl_chain *c)
@@ -493,6 +479,7 @@ void nft_fake_cache(struct nft_handle *h)
 	fetch_table_cache(h);
 
 	h->cache_level = NFT_CL_FAKE;
+	h->cache_init = true;
 	mnl_genid_get(h, &h->nft_genid);
 }
 
@@ -593,26 +580,29 @@ static int flush_cache(struct nft_handle *h, struct nft_cache *c,
 
 void flush_chain_cache(struct nft_handle *h, const char *tablename)
 {
-	if (!h->cache_level)
+	if (!h->cache_init)
 		return;
 
 	if (flush_cache(h, h->cache, tablename))
-		h->cache_level = NFT_CL_NONE;
+		h->cache_init = false;
 }
 
 void nft_rebuild_cache(struct nft_handle *h)
 {
-	enum nft_cache_level level = h->cache_level;
-
-	if (h->cache_level)
+	if (h->cache_init) {
 		__nft_flush_cache(h);
+		h->cache_init = false;
+	}
 
-	if (h->cache_level == NFT_CL_FAKE) {
+	if (h->cache_level == NFT_CL_FAKE)
 		nft_fake_cache(h);
-	} else {
-		h->cache_level = NFT_CL_NONE;
-		__nft_build_cache(h, level, NULL, NULL, NULL);
-	}
+	else
+		__nft_build_cache(h, h->cache_level, NULL, NULL, NULL);
+}
+
+void nft_cache_build(struct nft_handle *h)
+{
+	__nft_build_cache(h, h->cache_level, NULL, NULL, NULL);
 }
 
 void nft_release_cache(struct nft_handle *h)
diff --git a/iptables/nft-cache.h b/iptables/nft-cache.h
index ed498835676e2..cf28808e22c72 100644
--- a/iptables/nft-cache.h
+++ b/iptables/nft-cache.h
@@ -3,6 +3,7 @@
 
 struct nft_handle;
 
+void nft_cache_level_set(struct nft_handle *h, int level);
 void nft_fake_cache(struct nft_handle *h);
 void nft_build_cache(struct nft_handle *h, struct nftnl_chain *c);
 void nft_rebuild_cache(struct nft_handle *h);
@@ -10,6 +11,7 @@ void nft_release_cache(struct nft_handle *h);
 void flush_chain_cache(struct nft_handle *h, const char *tablename);
 int flush_rule_cache(struct nft_handle *h, const char *table,
 		     struct nftnl_chain *c);
+void nft_cache_build(struct nft_handle *h);
 
 struct nftnl_chain_list *
 nft_chain_list_get(struct nft_handle *h, const char *table, const char *chain);
diff --git a/iptables/nft-cmd.c b/iptables/nft-cmd.c
index 1d9834c0f94d7..be9fbbf5a19bd 100644
--- a/iptables/nft-cmd.c
+++ b/iptables/nft-cmd.c
@@ -63,12 +63,33 @@ void nft_cmd_free(struct nft_cmd *cmd)
 	free(cmd);
 }
 
+static void nft_cmd_rule_bridge(struct nft_handle *h, const char *chain,
+				const char *table)
+{
+	const struct builtin_table *t;
+
+	t = nft_table_builtin_find(h, table);
+	if (!t)
+		return;
+
+	/* Since ebtables user-defined chain policies are implemented as last
+	 * rule in nftables, rule cache is required here to treat them right.
+	 */
+	if (h->family == NFPROTO_BRIDGE &&
+	    !nft_chain_builtin_find(t, chain))
+		nft_cache_level_set(h, NFT_CL_RULES);
+	else
+		nft_cache_level_set(h, NFT_CL_CHAINS);
+}
+
 int nft_cmd_rule_append(struct nft_handle *h, const char *chain,
 			const char *table, struct iptables_command_state *state,
 			void *ref, bool verbose)
 {
 	struct nft_cmd *cmd;
 
+	nft_cmd_rule_bridge(h, chain, table);
+
 	cmd = nft_cmd_new(h, NFT_COMPAT_RULE_APPEND, table, chain, state, -1,
 			  verbose);
 	if (!cmd)
@@ -83,11 +104,18 @@ int nft_cmd_rule_insert(struct nft_handle *h, const char *chain,
 {
 	struct nft_cmd *cmd;
 
+	nft_cmd_rule_bridge(h, chain, table);
+
 	cmd = nft_cmd_new(h, NFT_COMPAT_RULE_INSERT, table, chain, state,
 			  rulenum, verbose);
 	if (!cmd)
 		return 0;
 
+	if (cmd->rulenum > 0)
+		nft_cache_level_set(h, NFT_CL_RULES);
+	else
+		nft_cache_level_set(h, NFT_CL_CHAINS);
+
 	return 1;
 }
 
@@ -102,6 +130,8 @@ int nft_cmd_rule_delete(struct nft_handle *h, const char *chain,
 	if (!cmd)
 		return 0;
 
+	nft_cache_level_set(h, NFT_CL_RULES);
+
 	return 1;
 }
 
@@ -115,6 +145,8 @@ int nft_cmd_rule_delete_num(struct nft_handle *h, const char *chain,
 	if (!cmd)
 		return 0;
 
+	nft_cache_level_set(h, NFT_CL_RULES);
+
 	return 1;
 }
 
@@ -128,6 +160,8 @@ int nft_cmd_rule_flush(struct nft_handle *h, const char *chain,
 	if (!cmd)
 		return 0;
 
+	nft_cache_level_set(h, NFT_CL_CHAINS);
+
 	return 1;
 }
 
@@ -141,6 +175,8 @@ int nft_cmd_chain_zero_counters(struct nft_handle *h, const char *chain,
 	if (!cmd)
 		return 0;
 
+	nft_cache_level_set(h, NFT_CL_CHAINS);
+
 	return 1;
 }
 
@@ -154,6 +190,8 @@ int nft_cmd_chain_user_add(struct nft_handle *h, const char *chain,
 	if (!cmd)
 		return 0;
 
+	nft_cache_level_set(h, NFT_CL_CHAINS);
+
 	return 1;
 }
 
@@ -167,6 +205,14 @@ int nft_cmd_chain_user_del(struct nft_handle *h, const char *chain,
 	if (!cmd)
 		return 0;
 
+	/* This triggers nft_bridge_chain_postprocess() when fetching the
+	 * rule cache.
+	 */
+	if (h->family == NFPROTO_BRIDGE)
+		nft_cache_level_set(h, NFT_CL_RULES);
+	else
+		nft_cache_level_set(h, NFT_CL_CHAINS);
+
 	return 1;
 }
 
@@ -182,6 +228,8 @@ int nft_cmd_chain_user_rename(struct nft_handle *h,const char *chain,
 
 	cmd->rename = strdup(newname);
 
+	nft_cache_level_set(h, NFT_CL_CHAINS);
+
 	return 1;
 }
 
@@ -197,6 +245,8 @@ int nft_cmd_rule_list(struct nft_handle *h, const char *chain,
 
 	cmd->format = format;
 
+	nft_cache_level_set(h, NFT_CL_RULES);
+
 	return 1;
 }
 
@@ -211,6 +261,8 @@ int nft_cmd_rule_replace(struct nft_handle *h, const char *chain,
 	if (!cmd)
 		return 0;
 
+	nft_cache_level_set(h, NFT_CL_RULES);
+
 	return 1;
 }
 
@@ -224,6 +276,8 @@ int nft_cmd_rule_check(struct nft_handle *h, const char *chain,
 	if (!cmd)
 		return 0;
 
+	nft_cache_level_set(h, NFT_CL_RULES);
+
 	return 1;
 }
 
@@ -242,6 +296,8 @@ int nft_cmd_chain_set(struct nft_handle *h, const char *table,
 	if (counters)
 		cmd->counters = *counters;
 
+	nft_cache_level_set(h, NFT_CL_CHAINS);
+
 	return 1;
 }
 
@@ -254,6 +310,8 @@ int nft_cmd_table_flush(struct nft_handle *h, const char *table)
 	if (!cmd)
 		return 0;
 
+	nft_cache_level_set(h, NFT_CL_TABLES);
+
 	return 1;
 }
 
@@ -267,6 +325,8 @@ int nft_cmd_chain_restore(struct nft_handle *h, const char *chain,
 	if (!cmd)
 		return 0;
 
+	nft_cache_level_set(h, NFT_CL_CHAINS);
+
 	return 1;
 }
 
@@ -280,6 +340,8 @@ int nft_cmd_rule_zero_counters(struct nft_handle *h, const char *chain,
 	if (!cmd)
 		return 0;
 
+	nft_cache_level_set(h, NFT_CL_RULES);
+
 	return 1;
 }
 
@@ -295,6 +357,8 @@ int nft_cmd_rule_list_save(struct nft_handle *h, const char *chain,
 
 	cmd->counters_save = counters;
 
+	nft_cache_level_set(h, NFT_CL_RULES);
+
 	return 1;
 }
 
@@ -310,6 +374,8 @@ int ebt_cmd_user_chain_policy(struct nft_handle *h, const char *table,
 
 	cmd->policy = strdup(policy);
 
+	nft_cache_level_set(h, NFT_CL_RULES);
+
 	return 1;
 }
 
diff --git a/iptables/nft-cmd.h b/iptables/nft-cmd.h
index 33ee766ae823f..0e1776ce088bf 100644
--- a/iptables/nft-cmd.h
+++ b/iptables/nft-cmd.h
@@ -18,6 +18,7 @@ struct nft_cmd {
 	unsigned int			format;
 	struct {
 		struct nftnl_rule	*rule;
+		struct nftnl_set	*set;
 	} obj;
 	const char			*policy;
 	struct xt_counters		counters;
diff --git a/iptables/nft.c b/iptables/nft.c
index bbbf7c6166ac6..f069396a05190 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -954,6 +954,7 @@ static struct nftnl_set *add_anon_set(struct nft_handle *h, const char *table,
 {
 	static uint32_t set_id = 0;
 	struct nftnl_set *s;
+	struct nft_cmd *cmd;
 
 	s = nftnl_set_alloc();
 	if (!s)
@@ -969,7 +970,14 @@ static struct nftnl_set *add_anon_set(struct nft_handle *h, const char *table,
 	nftnl_set_set_u32(s, NFTNL_SET_KEY_LEN, key_len);
 	nftnl_set_set_u32(s, NFTNL_SET_DESC_SIZE, size);
 
-	return batch_set_add(h, NFT_COMPAT_SET_ADD, s) ? s : NULL;
+	cmd = nft_cmd_new(h, NFT_COMPAT_SET_ADD, table, NULL, NULL, -1, false);
+	if (!cmd) {
+		nftnl_set_free(s);
+		return NULL;
+	}
+	cmd->obj.set = s;
+
+	return s;
 }
 
 static struct nftnl_expr *
@@ -2996,6 +3004,8 @@ static int nft_prepare(struct nft_handle *h)
 	struct nft_cmd *cmd, *next;
 	int ret = 1;
 
+	nft_cache_build(h);
+
 	list_for_each_entry_safe(cmd, next, &h->cmd_list, head) {
 		switch (cmd->command) {
 		case NFT_COMPAT_TABLE_FLUSH:
@@ -3081,9 +3091,12 @@ static int nft_prepare(struct nft_handle *h)
 			nft_xt_builtin_init(h, cmd->table);
 			ret = 1;
 			break;
+		case NFT_COMPAT_SET_ADD:
+			batch_set_add(h, NFT_COMPAT_SET_ADD, cmd->obj.set);
+			ret = 1;
+			break;
 		case NFT_COMPAT_TABLE_ADD:
 		case NFT_COMPAT_CHAIN_ADD:
-		case NFT_COMPAT_SET_ADD:
 			assert(0);
 			break;
 		}
diff --git a/iptables/nft.h b/iptables/nft.h
index 7ddc3a8bbb042..d61a40979d5bc 100644
--- a/iptables/nft.h
+++ b/iptables/nft.h
@@ -30,7 +30,6 @@ struct builtin_table {
 };
 
 enum nft_cache_level {
-	NFT_CL_NONE,
 	NFT_CL_TABLES,
 	NFT_CL_CHAINS,
 	NFT_CL_SETS,
@@ -95,6 +94,7 @@ struct nft_handle {
 	bool			noflush;
 	int8_t			config_done;
 	struct list_head	cmd_list;
+	bool			cache_init;
 
 	/* meta data, for error reporting */
 	struct {
diff --git a/iptables/xtables-restore.c b/iptables/xtables-restore.c
index c00ccb558784c..a58c6a5bdca7a 100644
--- a/iptables/xtables-restore.c
+++ b/iptables/xtables-restore.c
@@ -252,99 +252,16 @@ static void xtables_restore_parse_line(struct nft_handle *h,
 	}
 }
 
-/* Return true if given iptables-restore line will require a full cache.
- * Typically these are commands referring to an existing rule
- * (either by number or content) or commands listing the ruleset. */
-static bool cmd_needs_full_cache(char *cmd)
-{
-	char c, chain[32];
-	int rulenum, mcount;
-
-	mcount = sscanf(cmd, "-%c %31s %d", &c, chain, &rulenum);
-
-	if (mcount == 3)
-		return true;
-	if (mcount < 1)
-		return false;
-
-	switch (c) {
-	case 'D':
-	case 'C':
-	case 'S':
-	case 'L':
-	case 'Z':
-		return true;
-	}
-
-	return false;
-}
-
-#define PREBUFSIZ	65536
-
 void xtables_restore_parse(struct nft_handle *h,
 			   const struct nft_xt_restore_parse *p)
 {
 	struct nft_xt_restore_state state = {};
-	char preload_buffer[PREBUFSIZ] = {}, buffer[10240] = {}, *ptr;
+	char buffer[10240] = {};
 
-	if (!h->noflush) {
+	if (!h->noflush)
 		nft_fake_cache(h);
-	} else {
-		ssize_t pblen = sizeof(preload_buffer);
-		bool do_cache = false;
-
-		ptr = preload_buffer;
-		while (fgets(buffer, sizeof(buffer), p->in)) {
-			size_t blen = strlen(buffer);
-
-			/* Drop trailing newline; xtables_restore_parse_line()
-			 * uses strtok() which replaces them by nul-characters,
-			 * causing unpredictable string delimiting in
-			 * preload_buffer.
-			 * Unless this is an empty line which would fold into a
-			 * spurious EoB indicator (double nul-char). */
-			if (buffer[blen - 1] == '\n' && blen > 1)
-				buffer[blen - 1] = '\0';
-			else
-				blen++;
-
-			pblen -= blen;
-			if (pblen <= 0) {
-				/* buffer exhausted */
-				do_cache = true;
-				break;
-			}
-
-			if (cmd_needs_full_cache(buffer)) {
-				do_cache = true;
-				break;
-			}
-
-			/* copy string including terminating nul-char */
-			memcpy(ptr, buffer, blen);
-			ptr += blen;
-			buffer[0] = '\0';
-		}
-
-		if (do_cache)
-			nft_build_cache(h, NULL);
-	}
 
 	line = 0;
-	ptr = preload_buffer;
-	while (*ptr) {
-		size_t len = strlen(ptr);
-
-		h->error.lineno = ++line;
-		DEBUGP("%s: buffered line %d: '%s'\n", __func__, line, ptr);
-		xtables_restore_parse_line(h, p, &state, ptr);
-		ptr += len + 1;
-	}
-	if (*buffer) {
-		h->error.lineno = ++line;
-		DEBUGP("%s: overrun line %d: '%s'\n", __func__, line, buffer);
-		xtables_restore_parse_line(h, p, &state, buffer);
-	}
 	while (fgets(buffer, sizeof(buffer), p->in)) {
 		h->error.lineno = ++line;
 		DEBUGP("%s: input line %d: '%s'\n", __func__, line, buffer);
diff --git a/iptables/xtables-save.c b/iptables/xtables-save.c
index 28f7490275ce5..f927aa6e9e404 100644
--- a/iptables/xtables-save.c
+++ b/iptables/xtables-save.c
@@ -239,6 +239,9 @@ xtables_save_main(int family, int argc, char *argv[],
 		exit(EXIT_FAILURE);
 	}
 
+	nft_cache_level_set(&h, NFT_CL_RULES);
+	nft_cache_build(&h);
+
 	ret = do_output(&h, tablename, &d);
 	nft_fini(&h);
 	if (dump)
-- 
2.25.1

[iptables PATCH v2 08/18] nft: restore among support

[Phil Sutter]

From: Pablo Neira Ayuso <pablo@netfilter.org>

Update among support to work again with the new parser and cache logic.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft-bridge.c | 13 +++++++++++--
 iptables/nft.c        | 15 +++++++++++++++
 iptables/nft.h        |  6 ++++++
 3 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/iptables/nft-bridge.c b/iptables/nft-bridge.c
index a5aaa3f87187e..80d7f91710c16 100644
--- a/iptables/nft-bridge.c
+++ b/iptables/nft-bridge.c
@@ -421,11 +421,20 @@ static struct nftnl_set *set_from_lookup_expr(struct nft_xt_ctx *ctx,
 					      const struct nftnl_expr *e)
 {
 	const char *set_name = nftnl_expr_get_str(e, NFTNL_EXPR_LOOKUP_SET);
+	uint32_t set_id = nftnl_expr_get_u32(e, NFTNL_EXPR_LOOKUP_SET_ID);
 	struct nftnl_set_list *slist;
+	struct nftnl_set *set;
 
 	slist = nft_set_list_get(ctx->h, ctx->table, set_name);
-	if (slist)
-		return nftnl_set_list_lookup_byname(slist, set_name);
+	if (slist) {
+		set = nftnl_set_list_lookup_byname(slist, set_name);
+		if (set)
+			return set;
+
+		set = nft_set_batch_lookup_byid(ctx->h, set_id);
+		if (set)
+			return set;
+	}
 
 	return NULL;
 }
diff --git a/iptables/nft.c b/iptables/nft.c
index f069396a05190..9771bcc9add02 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -1613,6 +1613,20 @@ int nft_rule_save(struct nft_handle *h, const char *table, unsigned int format)
 	return ret == 0 ? 1 : 0;
 }
 
+struct nftnl_set *nft_set_batch_lookup_byid(struct nft_handle *h,
+					    uint32_t set_id)
+{
+	struct obj_update *n;
+
+	list_for_each_entry(n, &h->obj_list, head) {
+		if (n->type == NFT_COMPAT_SET_ADD &&
+		    nftnl_set_get_u32(n->set, NFTNL_SET_ID) == set_id)
+			return n->set;
+	}
+
+	return NULL;
+}
+
 static void
 __nft_rule_flush(struct nft_handle *h, const char *table,
 		 const char *chain, bool verbose, bool implicit)
@@ -3092,6 +3106,7 @@ static int nft_prepare(struct nft_handle *h)
 			ret = 1;
 			break;
 		case NFT_COMPAT_SET_ADD:
+			nft_xt_builtin_init(h, cmd->table);
 			batch_set_add(h, NFT_COMPAT_SET_ADD, cmd->obj.set);
 			ret = 1;
 			break;
diff --git a/iptables/nft.h b/iptables/nft.h
index d61a40979d5bc..89c3620e7b7d7 100644
--- a/iptables/nft.h
+++ b/iptables/nft.h
@@ -144,6 +144,12 @@ void nft_bridge_chain_postprocess(struct nft_handle *h,
 				  struct nftnl_chain *c);
 
 
+/*
+ * Operations with sets.
+ */
+struct nftnl_set *nft_set_batch_lookup_byid(struct nft_handle *h,
+					    uint32_t set_id);
+
 /*
  * Operations with rule-set.
  */
-- 
2.25.1

[iptables PATCH v2 09/18] nft: remove cache build calls

[Phil Sutter]

From: Pablo Neira Ayuso <pablo@netfilter.org>

The cache requirements are now calculated once from the parsing phase.
There is no need to call __nft_build_cache() from several spots in the
codepath anymore.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v1:
- Drop now unused nft_build_cache() function.
---
 iptables/nft-cache.c | 20 --------------------
 iptables/nft-cache.h |  1 -
 iptables/nft.c       | 21 ---------------------
 3 files changed, 42 deletions(-)

diff --git a/iptables/nft-cache.c b/iptables/nft-cache.c
index 38e353bd7231f..6db261fbba4b3 100644
--- a/iptables/nft-cache.c
+++ b/iptables/nft-cache.c
@@ -460,20 +460,6 @@ __nft_build_cache(struct nft_handle *h, enum nft_cache_level level,
 		fetch_rule_cache(h, t, chain);
 }
 
-void nft_build_cache(struct nft_handle *h, struct nftnl_chain *c)
-{
-	const struct builtin_table *t;
-	const char *table, *chain;
-
-	if (!c)
-		return __nft_build_cache(h, NFT_CL_RULES, NULL, NULL, NULL);
-
-	table = nftnl_chain_get_str(c, NFTNL_CHAIN_TABLE);
-	chain = nftnl_chain_get_str(c, NFTNL_CHAIN_NAME);
-	t = nft_table_builtin_find(h, table);
-	__nft_build_cache(h, NFT_CL_RULES, t, NULL, chain);
-}
-
 void nft_fake_cache(struct nft_handle *h)
 {
 	fetch_table_cache(h);
@@ -619,8 +605,6 @@ void nft_release_cache(struct nft_handle *h)
 
 struct nftnl_table_list *nftnl_table_list_get(struct nft_handle *h)
 {
-	__nft_build_cache(h, NFT_CL_TABLES, NULL, NULL, NULL);
-
 	return h->cache->tables;
 }
 
@@ -633,8 +617,6 @@ nft_set_list_get(struct nft_handle *h, const char *table, const char *set)
 	if (!t)
 		return NULL;
 
-	__nft_build_cache(h, NFT_CL_RULES, t, set, NULL);
-
 	return h->cache->table[t->type].sets;
 }
 
@@ -647,8 +629,6 @@ nft_chain_list_get(struct nft_handle *h, const char *table, const char *chain)
 	if (!t)
 		return NULL;
 
-	__nft_build_cache(h, NFT_CL_CHAINS, t, NULL, chain);
-
 	return h->cache->table[t->type].chains;
 }
 
diff --git a/iptables/nft-cache.h b/iptables/nft-cache.h
index cf28808e22c72..8c63d8d566c19 100644
--- a/iptables/nft-cache.h
+++ b/iptables/nft-cache.h
@@ -5,7 +5,6 @@ struct nft_handle;
 
 void nft_cache_level_set(struct nft_handle *h, int level);
 void nft_fake_cache(struct nft_handle *h);
-void nft_build_cache(struct nft_handle *h, struct nftnl_chain *c);
 void nft_rebuild_cache(struct nft_handle *h);
 void nft_release_cache(struct nft_handle *h);
 void flush_chain_cache(struct nft_handle *h, const char *tablename);
diff --git a/iptables/nft.c b/iptables/nft.c
index 9771bcc9add02..f9e53316ab7cf 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -1367,14 +1367,6 @@ nft_rule_append(struct nft_handle *h, const char *chain, const char *table,
 
 	nft_xt_builtin_init(h, table);
 
-	/* Since ebtables user-defined chain policies are implemented as last
-	 * rule in nftables, rule cache is required here to treat them right. */
-	if (h->family == NFPROTO_BRIDGE) {
-		c = nft_chain_find(h, table, chain);
-		if (c && !nft_chain_builtin(c))
-			nft_build_cache(h, c);
-	}
-
 	nft_fn = nft_rule_append;
 
 	if (ref) {
@@ -1599,7 +1591,6 @@ int nft_rule_save(struct nft_handle *h, const char *table, unsigned int format)
 
 	c = nftnl_chain_list_iter_next(iter);
 	while (c) {
-		nft_build_cache(h, c);
 		ret = nft_chain_save_rules(h, c, format);
 		if (ret != 0)
 			break;
@@ -1807,10 +1798,6 @@ static int __nft_chain_user_del(struct nftnl_chain *c, void *data)
 		fprintf(stdout, "Deleting chain `%s'\n",
 			nftnl_chain_get_str(c, NFTNL_CHAIN_NAME));
 
-	/* This triggers required policy rule deletion. */
-	if (h->family == NFPROTO_BRIDGE)
-		nft_build_cache(h, c);
-
 	/* XXX This triggers a fast lookup from the kernel. */
 	nftnl_chain_unset(c, NFTNL_CHAIN_HANDLE);
 	ret = batch_chain_add(h, NFT_COMPAT_CHAIN_USER_DEL, c);
@@ -2093,8 +2080,6 @@ nft_rule_find(struct nft_handle *h, struct nftnl_chain *c,
 	struct nftnl_rule_iter *iter;
 	bool found = false;
 
-	nft_build_cache(h, c);
-
 	if (rulenum >= 0)
 		/* Delete by rule number case */
 		return nftnl_rule_lookup_byindex(c, rulenum);
@@ -2979,8 +2964,6 @@ int ebt_set_user_chain_policy(struct nft_handle *h, const char *table,
 	else
 		return 0;
 
-	nft_build_cache(h, c);
-
 	nftnl_chain_set_u32(c, NFTNL_CHAIN_POLICY, pval);
 	return 1;
 }
@@ -3333,8 +3316,6 @@ static int __nft_chain_zero_counters(struct nftnl_chain *c, void *data)
 			return -1;
 	}
 
-	nft_build_cache(h, c);
-
 	iter = nftnl_rule_iter_create(c);
 	if (iter == NULL)
 		return -1;
@@ -3471,8 +3452,6 @@ static int nft_is_chain_compatible(struct nftnl_chain *c, void *data)
 	enum nf_inet_hooks hook;
 	int prio;
 
-	nft_build_cache(h, c);
-
 	if (nftnl_rule_foreach(c, nft_is_rule_compatible, NULL))
 		return -1;
 
-- 
2.25.1

[iptables PATCH v2 10/18] nft: missing nft_fini() call in bridge family

[Phil Sutter]

From: Pablo Neira Ayuso <pablo@netfilter.org>

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/xtables-eb-standalone.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/iptables/xtables-eb-standalone.c b/iptables/xtables-eb-standalone.c
index a9081c78c3bc3..ff74ddbb37334 100644
--- a/iptables/xtables-eb-standalone.c
+++ b/iptables/xtables-eb-standalone.c
@@ -53,6 +53,8 @@ int xtables_eb_main(int argc, char *argv[])
 	if (ret)
 		ret = nft_bridge_commit(&h);
 
+	nft_fini(&h);
+
 	if (!ret)
 		fprintf(stderr, "ebtables: %s\n", nft_strerror(errno));
 
-- 
2.25.1

[iptables PATCH v2 11/18] nft: cache: Simplify rule and set fetchers

[Phil Sutter]

Since no incremental cache fetching happens anymore, code fetching rules
for chains or elements for sets may safely assume that whatever is in
cache also didn't get populated with rules or elements before.

Therefore no (optional) chain name needs to be passed on to
fetch_rule_cache() and fetch_set_cache() doesn't have to select for
which sets in a table to call set_fetch_elem_cb().

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft-cache.c | 20 +++++---------------
 1 file changed, 5 insertions(+), 15 deletions(-)

diff --git a/iptables/nft-cache.c b/iptables/nft-cache.c
index 6db261fbba4b3..e0c1387071330 100644
--- a/iptables/nft-cache.c
+++ b/iptables/nft-cache.c
@@ -295,11 +295,7 @@ static int fetch_set_cache(struct nft_handle *h,
 		return ret;
 	}
 
-	if (t && set) {
-		s = nftnl_set_list_lookup_byname(h->cache->table[t->type].sets,
-						 set);
-		set_fetch_elem_cb(s, h);
-	} else if (t) {
+	if (t) {
 		nftnl_set_list_foreach(h->cache->table[t->type].sets,
 				       set_fetch_elem_cb, h);
 	} else {
@@ -409,20 +405,14 @@ static int nft_rule_list_update(struct nftnl_chain *c, void *data)
 }
 
 static int fetch_rule_cache(struct nft_handle *h,
-			    const struct builtin_table *t, const char *chain)
+			    const struct builtin_table *t)
 {
 	int i;
 
 	if (t) {
-		struct nftnl_chain_list *list;
-		struct nftnl_chain *c;
-
-		list = h->cache->table[t->type].chains;
+		struct nftnl_chain_list *list =
+			h->cache->table[t->type].chains;
 
-		if (chain) {
-			c = nftnl_chain_list_lookup_byname(list, chain);
-			return nft_rule_list_update(c, h);
-		}
 		return nftnl_chain_list_foreach(list, nft_rule_list_update, h);
 	}
 
@@ -457,7 +447,7 @@ __nft_build_cache(struct nft_handle *h, enum nft_cache_level level,
 	if (h->cache_level >= NFT_CL_SETS)
 		fetch_set_cache(h, t, set);
 	if (h->cache_level >= NFT_CL_RULES)
-		fetch_rule_cache(h, t, chain);
+		fetch_rule_cache(h, t);
 }
 
 void nft_fake_cache(struct nft_handle *h)
-- 
2.25.1

[iptables PATCH v2 12/18] nft: cache: Improve fake cache integration

[Phil Sutter]

With NFT_CL_FAKE being highest cache level while at the same time
__nft_build_cache() treating it equal to NFT_CL_TABLES, no special
handling for fake cache is required anymore.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft-cache.c       | 16 +++-------------
 iptables/nft-cache.h       |  1 -
 iptables/xtables-restore.c |  2 +-
 3 files changed, 4 insertions(+), 15 deletions(-)

diff --git a/iptables/nft-cache.c b/iptables/nft-cache.c
index e0c1387071330..2c6a170881eb3 100644
--- a/iptables/nft-cache.c
+++ b/iptables/nft-cache.c
@@ -442,6 +442,8 @@ __nft_build_cache(struct nft_handle *h, enum nft_cache_level level,
 
 	if (h->cache_level >= NFT_CL_TABLES)
 		fetch_table_cache(h);
+	if (h->cache_level == NFT_CL_FAKE)
+		return;
 	if (h->cache_level >= NFT_CL_CHAINS)
 		fetch_chain_cache(h, t, chain);
 	if (h->cache_level >= NFT_CL_SETS)
@@ -450,15 +452,6 @@ __nft_build_cache(struct nft_handle *h, enum nft_cache_level level,
 		fetch_rule_cache(h, t);
 }
 
-void nft_fake_cache(struct nft_handle *h)
-{
-	fetch_table_cache(h);
-
-	h->cache_level = NFT_CL_FAKE;
-	h->cache_init = true;
-	mnl_genid_get(h, &h->nft_genid);
-}
-
 static void __nft_flush_cache(struct nft_handle *h)
 {
 	if (!h->cache_index) {
@@ -570,10 +563,7 @@ void nft_rebuild_cache(struct nft_handle *h)
 		h->cache_init = false;
 	}
 
-	if (h->cache_level == NFT_CL_FAKE)
-		nft_fake_cache(h);
-	else
-		__nft_build_cache(h, h->cache_level, NULL, NULL, NULL);
+	__nft_build_cache(h, h->cache_level, NULL, NULL, NULL);
 }
 
 void nft_cache_build(struct nft_handle *h)
diff --git a/iptables/nft-cache.h b/iptables/nft-cache.h
index 8c63d8d566c19..01dd15e145fd4 100644
--- a/iptables/nft-cache.h
+++ b/iptables/nft-cache.h
@@ -4,7 +4,6 @@
 struct nft_handle;
 
 void nft_cache_level_set(struct nft_handle *h, int level);
-void nft_fake_cache(struct nft_handle *h);
 void nft_rebuild_cache(struct nft_handle *h);
 void nft_release_cache(struct nft_handle *h);
 void flush_chain_cache(struct nft_handle *h, const char *tablename);
diff --git a/iptables/xtables-restore.c b/iptables/xtables-restore.c
index a58c6a5bdca7a..ca01d17eba566 100644
--- a/iptables/xtables-restore.c
+++ b/iptables/xtables-restore.c
@@ -259,7 +259,7 @@ void xtables_restore_parse(struct nft_handle *h,
 	char buffer[10240] = {};
 
 	if (!h->noflush)
-		nft_fake_cache(h);
+		nft_cache_level_set(h, NFT_CL_FAKE);
 
 	line = 0;
 	while (fgets(buffer, sizeof(buffer), p->in)) {
-- 
2.25.1

[iptables PATCH v2 13/18] nft: cache: Introduce struct nft_cache_req

[Phil Sutter]

This embedded struct collects cache requirement info gathered from parsed
nft_cmds and is interpreted by __nft_build_cache().

While being at it, remove unused parameters passed to the latter
function, nft_handle pointer is sufficient.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft-cache.c | 32 +++++++++++++++++---------------
 iptables/nft.h       |  6 +++++-
 2 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/iptables/nft-cache.c b/iptables/nft-cache.c
index 2c6a170881eb3..305f2c12307f7 100644
--- a/iptables/nft-cache.c
+++ b/iptables/nft-cache.c
@@ -26,10 +26,12 @@
 
 void nft_cache_level_set(struct nft_handle *h, int level)
 {
-	if (level <= h->cache_level)
+	struct nft_cache_req *req = &h->cache_req;
+
+	if (level <= req->level)
 		return;
 
-	h->cache_level = level;
+	req->level = level;
 }
 
 static int genid_cb(const struct nlmsghdr *nlh, void *data)
@@ -430,26 +432,26 @@ static int fetch_rule_cache(struct nft_handle *h,
 }
 
 static void
-__nft_build_cache(struct nft_handle *h, enum nft_cache_level level,
-		  const struct builtin_table *t, const char *set,
-		  const char *chain)
+__nft_build_cache(struct nft_handle *h)
 {
+	struct nft_cache_req *req = &h->cache_req;
+
 	if (h->cache_init)
 		return;
 
 	h->cache_init = true;
 	mnl_genid_get(h, &h->nft_genid);
 
-	if (h->cache_level >= NFT_CL_TABLES)
+	if (req->level >= NFT_CL_TABLES)
 		fetch_table_cache(h);
-	if (h->cache_level == NFT_CL_FAKE)
+	if (req->level == NFT_CL_FAKE)
 		return;
-	if (h->cache_level >= NFT_CL_CHAINS)
-		fetch_chain_cache(h, t, chain);
-	if (h->cache_level >= NFT_CL_SETS)
-		fetch_set_cache(h, t, set);
-	if (h->cache_level >= NFT_CL_RULES)
-		fetch_rule_cache(h, t);
+	if (req->level >= NFT_CL_CHAINS)
+		fetch_chain_cache(h, NULL, NULL);
+	if (req->level >= NFT_CL_SETS)
+		fetch_set_cache(h, NULL, NULL);
+	if (req->level >= NFT_CL_RULES)
+		fetch_rule_cache(h, NULL);
 }
 
 static void __nft_flush_cache(struct nft_handle *h)
@@ -563,12 +565,12 @@ void nft_rebuild_cache(struct nft_handle *h)
 		h->cache_init = false;
 	}
 
-	__nft_build_cache(h, h->cache_level, NULL, NULL, NULL);
+	__nft_build_cache(h);
 }
 
 void nft_cache_build(struct nft_handle *h)
 {
-	__nft_build_cache(h, h->cache_level, NULL, NULL, NULL);
+	__nft_build_cache(h);
 }
 
 void nft_release_cache(struct nft_handle *h)
diff --git a/iptables/nft.h b/iptables/nft.h
index 89c3620e7b7d7..c6aece7d1dac8 100644
--- a/iptables/nft.h
+++ b/iptables/nft.h
@@ -71,6 +71,10 @@ enum obj_update_type {
 	NFT_COMPAT_TABLE_NEW,
 };
 
+struct nft_cache_req {
+	enum nft_cache_level	level;
+};
+
 struct nft_handle {
 	int			family;
 	struct mnl_socket	*nl;
@@ -89,7 +93,7 @@ struct nft_handle {
 	unsigned int		cache_index;
 	struct nft_cache	__cache[2];
 	struct nft_cache	*cache;
-	enum nft_cache_level	cache_level;
+	struct nft_cache_req	cache_req;
 	bool			restore;
 	bool			noflush;
 	int8_t			config_done;
-- 
2.25.1

[iptables PATCH v2 14/18] nft-cache: Fetch cache per table

[Phil Sutter]

Restore per-table operation of cache routines as initially implemented
in commit e2883c5531e6e ("nft-cache: Support partial cache per table").

As before, this doesn't limit fetching of tables (their number is
supposed to be low) but instead limits fetching of sets, chains and
rules to the specified table.

For this to behave correctly when restoring without flushing over
multiple tables, cache must be freed fully after each commit - otherwise
the previous table's cache level is reused for the current one. The
exception being fake cache, used for flushing restore: NFT_CL_FAKE is
set just once at program startup, so it must stay set otherwise
consecutive tables cause pointless cache fetching.

The sole use-case requiring a multi-table cache, iptables-save, is
indicated by req->table being NULL. Therefore, req->table assignment is
a bit sloppy: All calls to nft_cache_level_set() are assumed to set the
same table value, no collision detection happens.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft-cache.c       | 33 ++++++++++++++--------
 iptables/nft-cache.h       |  4 ++-
 iptables/nft-cmd.c         | 57 +++++++++++++++++++-------------------
 iptables/nft.h             |  1 +
 iptables/xtables-restore.c |  2 +-
 iptables/xtables-save.c    |  2 +-
 6 files changed, 56 insertions(+), 43 deletions(-)

diff --git a/iptables/nft-cache.c b/iptables/nft-cache.c
index 305f2c12307f7..5cbe7b80d084d 100644
--- a/iptables/nft-cache.c
+++ b/iptables/nft-cache.c
@@ -24,14 +24,18 @@
 #include "nft.h"
 #include "nft-cache.h"
 
-void nft_cache_level_set(struct nft_handle *h, int level)
+void nft_cache_level_set(struct nft_handle *h, int level,
+			 const struct nft_cmd *cmd)
 {
 	struct nft_cache_req *req = &h->cache_req;
 
-	if (level <= req->level)
+	if (level > req->level)
+		req->level = level;
+
+	if (!cmd)
 		return;
 
-	req->level = level;
+	req->table = cmd->table;
 }
 
 static int genid_cb(const struct nlmsghdr *nlh, void *data)
@@ -435,10 +439,14 @@ static void
 __nft_build_cache(struct nft_handle *h)
 {
 	struct nft_cache_req *req = &h->cache_req;
+	const struct builtin_table *t = NULL;
 
 	if (h->cache_init)
 		return;
 
+	if (req->table)
+		t = nft_table_builtin_find(h, req->table);
+
 	h->cache_init = true;
 	mnl_genid_get(h, &h->nft_genid);
 
@@ -447,11 +455,11 @@ __nft_build_cache(struct nft_handle *h)
 	if (req->level == NFT_CL_FAKE)
 		return;
 	if (req->level >= NFT_CL_CHAINS)
-		fetch_chain_cache(h, NULL, NULL);
+		fetch_chain_cache(h, t, NULL);
 	if (req->level >= NFT_CL_SETS)
-		fetch_set_cache(h, NULL, NULL);
+		fetch_set_cache(h, t, NULL);
 	if (req->level >= NFT_CL_RULES)
-		fetch_rule_cache(h, NULL);
+		fetch_rule_cache(h, t);
 }
 
 static void __nft_flush_cache(struct nft_handle *h)
@@ -575,14 +583,17 @@ void nft_cache_build(struct nft_handle *h)
 
 void nft_release_cache(struct nft_handle *h)
 {
-	if (!h->cache_index)
-		return;
+	struct nft_cache_req *req = &h->cache_req;
 
+	while (h->cache_index)
+		flush_cache(h, &h->__cache[h->cache_index--], NULL);
 	flush_cache(h, &h->__cache[0], NULL);
-	memcpy(&h->__cache[0], &h->__cache[1], sizeof(h->__cache[0]));
-	memset(&h->__cache[1], 0, sizeof(h->__cache[1]));
-	h->cache_index = 0;
 	h->cache = &h->__cache[0];
+	h->cache_init = false;
+
+	if (req->level != NFT_CL_FAKE)
+		req->level = NFT_CL_TABLES;
+	req->table = NULL;
 }
 
 struct nftnl_table_list *nftnl_table_list_get(struct nft_handle *h)
diff --git a/iptables/nft-cache.h b/iptables/nft-cache.h
index 01dd15e145fd4..f429118041be4 100644
--- a/iptables/nft-cache.h
+++ b/iptables/nft-cache.h
@@ -2,8 +2,10 @@
 #define _NFT_CACHE_H_
 
 struct nft_handle;
+struct nft_cmd;
 
-void nft_cache_level_set(struct nft_handle *h, int level);
+void nft_cache_level_set(struct nft_handle *h, int level,
+			 const struct nft_cmd *cmd);
 void nft_rebuild_cache(struct nft_handle *h);
 void nft_release_cache(struct nft_handle *h);
 void flush_chain_cache(struct nft_handle *h, const char *tablename);
diff --git a/iptables/nft-cmd.c b/iptables/nft-cmd.c
index be9fbbf5a19bd..64889f5eb6196 100644
--- a/iptables/nft-cmd.c
+++ b/iptables/nft-cmd.c
@@ -63,12 +63,11 @@ void nft_cmd_free(struct nft_cmd *cmd)
 	free(cmd);
 }
 
-static void nft_cmd_rule_bridge(struct nft_handle *h, const char *chain,
-				const char *table)
+static void nft_cmd_rule_bridge(struct nft_handle *h, const struct nft_cmd *cmd)
 {
 	const struct builtin_table *t;
 
-	t = nft_table_builtin_find(h, table);
+	t = nft_table_builtin_find(h, cmd->table);
 	if (!t)
 		return;
 
@@ -76,10 +75,10 @@ static void nft_cmd_rule_bridge(struct nft_handle *h, const char *chain,
 	 * rule in nftables, rule cache is required here to treat them right.
 	 */
 	if (h->family == NFPROTO_BRIDGE &&
-	    !nft_chain_builtin_find(t, chain))
-		nft_cache_level_set(h, NFT_CL_RULES);
+	    !nft_chain_builtin_find(t, cmd->chain))
+		nft_cache_level_set(h, NFT_CL_RULES, cmd);
 	else
-		nft_cache_level_set(h, NFT_CL_CHAINS);
+		nft_cache_level_set(h, NFT_CL_CHAINS, cmd);
 }
 
 int nft_cmd_rule_append(struct nft_handle *h, const char *chain,
@@ -88,13 +87,13 @@ int nft_cmd_rule_append(struct nft_handle *h, const char *chain,
 {
 	struct nft_cmd *cmd;
 
-	nft_cmd_rule_bridge(h, chain, table);
-
 	cmd = nft_cmd_new(h, NFT_COMPAT_RULE_APPEND, table, chain, state, -1,
 			  verbose);
 	if (!cmd)
 		return 0;
 
+	nft_cmd_rule_bridge(h, cmd);
+
 	return 1;
 }
 
@@ -104,17 +103,17 @@ int nft_cmd_rule_insert(struct nft_handle *h, const char *chain,
 {
 	struct nft_cmd *cmd;
 
-	nft_cmd_rule_bridge(h, chain, table);
-
 	cmd = nft_cmd_new(h, NFT_COMPAT_RULE_INSERT, table, chain, state,
 			  rulenum, verbose);
 	if (!cmd)
 		return 0;
 
+	nft_cmd_rule_bridge(h, cmd);
+
 	if (cmd->rulenum > 0)
-		nft_cache_level_set(h, NFT_CL_RULES);
+		nft_cache_level_set(h, NFT_CL_RULES, cmd);
 	else
-		nft_cache_level_set(h, NFT_CL_CHAINS);
+		nft_cache_level_set(h, NFT_CL_CHAINS, cmd);
 
 	return 1;
 }
@@ -130,7 +129,7 @@ int nft_cmd_rule_delete(struct nft_handle *h, const char *chain,
 	if (!cmd)
 		return 0;
 
-	nft_cache_level_set(h, NFT_CL_RULES);
+	nft_cache_level_set(h, NFT_CL_RULES, cmd);
 
 	return 1;
 }
@@ -145,7 +144,7 @@ int nft_cmd_rule_delete_num(struct nft_handle *h, const char *chain,
 	if (!cmd)
 		return 0;
 
-	nft_cache_level_set(h, NFT_CL_RULES);
+	nft_cache_level_set(h, NFT_CL_RULES, cmd);
 
 	return 1;
 }
@@ -160,7 +159,7 @@ int nft_cmd_rule_flush(struct nft_handle *h, const char *chain,
 	if (!cmd)
 		return 0;
 
-	nft_cache_level_set(h, NFT_CL_CHAINS);
+	nft_cache_level_set(h, NFT_CL_CHAINS, cmd);
 
 	return 1;
 }
@@ -175,7 +174,7 @@ int nft_cmd_chain_zero_counters(struct nft_handle *h, const char *chain,
 	if (!cmd)
 		return 0;
 
-	nft_cache_level_set(h, NFT_CL_CHAINS);
+	nft_cache_level_set(h, NFT_CL_CHAINS, cmd);
 
 	return 1;
 }
@@ -190,7 +189,7 @@ int nft_cmd_chain_user_add(struct nft_handle *h, const char *chain,
 	if (!cmd)
 		return 0;
 
-	nft_cache_level_set(h, NFT_CL_CHAINS);
+	nft_cache_level_set(h, NFT_CL_CHAINS, cmd);
 
 	return 1;
 }
@@ -209,9 +208,9 @@ int nft_cmd_chain_user_del(struct nft_handle *h, const char *chain,
 	 * rule cache.
 	 */
 	if (h->family == NFPROTO_BRIDGE)
-		nft_cache_level_set(h, NFT_CL_RULES);
+		nft_cache_level_set(h, NFT_CL_RULES, cmd);
 	else
-		nft_cache_level_set(h, NFT_CL_CHAINS);
+		nft_cache_level_set(h, NFT_CL_CHAINS, cmd);
 
 	return 1;
 }
@@ -228,7 +227,7 @@ int nft_cmd_chain_user_rename(struct nft_handle *h,const char *chain,
 
 	cmd->rename = strdup(newname);
 
-	nft_cache_level_set(h, NFT_CL_CHAINS);
+	nft_cache_level_set(h, NFT_CL_CHAINS, cmd);
 
 	return 1;
 }
@@ -245,7 +244,7 @@ int nft_cmd_rule_list(struct nft_handle *h, const char *chain,
 
 	cmd->format = format;
 
-	nft_cache_level_set(h, NFT_CL_RULES);
+	nft_cache_level_set(h, NFT_CL_RULES, cmd);
 
 	return 1;
 }
@@ -261,7 +260,7 @@ int nft_cmd_rule_replace(struct nft_handle *h, const char *chain,
 	if (!cmd)
 		return 0;
 
-	nft_cache_level_set(h, NFT_CL_RULES);
+	nft_cache_level_set(h, NFT_CL_RULES, cmd);
 
 	return 1;
 }
@@ -276,7 +275,7 @@ int nft_cmd_rule_check(struct nft_handle *h, const char *chain,
 	if (!cmd)
 		return 0;
 
-	nft_cache_level_set(h, NFT_CL_RULES);
+	nft_cache_level_set(h, NFT_CL_RULES, cmd);
 
 	return 1;
 }
@@ -296,7 +295,7 @@ int nft_cmd_chain_set(struct nft_handle *h, const char *table,
 	if (counters)
 		cmd->counters = *counters;
 
-	nft_cache_level_set(h, NFT_CL_CHAINS);
+	nft_cache_level_set(h, NFT_CL_CHAINS, cmd);
 
 	return 1;
 }
@@ -310,7 +309,7 @@ int nft_cmd_table_flush(struct nft_handle *h, const char *table)
 	if (!cmd)
 		return 0;
 
-	nft_cache_level_set(h, NFT_CL_TABLES);
+	nft_cache_level_set(h, NFT_CL_TABLES, cmd);
 
 	return 1;
 }
@@ -325,7 +324,7 @@ int nft_cmd_chain_restore(struct nft_handle *h, const char *chain,
 	if (!cmd)
 		return 0;
 
-	nft_cache_level_set(h, NFT_CL_CHAINS);
+	nft_cache_level_set(h, NFT_CL_CHAINS, cmd);
 
 	return 1;
 }
@@ -340,7 +339,7 @@ int nft_cmd_rule_zero_counters(struct nft_handle *h, const char *chain,
 	if (!cmd)
 		return 0;
 
-	nft_cache_level_set(h, NFT_CL_RULES);
+	nft_cache_level_set(h, NFT_CL_RULES, cmd);
 
 	return 1;
 }
@@ -357,7 +356,7 @@ int nft_cmd_rule_list_save(struct nft_handle *h, const char *chain,
 
 	cmd->counters_save = counters;
 
-	nft_cache_level_set(h, NFT_CL_RULES);
+	nft_cache_level_set(h, NFT_CL_RULES, cmd);
 
 	return 1;
 }
@@ -374,7 +373,7 @@ int ebt_cmd_user_chain_policy(struct nft_handle *h, const char *table,
 
 	cmd->policy = strdup(policy);
 
-	nft_cache_level_set(h, NFT_CL_RULES);
+	nft_cache_level_set(h, NFT_CL_RULES, cmd);
 
 	return 1;
 }
diff --git a/iptables/nft.h b/iptables/nft.h
index c6aece7d1dac8..50bcc0dfebecf 100644
--- a/iptables/nft.h
+++ b/iptables/nft.h
@@ -73,6 +73,7 @@ enum obj_update_type {
 
 struct nft_cache_req {
 	enum nft_cache_level	level;
+	const char		*table;
 };
 
 struct nft_handle {
diff --git a/iptables/xtables-restore.c b/iptables/xtables-restore.c
index ca01d17eba566..44eaf8ab6c483 100644
--- a/iptables/xtables-restore.c
+++ b/iptables/xtables-restore.c
@@ -259,7 +259,7 @@ void xtables_restore_parse(struct nft_handle *h,
 	char buffer[10240] = {};
 
 	if (!h->noflush)
-		nft_cache_level_set(h, NFT_CL_FAKE);
+		nft_cache_level_set(h, NFT_CL_FAKE, NULL);
 
 	line = 0;
 	while (fgets(buffer, sizeof(buffer), p->in)) {
diff --git a/iptables/xtables-save.c b/iptables/xtables-save.c
index f927aa6e9e404..0ce66e5d15cee 100644
--- a/iptables/xtables-save.c
+++ b/iptables/xtables-save.c
@@ -239,7 +239,7 @@ xtables_save_main(int family, int argc, char *argv[],
 		exit(EXIT_FAILURE);
 	}
 
-	nft_cache_level_set(&h, NFT_CL_RULES);
+	nft_cache_level_set(&h, NFT_CL_RULES, NULL);
 	nft_cache_build(&h);
 
 	ret = do_output(&h, tablename, &d);
-- 
2.25.1

[iptables PATCH v2 15/18] nft-cache: Introduce __fetch_chain_cache()

[Phil Sutter]

Extract the inner part of fetch_chain_cache() into a dedicated function,
preparing for individual chain caching.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft-cache.c | 50 ++++++++++++++++++++++++++------------------
 1 file changed, 30 insertions(+), 20 deletions(-)

diff --git a/iptables/nft-cache.c b/iptables/nft-cache.c
index 5cbe7b80d084d..904c9a8217dac 100644
--- a/iptables/nft-cache.c
+++ b/iptables/nft-cache.c
@@ -318,9 +318,9 @@ static int fetch_set_cache(struct nft_handle *h,
 	return ret;
 }
 
-static int fetch_chain_cache(struct nft_handle *h,
-			     const struct builtin_table *t,
-			     const char *chain)
+static int __fetch_chain_cache(struct nft_handle *h,
+			       const struct builtin_table *t,
+			       const struct nftnl_chain *c)
 {
 	struct nftnl_chain_list_cb_data d = {
 		.h = h,
@@ -330,24 +330,10 @@ static int fetch_chain_cache(struct nft_handle *h,
 	struct nlmsghdr *nlh;
 	int ret;
 
-	if (t && chain) {
-		struct nftnl_chain *c = nftnl_chain_alloc();
-
-		if (!c)
-			return -1;
-
-		nlh = nftnl_chain_nlmsg_build_hdr(buf, NFT_MSG_GETCHAIN,
-						  h->family, NLM_F_ACK,
-						  h->seq);
-		nftnl_chain_set_str(c, NFTNL_CHAIN_TABLE, t->name);
-		nftnl_chain_set_str(c, NFTNL_CHAIN_NAME, chain);
+	nlh = nftnl_chain_nlmsg_build_hdr(buf, NFT_MSG_GETCHAIN, h->family,
+					  c ? NLM_F_ACK : NLM_F_DUMP, h->seq);
+	if (c)
 		nftnl_chain_nlmsg_build_payload(nlh, c);
-		nftnl_chain_free(c);
-	} else {
-		nlh = nftnl_chain_nlmsg_build_hdr(buf, NFT_MSG_GETCHAIN,
-						  h->family, NLM_F_DUMP,
-						  h->seq);
-	}
 
 	ret = mnl_talk(h, nlh, nftnl_chain_list_cb, &d);
 	if (ret < 0 && errno == EINTR)
@@ -356,6 +342,30 @@ static int fetch_chain_cache(struct nft_handle *h,
 	return ret;
 }
 
+static int fetch_chain_cache(struct nft_handle *h,
+			     const struct builtin_table *t,
+			     const char *chain)
+{
+	struct nftnl_chain *c;
+	int ret;
+
+	if (!chain)
+		return __fetch_chain_cache(h, t, NULL);
+
+	assert(t);
+
+	c = nftnl_chain_alloc();
+	if (!c)
+		return -1;
+
+	nftnl_chain_set_str(c, NFTNL_CHAIN_TABLE, t->name);
+	nftnl_chain_set_str(c, NFTNL_CHAIN_NAME, chain);
+	ret = __fetch_chain_cache(h, t, c);
+
+	nftnl_chain_free(c);
+	return ret;
+}
+
 static int nftnl_rule_list_cb(const struct nlmsghdr *nlh, void *data)
 {
 	struct nftnl_chain *c = data;
-- 
2.25.1

[iptables PATCH v2 16/18] nft: cache: Fetch cache for specific chains

[Phil Sutter]

Iterate over command list and collect chains to cache. Insert them into
a sorted list to pass to __nft_build_cache().

If a command is interested in all chains (e.g., --list), cmd->chain
remains unset. To record this case reliably, use a boolean
('all_chains'). Otherwise, it is hard to distinguish between first call
to nft_cache_level_set() and previous command with NULL cmd->chain
value.

When caching only specific chains, manually add builtin ones for the
given table as well - otherwise nft_xt_builtin_init() will act as if
they don't exist and possibly override non-default chain policies.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft-cache.c | 82 +++++++++++++++++++++++++++++++++++++++-----
 iptables/nft.c       |  1 +
 iptables/nft.h       |  7 ++++
 3 files changed, 82 insertions(+), 8 deletions(-)

diff --git a/iptables/nft-cache.c b/iptables/nft-cache.c
index 904c9a8217dac..83af9a2f689e1 100644
--- a/iptables/nft-cache.c
+++ b/iptables/nft-cache.c
@@ -11,6 +11,7 @@
 
 #include <assert.h>
 #include <errno.h>
+#include <stdlib.h>
 #include <string.h>
 #include <xtables.h>
 
@@ -24,6 +25,24 @@
 #include "nft.h"
 #include "nft-cache.h"
 
+static void cache_chain_list_insert(struct list_head *list, const char *name)
+{
+	struct cache_chain *pos = NULL, *new;
+
+	list_for_each_entry(pos, list, head) {
+		int cmp = strcmp(pos->name, name);
+
+		if (!cmp)
+			return;
+		if (cmp > 0)
+			break;
+	}
+
+	new = xtables_malloc(sizeof(*new));
+	new->name = name;
+	list_add_tail(&new->head, pos ? &pos->head : list);
+}
+
 void nft_cache_level_set(struct nft_handle *h, int level,
 			 const struct nft_cmd *cmd)
 {
@@ -32,10 +51,21 @@ void nft_cache_level_set(struct nft_handle *h, int level,
 	if (level > req->level)
 		req->level = level;
 
-	if (!cmd)
+	if (!cmd || !cmd->table || req->all_chains)
 		return;
 
 	req->table = cmd->table;
+
+	if (!cmd->chain) {
+		req->all_chains = true;
+		return;
+	}
+
+	cache_chain_list_insert(&req->chain_list, cmd->chain);
+	if (cmd->rename)
+		cache_chain_list_insert(&req->chain_list, cmd->rename);
+	if (cmd->jumpto)
+		cache_chain_list_insert(&req->chain_list, cmd->jumpto);
 }
 
 static int genid_cb(const struct nlmsghdr *nlh, void *data)
@@ -344,12 +374,13 @@ static int __fetch_chain_cache(struct nft_handle *h,
 
 static int fetch_chain_cache(struct nft_handle *h,
 			     const struct builtin_table *t,
-			     const char *chain)
+			     struct list_head *chains)
 {
+	struct cache_chain *cc;
 	struct nftnl_chain *c;
-	int ret;
+	int rc, ret = 0;
 
-	if (!chain)
+	if (!chains)
 		return __fetch_chain_cache(h, t, NULL);
 
 	assert(t);
@@ -359,8 +390,13 @@ static int fetch_chain_cache(struct nft_handle *h,
 		return -1;
 
 	nftnl_chain_set_str(c, NFTNL_CHAIN_TABLE, t->name);
-	nftnl_chain_set_str(c, NFTNL_CHAIN_NAME, chain);
-	ret = __fetch_chain_cache(h, t, c);
+
+	list_for_each_entry(cc, chains, head) {
+		nftnl_chain_set_str(c, NFTNL_CHAIN_NAME, cc->name);
+		rc = __fetch_chain_cache(h, t, c);
+		if (rc)
+			ret = rc;
+	}
 
 	nftnl_chain_free(c);
 	return ret;
@@ -450,12 +486,16 @@ __nft_build_cache(struct nft_handle *h)
 {
 	struct nft_cache_req *req = &h->cache_req;
 	const struct builtin_table *t = NULL;
+	struct list_head *chains = NULL;
 
 	if (h->cache_init)
 		return;
 
-	if (req->table)
+	if (req->table) {
 		t = nft_table_builtin_find(h, req->table);
+		if (!req->all_chains)
+			chains = &req->chain_list;
+	}
 
 	h->cache_init = true;
 	mnl_genid_get(h, &h->nft_genid);
@@ -465,7 +505,7 @@ __nft_build_cache(struct nft_handle *h)
 	if (req->level == NFT_CL_FAKE)
 		return;
 	if (req->level >= NFT_CL_CHAINS)
-		fetch_chain_cache(h, t, NULL);
+		fetch_chain_cache(h, t, chains);
 	if (req->level >= NFT_CL_SETS)
 		fetch_set_cache(h, t, NULL);
 	if (req->level >= NFT_CL_RULES)
@@ -588,12 +628,32 @@ void nft_rebuild_cache(struct nft_handle *h)
 
 void nft_cache_build(struct nft_handle *h)
 {
+	struct nft_cache_req *req = &h->cache_req;
+	const struct builtin_table *t = NULL;
+	int i;
+
+	if (req->table)
+		t = nft_table_builtin_find(h, req->table);
+
+	/* fetch builtin chains as well (if existing) so nft_xt_builtin_init()
+	 * doesn't override policies by accident */
+	if (t && !req->all_chains) {
+		for (i = 0; i < NF_INET_NUMHOOKS; i++) {
+			const char *cname = t->chains[i].name;
+
+			if (!cname)
+				break;
+			cache_chain_list_insert(&req->chain_list, cname);
+		}
+	}
+
 	__nft_build_cache(h);
 }
 
 void nft_release_cache(struct nft_handle *h)
 {
 	struct nft_cache_req *req = &h->cache_req;
+	struct cache_chain *cc, *cc_tmp;
 
 	while (h->cache_index)
 		flush_cache(h, &h->__cache[h->cache_index--], NULL);
@@ -604,6 +664,12 @@ void nft_release_cache(struct nft_handle *h)
 	if (req->level != NFT_CL_FAKE)
 		req->level = NFT_CL_TABLES;
 	req->table = NULL;
+
+	req->all_chains = false;
+	list_for_each_entry_safe(cc, cc_tmp, &req->chain_list, head) {
+		list_del(&cc->head);
+		free(cc);
+	}
 }
 
 struct nftnl_table_list *nftnl_table_list_get(struct nft_handle *h)
diff --git a/iptables/nft.c b/iptables/nft.c
index f9e53316ab7cf..5b255477f27f7 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -805,6 +805,7 @@ int nft_init(struct nft_handle *h, int family, const struct builtin_table *t)
 	INIT_LIST_HEAD(&h->obj_list);
 	INIT_LIST_HEAD(&h->err_list);
 	INIT_LIST_HEAD(&h->cmd_list);
+	INIT_LIST_HEAD(&h->cache_req.chain_list);
 
 	return 0;
 }
diff --git a/iptables/nft.h b/iptables/nft.h
index 50bcc0dfebecf..045393da7c179 100644
--- a/iptables/nft.h
+++ b/iptables/nft.h
@@ -71,9 +71,16 @@ enum obj_update_type {
 	NFT_COMPAT_TABLE_NEW,
 };
 
+struct cache_chain {
+	struct list_head head;
+	const char *name;
+};
+
 struct nft_cache_req {
 	enum nft_cache_level	level;
 	const char		*table;
+	bool			all_chains;
+	struct list_head	chain_list;
 };
 
 struct nft_handle {
-- 
2.25.1

[iptables PATCH v2 17/18] nft: cache: Optimize caching for flush command

[Phil Sutter]

When flushing all chains and verbose mode is not enabled,
nft_rule_flush() uses a shortcut: It doesn't specify a chain name for
NFT_MSG_DELRULE, so the kernel will flush all existing chains without
user space needing to know which they are.

The above allows to avoid a chain cache, but there's a caveat:
nft_xt_builtin_init() will create base chains as it assumes they are
missing and thereby possibly overrides any non-default chain policies.

Solve this by making nft_xt_builtin_init() cache-aware: If a command
doesn't need a chain cache, there's no need to bother with creating any
non-existing builtin chains, either. For the sake of completeness, also
do nothing if cache is not initialized (although that shouldn't happen).

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft-cmd.c                            |  5 +++-
 iptables/nft.c                                |  6 ++++
 .../testcases/nft-only/0006-policy-override_0 | 29 +++++++++++++++++++
 3 files changed, 39 insertions(+), 1 deletion(-)
 create mode 100755 iptables/tests/shell/testcases/nft-only/0006-policy-override_0

diff --git a/iptables/nft-cmd.c b/iptables/nft-cmd.c
index 64889f5eb6196..3c0c6a34515e4 100644
--- a/iptables/nft-cmd.c
+++ b/iptables/nft-cmd.c
@@ -159,7 +159,10 @@ int nft_cmd_rule_flush(struct nft_handle *h, const char *chain,
 	if (!cmd)
 		return 0;
 
-	nft_cache_level_set(h, NFT_CL_CHAINS, cmd);
+	if (chain || verbose)
+		nft_cache_level_set(h, NFT_CL_CHAINS, cmd);
+	else
+		nft_cache_level_set(h, NFT_CL_TABLES, cmd);
 
 	return 1;
 }
diff --git a/iptables/nft.c b/iptables/nft.c
index 5b255477f27f7..67b8466b50692 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -737,6 +737,9 @@ static int nft_xt_builtin_init(struct nft_handle *h, const char *table)
 {
 	const struct builtin_table *t;
 
+	if (!h->cache_init)
+		return 0;
+
 	t = nft_table_builtin_find(h, table);
 	if (t == NULL)
 		return -1;
@@ -747,6 +750,9 @@ static int nft_xt_builtin_init(struct nft_handle *h, const char *table)
 	if (nft_table_builtin_add(h, t) < 0)
 		return -1;
 
+	if (h->cache_req.level < NFT_CL_CHAINS)
+		return 0;
+
 	nft_chain_builtin_init(h, t);
 
 	h->cache->table[t->type].initialized = true;
diff --git a/iptables/tests/shell/testcases/nft-only/0006-policy-override_0 b/iptables/tests/shell/testcases/nft-only/0006-policy-override_0
new file mode 100755
index 0000000000000..68e2019b83119
--- /dev/null
+++ b/iptables/tests/shell/testcases/nft-only/0006-policy-override_0
@@ -0,0 +1,29 @@
+#!/bin/bash
+
+[[ $XT_MULTI == *xtables-nft-multi ]] || { echo "skip $XT_MULTI"; exit 0; }
+
+# make sure none of the commands invoking nft_xt_builtin_init() override
+# non-default chain policies via needless chain add.
+
+RC=0
+
+do_test() {
+	$XT_MULTI $@
+	$XT_MULTI iptables -S | grep -q -- '-P FORWARD DROP' && return
+
+	echo "command '$@' kills chain policies"
+	$XT_MULTI iptables -P FORWARD DROP
+	RC=1
+}
+
+$XT_MULTI iptables -P FORWARD DROP
+
+do_test iptables -A OUTPUT -j ACCEPT
+do_test iptables -F
+do_test iptables -N foo
+do_test iptables -E foo foo2
+do_test iptables -I OUTPUT -j ACCEPT
+do_test iptables -nL
+do_test iptables -S
+
+exit $RC
-- 
2.25.1

[iptables PATCH v2 18/18] nft: Fix for '-F' in iptables dumps

[Phil Sutter]

When restoring a dump which contains an explicit flush command,
previously added rules are removed from cache and the following commit
will try to create netlink messages based on freed memory.

Fix this by weeding any rule-based commands from obj_list if they
address the same chain.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/iptables/nft.c b/iptables/nft.c
index 67b8466b50692..d2796fcd8ad26 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -402,6 +402,38 @@ batch_rule_add(struct nft_handle *h, enum obj_update_type type,
 	return batch_add(h, type, r);
 }
 
+static void batch_obj_del(struct nft_handle *h, struct obj_update *o);
+
+static void batch_chain_flush(struct nft_handle *h,
+			      const char *table, const char *chain)
+{
+	struct obj_update *obj, *tmp;
+
+	list_for_each_entry_safe(obj, tmp, &h->obj_list, head) {
+		struct nftnl_rule *r = obj->ptr;
+
+		switch (obj->type) {
+		case NFT_COMPAT_RULE_APPEND:
+		case NFT_COMPAT_RULE_INSERT:
+		case NFT_COMPAT_RULE_REPLACE:
+		case NFT_COMPAT_RULE_DELETE:
+			break;
+		default:
+			continue;
+		}
+
+		if (table &&
+		    strcmp(table, nftnl_rule_get_str(r, NFTNL_RULE_TABLE)))
+			continue;
+
+		if (chain &&
+		    strcmp(chain, nftnl_rule_get_str(r, NFTNL_RULE_CHAIN)))
+			continue;
+
+		batch_obj_del(h, obj);
+	}
+}
+
 const struct builtin_table xtables_ipv4[NFT_TABLE_MAX] = {
 	[NFT_TABLE_RAW] = {
 		.name	= "raw",
@@ -1681,6 +1713,7 @@ int nft_rule_flush(struct nft_handle *h, const char *chain, const char *table,
 	}
 
 	if (chain || !verbose) {
+		batch_chain_flush(h, table, chain);
 		__nft_rule_flush(h, table, chain, verbose, false);
 		flush_rule_cache(h, table, c);
 		return 1;
@@ -1696,6 +1729,7 @@ int nft_rule_flush(struct nft_handle *h, const char *chain, const char *table,
 	while (c != NULL) {
 		chain = nftnl_chain_get_str(c, NFTNL_CHAIN_NAME);
 
+		batch_chain_flush(h, table, chain);
 		__nft_rule_flush(h, table, chain, verbose, false);
 		flush_rule_cache(h, table, c);
 		c = nftnl_chain_list_iter_next(iter);
-- 
2.25.1

Re: [iptables PATCH v2 00/18] iptables: introduce cache evaluation phase

[Pablo Neira Ayuso]

Hi Phil,

On Tue, Apr 28, 2020 at 02:09:55PM +0200, Phil Sutter wrote:
> Hi Pablo,
> 
> As promised, here's a revised version of your cache rework series from
> January. It restores performance according to my tests (which are yet to
> be published somewhere) and passes the testsuites.

I did not test this yet, and I made a few rounds of quick reviews
alrady, but this series LGTM. Thank you for working on this.

Re: [iptables PATCH v2 00/18] iptables: introduce cache evaluation phase

[Phil Sutter]

Hi Pablo,

On Wed, Apr 29, 2020 at 11:36:09PM +0200, Pablo Neira Ayuso wrote:
> On Tue, Apr 28, 2020 at 02:09:55PM +0200, Phil Sutter wrote:
> > Hi Pablo,
> > 
> > As promised, here's a revised version of your cache rework series from
> > January. It restores performance according to my tests (which are yet to
> > be published somewhere) and passes the testsuites.
> 
> I did not test this yet, and I made a few rounds of quick reviews
> alrady, but this series LGTM. Thank you for working on this.

Cool! Should I push it or do you want to have a closer look first?

Cheers, Phil

Re: [iptables PATCH v2 00/18] iptables: introduce cache evaluation phase

[Pablo Neira Ayuso]

On Thu, Apr 30, 2020 at 03:53:00PM +0200, Phil Sutter wrote:
> Hi Pablo,
>
> On Wed, Apr 29, 2020 at 11:36:09PM +0200, Pablo Neira Ayuso wrote:
> > On Tue, Apr 28, 2020 at 02:09:55PM +0200, Phil Sutter wrote:
> > > Hi Pablo,
> > >
> > > As promised, here's a revised version of your cache rework series from
> > > January. It restores performance according to my tests (which are yet to
> > > be published somewhere) and passes the testsuites.
> >
> > I did not test this yet, and I made a few rounds of quick reviews
> > alrady, but this series LGTM. Thank you for working on this.
>
> Cool! Should I push it or do you want to have a closer look first?

You already took the time to test this, so I think it's fine if you
push out. Problems can be fixed from master. It would also good a few
runs to valgrind.

BTW, this cache consistency check

commit 200bc399651499f502ac0de45f4d4aa4c9d37ab6
Author: Phil Sutter <phil@nwl.cc>
Date:   Fri Mar 13 13:02:12 2020 +0100

    nft: cache: Fix iptables-save segfault under stress

is already restored in this series, right?

Thanks.

Re: [iptables PATCH v2 00/18] iptables: introduce cache evaluation phase

[Phil Sutter]

Hi,

On Thu, Apr 30, 2020 at 05:08:31PM +0200, Pablo Neira Ayuso wrote:
> On Thu, Apr 30, 2020 at 03:53:00PM +0200, Phil Sutter wrote:
> > Hi Pablo,
> >
> > On Wed, Apr 29, 2020 at 11:36:09PM +0200, Pablo Neira Ayuso wrote:
> > > On Tue, Apr 28, 2020 at 02:09:55PM +0200, Phil Sutter wrote:
> > > > Hi Pablo,
> > > >
> > > > As promised, here's a revised version of your cache rework series from
> > > > January. It restores performance according to my tests (which are yet to
> > > > be published somewhere) and passes the testsuites.
> > >
> > > I did not test this yet, and I made a few rounds of quick reviews
> > > alrady, but this series LGTM. Thank you for working on this.
> >
> > Cool! Should I push it or do you want to have a closer look first?
> 
> You already took the time to test this, so I think it's fine if you
> push out. Problems can be fixed from master. It would also good a few
> runs to valgrind.

OK, I'll play a bit with valgrind just to be sure and then push it out.

> BTW, this cache consistency check
> 
> commit 200bc399651499f502ac0de45f4d4aa4c9d37ab6
> Author: Phil Sutter <phil@nwl.cc>
> Date:   Fri Mar 13 13:02:12 2020 +0100
> 
>     nft: cache: Fix iptables-save segfault under stress
> 
> is already restored in this series, right?

Yes, IIRC this was the reason why I got a merge conflict upon rebase.
But the problem shouldn't exist with the new logic: We fetch cache just
once, so there is no cache update (and potential cache free) happening
while iterating through chain lists or anything.

Cheers, Phil

Re: [iptables PATCH v2 00/18] iptables: introduce cache evaluation phase

[Pablo Neira Ayuso]

On Thu, Apr 30, 2020 at 05:26:07PM +0200, Phil Sutter wrote:
[...]
> > BTW, this cache consistency check
> > 
> > commit 200bc399651499f502ac0de45f4d4aa4c9d37ab6
> > Author: Phil Sutter <phil@nwl.cc>
> > Date:   Fri Mar 13 13:02:12 2020 +0100
> > 
> >     nft: cache: Fix iptables-save segfault under stress
> > 
> > is already restored in this series, right?
> 
> Yes, IIRC this was the reason why I got a merge conflict upon rebase.
> But the problem shouldn't exist with the new logic: We fetch cache just
> once, so there is no cache update (and potential cache free) happening
> while iterating through chain lists or anything.

Still another process might be competing to update the ruleset, right?

Re: [iptables PATCH v2 00/18] iptables: introduce cache evaluation phase

[Pablo Neira Ayuso]

On Thu, Apr 30, 2020 at 05:44:40PM +0200, Pablo Neira Ayuso wrote:
> On Thu, Apr 30, 2020 at 05:26:07PM +0200, Phil Sutter wrote:
> [...]
> > > BTW, this cache consistency check
> > > 
> > > commit 200bc399651499f502ac0de45f4d4aa4c9d37ab6
> > > Author: Phil Sutter <phil@nwl.cc>
> > > Date:   Fri Mar 13 13:02:12 2020 +0100
> > > 
> > >     nft: cache: Fix iptables-save segfault under stress
> > > 
> > > is already restored in this series, right?
> > 
> > Yes, IIRC this was the reason why I got a merge conflict upon rebase.
> > But the problem shouldn't exist with the new logic: We fetch cache just
> > once, so there is no cache update (and potential cache free) happening
> > while iterating through chain lists or anything.
> 
> Still another process might be competing to update the ruleset, right?

I mean this case:

-       mnl_genid_get(h, &genid_stop);
-       if (genid_start != genid_stop) {
-               flush_chain_cache(h, NULL);
-               goto retry;
-       }

if the cache is inconsistent (another process stepped in and updated
the ruleset), the discard this cache and fetch it again.

Re: [iptables PATCH v2 00/18] iptables: introduce cache evaluation phase

[Phil Sutter]

On Thu, Apr 30, 2020 at 05:48:34PM +0200, Pablo Neira Ayuso wrote:
> On Thu, Apr 30, 2020 at 05:44:40PM +0200, Pablo Neira Ayuso wrote:
> > On Thu, Apr 30, 2020 at 05:26:07PM +0200, Phil Sutter wrote:
> > [...]
> > > > BTW, this cache consistency check
> > > > 
> > > > commit 200bc399651499f502ac0de45f4d4aa4c9d37ab6
> > > > Author: Phil Sutter <phil@nwl.cc>
> > > > Date:   Fri Mar 13 13:02:12 2020 +0100
> > > > 
> > > >     nft: cache: Fix iptables-save segfault under stress
> > > > 
> > > > is already restored in this series, right?
> > > 
> > > Yes, IIRC this was the reason why I got a merge conflict upon rebase.
> > > But the problem shouldn't exist with the new logic: We fetch cache just
> > > once, so there is no cache update (and potential cache free) happening
> > > while iterating through chain lists or anything.
> > 
> > Still another process might be competing to update the ruleset, right?
> 
> I mean this case:
> 
> -       mnl_genid_get(h, &genid_stop);
> -       if (genid_start != genid_stop) {
> -               flush_chain_cache(h, NULL);
> -               goto retry;
> -       }
> 
> if the cache is inconsistent (another process stepped in and updated
> the ruleset), the discard this cache and fetch it again.

Ah, I understand your point now. Let me try to reenable this so
__nft_build_cache() returns a cache in which all entries "match"
regarding genid. I think this shouldn't cause problems anymore, even if
cache is freed in between attempts.

Thanks, Phil