[iptables PATCH 0/3] nft: cache: Minor review

[iptables PATCH 0/3] nft: cache: Minor review

[Phil Sutter]

Minor code simplification in patches 1 and 2, a small tweak to set
fetching in patch 3.

Basically these are fall-out from working at rewritten cache logic.

Phil Sutter (3):
  nft: cache: Eliminate init_chain_cache()
  nft: cache: Init per table set list along with chain list
  nft: cache: Fetch sets per table

 iptables/nft-cache.c | 57 ++++++++++++++++----------------------------
 1 file changed, 21 insertions(+), 36 deletions(-)

-- 
2.25.1

[iptables PATCH 1/3] 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

[iptables PATCH 2/3] 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

[iptables PATCH 3/3] 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 0/3] nft: cache: Minor review

[Pablo Neira Ayuso]

On Tue, Apr 07, 2020 at 04:34:42PM +0200, Phil Sutter wrote:
> Minor code simplification in patches 1 and 2, a small tweak to set
> fetching in patch 3.
> 
> Basically these are fall-out from working at rewritten cache logic.

LGTM.

These are not clashing with my pending patches, right? :-)

Thanks.