[iptables PATCH v3 0/9] nft: Sorted chain listing et al.

[iptables PATCH v3 0/9] nft: Sorted chain listing et al.

[Phil Sutter]

This is a respin of my original series after getting rid of a few
initial ("fallout") patches. It implements structs nft_chain and
nft_chain_list to avoid changes to libnftnl as requested. Obviously this
introduces some code duplication as some bits from libnftnl have to be
replicated within iptables now.

Changes since v2:

* Reworded patch 1 comment to clarify what it fixes.

* Reordered patches so that nft_chain_foreach() introduced in patch
  3 replaces nft_chain_list_get().

* Drop getters previously introduced along with struct nft_chain to
  reduce size of patch 5. Extracting data from embedded nftnl_chain into
  nft_chain and back if needed is future work.

Phil Sutter (9):
  nft: Fix selective chain compatibility checks
  nft: cache: Introduce nft_cache_add_chain()
  nft: Implement nft_chain_foreach()
  nft: cache: Move nft_chain_find() over
  nft: Introduce struct nft_chain
  nft: Introduce a dedicated base chain array
  nft: cache: Sort custom chains by name
  tests: shell: Drop any dump sorting in place
  nft: Avoid pointless table/chain creation

 iptables/Makefile.am                          |   2 +-
 iptables/nft-cache.c                          | 162 ++++++---
 iptables/nft-cache.h                          |  11 +-
 iptables/nft-chain.c                          |  59 ++++
 iptables/nft-chain.h                          |  29 ++
 iptables/nft.c                                | 322 +++++++++++-------
 iptables/nft.h                                |  10 +-
 .../ebtables/0002-ebtables-save-restore_0     |   2 +-
 .../firewalld-restore/0001-firewalld_0        |  17 +-
 .../ipt-restore/0007-flush-noflush_0          |   4 +-
 .../ipt-restore/0014-verbose-restore_0        |   2 +-
 iptables/xtables-save.c                       |   8 +-
 12 files changed, 421 insertions(+), 207 deletions(-)
 create mode 100644 iptables/nft-chain.c
 create mode 100644 iptables/nft-chain.h

-- 
2.28.0

[iptables PATCH v3 1/9] nft: Fix selective chain compatibility checks

[Phil Sutter]

Since commit 80251bc2a56ed ("nft: remove cache build calls"), 'chain'
parameter passed to nft_chain_list_get() is no longer effective.
Before, it was used to fetch only that single chain from kernel when
populating the cache. So the returned list of chains for which
compatibility checks are done would contain only that single chain.

Re-establish the single chain compat checking by introducing a dedicated
code path to nft_is_chain_compatible() doing so.

