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 <p...@nwl.cc>
---
 iptables/nft.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

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;
        }
-- 
2.23.0

Reply via email to