All of lore.kernel.org
 help / color / mirror / Atom feed
From: Phil Sutter <phil@nwl.cc>
To: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: netfilter-devel@vger.kernel.org
Subject: [iptables PATCH v2 14/18] nft-cache: Fetch cache per table
Date: Tue, 28 Apr 2020 14:10:09 +0200	[thread overview]
Message-ID: <20200428121013.24507-15-phil@nwl.cc> (raw)
In-Reply-To: <20200428121013.24507-1-phil@nwl.cc>

Restore per-table operation of cache routines as initially implemented
in commit e2883c5531e6e ("nft-cache: Support partial cache per table").

As before, this doesn't limit fetching of tables (their number is
supposed to be low) but instead limits fetching of sets, chains and
rules to the specified table.

For this to behave correctly when restoring without flushing over
multiple tables, cache must be freed fully after each commit - otherwise
the previous table's cache level is reused for the current one. The
exception being fake cache, used for flushing restore: NFT_CL_FAKE is
set just once at program startup, so it must stay set otherwise
consecutive tables cause pointless cache fetching.

The sole use-case requiring a multi-table cache, iptables-save, is
indicated by req->table being NULL. Therefore, req->table assignment is
a bit sloppy: All calls to nft_cache_level_set() are assumed to set the
same table value, no collision detection happens.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft-cache.c       | 33 ++++++++++++++--------
 iptables/nft-cache.h       |  4 ++-
 iptables/nft-cmd.c         | 57 +++++++++++++++++++-------------------
 iptables/nft.h             |  1 +
 iptables/xtables-restore.c |  2 +-
 iptables/xtables-save.c    |  2 +-
 6 files changed, 56 insertions(+), 43 deletions(-)

diff --git a/iptables/nft-cache.c b/iptables/nft-cache.c
index 305f2c12307f7..5cbe7b80d084d 100644
--- a/iptables/nft-cache.c
+++ b/iptables/nft-cache.c
@@ -24,14 +24,18 @@
 #include "nft.h"
 #include "nft-cache.h"
 
-void nft_cache_level_set(struct nft_handle *h, int level)
+void nft_cache_level_set(struct nft_handle *h, int level,
+			 const struct nft_cmd *cmd)
 {
 	struct nft_cache_req *req = &h->cache_req;
 
-	if (level <= req->level)
+	if (level > req->level)
+		req->level = level;
+
+	if (!cmd)
 		return;
 
-	req->level = level;
+	req->table = cmd->table;
 }
 
 static int genid_cb(const struct nlmsghdr *nlh, void *data)
@@ -435,10 +439,14 @@ static void
 __nft_build_cache(struct nft_handle *h)
 {
 	struct nft_cache_req *req = &h->cache_req;
+	const struct builtin_table *t = NULL;
 
 	if (h->cache_init)
 		return;
 
+	if (req->table)
+		t = nft_table_builtin_find(h, req->table);
+
 	h->cache_init = true;
 	mnl_genid_get(h, &h->nft_genid);
 
@@ -447,11 +455,11 @@ __nft_build_cache(struct nft_handle *h)
 	if (req->level == NFT_CL_FAKE)
 		return;
 	if (req->level >= NFT_CL_CHAINS)
-		fetch_chain_cache(h, NULL, NULL);
+		fetch_chain_cache(h, t, NULL);
 	if (req->level >= NFT_CL_SETS)
-		fetch_set_cache(h, NULL, NULL);
+		fetch_set_cache(h, t, NULL);
 	if (req->level >= NFT_CL_RULES)
-		fetch_rule_cache(h, NULL);
+		fetch_rule_cache(h, t);
 }
 
 static void __nft_flush_cache(struct nft_handle *h)
@@ -575,14 +583,17 @@ void nft_cache_build(struct nft_handle *h)
 
 void nft_release_cache(struct nft_handle *h)
 {
-	if (!h->cache_index)
-		return;
+	struct nft_cache_req *req = &h->cache_req;
 
+	while (h->cache_index)
+		flush_cache(h, &h->__cache[h->cache_index--], NULL);
 	flush_cache(h, &h->__cache[0], NULL);
-	memcpy(&h->__cache[0], &h->__cache[1], sizeof(h->__cache[0]));
-	memset(&h->__cache[1], 0, sizeof(h->__cache[1]));
-	h->cache_index = 0;
 	h->cache = &h->__cache[0];
+	h->cache_init = false;
+
+	if (req->level != NFT_CL_FAKE)
+		req->level = NFT_CL_TABLES;
+	req->table = NULL;
 }
 
 struct nftnl_table_list *nftnl_table_list_get(struct nft_handle *h)