Fixes: 80251bc2a56ed ("nft: remove cache build calls")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/iptables/nft.c b/iptables/nft.c
index 411e2597205c9..24e49db4ab919 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -3456,6 +3456,12 @@ bool nft_is_table_compatible(struct nft_handle *h,
 {
 	struct nftnl_chain_list *clist;
 
+	if (chain) {
+		struct nftnl_chain *c = nft_chain_find(h, table, chain);
+
+		return c && !nft_is_chain_compatible(c, h);
+	}
+
 	clist = nft_chain_list_get(h, table, chain);
 	if (clist == NULL)
 		return false;
-- 
2.28.0

[iptables PATCH v3 2/9] nft: cache: Introduce nft_cache_add_chain()

[Phil Sutter]

This is a convenience function for adding a chain to cache, for now just
a simple wrapper around nftnl_chain_list_add_tail().

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

diff --git a/iptables/nft-cache.c b/iptables/nft-cache.c
index 32cfd6cf0989a..afa655d73bc63 100644
--- a/iptables/nft-cache.c
+++ b/iptables/nft-cache.c
@@ -165,6 +165,13 @@ static int fetch_table_cache(struct nft_handle *h)
 	return 1;
 }
 
+int nft_cache_add_chain(struct nft_handle *h, const struct builtin_table *t,
+			struct nftnl_chain *c)
+{
+	nftnl_chain_list_add_tail(c, h->cache->table[t->type].chains);
+	return 0;
+}
+
 struct nftnl_chain_list_cb_data {
 	struct nft_handle *h;
 	const struct builtin_table *t;
@@ -174,7 +181,6 @@ static int nftnl_chain_list_cb(const struct nlmsghdr *nlh, void *data)
 {
 	struct nftnl_chain_list_cb_data *d = data;
 	const struct builtin_table *t = d->t;
-	struct nftnl_chain_list *list;
 	struct nft_handle *h = d->h;
 	struct nftnl_chain *c;
 	const char *tname;
@@ -196,8 +202,8 @@ static int nftnl_chain_list_cb(const struct nlmsghdr *nlh, void *data)
 		goto out;
 	}
 
-	list = h->cache->table[t->type].chains;
-	nftnl_chain_list_add_tail(c, list);
+	if (nft_cache_add_chain(h, t, c))
+		goto out;
 
 	return MNL_CB_OK;
 out:
diff --git a/iptables/nft-cache.h b/iptables/nft-cache.h
index 76f9fbb6c8ccc..d97f8de255f02 100644
--- a/iptables/nft-cache.h
+++ b/iptables/nft-cache.h
@@ -3,6 +3,7 @@
 
 struct nft_handle;
 struct nft_cmd;
+struct builtin_table;
 
 void nft_cache_level_set(struct nft_handle *h, int level,
 			 const struct nft_cmd *cmd);
@@ -12,6 +13,8 @@ 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);
+int nft_cache_add_chain(struct nft_handle *h, const struct builtin_table *t,
+			struct nftnl_chain *c);
 
 struct nftnl_chain_list *
 nft_chain_list_get(struct nft_handle *h, const char *table, const char *chain);
diff --git a/iptables/nft.c b/iptables/nft.c
index 24e49db4ab919..d1f6d417785b6 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -697,7 +697,7 @@ static void nft_chain_builtin_add(struct nft_handle *h,
 		return;
 
 	batch_chain_add(h, NFT_COMPAT_CHAIN_ADD, c);
-	nftnl_chain_list_add_tail(c, h->cache->table[table->type].chains);
+	nft_cache_add_chain(h, table, c);
 }
 
 /* find if built-in table already exists */
@@ -1712,7 +1712,7 @@ err:
 
 int nft_chain_user_add(struct nft_handle *h, const char *chain, const char *table)
 {
-	struct nftnl_chain_list *list;
+	const struct builtin_table *t;
 	struct nftnl_chain *c;
 
 	nft_fn = nft_chain_user_add;
@@ -1736,9 +1736,8 @@ int nft_chain_user_add(struct nft_handle *h, const char *chain, const char *tabl
 	if (!batch_chain_add(h, NFT_COMPAT_CHAIN_USER_ADD, c))
 		return 0;
 
-	list = nft_chain_list_get(h, table, chain);
-	if (list)
-		nftnl_chain_list_add(c, list);
+	t = nft_table_builtin_find(h, table);
+	nft_cache_add_chain(h, t, c);
 
 	/* the core expects 1 for success and 0 for error */
 	return 1;
@@ -1746,7 +1745,7 @@ int nft_chain_user_add(struct nft_handle *h, const char *chain, const char *tabl
 
 int nft_chain_restore(struct nft_handle *h, const char *chain, const char *table)
 {
-	struct nftnl_chain_list *list;
+	const struct builtin_table *t;
 	struct obj_update *obj;
 	struct nftnl_chain *c;
 	bool created = false;
@@ -1763,9 +1762,8 @@ int nft_chain_restore(struct nft_handle *h, const char *chain, const char *table
 		nftnl_chain_set_str(c, NFTNL_CHAIN_NAME, chain);
 		created = true;
 
-		list = nft_chain_list_get(h, table, chain);
-		if (list)
-			nftnl_chain_list_add(c, list);
+		t = nft_table_builtin_find(h, table);
+		nft_cache_add_chain(h, t, c);
 	} else {
 		/* If the chain should vanish meanwhile, kernel genid changes
 		 * and the transaction is refreshed enabling the chain add
-- 
2.28.0

[iptables PATCH v3 3/9] nft: Implement nft_chain_foreach()

[Phil Sutter]

This is just a fancy wrapper around nftnl_chain_list_foreach() with the
added benefit of detecting invalid table names or uninitialized chain
lists. This in turn allows to drop the checks in flush_rule_cache() and
ignore the return code of nft_chain_foreach() as it fails only if the
dropped checks had failed, too.

Since this wrapper does the chain list lookup by itself, use of
nft_chain_list_get() shrinks down to a single place, namely inside
nft_chain_find(). Therefore fold it into the latter.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft-cache.c    | 42 +++++---------------
 iptables/nft-cache.h    |  2 -
 iptables/nft.c          | 88 ++++++++++++++++-------------------------
 iptables/nft.h          |  3 ++
 iptables/xtables-save.c |  7 +---
 5 files changed, 46 insertions(+), 96 deletions(-)

diff --git a/iptables/nft-cache.c b/iptables/nft-cache.c
index afa655d73bc63..109524c3fbc79 100644
--- a/iptables/nft-cache.c
+++ b/iptables/nft-cache.c
@@ -461,21 +461,16 @@ static int fetch_rule_cache(struct nft_handle *h,
 {
 	int i;
 
-	if (t) {
-		struct nftnl_chain_list *list =
-			h->cache->table[t->type].chains;
-
-		return nftnl_chain_list_foreach(list, nft_rule_list_update, h);
-	}
+	if (t)
+		return nft_chain_foreach(h, t->name, nft_rule_list_update, h);
 
 	for (i = 0; i < NFT_TABLE_MAX; i++) {
-		enum nft_table_type type = h->tables[i].type;
 
 		if (!h->tables[i].name)
 			continue;
 
-		if (nftnl_chain_list_foreach(h->cache->table[type].chains,
-					     nft_rule_list_update, h))
+		if (nft_chain_foreach(h, h->tables[i].name,
+				      nft_rule_list_update, h))
 			return -1;
 	}
 	return 0;
@@ -549,17 +544,11 @@ static int __flush_rule_cache(struct nftnl_chain *c, void *data)
 int flush_rule_cache(struct nft_handle *h, const char *table,
 		     struct nftnl_chain *c)
 {
-	const struct builtin_table *t;
-
 	if (c)
 		return __flush_rule_cache(c, NULL);
 
-	t = nft_table_builtin_find(h, table);
-	if (!t || !h->cache->table[t->type].chains)
-		return 0;
-
-	return nftnl_chain_list_foreach(h->cache->table[t->type].chains,
-					__flush_rule_cache, NULL);
+	nft_chain_foreach(h, table, __flush_rule_cache, NULL);
+	return 0;
 }
 
 static int __flush_chain_cache(struct nftnl_chain *c, void *data)
@@ -588,9 +577,9 @@ static int flush_cache(struct nft_handle *h, struct nft_cache *c,
 		table = nft_table_builtin_find(h, tablename);
 		if (!table)
 			return 0;
-		if (c->table[table->type].chains)
-			nftnl_chain_list_foreach(c->table[table->type].chains,
-						 __flush_chain_cache, NULL);
+
+		nft_chain_foreach(h, tablename, __flush_chain_cache, NULL);
+
 		if (c->table[table->type].sets)
 			nftnl_set_list_foreach(c->table[table->type].sets,
 					       __flush_set_cache, NULL);
@@ -695,16 +684,3 @@ nft_set_list_get(struct nft_handle *h, const char *table, const char *set)
 
 	return h->cache->table[t->type].sets;
 }
-
-struct nftnl_chain_list *
-nft_chain_list_get(struct nft_handle *h, const char *table, const char *chain)
-{
-	const struct builtin_table *t;
-
-	t = nft_table_builtin_find(h, table);
-	if (!t)
-		return NULL;
-
-	return h->cache->table[t->type].chains;
-}
-
diff --git a/iptables/nft-cache.h b/iptables/nft-cache.h
index d97f8de255f02..52ad2d396199e 100644
--- a/iptables/nft-cache.h
+++ b/iptables/nft-cache.h
@@ -16,8 +16,6 @@ void nft_cache_build(struct nft_handle *h);
 int nft_cache_add_chain(struct nft_handle *h, const struct builtin_table *t,
 			struct nftnl_chain *c);
 
-struct nftnl_chain_list *
-nft_chain_list_get(struct nft_handle *h, const char *table, const char *chain);
 struct nftnl_set_list *
 nft_set_list_get(struct nft_handle *h, const char *table, const char *set);
 
diff --git a/iptables/nft.c b/iptables/nft.c
index d1f6d417785b6..afe7fe9ed05c5 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -1593,14 +1593,9 @@ int nft_rule_save(struct nft_handle *h, const char *table, unsigned int format)
 		.h = h,
 		.format = format,
 	};
-	struct nftnl_chain_list *list;
 	int ret;
 
-	list = nft_chain_list_get(h, table, NULL);
-	if (!list)
-		return 0;
-
-	ret = nftnl_chain_list_foreach(list, nft_rule_save_cb, &d);
+	ret = nft_chain_foreach(h, table, nft_rule_save_cb, &d);
 
 	/* the core expects 1 for success and 0 for error */
 	return ret == 0 ? 1 : 0;
@@ -1672,7 +1667,6 @@ int nft_rule_flush(struct nft_handle *h, const char *chain, const char *table,
 		.table = table,
 		.verbose = verbose,
 	};
-	struct nftnl_chain_list *list;
 	struct nftnl_chain *c = NULL;
 	int ret = 0;
 
@@ -1698,14 +1692,8 @@ int nft_rule_flush(struct nft_handle *h, const char *chain, const char *table,
 		return 1;
 	}
 
-	list = nft_chain_list_get(h, table, chain);
-	if (list == NULL) {
-		ret = 1;
-		goto err;
-	}
+	ret = nft_chain_foreach(h, table, nft_rule_flush_cb, &d);
 
-	ret = nftnl_chain_list_foreach(list, nft_rule_flush_cb, &d);
-err:
 	/* the core expects 1 for success and 0 for error */
 	return ret == 0 ? 1 : 0;
 }
@@ -1825,18 +1813,13 @@ int nft_chain_user_del(struct nft_handle *h, const char *chain,
 		.handle = h,
 		.verbose = verbose,
 	};
-	struct nftnl_chain_list *list;
 	struct nftnl_chain *c;
 	int ret = 0;
 
 	nft_fn = nft_chain_user_del;
 
-	list = nft_chain_list_get(h, table, chain);
-	if (list == NULL)
-		return 0;
-
 	if (chain) {
-		c = nftnl_chain_list_lookup_byname(list, chain);
+		c = nft_chain_find(h, table, chain);
 		if (!c) {
 			errno = ENOENT;
 			return 0;
@@ -1848,7 +1831,7 @@ int nft_chain_user_del(struct nft_handle *h, const char *chain,
 		goto out;
 	}
 
-	ret = nftnl_chain_list_foreach(list, __nft_chain_user_del, &d);
+	ret = nft_chain_foreach(h, table, __nft_chain_user_del, &d);
 out:
 	/* the core expects 1 for success and 0 for error */
 	return ret == 0 ? 1 : 0;
@@ -1857,13 +1840,15 @@ out:
 static struct nftnl_chain *
 nft_chain_find(struct nft_handle *h, const char *table, const char *chain)
 {
+	const struct builtin_table *t;
 	struct nftnl_chain_list *list;
 
-	list = nft_chain_list_get(h, table, chain);
-	if (list == NULL)
+	t = nft_table_builtin_find(h, table);
+	if (!t)
 		return NULL;
 
-	return nftnl_chain_list_lookup_byname(list, chain);
+	list = h->cache->table[t->type].chains;
+	return list ? nftnl_chain_list_lookup_byname(list, chain) : NULL;
 }
 
 bool nft_chain_exists(struct nft_handle *h,
@@ -2375,7 +2360,6 @@ int nft_rule_list(struct nft_handle *h, const char *chain, const char *table,
 		.rulenum = rulenum,
 		.cb = ops->print_rule,
 	};
-	struct nftnl_chain_list *list;
 	struct nftnl_chain *c;
 
 	nft_xt_builtin_init(h, table);
@@ -2395,14 +2379,10 @@ int nft_rule_list(struct nft_handle *h, const char *chain, const char *table,
 		return 1;
 	}
 
-	list = nft_chain_list_get(h, table, chain);
-	if (!list)
-		return 0;
-
 	if (ops->print_table_header)
 		ops->print_table_header(table);
 
-	nftnl_chain_list_foreach(list, nft_rule_list_cb, &d);
+	nft_chain_foreach(h, table, nft_rule_list_cb, &d);
 	return 1;
 }
 
@@ -2413,6 +2393,23 @@ list_save(struct nft_handle *h, struct nftnl_rule *r,
 	nft_rule_print_save(h, r, NFT_RULE_APPEND, format);
 }
 
+int nft_chain_foreach(struct nft_handle *h, const char *table,
+		      int (*cb)(struct nftnl_chain *c, void *data),
+		      void *data)
+{
+	const struct builtin_table *t;
+
+	t = nft_table_builtin_find(h, table);
+	if (!t)
+		return -1;
+
+	if (!h->cache->table[t->type].chains)
+		return -1;
+
+	return nftnl_chain_list_foreach(h->cache->table[t->type].chains,
+					cb, data);
+}
+
 static int nft_rule_list_chain_save(struct nftnl_chain *c, void *data)
 {
 	const char *chain_name = nftnl_chain_get_str(c, NFTNL_CHAIN_NAME);
@@ -2444,24 +2441,19 @@ int nft_rule_list_save(struct nft_handle *h, const char *chain,
 		.save_fmt = true,
 		.cb = list_save,
 	};
-	struct nftnl_chain_list *list;
 	struct nftnl_chain *c;
 	int ret = 0;
 
 	nft_xt_builtin_init(h, table);
 	nft_assert_table_compatible(h, table, chain);
 
-	list = nft_chain_list_get(h, table, chain);
-	if (!list)
-		return 0;
-
 	if (counters < 0)
 		d.format = FMT_C_COUNTS;
 	else if (counters == 0)
 		d.format = FMT_NOCOUNTS;
 
 	if (chain) {
-		c = nftnl_chain_list_lookup_byname(list, chain);
+		c = nft_chain_find(h, table, chain);
 		if (!c)
 			return 0;
 
@@ -2472,10 +2464,10 @@ int nft_rule_list_save(struct nft_handle *h, const char *chain,
 	}
 
 	/* Dump policies and custom chains first */
-	nftnl_chain_list_foreach(list, nft_rule_list_chain_save, &counters);
+	nft_chain_foreach(h, table, nft_rule_list_chain_save, &counters);
 
 	/* Now dump out rules in this table */
-	ret = nftnl_chain_list_foreach(list, nft_rule_list_cb, &d);
+	ret = nft_chain_foreach(h, table, nft_rule_list_cb, &d);
 	return ret == 0 ? 1 : 0;
 }
 
@@ -3340,7 +3332,6 @@ static int __nft_chain_zero_counters(struct nftnl_chain *c, void *data)
 int nft_chain_zero_counters(struct nft_handle *h, const char *chain,
 			    const char *table, bool verbose)
 {
-	struct nftnl_chain_list *list;
 	struct chain_zero_data d = {
 		.handle = h,
 		.verbose = verbose,
@@ -3348,12 +3339,8 @@ int nft_chain_zero_counters(struct nft_handle *h, const char *chain,
 	struct nftnl_chain *c;
 	int ret = 0;
 
-	list = nft_chain_list_get(h, table, chain);
-	if (list == NULL)
-		goto err;
-
 	if (chain) {
-		c = nftnl_chain_list_lookup_byname(list, chain);
+		c = nft_chain_find(h, table, chain);
 		if (!c) {
 			errno = ENOENT;
 			return 0;
@@ -3363,7 +3350,7 @@ int nft_chain_zero_counters(struct nft_handle *h, const char *chain,
 		goto err;
 	}
 
-	ret = nftnl_chain_list_foreach(list, __nft_chain_zero_counters, &d);
+	ret = nft_chain_foreach(h, table, __nft_chain_zero_counters, &d);
 err:
 	/* the core expects 1 for success and 0 for error */
 	return ret == 0 ? 1 : 0;
@@ -3452,22 +3439,13 @@ static int nft_is_chain_compatible(struct nftnl_chain *c, void *data)
 bool nft_is_table_compatible(struct nft_handle *h,
 			     const char *table, const char *chain)
 {
-	struct nftnl_chain_list *clist;
-
 	if (chain) {
 		struct nftnl_chain *c = nft_chain_find(h, table, chain);
 
 		return c && !nft_is_chain_compatible(c, h);
 	}
 
-	clist = nft_chain_list_get(h, table, chain);
-	if (clist == NULL)
-		return false;
-
-	if (nftnl_chain_list_foreach(clist, nft_is_chain_compatible, h))
-		return false;
-
-	return true;
+	return !nft_chain_foreach(h, table, nft_is_chain_compatible, h);
 }
 
 void nft_assert_table_compatible(struct nft_handle *h,
diff --git a/iptables/nft.h b/iptables/nft.h
index 128e09beb805e..949d9d077f23b 100644
--- a/iptables/nft.h
+++ b/iptables/nft.h
@@ -151,6 +151,9 @@ const struct builtin_chain *nft_chain_builtin_find(const struct builtin_table *t
 bool nft_chain_exists(struct nft_handle *h, const char *table, const char *chain);
 void nft_bridge_chain_postprocess(struct nft_handle *h,
 				  struct nftnl_chain *c);
+int nft_chain_foreach(struct nft_handle *h, const char *table,
+		      int (*cb)(struct nftnl_chain *c, void *data),
+		      void *data);
 
 
 /*
diff --git a/iptables/xtables-save.c b/iptables/xtables-save.c
index 92b0c911c5f1c..bf00b0324cc4f 100644
--- a/iptables/xtables-save.c
+++ b/iptables/xtables-save.c
@@ -68,7 +68,6 @@ struct do_output_data {
 static int
 __do_output(struct nft_handle *h, const char *tablename, void *data)
 {
-	struct nftnl_chain_list *chain_list;
 	struct do_output_data *d = data;
 	time_t now;
 
@@ -81,10 +80,6 @@ __do_output(struct nft_handle *h, const char *tablename, void *data)
 		return 0;
 	}
 
-	chain_list = nft_chain_list_get(h, tablename, NULL);
-	if (!chain_list)
-		return 0;
-
 	now = time(NULL);
 	printf("# Generated by %s v%s on %s", prog_name,
 	       prog_vers, ctime(&now));
@@ -92,7 +87,7 @@ __do_output(struct nft_handle *h, const char *tablename, void *data)
 	printf("*%s\n", tablename);
 	/* Dump out chain names first,
 	 * thereby preventing dependency conflicts */
-	nftnl_chain_list_foreach(chain_list, nft_chain_save, h);
+	nft_chain_foreach(h, tablename, nft_chain_save, h);
 	nft_rule_save(h, tablename, d->format);
 	if (d->commit)
 		printf("COMMIT\n");
-- 
2.28.0

[iptables PATCH v3 4/9] nft: cache: Move nft_chain_find() over

[Phil Sutter]

It is basically just a cache lookup, hence fits better in here.

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

diff --git a/iptables/nft-cache.c b/iptables/nft-cache.c
index 109524c3fbc79..929fa0fa152c1 100644
--- a/iptables/nft-cache.c
+++ b/iptables/nft-cache.c
@@ -165,6 +165,20 @@ static int fetch_table_cache(struct nft_handle *h)
 	return 1;
 }
 
+struct nftnl_chain *
+nft_chain_find(struct nft_handle *h, const char *table, const char *chain)
+{
+	const struct builtin_table *t;
+	struct nftnl_chain_list *list;
+
+	t = nft_table_builtin_find(h, table);
+	if (!t)
+		return NULL;
+
+	list = h->cache->table[t->type].chains;
+	return list ? nftnl_chain_list_lookup_byname(list, chain) : NULL;
+}
+
 int nft_cache_add_chain(struct nft_handle *h, const struct builtin_table *t,
 			struct nftnl_chain *c)
 {
diff --git a/iptables/nft-cache.h b/iptables/nft-cache.h
index 52ad2d396199e..085594c26457b 100644
--- a/iptables/nft-cache.h
+++ b/iptables/nft-cache.h
@@ -16,6 +16,9 @@ void nft_cache_build(struct nft_handle *h);
 int nft_cache_add_chain(struct nft_handle *h, const struct builtin_table *t,
 			struct nftnl_chain *c);
 
+struct nftnl_chain *
+nft_chain_find(struct nft_handle *h, const char *table, const char *chain);
+
 struct nftnl_set_list *
 nft_set_list_get(struct nft_handle *h, const char *table, const char *set);
 
diff --git a/iptables/nft.c b/iptables/nft.c
index afe7fe9ed05c5..9b40b3c3b9631 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -738,9 +738,6 @@ nft_chain_builtin_find(const struct builtin_table *t, const char *chain)
 	return found ? &t->chains[i] : NULL;
 }
 
-static struct nftnl_chain *
-nft_chain_find(struct nft_handle *h, const char *table, const char *chain);
-
 static void nft_chain_builtin_init(struct nft_handle *h,
 				   const struct builtin_table *table)
 {
@@ -1837,20 +1834,6 @@ out:
 	return ret == 0 ? 1 : 0;
 }
 
-static struct nftnl_chain *
-nft_chain_find(struct nft_handle *h, const char *table, const char *chain)
-{
-	const struct builtin_table *t;
-	struct nftnl_chain_list *list;
-
-	t = nft_table_builtin_find(h, table);
-	if (!t)
-		return NULL;
-
-	list = h->cache->table[t->type].chains;
-	return list ? nftnl_chain_list_lookup_byname(list, chain) : NULL;
-}
-
 bool nft_chain_exists(struct nft_handle *h,
 		      const char *table, const char *chain)
 {
-- 
2.28.0

[iptables PATCH v3 5/9] nft: Introduce struct nft_chain

[Phil Sutter]

Preparing for ordered output of user-defined chains, introduce a local
datatype wrapping nftnl_chain. In order to maintain the chain name hash
table, introduce nft_chain_list as well and use it instead of
nftnl_chain_list.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/Makefile.am |   2 +-
 iptables/nft-cache.c |  61 +++++++++++++++++------
 iptables/nft-cache.h |   5 +-
 iptables/nft-chain.c |  59 ++++++++++++++++++++++
 iptables/nft-chain.h |  29 +++++++++++
 iptables/nft.c       | 115 ++++++++++++++++++++++++++-----------------
 iptables/nft.h       |   7 +--
 7 files changed, 212 insertions(+), 66 deletions(-)
 create mode 100644 iptables/nft-chain.c
 create mode 100644 iptables/nft-chain.h

diff --git a/iptables/Makefile.am b/iptables/Makefile.am
index 4bf5742c9dc95..f789521042f87 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-cmd.c \
+				nft-bridge.c nft-cmd.c nft-chain.c \
 				xtables-eb-standalone.c xtables-eb.c \
 				xtables-eb-translate.c \
 				xtables-translate.c
diff --git a/iptables/nft-cache.c b/iptables/nft-cache.c
index 929fa0fa152c1..f62e5100cd67b 100644
--- a/iptables/nft-cache.c
+++ b/iptables/nft-cache.c
@@ -24,6 +24,7 @@
 
 #include "nft.h"
 #include "nft-cache.h"
+#include "nft-chain.h"
 
 static void cache_chain_list_insert(struct list_head *list, const char *name)
 {
@@ -153,9 +154,7 @@ static int fetch_table_cache(struct nft_handle *h)
 		if (!h->tables[i].name)
 			continue;
 
-		h->cache->table[type].chains = nftnl_chain_list_alloc();
-		if (!h->cache->table[type].chains)
-			return 0;
+		h->cache->table[type].chains = nft_chain_list_alloc();
 
 		h->cache->table[type].sets = nftnl_set_list_alloc();
 		if (!h->cache->table[type].sets)
@@ -165,24 +164,52 @@ static int fetch_table_cache(struct nft_handle *h)
 	return 1;
 }
 
-struct nftnl_chain *
+static uint32_t djb_hash(const char *key)
+{
+	uint32_t i, hash = 5381;
+
+	for (i = 0; i < strlen(key); i++)
+		hash = ((hash << 5) + hash) + key[i];
+
+	return hash;
+}
+
+static struct hlist_head *chain_name_hlist(struct nft_handle *h,
+					   const struct builtin_table *t,
+					   const char *chain)
+{
+	int key = djb_hash(chain) % CHAIN_NAME_HSIZE;
+
+	return &h->cache->table[t->type].chains->names[key];
+}
+
+struct nft_chain *
 nft_chain_find(struct nft_handle *h, const char *table, const char *chain)
 {
 	const struct builtin_table *t;
-	struct nftnl_chain_list *list;
+	struct hlist_node *node;
+	struct nft_chain *c;
 
 	t = nft_table_builtin_find(h, table);
 	if (!t)
 		return NULL;
 
-	list = h->cache->table[t->type].chains;
-	return list ? nftnl_chain_list_lookup_byname(list, chain) : NULL;
+	hlist_for_each_entry(c, node, chain_name_hlist(h, t, chain), hnode) {
+		if (!strcmp(nftnl_chain_get_str(c->nftnl, NFTNL_CHAIN_NAME),
+			    chain))
+			return c;
+	}
+	return NULL;
 }
 
 int nft_cache_add_chain(struct nft_handle *h, const struct builtin_table *t,
 			struct nftnl_chain *c)
 {
-	nftnl_chain_list_add_tail(c, h->cache->table[t->type].chains);
+	const char *cname = nftnl_chain_get_str(c, NFTNL_CHAIN_NAME);
+	struct nft_chain *nc = nft_chain_alloc(c);
+
+	list_add_tail(&nc->head, &h->cache->table[t->type].chains->list);
+	hlist_add_head(&nc->hnode, chain_name_hlist(h, t, cname));
 	return 0;
 }
 
@@ -434,8 +461,9 @@ static int nftnl_rule_list_cb(const struct nlmsghdr *nlh, void *data)
 	return MNL_CB_OK;
 }
 
-static int nft_rule_list_update(struct nftnl_chain *c, void *data)
+static int nft_rule_list_update(struct nft_chain *nc, void *data)
 {
+	struct nftnl_chain *c = nc->nftnl;
 	struct nft_handle *h = data;
 	char buf[16536];
 	struct nlmsghdr *nlh;
@@ -550,13 +578,13 @@ static int ____flush_rule_cache(struct nftnl_rule *r, void *data)
 	return 0;
 }
 
-static int __flush_rule_cache(struct nftnl_chain *c, void *data)
+static int __flush_rule_cache(struct nft_chain *c, void *data)
 {
-	return nftnl_rule_foreach(c, ____flush_rule_cache, NULL);
+	return nftnl_rule_foreach(c->nftnl, ____flush_rule_cache, NULL);
 }
 
 int flush_rule_cache(struct nft_handle *h, const char *table,
-		     struct nftnl_chain *c)
+		     struct nft_chain *c)
 {
 	if (c)
 		return __flush_rule_cache(c, NULL);
@@ -565,10 +593,10 @@ int flush_rule_cache(struct nft_handle *h, const char *table,
 	return 0;
 }
 
-static int __flush_chain_cache(struct nftnl_chain *c, void *data)
+static int __flush_chain_cache(struct nft_chain *c, void *data)
 {
-	nftnl_chain_list_del(c);
-	nftnl_chain_free(c);
+	nft_chain_list_del(c);
+	nft_chain_free(c);
 
 	return 0;
 }
@@ -605,9 +633,10 @@ static int flush_cache(struct nft_handle *h, struct nft_cache *c,
 			continue;
 
 		if (c->table[i].chains) {
-			nftnl_chain_list_free(c->table[i].chains);
+			nft_chain_list_free(c->table[i].chains);
 			c->table[i].chains = NULL;
 		}
+
 		if (c->table[i].sets) {
 			nftnl_set_list_free(c->table[i].sets);
 			c->table[i].sets = NULL;
diff --git a/iptables/nft-cache.h b/iptables/nft-cache.h
index 085594c26457b..20d96beede876 100644
--- a/iptables/nft-cache.h
+++ b/iptables/nft-cache.h
@@ -2,6 +2,7 @@
 #define _NFT_CACHE_H_
 
 struct nft_handle;
+struct nft_chain;
 struct nft_cmd;
 struct builtin_table;
 
@@ -11,12 +12,12 @@ 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);
 int flush_rule_cache(struct nft_handle *h, const char *table,
-		     struct nftnl_chain *c);
+		     struct nft_chain *c);
 void nft_cache_build(struct nft_handle *h);
 int nft_cache_add_chain(struct nft_handle *h, const struct builtin_table *t,
 			struct nftnl_chain *c);
 
-struct nftnl_chain *
+struct nft_chain *
 nft_chain_find(struct nft_handle *h, const char *table, const char *chain);
 
 struct nftnl_set_list *
diff --git a/iptables/nft-chain.c b/iptables/nft-chain.c
new file mode 100644
index 0000000000000..e954170fa7312
--- /dev/null
+++ b/iptables/nft-chain.c
@@ -0,0 +1,59 @@
+/*
+ * Copyright (c) 2020  Red Hat GmbH.  Author: Phil Sutter <phil@nwl.cc>
+ *
+ * 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.
+ */
+
+#include <stdlib.h>
+#include <xtables.h>
+
+#include "nft-chain.h"
+
+struct nft_chain *nft_chain_alloc(struct nftnl_chain *nftnl)
+{
+	struct nft_chain *c = xtables_malloc(sizeof(*c));
+
+	INIT_LIST_HEAD(&c->head);
+	c->nftnl = nftnl;
+
+	return c;
+}
+
+void nft_chain_free(struct nft_chain *c)
+{
+	if (c->nftnl)
+		nftnl_chain_free(c->nftnl);
+	free(c);
+}
+
+struct nft_chain_list *nft_chain_list_alloc(void)
+{
+	struct nft_chain_list *list = xtables_malloc(sizeof(*list));
+	int i;
+
+	INIT_LIST_HEAD(&list->list);
+	for (i = 0; i < CHAIN_NAME_HSIZE; i++)
+		INIT_HLIST_HEAD(&list->names[i]);
+
+	return list;
+}
+
+void nft_chain_list_del(struct nft_chain *c)
+{
+	list_del(&c->head);
+	hlist_del(&c->hnode);
+}
+
+void nft_chain_list_free(struct nft_chain_list *list)
+{
+	struct nft_chain *c, *c2;
+
+	list_for_each_entry_safe(c, c2, &list->list, head) {
+		nft_chain_list_del(c);
+		nft_chain_free(c);
+	}
+	free(list);
+}
diff --git a/iptables/nft-chain.h b/iptables/nft-chain.h
new file mode 100644
index 0000000000000..137f4b7f90085
--- /dev/null
+++ b/iptables/nft-chain.h
@@ -0,0 +1,29 @@
+#ifndef _NFT_CHAIN_H_
+#define _NFT_CHAIN_H_
+
+#include <libnftnl/chain.h>
+#include <libiptc/linux_list.h>
+
+struct nft_handle;
+
+struct nft_chain {
+	struct list_head	head;
+	struct hlist_node	hnode;
+	struct nftnl_chain	*nftnl;
+};
+
+#define CHAIN_NAME_HSIZE	512
+
+struct nft_chain_list {
+	struct list_head	list;
+	struct hlist_head	names[CHAIN_NAME_HSIZE];
+};
+
+struct nft_chain *nft_chain_alloc(struct nftnl_chain *nftnl);
+void nft_chain_free(struct nft_chain *c);
+
+struct nft_chain_list *nft_chain_list_alloc(void);
+void nft_chain_list_free(struct nft_chain_list *list);
+void nft_chain_list_del(struct nft_chain *c);
+
+#endif /* _NFT_CHAIN_H_ */
diff --git a/iptables/nft.c b/iptables/nft.c
index 9b40b3c3b9631..1b7400050c0ed 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -1388,7 +1388,7 @@ int
 nft_rule_append(struct nft_handle *h, const char *chain, const char *table,
 		struct nftnl_rule *r, struct nftnl_rule *ref, bool verbose)
 {
-	struct nftnl_chain *c;
+	struct nft_chain *c;
 	int type;
 
 	nft_xt_builtin_init(h, table);
@@ -1418,7 +1418,7 @@ nft_rule_append(struct nft_handle *h, const char *chain, const char *table,
 			errno = ENOENT;
 			return 0;
 		}
-		nftnl_chain_rule_add_tail(r, c);
+		nftnl_chain_rule_add_tail(r, c->nftnl);
 	}
 
 	return 1;
@@ -1540,8 +1540,9 @@ static const char *policy_name[NF_ACCEPT+1] = {
 	[NF_ACCEPT] = "ACCEPT",
 };
 
-int nft_chain_save(struct nftnl_chain *c, void *data)
+int nft_chain_save(struct nft_chain *nc, void *data)
 {
+	struct nftnl_chain *c = nc->nftnl;
 	struct nft_handle *h = data;
 	const char *policy = NULL;
 
@@ -1564,13 +1565,13 @@ struct nft_rule_save_data {
 	unsigned int format;
 };
 
-static int nft_rule_save_cb(struct nftnl_chain *c, void *data)
+static int nft_rule_save_cb(struct nft_chain *c, void *data)
 {
 	struct nft_rule_save_data *d = data;
 	struct nftnl_rule_iter *iter;
 	struct nftnl_rule *r;
 
-	iter = nftnl_rule_iter_create(c);
+	iter = nftnl_rule_iter_create(c->nftnl);
 	if (iter == NULL)
 		return 1;
 
@@ -1645,9 +1646,9 @@ struct nft_rule_flush_data {
 	bool verbose;
 };
 
-static int nft_rule_flush_cb(struct nftnl_chain *c, void *data)
+static int nft_rule_flush_cb(struct nft_chain *c, void *data)
 {
-	const char *chain = nftnl_chain_get_str(c, NFTNL_CHAIN_NAME);
+	const char *chain = nftnl_chain_get_str(c->nftnl, NFTNL_CHAIN_NAME);
 	struct nft_rule_flush_data *d = data;
 
 	batch_chain_flush(d->h, d->table, chain);
@@ -1664,7 +1665,7 @@ int nft_rule_flush(struct nft_handle *h, const char *chain, const char *table,
 		.table = table,
 		.verbose = verbose,
 	};
-	struct nftnl_chain *c = NULL;
+	struct nft_chain *c = NULL;
 	int ret = 0;
 
 	nft_fn = nft_rule_flush;
@@ -1733,12 +1734,13 @@ int nft_chain_restore(struct nft_handle *h, const char *chain, const char *table
 	const struct builtin_table *t;
 	struct obj_update *obj;
 	struct nftnl_chain *c;
+	struct nft_chain *nc;
 	bool created = false;
 
 	nft_xt_builtin_init(h, table);
 
-	c = nft_chain_find(h, table, chain);
-	if (!c) {
+	nc = nft_chain_find(h, table, chain);
+	if (!nc) {
 		c = nftnl_chain_alloc();
 		if (!c)
 			return 0;
@@ -1750,6 +1752,8 @@ int nft_chain_restore(struct nft_handle *h, const char *chain, const char *table
 		t = nft_table_builtin_find(h, table);
 		nft_cache_add_chain(h, t, c);
 	} else {
+		c = nc->nftnl;
+
 		/* If the chain should vanish meanwhile, kernel genid changes
 		 * and the transaction is refreshed enabling the chain add
 		 * object. With the handle still set, kernel interprets it as a
@@ -1781,9 +1785,10 @@ struct chain_user_del_data {
 	int			builtin_err;
 };
 
-static int __nft_chain_user_del(struct nftnl_chain *c, void *data)
+static int __nft_chain_user_del(struct nft_chain *nc, void *data)
 {
 	struct chain_user_del_data *d = data;
+	struct nftnl_chain *c = nc->nftnl;
 	struct nft_handle *h = d->handle;
 
 	/* don't delete built-in chain */
@@ -1794,12 +1799,17 @@ 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));
 
+
 	/* XXX This triggers a fast lookup from the kernel. */
 	nftnl_chain_unset(c, NFTNL_CHAIN_HANDLE);
 	if (!batch_chain_add(h, NFT_COMPAT_CHAIN_USER_DEL, c))
 		return -1;
 
-	nftnl_chain_list_del(c);
+	/* nftnl_chain is freed when deleting the batch object */
+	nc->nftnl = NULL;
+
+	nft_chain_list_del(nc);
+	nft_chain_free(nc);
 	return 0;
 }
 
@@ -1810,7 +1820,7 @@ int nft_chain_user_del(struct nft_handle *h, const char *chain,
 		.handle = h,
 		.verbose = verbose,
 	};
-	struct nftnl_chain *c;
+	struct nft_chain *c;
 	int ret = 0;
 
 	nft_fn = nft_chain_user_del;
@@ -1853,6 +1863,7 @@ int nft_chain_user_rename(struct nft_handle *h,const char *chain,
 			  const char *table, const char *newname)
 {
 	struct nftnl_chain *c;
+	struct nft_chain *nc;
 	uint64_t handle;
 
 	nft_fn = nft_chain_user_rename;
@@ -1863,12 +1874,12 @@ int nft_chain_user_rename(struct nft_handle *h,const char *chain,
 	}
 
 	/* Find the old chain to be renamed */
-	c = nft_chain_find(h, table, chain);
-	if (c == NULL) {
+	nc = nft_chain_find(h, table, chain);
+	if (nc == NULL) {
 		errno = ENOENT;
 		return 0;
 	}
-	handle = nftnl_chain_get_u64(c, NFTNL_CHAIN_HANDLE);
+	handle = nftnl_chain_get_u64(nc->nftnl, NFTNL_CHAIN_HANDLE);
 
 	/* Now prepare the new name for the chain */
 	c = nftnl_chain_alloc();
@@ -2017,9 +2028,10 @@ out:
 }
 
 static struct nftnl_rule *
-nft_rule_find(struct nft_handle *h, struct nftnl_chain *c,
+nft_rule_find(struct nft_handle *h, struct nft_chain *nc,
 	      struct nftnl_rule *rule, int rulenum)
 {
+	struct nftnl_chain *c = nc->nftnl;
 	struct nftnl_rule *r;
 	struct nftnl_rule_iter *iter;
 	bool found = false;
@@ -2048,8 +2060,8 @@ nft_rule_find(struct nft_handle *h, struct nftnl_chain *c,
 int nft_rule_check(struct nft_handle *h, const char *chain,
 		   const char *table, struct nftnl_rule *rule, bool verbose)
 {
-	struct nftnl_chain *c;
 	struct nftnl_rule *r;
+	struct nft_chain *c;
 
 	nft_fn = nft_rule_check;
 
@@ -2074,8 +2086,8 @@ int nft_rule_delete(struct nft_handle *h, const char *chain,
 		    const char *table, struct nftnl_rule *rule, bool verbose)
 {
 	int ret = 0;
-	struct nftnl_chain *c;
 	struct nftnl_rule *r;
+	struct nft_chain *c;
 
 	nft_fn = nft_rule_delete;
 
@@ -2135,7 +2147,7 @@ int nft_rule_insert(struct nft_handle *h, const char *chain,
 		    bool verbose)
 {
 	struct nftnl_rule *r = NULL;
-	struct nftnl_chain *c;
+	struct nft_chain *c;
 
 	nft_xt_builtin_init(h, table);
 
@@ -2170,7 +2182,7 @@ int nft_rule_insert(struct nft_handle *h, const char *chain,
 	if (r)
 		nftnl_chain_rule_insert_at(new_rule, r);
 	else
-		nftnl_chain_rule_add(new_rule, c);
+		nftnl_chain_rule_add(new_rule, c->nftnl);
 
 	return 1;
 err:
@@ -2181,8 +2193,8 @@ int nft_rule_delete_num(struct nft_handle *h, const char *chain,
 			const char *table, int rulenum, bool verbose)
 {
 	int ret = 0;
-	struct nftnl_chain *c;
 	struct nftnl_rule *r;
+	struct nft_chain *c;
 
 	nft_fn = nft_rule_delete_num;
 
@@ -2209,8 +2221,8 @@ int nft_rule_replace(struct nft_handle *h, const char *chain,
 		     int rulenum, bool verbose)
 {
 	int ret = 0;
-	struct nftnl_chain *c;
 	struct nftnl_rule *r;
+	struct nft_chain *c;
 
 	nft_fn = nft_rule_replace;
 
@@ -2289,8 +2301,9 @@ static int nft_rule_count(struct nft_handle *h, struct nftnl_chain *c)
 }
 
 static void __nft_print_header(struct nft_handle *h,
-			       struct nftnl_chain *c, unsigned int format)
+			       struct nft_chain *nc, unsigned int format)
 {
+	struct nftnl_chain *c = nc->nftnl;
 	const char *chain_name = nftnl_chain_get_str(c, NFTNL_CHAIN_NAME);
 	bool basechain = !!nftnl_chain_get(c, NFTNL_CHAIN_HOOKNUM);
 	uint32_t refs = nftnl_chain_get_u32(c, NFTNL_CHAIN_USE);
@@ -2318,7 +2331,7 @@ struct nft_rule_list_cb_data {
 		   unsigned int num, unsigned int format);
 };
 
-static int nft_rule_list_cb(struct nftnl_chain *c, void *data)
+static int nft_rule_list_cb(struct nft_chain *c, void *data)
 {
 	struct nft_rule_list_cb_data *d = data;
 
@@ -2330,7 +2343,7 @@ static int nft_rule_list_cb(struct nftnl_chain *c, void *data)
 		__nft_print_header(d->h, c, d->format);
 	}
 
-	return __nft_rule_list(d->h, c, d->rulenum, d->format, d->cb);
+	return __nft_rule_list(d->h, c->nftnl, d->rulenum, d->format, d->cb);
 }
 
 int nft_rule_list(struct nft_handle *h, const char *chain, const char *table,
@@ -2343,7 +2356,7 @@ int nft_rule_list(struct nft_handle *h, const char *chain, const char *table,
 		.rulenum = rulenum,
 		.cb = ops->print_rule,
 	};
-	struct nftnl_chain *c;
+	struct nft_chain *c;
 
 	nft_xt_builtin_init(h, table);
 	nft_assert_table_compatible(h, table, chain);
@@ -2377,24 +2390,33 @@ list_save(struct nft_handle *h, struct nftnl_rule *r,
 }
 
 int nft_chain_foreach(struct nft_handle *h, const char *table,
-		      int (*cb)(struct nftnl_chain *c, void *data),
+		      int (*cb)(struct nft_chain *c, void *data),
 		      void *data)
 {
 	const struct builtin_table *t;
+	struct nft_chain_list *list;
+	struct nft_chain *c, *c_bak;
+	int ret;
 
 	t = nft_table_builtin_find(h, table);
 	if (!t)
 		return -1;
 
-	if (!h->cache->table[t->type].chains)
+	list = h->cache->table[t->type].chains;
+	if (!list)
 		return -1;
 
-	return nftnl_chain_list_foreach(h->cache->table[t->type].chains,
-					cb, data);
+	list_for_each_entry_safe(c, c_bak, &list->list, head) {
+		ret = cb(c, data);
+		if (ret < 0)
+			return ret;
+	}
+	return 0;
 }
 
-static int nft_rule_list_chain_save(struct nftnl_chain *c, void *data)
+static int nft_rule_list_chain_save(struct nft_chain *nc, void *data)
 {
+	struct nftnl_chain *c = nc->nftnl;
 	const char *chain_name = nftnl_chain_get_str(c, NFTNL_CHAIN_NAME);
 	uint32_t policy = nftnl_chain_get_u32(c, NFTNL_CHAIN_POLICY);
 	int *counters = data;
@@ -2424,7 +2446,7 @@ int nft_rule_list_save(struct nft_handle *h, const char *chain,
 		.save_fmt = true,
 		.cb = list_save,
 	};
-	struct nftnl_chain *c;
+	struct nft_chain *c;
 	int ret = 0;
 
 	nft_xt_builtin_init(h, table);
@@ -2459,7 +2481,7 @@ int nft_rule_zero_counters(struct nft_handle *h, const char *chain,
 {
 	struct iptables_command_state cs = {};
 	struct nftnl_rule *r, *new_rule;
-	struct nftnl_chain *c;
+	struct nft_chain *c;
 	int ret = 0;
 
 	nft_fn = nft_rule_delete;
@@ -2601,7 +2623,7 @@ static void batch_obj_del(struct nft_handle *h, struct obj_update *o)
 static void nft_refresh_transaction(struct nft_handle *h)
 {
 	const char *tablename, *chainname;
-	const struct nftnl_chain *c;
+	const struct nft_chain *c;
 	struct obj_update *n, *tmp;
 	bool exists;
 
@@ -2898,7 +2920,7 @@ err_free_rule:
 int ebt_set_user_chain_policy(struct nft_handle *h, const char *table,
 			      const char *chain, const char *policy)
 {
-	struct nftnl_chain *c = nft_chain_find(h, table, chain);
+	struct nft_chain *c = nft_chain_find(h, table, chain);
 	int pval;
 
 	if (!c)
@@ -2913,14 +2935,15 @@ int ebt_set_user_chain_policy(struct nft_handle *h, const char *table,
 	else
 		return 0;
 
-	nftnl_chain_set_u32(c, NFTNL_CHAIN_POLICY, pval);
+	nftnl_chain_set_u32(c->nftnl, NFTNL_CHAIN_POLICY, pval);
 	return 1;
 }
 
 static void nft_bridge_commit_prepare(struct nft_handle *h)
 {
 	const struct builtin_table *t;
-	struct nftnl_chain_list *list;
+	struct nft_chain_list *list;
+	struct nft_chain *c;
 	int i;
 
 	for (i = 0; i < NFT_TABLE_MAX; i++) {
@@ -2933,7 +2956,9 @@ static void nft_bridge_commit_prepare(struct nft_handle *h)
 		if (!list)
 			continue;
 
-		nftnl_chain_list_foreach(list, ebt_add_policy_rule, h);
+		list_for_each_entry(c, &list->list, head) {
+			ebt_add_policy_rule(c->nftnl, h);
+		}
 	}
 }
 
@@ -3241,8 +3266,9 @@ struct chain_zero_data {
 	bool			verbose;
 };
 
-static int __nft_chain_zero_counters(struct nftnl_chain *c, void *data)
+static int __nft_chain_zero_counters(struct nft_chain *nc, void *data)
 {
+	struct nftnl_chain *c = nc->nftnl;
 	struct chain_zero_data *d = data;
 	struct nft_handle *h = d->handle;
 	struct nftnl_rule_iter *iter;
@@ -3250,7 +3276,7 @@ static int __nft_chain_zero_counters(struct nftnl_chain *c, void *data)
 
 	if (d->verbose)
 		fprintf(stdout, "Zeroing chain `%s'\n",
-			nftnl_chain_get_str(c, NFTNL_CHAIN_NAME));
+		        nftnl_chain_get_str(c, NFTNL_CHAIN_NAME));
 
 	if (nftnl_chain_is_set(c, NFTNL_CHAIN_HOOKNUM)) {
 		/* zero base chain counters. */
@@ -3319,7 +3345,7 @@ int nft_chain_zero_counters(struct nft_handle *h, const char *chain,
 		.handle = h,
 		.verbose = verbose,
 	};
-	struct nftnl_chain *c;
+	struct nft_chain *c;
 	int ret = 0;
 
 	if (chain) {
@@ -3383,8 +3409,9 @@ static int nft_is_rule_compatible(struct nftnl_rule *rule, void *data)
 	return nftnl_expr_foreach(rule, nft_is_expr_compatible, NULL);
 }
 
-static int nft_is_chain_compatible(struct nftnl_chain *c, void *data)
+static int nft_is_chain_compatible(struct nft_chain *nc, void *data)
 {
+	struct nftnl_chain *c = nc->nftnl;
 	const struct builtin_table *table;
 	const struct builtin_chain *chain;
 	const char *tname, *cname, *type;
@@ -3423,7 +3450,7 @@ bool nft_is_table_compatible(struct nft_handle *h,
 			     const char *table, const char *chain)
 {
 	if (chain) {
-		struct nftnl_chain *c = nft_chain_find(h, table, chain);
+		struct nft_chain *c = nft_chain_find(h, table, chain);
 
 		return c && !nft_is_chain_compatible(c, h);
 	}
diff --git a/iptables/nft.h b/iptables/nft.h
index 949d9d077f23b..ac227b4c6c581 100644
--- a/iptables/nft.h
+++ b/iptables/nft.h
@@ -4,6 +4,7 @@
 #include "xshared.h"
 #include "nft-shared.h"
 #include "nft-cache.h"
+#include "nft-chain.h"
 #include "nft-cmd.h"
 #include <libiptc/linux_list.h>
 
@@ -39,7 +40,7 @@ enum nft_cache_level {
 
 struct nft_cache {
 	struct {
-		struct nftnl_chain_list *chains;
+		struct nft_chain_list	*chains;
 		struct nftnl_set_list	*sets;
 		bool			exists;
 	} table[NFT_TABLE_MAX];
@@ -141,7 +142,7 @@ const struct builtin_table *nft_table_builtin_find(struct nft_handle *h, const c
 struct nftnl_chain;
 
 int nft_chain_set(struct nft_handle *h, const char *table, const char *chain, const char *policy, const struct xt_counters *counters);
-int nft_chain_save(struct nftnl_chain *c, void *data);
+int nft_chain_save(struct nft_chain *c, void *data);
 int nft_chain_user_add(struct nft_handle *h, const char *chain, const char *table);
 int nft_chain_user_del(struct nft_handle *h, const char *chain, const char *table, bool verbose);
 int nft_chain_restore(struct nft_handle *h, const char *chain, const char *table);
@@ -152,7 +153,7 @@ bool nft_chain_exists(struct nft_handle *h, const char *table, const char *chain
 void nft_bridge_chain_postprocess(struct nft_handle *h,
 				  struct nftnl_chain *c);
 int nft_chain_foreach(struct nft_handle *h, const char *table,
-		      int (*cb)(struct nftnl_chain *c, void *data),
+		      int (*cb)(struct nft_chain *c, void *data),
 		      void *data);
 
 
-- 
2.28.0

[iptables PATCH v3 6/9] nft: Introduce a dedicated base chain array

[Phil Sutter]

Preparing for sorted chain output, introduce a per-table array holding
base chains indexed by nf_inet_hooks value. Since the latter is ordered
correctly, iterating over the array will return base chains in expected
order.

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

diff --git a/iptables/nft-cache.c b/iptables/nft-cache.c
index f62e5100cd67b..bd19b6dfc4d8a 100644
--- a/iptables/nft-cache.c
+++ b/iptables/nft-cache.c
@@ -208,7 +208,24 @@ int nft_cache_add_chain(struct nft_handle *h, const struct builtin_table *t,
 	const char *cname = nftnl_chain_get_str(c, NFTNL_CHAIN_NAME);
 	struct nft_chain *nc = nft_chain_alloc(c);
 
-	list_add_tail(&nc->head, &h->cache->table[t->type].chains->list);
+	if (nftnl_chain_is_set(c, NFTNL_CHAIN_HOOKNUM)) {
+		uint32_t hooknum = nftnl_chain_get_u32(c, NFTNL_CHAIN_HOOKNUM);
+
+		if (hooknum >= NF_INET_NUMHOOKS) {
+			nft_chain_free(nc);
+			return -EINVAL;
+		}
+
+		if (h->cache->table[t->type].base_chains[hooknum]) {
+			nft_chain_free(nc);
+			return -EEXIST;
+		}
+
+		h->cache->table[t->type].base_chains[hooknum] = nc;
+	} else {
+		list_add_tail(&nc->head,
+			      &h->cache->table[t->type].chains->list);
+	}
 	hlist_add_head(&nc->hnode, chain_name_hlist(h, t, cname));
 	return 0;
 }
@@ -609,6 +626,19 @@ static int __flush_set_cache(struct nftnl_set *s, void *data)
 	return 0;
 }
 
+static void flush_base_chain_cache(struct nft_chain **base_chains)
+{
+	int i;
+
+	for (i = 0; i < NF_INET_NUMHOOKS; i++) {
+		if (!base_chains[i])
+			continue;
+		hlist_del(&base_chains[i]->hnode);
+		nft_chain_free(base_chains[i]);
+		base_chains[i] = NULL;
+	}
+}
+
 static int flush_cache(struct nft_handle *h, struct nft_cache *c,
 		       const char *tablename)
 {
@@ -620,6 +650,7 @@ static int flush_cache(struct nft_handle *h, struct nft_cache *c,
 		if (!table)
 			return 0;
 
+		flush_base_chain_cache(c->table[table->type].base_chains);
 		nft_chain_foreach(h, tablename, __flush_chain_cache, NULL);
 
 		if (c->table[table->type].sets)
@@ -632,6 +663,7 @@ static int flush_cache(struct nft_handle *h, struct nft_cache *c,
 		if (h->tables[i].name == NULL)
 			continue;
 
+		flush_base_chain_cache(c->table[i].base_chains);
 		if (c->table[i].chains) {
 			nft_chain_list_free(c->table[i].chains);
 			c->table[i].chains = NULL;
diff --git a/iptables/nft.c b/iptables/nft.c
index 1b7400050c0ed..4187e691d8926 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -2396,12 +2396,22 @@ int nft_chain_foreach(struct nft_handle *h, const char *table,
 	const struct builtin_table *t;
 	struct nft_chain_list *list;
 	struct nft_chain *c, *c_bak;
-	int ret;
+	int i, ret;
 
 	t = nft_table_builtin_find(h, table);
 	if (!t)
 		return -1;
 
+	for (i = 0; i < NF_INET_NUMHOOKS; i++) {
+		c = h->cache->table[t->type].base_chains[i];
+		if (!c)
+			continue;
+
+		ret = cb(c, data);
+		if (ret < 0)
+			return ret;
+	}
+
 	list = h->cache->table[t->type].chains;
 	if (!list)
 		return -1;
diff --git a/iptables/nft.h b/iptables/nft.h
index ac227b4c6c581..1a2506eea7b6c 100644
--- a/iptables/nft.h
+++ b/iptables/nft.h
@@ -40,6 +40,7 @@ enum nft_cache_level {
 
 struct nft_cache {
 	struct {
+		struct nft_chain	*base_chains[NF_INET_NUMHOOKS];
 		struct nft_chain_list	*chains;
 		struct nftnl_set_list	*sets;
 		bool			exists;
-- 
2.28.0

[iptables PATCH v3 7/9] nft: cache: Sort custom chains by name

[Phil Sutter]

With base chains no longer residing in the tables' chain lists, they can
easily be sorted upon insertion. This on one hand aligns custom chain
ordering with legacy iptables and on the other makes it predictable,
which is very helpful when manually comparing ruleset dumps for
instance.

Adjust the one ebtables-nft test case this change breaks (as wrong
ordering is expected in there). The manual output sorting done for tests
which apply to legacy as well as nft is removed in a separate patch.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft-cache.c                              | 15 +++++++++++++--
 .../ebtables/0002-ebtables-save-restore_0         |  2 +-
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/iptables/nft-cache.c b/iptables/nft-cache.c
index bd19b6dfc4d8a..6b6e6da40a826 100644
--- a/iptables/nft-cache.c
+++ b/iptables/nft-cache.c
@@ -223,8 +223,19 @@ int nft_cache_add_chain(struct nft_handle *h, const struct builtin_table *t,
 
 		h->cache->table[t->type].base_chains[hooknum] = nc;
 	} else {
-		list_add_tail(&nc->head,
-			      &h->cache->table[t->type].chains->list);
+		struct nft_chain_list *clist = h->cache->table[t->type].chains;
+		struct list_head *pos = &clist->list;
+		struct nft_chain *cur;
+		const char *n;
+
+		list_for_each_entry(cur, &clist->list, head) {
+			n = nftnl_chain_get_str(cur->nftnl, NFTNL_CHAIN_NAME);
+			if (strcmp(cname, n) <= 0) {
+				pos = &cur->head;
+				break;
+			}
+		}
+		list_add_tail(&nc->head, pos);
 	}
 	hlist_add_head(&nc->hnode, chain_name_hlist(h, t, cname));
 	return 0;
diff --git a/iptables/tests/shell/testcases/ebtables/0002-ebtables-save-restore_0 b/iptables/tests/shell/testcases/ebtables/0002-ebtables-save-restore_0
index b84f63a7c3672..ccdef19cfb215 100755
--- a/iptables/tests/shell/testcases/ebtables/0002-ebtables-save-restore_0
+++ b/iptables/tests/shell/testcases/ebtables/0002-ebtables-save-restore_0
@@ -70,8 +70,8 @@ DUMP='*filter
 :INPUT ACCEPT
 :FORWARD DROP
 :OUTPUT ACCEPT
-:foo ACCEPT
 :bar RETURN
+:foo ACCEPT
 -A INPUT -p IPv4 -i lo -j ACCEPT
 -A FORWARD -j foo
 -A OUTPUT -s Broadcast -j DROP
-- 
2.28.0

[iptables PATCH v3 8/9] tests: shell: Drop any dump sorting in place

[Phil Sutter]

With iptables-nft-save output now sorted just like legacy one, no
sorting to unify them is needed anymore.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 .../firewalld-restore/0001-firewalld_0          | 17 ++---------------
 .../testcases/ipt-restore/0007-flush-noflush_0  |  4 ++--
 .../ipt-restore/0014-verbose-restore_0          |  2 +-
 3 files changed, 5 insertions(+), 18 deletions(-)

diff --git a/iptables/tests/shell/testcases/firewalld-restore/0001-firewalld_0 b/iptables/tests/shell/testcases/firewalld-restore/0001-firewalld_0
index 0174b03f4ebc7..4900554e7d9e6 100755
--- a/iptables/tests/shell/testcases/firewalld-restore/0001-firewalld_0
+++ b/iptables/tests/shell/testcases/firewalld-restore/0001-firewalld_0
@@ -230,21 +230,8 @@ for table in nat mangle raw filter;do
 	$XT_MULTI iptables-save -t $table | grep -v '^#' >> "$tmpfile"
 done
 
-case "$XT_MULTI" in
-*xtables-nft-multi)
-	# nft-multi displays chain names in different order, work around this for now
-	tmpfile2=$(mktemp)
-	sort "$tmpfile" > "$tmpfile2"
-	sort $(dirname "$0")/dumps/ipt-save-completed.txt > "$tmpfile"
-	diff -u $tmpfile $tmpfile2
-	RET=$?
-	rm -f "$tmpfile2"
-	;;
-*)
-	diff -u $tmpfile  $(dirname "$0")/dumps/ipt-save-completed.txt
-	RET=$?
-	;;
-esac
+diff -u $tmpfile  $(dirname "$0")/dumps/ipt-save-completed.txt
+RET=$?
 
 rm -f "$tmpfile"
 
diff --git a/iptables/tests/shell/testcases/ipt-restore/0007-flush-noflush_0 b/iptables/tests/shell/testcases/ipt-restore/0007-flush-noflush_0
index 029db2235b9a4..e705b28c87359 100755
--- a/iptables/tests/shell/testcases/ipt-restore/0007-flush-noflush_0
+++ b/iptables/tests/shell/testcases/ipt-restore/0007-flush-noflush_0
@@ -18,7 +18,7 @@ EXPECT="*nat
 :POSTROUTING ACCEPT [0:0]
 -A POSTROUTING -j ACCEPT
 COMMIT"
-diff -u -Z <(echo -e "$EXPECT" | sort) <($XT_MULTI iptables-save | grep -v '^#' | sort)
+diff -u -Z <(echo -e "$EXPECT") <($XT_MULTI iptables-save | grep -v '^#')
 
 $XT_MULTI iptables-restore <<EOF
 *filter
@@ -39,4 +39,4 @@ COMMIT
 :POSTROUTING ACCEPT [0:0]
 -A POSTROUTING -j ACCEPT
 COMMIT"
-diff -u -Z <(echo -e "$EXPECT" | sort) <($XT_MULTI iptables-save | grep -v '^#' | sort)
+diff -u -Z <(echo -e "$EXPECT") <($XT_MULTI iptables-save | grep -v '^#')
diff --git a/iptables/tests/shell/testcases/ipt-restore/0014-verbose-restore_0 b/iptables/tests/shell/testcases/ipt-restore/0014-verbose-restore_0
index 94bed0ec29c6b..fc8559c5bac9e 100755
--- a/iptables/tests/shell/testcases/ipt-restore/0014-verbose-restore_0
+++ b/iptables/tests/shell/testcases/ipt-restore/0014-verbose-restore_0
@@ -59,7 +59,7 @@ Flushing chain \`secfoo'
 Deleting chain \`secfoo'"
 
 for ipt in iptables-restore ip6tables-restore; do
-	diff -u -Z <(sort <<< "$EXPECT") <($XT_MULTI $ipt -v <<< "$DUMP" | sort)
+	diff -u -Z <(echo "$EXPECT") <($XT_MULTI $ipt -v <<< "$DUMP")
 done
 
 DUMP="*filter
-- 
2.28.0

[iptables PATCH v3 9/9] nft: Avoid pointless table/chain creation

[Phil Sutter]

Accept a chain name in nft_xt_builtin_init() to limit the base chain
creation to that specific chain only.

Introduce nft_xt_builtin_table_init() to create just the table for
situations where no builtin chains are needed but the command may still
succeed in an empty ruleset, particularly when creating a custom chain,
restoring base chains or adding a set for ebtables among match.

Introduce nft_xt_fake_builtin_chains(), a function to call after cache
has been populated to fill empty base chain slots. This keeps ruleset
listing output intact if some base chains do not exist (or even the
whole ruleset is completely empty).

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

diff --git a/iptables/nft.c b/iptables/nft.c
index 4187e691d8926..bde4ca72d3fcc 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -688,7 +688,8 @@ nft_chain_builtin_alloc(const struct builtin_table *table,
 
 static void nft_chain_builtin_add(struct nft_handle *h,
 				  const struct builtin_table *table,
-				  const struct builtin_chain *chain)
+				  const struct builtin_chain *chain,
+				  bool fake)
 {
 	struct nftnl_chain *c;
 
@@ -696,7 +697,8 @@ static void nft_chain_builtin_add(struct nft_handle *h,
 	if (c == NULL)
 		return;
 
-	batch_chain_add(h, NFT_COMPAT_CHAIN_ADD, c);
+	if (!fake)
+		batch_chain_add(h, NFT_COMPAT_CHAIN_ADD, c);
 	nft_cache_add_chain(h, table, c);
 }
 
@@ -748,29 +750,57 @@ static void nft_chain_builtin_init(struct nft_handle *h,
 		if (nft_chain_find(h, table->name, table->chains[i].name))
 			continue;
 
-		nft_chain_builtin_add(h, table, &table->chains[i]);
+		nft_chain_builtin_add(h, table, &table->chains[i], false);
 	}
 }
 
-static int nft_xt_builtin_init(struct nft_handle *h, const char *table)
+static const struct builtin_table *
+nft_xt_builtin_table_init(struct nft_handle *h, const char *table)
 {
 	const struct builtin_table *t;
 
 	if (!h->cache_init)
-		return 0;
+		return NULL;
 
 	t = nft_table_builtin_find(h, table);
 	if (t == NULL)
-		return -1;
+		return NULL;
 
 	if (nft_table_builtin_add(h, t) < 0)
+		return NULL;
+
+	return t;
+}
+
+static int nft_xt_builtin_init(struct nft_handle *h, const char *table,
+			       const char *chain)
+{
+	const struct builtin_table *t;
+	const struct builtin_chain *c;
+
+	if (!h->cache_init)
+		return 0;
+
+	t = nft_xt_builtin_table_init(h, table);
+	if (!t)
 		return -1;
 
 	if (h->cache_req.level < NFT_CL_CHAINS)
 		return 0;
 
-	nft_chain_builtin_init(h, t);
+	if (!chain) {
+		nft_chain_builtin_init(h, t);
+		return 0;
+	}
+
+	c = nft_chain_builtin_find(t, chain);
+	if (!c)
+		return -1;
+
+	if (h->cache->table[t->type].base_chains[c->hook])
+		return 0;
 
+	nft_chain_builtin_add(h, t, c, false);
 	return 0;
 }
 
@@ -782,6 +812,40 @@ static bool nft_chain_builtin(struct nftnl_chain *c)
 	return nftnl_chain_get(c, NFTNL_CHAIN_HOOKNUM) != NULL;
 }
 
+static int __nft_xt_fake_builtin_chains(struct nft_handle *h,
+				        const char *table, void *data)
+{
+	const char *chain = data ? *(const char **)data : NULL;
+	const struct builtin_table *t;
+	struct nft_chain **bcp;
+	int i;
+
+	t = nft_table_builtin_find(h, table);
+	if (!t)
+		return -1;
+
+	bcp = h->cache->table[t->type].base_chains;
+	for (i = 0; i < NF_INET_NUMHOOKS && t->chains[i].name; i++) {
+		if (bcp[t->chains[i].hook])
+			continue;
+
+		if (chain && strcmp(chain, t->chains[i].name))
+			continue;
+
+		nft_chain_builtin_add(h, t, &t->chains[i], true);
+	}
+	return 0;
+}
+
+int nft_xt_fake_builtin_chains(struct nft_handle *h,
+			       const char *table, const char *chain)
+{
+	if (table)
+		return __nft_xt_fake_builtin_chains(h, table, &chain);
+
+	return nft_for_each_table(h, __nft_xt_fake_builtin_chains, &chain);
+}
+
 int nft_restart(struct nft_handle *h)
 {
 	mnl_socket_close(h->nl);
@@ -874,7 +938,7 @@ static struct nftnl_chain *nft_chain_new(struct nft_handle *h,
 	}
 
 	/* if this built-in table does not exists, create it */
-	nft_xt_builtin_init(h, table);
+	nft_xt_builtin_init(h, table, chain);
 
 	_c = nft_chain_builtin_find(_t, chain);
 	if (_c != NULL) {
@@ -1391,7 +1455,7 @@ nft_rule_append(struct nft_handle *h, const char *chain, const char *table,
 	struct nft_chain *c;
 	int type;
 
-	nft_xt_builtin_init(h, table);
+	nft_xt_builtin_init(h, table, chain);
 
 	nft_fn = nft_rule_append;
 
@@ -1671,7 +1735,7 @@ int nft_rule_flush(struct nft_handle *h, const char *chain, const char *table,
 	nft_fn = nft_rule_flush;
 
 	if (chain || verbose)
-		nft_xt_builtin_init(h, table);
+		nft_xt_builtin_init(h, table, chain);
 	else if (!nft_table_find(h, table))
 		return 1;
 
@@ -1703,7 +1767,7 @@ int nft_chain_user_add(struct nft_handle *h, const char *chain, const char *tabl
 
 	nft_fn = nft_chain_user_add;
 
-	nft_xt_builtin_init(h, table);
+	t = nft_xt_builtin_table_init(h, table);
 
 	if (nft_chain_exists(h, table, chain)) {
 		errno = EEXIST;
@@ -1722,7 +1786,6 @@ int nft_chain_user_add(struct nft_handle *h, const char *chain, const char *tabl
 	if (!batch_chain_add(h, NFT_COMPAT_CHAIN_USER_ADD, c))
 		return 0;
 
-	t = nft_table_builtin_find(h, table);
 	nft_cache_add_chain(h, t, c);
 
 	/* the core expects 1 for success and 0 for error */
@@ -1737,7 +1800,7 @@ int nft_chain_restore(struct nft_handle *h, const char *chain, const char *table
 	struct nft_chain *nc;
 	bool created = false;
 
-	nft_xt_builtin_init(h, table);
+	t = nft_xt_builtin_table_init(h, table);
 
 	nc = nft_chain_find(h, table, chain);
 	if (!nc) {
@@ -1749,7 +1812,6 @@ int nft_chain_restore(struct nft_handle *h, const char *chain, const char *table
 		nftnl_chain_set_str(c, NFTNL_CHAIN_NAME, chain);
 		created = true;
 
-		t = nft_table_builtin_find(h, table);
 		nft_cache_add_chain(h, t, c);
 	} else {
 		c = nc->nftnl;
@@ -2149,7 +2211,7 @@ int nft_rule_insert(struct nft_handle *h, const char *chain,
 	struct nftnl_rule *r = NULL;
 	struct nft_chain *c;
 
-	nft_xt_builtin_init(h, table);
+	nft_xt_builtin_init(h, table, chain);
 
 	nft_fn = nft_rule_insert;
 
@@ -2358,7 +2420,7 @@ int nft_rule_list(struct nft_handle *h, const char *chain, const char *table,
 	};
 	struct nft_chain *c;
 
-	nft_xt_builtin_init(h, table);
+	nft_xt_fake_builtin_chains(h, table, chain);
 	nft_assert_table_compatible(h, table, chain);
 
 	if (chain) {
@@ -2459,7 +2521,7 @@ int nft_rule_list_save(struct nft_handle *h, const char *chain,
 	struct nft_chain *c;
 	int ret = 0;
 
-	nft_xt_builtin_init(h, table);
+	nft_xt_fake_builtin_chains(h, table, chain);
 	nft_assert_table_compatible(h, table, chain);
 
 	if (counters < 0)
@@ -3069,7 +3131,7 @@ static int nft_prepare(struct nft_handle *h)
 							cmd->chain, cmd->policy);
 			break;
 		case NFT_COMPAT_SET_ADD:
-			nft_xt_builtin_init(h, cmd->table);
+			nft_xt_builtin_table_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 1a2506eea7b6c..0910f82a2773c 100644
--- a/iptables/nft.h
+++ b/iptables/nft.h
@@ -136,6 +136,7 @@ bool nft_table_find(struct nft_handle *h, const char *tablename);
 int nft_table_purge_chains(struct nft_handle *h, const char *table, struct nftnl_chain_list *list);
 int nft_table_flush(struct nft_handle *h, const char *table);
 const struct builtin_table *nft_table_builtin_find(struct nft_handle *h, const char *table);
+int nft_xt_fake_builtin_chains(struct nft_handle *h, const char *table, const char *chain);
 
 /*
  * Operations with chains.
diff --git a/iptables/xtables-save.c b/iptables/xtables-save.c
index bf00b0324cc4f..d7901c650ea70 100644
--- a/iptables/xtables-save.c
+++ b/iptables/xtables-save.c
@@ -236,6 +236,7 @@ xtables_save_main(int family, int argc, char *argv[],
 
 	nft_cache_level_set(&h, NFT_CL_RULES, NULL);
 	nft_cache_build(&h);
+	nft_xt_fake_builtin_chains(&h, tablename, NULL);
 
 	ret = do_output(&h, tablename, &d);
 	nft_fini(&h);
-- 
2.28.0

Re: [iptables PATCH v3 0/9] nft: Sorted chain listing et al.

[Phil Sutter]

Hi,

On Thu, Dec 10, 2020 at 02:06:27PM +0100, Phil Sutter wrote:
[...]
> * Drop getters previously introduced along with struct nft_chain to
>   reduce size of patch 5. Extracting data from embedded nftnl_chain into
>   nft_chain and back if needed is future work.

In addition to a "common" review of my patches, I would like to ask you
to consider patch 5 and the code it adds separately as a direct result
of the premise to not add a sorting function to libnftnl (patch here[1])
in order to keep the library's size small.

A consequent continuation of patch 5 is the implementation of converters
from nftnl_chain to nft_chain and vice versa. While this should reduce
cache size a bit (struct nftnl_chain is pretty big), it adds overhead to
cache fetch and commit operations.

After all, I'm not sure if the direction is feasible given the
code-duplication it caused to manage a list of chains in iptables
instead of using the chain list functionality of libnftnl.

Cheers, Phil

[1] https://lore.kernel.org/netfilter-devel/20200711084505.23825-1-phil@nwl.cc/