diff mbox series

[iptables,14/14] nft: Reduce impact of nft_chain_builtin_init()

Message ID 20190916165000.18217-15-phil@nwl.cc
State Changes Requested
Delegated to: Pablo Neira
Headers show
Series Improve iptables-nft performance with large rulesets | expand

Commit Message

Phil Sutter Sept. 16, 2019, 4:50 p.m. UTC 
When initializing builtin chains, fetch only the chain to add from
kernel to reduce overhead in case kernel ruleset contains many chains.

Given the flexibility of nft_chain_list_get() this is not a problem, but
care has to be taken when updating the table's chain list: Since a
command like 'iptables-nft -F INPUT' causes two invocations of
nft_chain_list_get() with same arguments, the same chain will be fetched
from kernel twice. To handle this, simply clearing chain list from
fetch_chain_cache() is not an option as list elements might be
referenced by pending batch jobs. Instead abort in nftnl_chain_list_cb()
if a chain with same name already exists in the list. Also, the call to
fetch_table_cache() must happen conditionally to avoid leaking existing
table list.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)
diff mbox series

Patch

diff --git a/iptables/nft.c b/iptables/nft.c
index 5eb129461dab2..66011a8066ed9 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -750,15 +750,15 @@  nft_chain_builtin_find(const struct builtin_table *t, const char *chain)
 static void nft_chain_builtin_init(struct nft_handle *h,
 				   const struct builtin_table *table)
 {
-	struct nftnl_chain_list *list = nft_chain_list_get(h, table->name, NULL);
+	struct nftnl_chain_list *list;
 	struct nftnl_chain *c;
 	int i;
 
-	if (!list)
-		return;
-
 	/* Initialize built-in chains if they don't exist yet */
 	for (i=0; i < NF_INET_NUMHOOKS && table->chains[i].name != NULL; i++) {
+		list = nft_chain_list_get(h, table->name, table->chains[i].name);
+		if (!list)
+			continue;
 
 		c = nftnl_chain_list_lookup_byname(list, table->chains[i].name);
 		if (c != NULL)
@@ -1374,7 +1374,8 @@  static int nftnl_chain_list_cb(const struct nlmsghdr *nlh, void *data)
 {
 	struct nft_handle *h = data;
 	const struct builtin_table *t;
-	struct nftnl_chain *c;
+	struct nftnl_chain *c, *c2;
+	const char *tname, *cname;
 
 	c = nftnl_chain_alloc();
 	if (c == NULL)
@@ -1383,11 +1384,17 @@  static int nftnl_chain_list_cb(const struct nlmsghdr *nlh, void *data)
 	if (nftnl_chain_nlmsg_parse(nlh, c) < 0)
 		goto out;
 
-	t = nft_table_builtin_find(h,
-			nftnl_chain_get_str(c, NFTNL_CHAIN_TABLE));
+	tname = nftnl_chain_get_str(c, NFTNL_CHAIN_TABLE);
+	t = nft_table_builtin_find(h, tname);
 	if (!t)
 		goto out;
 
+	cname = nftnl_chain_get_str(c, NFTNL_CHAIN_NAME);
+	c2 = nftnl_chain_list_lookup_byname(h->cache->table[t->type].chains,
+					    cname);
+	if (c2)
+		goto out;
+
 	nftnl_chain_list_add_tail(c, h->cache->table[t->type].chains);
 
 	return MNL_CB_OK;
@@ -1447,7 +1454,8 @@  static int fetch_chain_cache(struct nft_handle *h, const char *table, const char
 	struct nlmsghdr *nlh;
 	int i, ret;
 
-	fetch_table_cache(h);
+	if (!h->cache->tables)
+		fetch_table_cache(h);
 
 	for (i = 0; i < NFT_TABLE_MAX; i++) {
 		enum nft_table_type type = h->tables[i].type;
@@ -1462,10 +1470,9 @@  static int fetch_chain_cache(struct nft_handle *h, const char *table, const char
 		if (chain && table && strcmp(table, h->tables[i].name))
 			continue;
 
-		if (h->cache->table[type].chains)
-			nftnl_chain_list_free(h->cache->table[type].chains);
+		if (!h->cache->table[type].chains)
+			h->cache->table[type].chains = nftnl_chain_list_alloc();
 
-		h->cache->table[type].chains = nftnl_chain_list_alloc();
 		if (!h->cache->table[type].chains)
 			return -1;
 	}