diff --git a/iptables/nft-cache.h b/iptables/nft-cache.h
index 01dd15e145fd4..f429118041be4 100644
--- a/iptables/nft-cache.h
+++ b/iptables/nft-cache.h
@@ -2,8 +2,10 @@
 #define _NFT_CACHE_H_
 
 struct nft_handle;
+struct nft_cmd;
 
-void nft_cache_level_set(struct nft_handle *h, int level);
+void nft_cache_level_set(struct nft_handle *h, int level,
+			 const struct nft_cmd *cmd);
 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);
diff --git a/iptables/nft-cmd.c b/iptables/nft-cmd.c
index be9fbbf5a19bd..64889f5eb6196 100644
--- a/iptables/nft-cmd.c
+++ b/iptables/nft-cmd.c
@@ -63,12 +63,11 @@ void nft_cmd_free(struct nft_cmd *cmd)
 	free(cmd);
 }
 
-static void nft_cmd_rule_bridge(struct nft_handle *h, const char *chain,
-				const char *table)
+static void nft_cmd_rule_bridge(struct nft_handle *h, const struct nft_cmd *cmd)
 {
 	const struct builtin_table *t;
 
-	t = nft_table_builtin_find(h, table);
+	t = nft_table_builtin_find(h, cmd->table);
 	if (!t)
 		return;
 
@@ -76,10 +75,10 @@ static void nft_cmd_rule_bridge(struct nft_handle *h, const char *chain,
 	 * rule in nftables, rule cache is required here to treat them right.
 	 */
 	if (h->family == NFPROTO_BRIDGE &&
-	    !nft_chain_builtin_find(t, chain))
-		nft_cache_level_set(h, NFT_CL_RULES);
+	    !nft_chain_builtin_find(t, cmd->chain))
+		nft_cache_level_set(h, NFT_CL_RULES, cmd);
 	else
-		nft_cache_level_set(h, NFT_CL_CHAINS);
+		nft_cache_level_set(h, NFT_CL_CHAINS, cmd);
 }
 
 int nft_cmd_rule_append(struct nft_handle *h, const char *chain,
@@ -88,13 +87,13 @@ int nft_cmd_rule_append(struct nft_handle *h, const char *chain,
 {
 	struct nft_cmd *cmd;
 
-	nft_cmd_rule_bridge(h, chain, table);
-
 	cmd = nft_cmd_new(h, NFT_COMPAT_RULE_APPEND, table, chain, state, -1,
 			  verbose);
 	if (!cmd)
 		return 0;
 
+	nft_cmd_rule_bridge(h, cmd);
+
 	return 1;
 }
 
@@ -104,17 +103,17 @@ int nft_cmd_rule_insert(struct nft_handle *h, const char *chain,
 {
 	struct nft_cmd *cmd;
 
-	nft_cmd_rule_bridge(h, chain, table);
-
 	cmd = nft_cmd_new(h, NFT_COMPAT_RULE_INSERT, table, chain, state,
 			  rulenum, verbose);
 	if (!cmd)
 		return 0;
 
+	nft_cmd_rule_bridge(h, cmd);
+
 	if (cmd->rulenum > 0)
-		nft_cache_level_set(h, NFT_CL_RULES);
+		nft_cache_level_set(h, NFT_CL_RULES, cmd);
 	else
-		nft_cache_level_set(h, NFT_CL_CHAINS);
+		nft_cache_level_set(h, NFT_CL_CHAINS, cmd);
 
 	return 1;
 }
@@ -130,7 +129,7 @@ int nft_cmd_rule_delete(struct nft_handle *h, const char *chain,
 	if (!cmd)
 		return 0;
 
-	nft_cache_level_set(h, NFT_CL_RULES);
+	nft_cache_level_set(h, NFT_CL_RULES, cmd);
 
 	return 1;
 }
@@ -145,7 +144,7 @@ int nft_cmd_rule_delete_num(struct nft_handle *h, const char *chain,
 	if (!cmd)
 		return 0;
 
-	nft_cache_level_set(h, NFT_CL_RULES);
+	nft_cache_level_set(h, NFT_CL_RULES, cmd);
 
 	return 1;
 }
@@ -160,7 +159,7 @@ int nft_cmd_rule_flush(struct nft_handle *h, const char *chain,
 	if (!cmd)
 		return 0;
 
-	nft_cache_level_set(h, NFT_CL_CHAINS);
+	nft_cache_level_set(h, NFT_CL_CHAINS, cmd);
 
 	return 1;
 }
@@ -175,7 +174,7 @@ int nft_cmd_chain_zero_counters(struct nft_handle *h, const char *chain,
 	if (!cmd)
 		return 0;
 
-	nft_cache_level_set(h, NFT_CL_CHAINS);
+	nft_cache_level_set(h, NFT_CL_CHAINS, cmd);
 
 	return 1;
 }
@@ -190,7 +189,7 @@ int nft_cmd_chain_user_add(struct nft_handle *h, const char *chain,
 	if (!cmd)
 		return 0;
 
-	nft_cache_level_set(h, NFT_CL_CHAINS);
+	nft_cache_level_set(h, NFT_CL_CHAINS, cmd);
 
 	return 1;
 }
@@ -209,9 +208,9 @@ int nft_cmd_chain_user_del(struct nft_handle *h, const char *chain,
 	 * rule cache.
 	 */
 	if (h->family == NFPROTO_BRIDGE)
-		nft_cache_level_set(h, NFT_CL_RULES);
+		nft_cache_level_set(h, NFT_CL_RULES, cmd);
 	else
-		nft_cache_level_set(h, NFT_CL_CHAINS);
+		nft_cache_level_set(h, NFT_CL_CHAINS, cmd);
 
 	return 1;
 }
@@ -228,7 +227,7 @@ int nft_cmd_chain_user_rename(struct nft_handle *h,const char *chain,
 
 	cmd->rename = strdup(newname);
 
-	nft_cache_level_set(h, NFT_CL_CHAINS);
+	nft_cache_level_set(h, NFT_CL_CHAINS, cmd);
 
 	return 1;
 }
@@ -245,7 +244,7 @@ int nft_cmd_rule_list(struct nft_handle *h, const char *chain,
 
 	cmd->format = format;
 
-	nft_cache_level_set(h, NFT_CL_RULES);
+	nft_cache_level_set(h, NFT_CL_RULES, cmd);
 
 	return 1;
 }
@@ -261,7 +260,7 @@ int nft_cmd_rule_replace(struct nft_handle *h, const char *chain,
 	if (!cmd)
 		return 0;
 
-	nft_cache_level_set(h, NFT_CL_RULES);
+	nft_cache_level_set(h, NFT_CL_RULES, cmd);
 
 	return 1;
 }
@@ -276,7 +275,7 @@ int nft_cmd_rule_check(struct nft_handle *h, const char *chain,
 	if (!cmd)
 		return 0;
 
-	nft_cache_level_set(h, NFT_CL_RULES);
+	nft_cache_level_set(h, NFT_CL_RULES, cmd);
 
 	return 1;
 }
@@ -296,7 +295,7 @@ int nft_cmd_chain_set(struct nft_handle *h, const char *table,
 	if (counters)
 		cmd->counters = *counters;
 
-	nft_cache_level_set(h, NFT_CL_CHAINS);
+	nft_cache_level_set(h, NFT_CL_CHAINS, cmd);
 
 	return 1;
 }
@@ -310,7 +309,7 @@ int nft_cmd_table_flush(struct nft_handle *h, const char *table)
 	if (!cmd)
 		return 0;
 
-	nft_cache_level_set(h, NFT_CL_TABLES);
+	nft_cache_level_set(h, NFT_CL_TABLES, cmd);
 
 	return 1;
 }
@@ -325,7 +324,7 @@ int nft_cmd_chain_restore(struct nft_handle *h, const char *chain,
 	if (!cmd)
 		return 0;
 
-	nft_cache_level_set(h, NFT_CL_CHAINS);
+	nft_cache_level_set(h, NFT_CL_CHAINS, cmd);
 
 	return 1;
 }
@@ -340,7 +339,7 @@ int nft_cmd_rule_zero_counters(struct nft_handle *h, const char *chain,
 	if (!cmd)
 		return 0;
 
-	nft_cache_level_set(h, NFT_CL_RULES);
+	nft_cache_level_set(h, NFT_CL_RULES, cmd);
 
 	return 1;
 }
@@ -357,7 +356,7 @@ int nft_cmd_rule_list_save(struct nft_handle *h, const char *chain,
 
 	cmd->counters_save = counters;
 
-	nft_cache_level_set(h, NFT_CL_RULES);
+	nft_cache_level_set(h, NFT_CL_RULES, cmd);
 
 	return 1;
 }
@@ -374,7 +373,7 @@ int ebt_cmd_user_chain_policy(struct nft_handle *h, const char *table,
 
 	cmd->policy = strdup(policy);
 
-	nft_cache_level_set(h, NFT_CL_RULES);
+	nft_cache_level_set(h, NFT_CL_RULES, cmd);
 
 	return 1;
 }
diff --git a/iptables/nft.h b/iptables/nft.h
index c6aece7d1dac8..50bcc0dfebecf 100644
--- a/iptables/nft.h
+++ b/iptables/nft.h
@@ -73,6 +73,7 @@ enum obj_update_type {
 
 struct nft_cache_req {
 	enum nft_cache_level	level;
+	const char		*table;
 };
 
 struct nft_handle {
diff --git a/iptables/xtables-restore.c b/iptables/xtables-restore.c
index ca01d17eba566..44eaf8ab6c483 100644
--- a/iptables/xtables-restore.c
+++ b/iptables/xtables-restore.c
@@ -259,7 +259,7 @@ void xtables_restore_parse(struct nft_handle *h,
 	char buffer[10240] = {};
 
 	if (!h->noflush)
-		nft_cache_level_set(h, NFT_CL_FAKE);
+		nft_cache_level_set(h, NFT_CL_FAKE, NULL);
 
 	line = 0;
 	while (fgets(buffer, sizeof(buffer), p->in)) {
diff --git a/iptables/xtables-save.c b/iptables/xtables-save.c
index f927aa6e9e404..0ce66e5d15cee 100644
--- a/iptables/xtables-save.c
+++ b/iptables/xtables-save.c
@@ -239,7 +239,7 @@ xtables_save_main(int family, int argc, char *argv[],
 		exit(EXIT_FAILURE);
 	}
 
-	nft_cache_level_set(&h, NFT_CL_RULES);
+	nft_cache_level_set(&h, NFT_CL_RULES, NULL);
 	nft_cache_build(&h);
 
 	ret = do_output(&h, tablename, &d);
-- 
2.25.1


  parent reply	other threads:[~2020-04-28 12:11 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-28 12:09 [iptables PATCH v2 00/18] iptables: introduce cache evaluation phase Phil Sutter
2020-04-28 12:09 ` [iptables PATCH v2 01/18] ebtables-restore: Drop custom table flush routine Phil Sutter
2020-04-28 12:14   ` Florian Westphal
2020-04-28 12:09 ` [iptables PATCH v2 02/18] nft: cache: Eliminate init_chain_cache() Phil Sutter
2020-04-28 12:14   ` Florian Westphal
2020-04-28 12:09 ` [iptables PATCH v2 03/18] nft: cache: Init per table set list along with chain list Phil Sutter
2020-04-28 12:15   ` Florian Westphal
2020-04-28 12:09 ` [iptables PATCH v2 04/18] nft: cache: Fetch sets per table Phil Sutter
2020-04-28 12:17   ` Florian Westphal
2020-04-28 12:10 ` [iptables PATCH v2 05/18] ebtables-restore: Table line to trigger implicit commit Phil Sutter
2020-04-28 12:10 ` [iptables PATCH v2 06/18] nft: split parsing from netlink commands Phil Sutter
2020-04-28 12:10 ` [iptables PATCH v2 07/18] nft: calculate cache requirements from list of commands Phil Sutter
2020-04-28 12:10 ` [iptables PATCH v2 08/18] nft: restore among support Phil Sutter
2020-04-28 12:10 ` [iptables PATCH v2 09/18] nft: remove cache build calls Phil Sutter
2020-04-28 12:10 ` [iptables PATCH v2 10/18] nft: missing nft_fini() call in bridge family Phil Sutter
2020-04-28 12:10 ` [iptables PATCH v2 11/18] nft: cache: Simplify rule and set fetchers Phil Sutter
2020-04-28 12:10 ` [iptables PATCH v2 12/18] nft: cache: Improve fake cache integration Phil Sutter
2020-04-28 12:10 ` [iptables PATCH v2 13/18] nft: cache: Introduce struct nft_cache_req Phil Sutter
2020-04-28 12:10 ` Phil Sutter [this message]
2020-04-28 12:10 ` [iptables PATCH v2 15/18] nft-cache: Introduce __fetch_chain_cache() Phil Sutter
2020-04-28 12:10 ` [iptables PATCH v2 16/18] nft: cache: Fetch cache for specific chains Phil Sutter
2020-04-28 12:10 ` [iptables PATCH v2 17/18] nft: cache: Optimize caching for flush command Phil Sutter
2020-04-28 12:10 ` [iptables PATCH v2 18/18] nft: Fix for '-F' in iptables dumps Phil Sutter
2020-04-29 21:36 ` [iptables PATCH v2 00/18] iptables: introduce cache evaluation phase Pablo Neira Ayuso
2020-04-30 13:53   ` Phil Sutter
2020-04-30 15:08     ` Pablo Neira Ayuso
2020-04-30 15:26       ` Phil Sutter
2020-04-30 15:44         ` Pablo Neira Ayuso
2020-04-30 15:48           ` Pablo Neira Ayuso
2020-04-30 15:52             ` Phil Sutter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200428121013.24507-15-phil@nwl.cc \
    --to=phil@nwl.cc \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=pablo@netfilter.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.