From c58ecf9f8bcb7619a27ef8ffaddf847a562475a5 Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Thu, 15 Nov 2018 14:53:02 +0100 Subject: xtables: Introduce per table chain caches Being able to omit the previously obligatory table name check when iterating over the chain cache might help restore performance with large rulesets in xtables-save and -restore. There is one subtle quirk in the code: flush_chain_cache() did free the global chain cache if not called with a table name but didn't if a table name was given even if it emptied the chain cache. In other places, chain_cache being non-NULL prevented a cache update from happening, so this patch establishes the same behaviour (for each individual chain cache) since otherwise unexpected cache updates lead to weird problems. Signed-off-by: Phil Sutter Signed-off-by: Pablo Neira Ayuso --- iptables/nft-shared.h | 3 +- iptables/nft.c | 160 ++++++++++++++++++++------------------------- iptables/nft.h | 10 +-- iptables/xtables-restore.c | 16 ++--- iptables/xtables-save.c | 12 ++-- 5 files changed, 95 insertions(+), 106 deletions(-) diff --git a/iptables/nft-shared.h b/iptables/nft-shared.h index e3ecdb4d..9a61d8d2 100644 --- a/iptables/nft-shared.h +++ b/iptables/nft-shared.h @@ -251,7 +251,8 @@ struct nftnl_chain_list; struct nft_xt_restore_cb { void (*table_new)(struct nft_handle *h, const char *table); - struct nftnl_chain_list *(*chain_list)(struct nft_handle *h); + struct nftnl_chain_list *(*chain_list)(struct nft_handle *h, + const char *table); void (*chain_del)(struct nftnl_chain_list *clist, const char *curtable, const char *chain); int (*chain_user_flush)(struct nft_handle *h, diff --git a/iptables/nft.c b/iptables/nft.c index e8538d38..5e55ec13 100644 --- a/iptables/nft.c +++ b/iptables/nft.c @@ -673,15 +673,17 @@ nft_chain_builtin_find(struct builtin_table *t, const char *chain) static void nft_chain_builtin_init(struct nft_handle *h, struct builtin_table *table) { - struct nftnl_chain_list *list = nft_chain_list_get(h); + struct nftnl_chain_list *list = nft_chain_list_get(h, table->name); 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++) { - c = nft_chain_list_find(list, table->name, - table->chains[i].name); + c = nft_chain_list_find(list, table->chains[i].name); if (c != NULL) continue; @@ -782,27 +784,33 @@ static void flush_rule_cache(struct nft_handle *h, const char *tablename) static int __flush_chain_cache(struct nftnl_chain *c, void *data) { - const char *tablename = data; - - if (!strcmp(nftnl_chain_get_str(c, NFTNL_CHAIN_TABLE), tablename)) { - nftnl_chain_list_del(c); - nftnl_chain_free(c); - } + nftnl_chain_list_del(c); + nftnl_chain_free(c); return 0; } static void flush_chain_cache(struct nft_handle *h, const char *tablename) { - if (!h->chain_cache) - return; + int i; - if (tablename) { - nftnl_chain_list_foreach(h->chain_cache, __flush_chain_cache, - (void *)tablename); - } else { - nftnl_chain_list_free(h->chain_cache); - h->chain_cache = NULL; + for (i = 0; i < NFT_TABLE_MAX; i++) { + if (h->tables[i].name == NULL) + continue; + + if (tablename && strcmp(h->tables[i].name, tablename)) + continue; + + if (h->tables[i].chain_cache) { + if (tablename) { + nftnl_chain_list_foreach(h->tables[i].chain_cache, + __flush_chain_cache, NULL); + break; + } else { + nftnl_chain_list_free(h->tables[i].chain_cache); + h->tables[i].chain_cache = NULL; + } + } } } @@ -1271,8 +1279,9 @@ nft_rule_print_save(const struct nftnl_rule *r, enum nft_rule_print type, static int nftnl_chain_list_cb(const struct nlmsghdr *nlh, void *data) { + struct nft_handle *h = data; + struct builtin_table *t; struct nftnl_chain *c; - struct nftnl_chain_list *list = data; c = nftnl_chain_alloc(); if (c == NULL) @@ -1281,7 +1290,18 @@ static int nftnl_chain_list_cb(const struct nlmsghdr *nlh, void *data) if (nftnl_chain_nlmsg_parse(nlh, c) < 0) goto out; - nftnl_chain_list_add_tail(c, list); + t = nft_table_builtin_find(h, + nftnl_chain_get_str(c, NFTNL_CHAIN_TABLE)); + if (!t) + goto out; + + if (!t->chain_cache) { + t->chain_cache = nftnl_chain_list_alloc(); + if (!t->chain_cache) + goto out; + } + + nftnl_chain_list_add_tail(c, t->chain_cache); return MNL_CB_OK; out: @@ -1290,35 +1310,34 @@ err: return MNL_CB_OK; } -struct nftnl_chain_list *nft_chain_list_get(struct nft_handle *h) +struct nftnl_chain_list *nft_chain_list_get(struct nft_handle *h, + const char *table) { char buf[16536]; struct nlmsghdr *nlh; - struct nftnl_chain_list *list; + struct builtin_table *t; int ret; - if (h->chain_cache) - return h->chain_cache; -retry: - list = nftnl_chain_list_alloc(); - if (list == NULL) { - errno = ENOMEM; + t = nft_table_builtin_find(h, table); + if (!t) return NULL; - } + if (t->chain_cache) + return t->chain_cache; +retry: 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, list); + ret = mnl_talk(h, nlh, nftnl_chain_list_cb, h); if (ret < 0 && errno == EINTR) { assert(nft_restart(h) >= 0); - nftnl_chain_list_free(list); goto retry; } - h->chain_cache = list; + if (!t->chain_cache) + t->chain_cache = nftnl_chain_list_alloc(); - return list; + return t->chain_cache; } static const char *policy_name[NF_ACCEPT+1] = { @@ -1326,8 +1345,7 @@ static const char *policy_name[NF_ACCEPT+1] = { [NF_ACCEPT] = "ACCEPT", }; -int nft_chain_save(struct nft_handle *h, struct nftnl_chain_list *list, - const char *table) +int nft_chain_save(struct nft_handle *h, struct nftnl_chain_list *list) { struct nftnl_chain_list_iter *iter; struct nft_family_ops *ops; @@ -1341,13 +1359,8 @@ int nft_chain_save(struct nft_handle *h, struct nftnl_chain_list *list, c = nftnl_chain_list_iter_next(iter); while (c != NULL) { - const char *chain_table = - nftnl_chain_get_str(c, NFTNL_CHAIN_TABLE); const char *policy = NULL; - if (strcmp(table, chain_table) != 0) - goto next; - if (nft_chain_builtin(c)) { uint32_t pol = NF_ACCEPT; @@ -1358,7 +1371,7 @@ int nft_chain_save(struct nft_handle *h, struct nftnl_chain_list *list, if (ops->save_chain) ops->save_chain(c, policy); -next: + c = nftnl_chain_list_iter_next(iter); } @@ -1529,7 +1542,7 @@ int nft_rule_flush(struct nft_handle *h, const char *chain, const char *table, nft_fn = nft_rule_flush; - list = nft_chain_list_get(h); + list = nft_chain_list_get(h, table); if (list == NULL) { ret = 1; goto err; @@ -1543,21 +1556,16 @@ int nft_rule_flush(struct nft_handle *h, const char *chain, const char *table, c = nftnl_chain_list_iter_next(iter); while (c != NULL) { - const char *table_name = - nftnl_chain_get_str(c, NFTNL_CHAIN_TABLE); const char *chain_name = nftnl_chain_get_str(c, NFTNL_CHAIN_NAME); - if (strcmp(table, table_name) != 0) - goto next; - if (chain != NULL && strcmp(chain, chain_name) != 0) goto next; if (verbose) fprintf(stdout, "Flushing chain `%s'\n", chain_name); - __nft_rule_flush(h, table_name, chain_name); + __nft_rule_flush(h, table, chain_name); if (chain != NULL) break; @@ -1573,6 +1581,7 @@ err: int nft_chain_user_add(struct nft_handle *h, const char *chain, const char *table) { + struct nftnl_chain_list *list; struct nftnl_chain *c; int ret; @@ -1591,9 +1600,9 @@ int nft_chain_user_add(struct nft_handle *h, const char *chain, const char *tabl ret = batch_chain_add(h, NFT_COMPAT_CHAIN_USER_ADD, c); - nft_chain_list_get(h); - - nftnl_chain_list_add(c, h->chain_cache); + list = nft_chain_list_get(h, table); + if (list) + nftnl_chain_list_add(c, list); /* the core expects 1 for success and 0 for error */ return ret == 0 ? 1 : 0; @@ -1615,7 +1624,7 @@ int nft_chain_user_del(struct nft_handle *h, const char *chain, nft_fn = nft_chain_user_del; - list = nft_chain_list_get(h); + list = nft_chain_list_get(h, table); if (list == NULL) goto err; @@ -1625,8 +1634,6 @@ int nft_chain_user_del(struct nft_handle *h, const char *chain, c = nftnl_chain_list_iter_next(iter); while (c != NULL) { - const char *table_name = - nftnl_chain_get_str(c, NFTNL_CHAIN_TABLE); const char *chain_name = nftnl_chain_get_str(c, NFTNL_CHAIN_NAME); @@ -1634,9 +1641,6 @@ int nft_chain_user_del(struct nft_handle *h, const char *chain, if (nft_chain_builtin(c)) goto next; - if (strcmp(table, table_name) != 0) - goto next; - if (chain != NULL && strcmp(chain, chain_name) != 0) goto next; @@ -1671,8 +1675,7 @@ err: } struct nftnl_chain * -nft_chain_list_find(struct nftnl_chain_list *list, - const char *table, const char *chain) +nft_chain_list_find(struct nftnl_chain_list *list, const char *chain) { struct nftnl_chain_list_iter *iter; struct nftnl_chain *c; @@ -1683,14 +1686,9 @@ nft_chain_list_find(struct nftnl_chain_list *list, c = nftnl_chain_list_iter_next(iter); while (c != NULL) { - const char *table_name = - nftnl_chain_get_str(c, NFTNL_CHAIN_TABLE); const char *chain_name = nftnl_chain_get_str(c, NFTNL_CHAIN_NAME); - if (strcmp(table, table_name) != 0) - goto next; - if (strcmp(chain, chain_name) != 0) goto next; @@ -1708,11 +1706,11 @@ nft_chain_find(struct nft_handle *h, const char *table, const char *chain) { struct nftnl_chain_list *list; - list = nft_chain_list_get(h); + list = nft_chain_list_get(h, table); if (list == NULL) return NULL; - return nft_chain_list_find(list, table, chain); + return nft_chain_list_find(list, chain); } bool nft_chain_exists(struct nft_handle *h, @@ -2324,7 +2322,9 @@ int nft_rule_list(struct nft_handle *h, const char *chain, const char *table, return 1; } - list = nft_chain_list_get(h); + list = nft_chain_list_get(h, table); + if (!list) + goto err; /* XXX: return 0 instead? */ iter = nftnl_chain_list_iter_create(list); if (iter == NULL) @@ -2335,8 +2335,6 @@ int nft_rule_list(struct nft_handle *h, const char *chain, const char *table, c = nftnl_chain_list_iter_next(iter); while (c != NULL) { - const char *chain_table = - nftnl_chain_get_str(c, NFTNL_CHAIN_TABLE); const char *chain_name = nftnl_chain_get_str(c, NFTNL_CHAIN_NAME); uint32_t policy = @@ -2353,8 +2351,6 @@ int nft_rule_list(struct nft_handle *h, const char *chain, const char *table, if (nftnl_chain_get(c, NFTNL_CHAIN_HOOKNUM)) basechain = true; - if (strcmp(table, chain_table) != 0) - goto next; if (chain) { if (strcmp(chain, chain_name) != 0) goto next; @@ -2469,7 +2465,9 @@ int nft_rule_list_save(struct nft_handle *h, const char *chain, return 0; } - list = nft_chain_list_get(h); + list = nft_chain_list_get(h, table); + if (!list) + goto err; /* XXX: correct? */ /* Dump policies and custom chains first */ if (!rulenum) @@ -2487,13 +2485,9 @@ int nft_rule_list_save(struct nft_handle *h, const char *chain, c = nftnl_chain_list_iter_next(iter); while (c != NULL) { - const char *chain_table = - nftnl_chain_get_str(c, NFTNL_CHAIN_TABLE); const char *chain_name = nftnl_chain_get_str(c, NFTNL_CHAIN_NAME); - if (strcmp(table, chain_table) != 0) - goto next; if (chain && strcmp(chain, chain_name) != 0) goto next; @@ -3072,7 +3066,7 @@ 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); + list = nft_chain_list_get(h, table); if (list == NULL) goto err; @@ -3084,11 +3078,6 @@ int nft_chain_zero_counters(struct nft_handle *h, const char *chain, while (c != NULL) { const char *chain_name = nftnl_chain_get(c, NFTNL_CHAIN_NAME); - const char *chain_table = - nftnl_chain_get(c, NFTNL_CHAIN_TABLE); - - if (strcmp(table, chain_table) != 0) - goto next; if (chain != NULL && strcmp(chain, chain_name) != 0) goto next; @@ -3229,7 +3218,7 @@ static int nft_are_chains_compatible(struct nft_handle *h, const char *tablename struct nftnl_chain *chain; int ret = 0; - list = nft_chain_list_get(h); + list = nft_chain_list_get(h, tablename); if (list == NULL) return -1; @@ -3239,12 +3228,7 @@ static int nft_are_chains_compatible(struct nft_handle *h, const char *tablename chain = nftnl_chain_list_iter_next(iter); while (chain != NULL) { - const char *chain_table; - - chain_table = nftnl_chain_get_str(chain, NFTNL_CHAIN_TABLE); - - if (strcmp(chain_table, tablename) || - !nft_chain_builtin(chain)) + if (!nft_chain_builtin(chain)) goto next; ret = nft_is_chain_compatible(h, chain); diff --git a/iptables/nft.h b/iptables/nft.h index 9b4ba5f9..980b38dc 100644 --- a/iptables/nft.h +++ b/iptables/nft.h @@ -25,6 +25,7 @@ struct builtin_table { const char *name; struct builtin_chain chains[NF_INET_NUMHOOKS]; bool initialized; + struct nftnl_chain_list *chain_cache; }; struct nft_handle { @@ -38,7 +39,6 @@ struct nft_handle { struct list_head err_list; struct nft_family_ops *ops; struct builtin_table *tables; - struct nftnl_chain_list *chain_cache; struct nftnl_rule_list *rule_cache; bool restore; int8_t config_done; @@ -78,9 +78,11 @@ struct builtin_table *nft_table_builtin_find(struct nft_handle *h, const char *t 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); -struct nftnl_chain_list *nft_chain_list_get(struct nft_handle *h); -struct nftnl_chain *nft_chain_list_find(struct nftnl_chain_list *list, const char *table, const char *chain); -int nft_chain_save(struct nft_handle *h, struct nftnl_chain_list *list, const char *table); +struct nftnl_chain_list *nft_chain_list_get(struct nft_handle *h, + const char *table); +struct nftnl_chain *nft_chain_list_find(struct nftnl_chain_list *list, + const char *chain); +int nft_chain_save(struct nft_handle *h, struct nftnl_chain_list *list); 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_user_flush(struct nft_handle *h, struct nftnl_chain_list *list, diff --git a/iptables/xtables-restore.c b/iptables/xtables-restore.c index f5297740..a46a9295 100644 --- a/iptables/xtables-restore.c +++ b/iptables/xtables-restore.c @@ -56,11 +56,12 @@ static void print_usage(const char *name, const char *version) " [ --ipv6 ]\n", name); } -static struct nftnl_chain_list *get_chain_list(struct nft_handle *h) +static struct nftnl_chain_list *get_chain_list(struct nft_handle *h, + const char *table) { struct nftnl_chain_list *chain_list; - chain_list = nft_chain_list_get(h); + chain_list = nft_chain_list_get(h, table); if (chain_list == NULL) xtables_error(OTHER_PROBLEM, "cannot retrieve chain list\n"); @@ -72,7 +73,7 @@ static void chain_delete(struct nftnl_chain_list *clist, const char *curtable, { struct nftnl_chain *chain_obj; - chain_obj = nft_chain_list_find(clist, curtable, chain); + chain_obj = nft_chain_list_find(clist, chain); /* This chain has been found, delete from list. Later * on, unvisited chains will be purged out. */ @@ -112,9 +113,6 @@ void xtables_restore_parse(struct nft_handle *h, line = 0; - if (cb->chain_list) - chain_list = cb->chain_list(h); - /* Grab standard input. */ while (fgets(buffer, sizeof(buffer), p->in)) { int ret = 0; @@ -165,6 +163,9 @@ void xtables_restore_parse(struct nft_handle *h, if (p->tablename && (strcmp(p->tablename, table) != 0)) continue; + if (cb->chain_list) + chain_list = cb->chain_list(h, table); + if (noflush == 0) { DEBUGP("Cleaning all chains of table '%s'\n", table); @@ -197,8 +198,7 @@ void xtables_restore_parse(struct nft_handle *h, if (cb->chain_del) cb->chain_del(chain_list, curtable->name, chain); - } else if (nft_chain_list_find(chain_list, - curtable->name, chain)) { + } else if (nft_chain_list_find(chain_list, chain)) { chain_exists = true; /* Apparently -n still flushes existing user * defined chains that are redefined. Otherwise, diff --git a/iptables/xtables-save.c b/iptables/xtables-save.c index bed3ee03..d121d50e 100644 --- a/iptables/xtables-save.c +++ b/iptables/xtables-save.c @@ -73,7 +73,9 @@ __do_output(struct nft_handle *h, const char *tablename, bool counters) return 0; } - chain_list = nft_chain_list_get(h); + chain_list = nft_chain_list_get(h, tablename); + if (!chain_list) + return 0; time_t now = time(NULL); @@ -83,7 +85,7 @@ __do_output(struct nft_handle *h, const char *tablename, bool counters) /* Dump out chain names first, * thereby preventing dependency conflicts */ - nft_chain_save(h, chain_list, tablename); + nft_chain_save(h, chain_list); nft_rule_save(h, tablename, counters ? 0 : FMT_NOCOUNTS); now = time(NULL); @@ -257,7 +259,7 @@ static int __ebt_save(struct nft_handle *h, const char *tablename, bool counters return 0; } - chain_list = nft_chain_list_get(h); + chain_list = nft_chain_list_get(h, tablename); if (first) { now = time(NULL); @@ -272,7 +274,7 @@ static int __ebt_save(struct nft_handle *h, const char *tablename, bool counters /* Dump out chain names first, * thereby preventing dependency conflicts */ - nft_chain_save(h, chain_list, tablename); + nft_chain_save(h, chain_list); nft_rule_save(h, tablename, format); printf("\n"); return 0; @@ -399,7 +401,7 @@ int xtables_arp_save_main(int argc, char **argv) } printf("*filter\n"); - nft_chain_save(&h, nft_chain_list_get(&h), "filter"); + nft_chain_save(&h, nft_chain_list_get(&h, "filter")); nft_rule_save(&h, "filter", show_counters ? 0 : FMT_NOCOUNTS); printf("\n"); nft_fini(&h); -- cgit v1.2.3