[PATCH nft,v2 1/5] src: remove useless parameter from cache_flush()

[PATCH nft,v2 1/5] src: remove useless parameter from cache_flush()

[Pablo Neira Ayuso]

Command type is never used in cache_flush().

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
v2: no changes.

 include/rule.h | 3 +--
 src/evaluate.c | 2 +-
 src/rule.c     | 2 +-
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/include/rule.h b/include/rule.h
index b41825d000d6..299485ffeeaa 100644
--- a/include/rule.h
+++ b/include/rule.h
@@ -639,8 +639,7 @@ extern int do_command(struct netlink_ctx *ctx, struct cmd *cmd);
 extern int cache_evaluate(struct nft_ctx *nft, struct list_head *cmds);
 extern int cache_update(struct nft_ctx *ctx, enum cmd_ops cmd,
 			struct list_head *msgs);
-extern void cache_flush(struct nft_ctx *ctx, enum cmd_ops cmd,
-			struct list_head *msgs);
+extern void cache_flush(struct nft_ctx *ctx, struct list_head *msgs);
 extern void cache_release(struct nft_cache *cache);
 extern bool cache_is_complete(struct nft_cache *cache, enum cmd_ops cmd);
 
diff --git a/src/evaluate.c b/src/evaluate.c
index 70c7e597f3b0..73a4be339ce1 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -3682,7 +3682,7 @@ static int cmd_evaluate_flush(struct eval_ctx *ctx, struct cmd *cmd)
 
 	switch (cmd->obj) {
 	case CMD_OBJ_RULESET:
-		cache_flush(ctx->nft, cmd->op, ctx->msgs);
+		cache_flush(ctx->nft, ctx->msgs);
 		break;
 	case CMD_OBJ_TABLE:
 		/* Flushing a table does not empty the sets in the table nor remove
diff --git a/src/rule.c b/src/rule.c
index 0c0fd07ec70c..4407b0b0ceaa 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -297,7 +297,7 @@ static void __cache_flush(struct list_head *table_list)
 	}
 }
 
-void cache_flush(struct nft_ctx *nft, enum cmd_ops cmd, struct list_head *msgs)
+void cache_flush(struct nft_ctx *nft, struct list_head *msgs)
 {
 	struct netlink_ctx ctx = {
 		.list		= LIST_HEAD_INIT(ctx.list),
-- 
2.11.0

[PATCH nft,v2 2/5] tests: shell: cannot use handle for non-existing rule in kernel

[Pablo Neira Ayuso]

This test invokes the 'replace rule ... handle 2' command. However,
there are no rules in the kernel, therefore it always fails.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
v2: no changes.

 tests/shell/testcases/nft-f/0006action_object_0 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/shell/testcases/nft-f/0006action_object_0 b/tests/shell/testcases/nft-f/0006action_object_0
index ffa6c9bda973..fab3070f493f 100755
--- a/tests/shell/testcases/nft-f/0006action_object_0
+++ b/tests/shell/testcases/nft-f/0006action_object_0
@@ -16,7 +16,7 @@ generate1()
 	add set $family t s {type inet_service;}
 	add element $family t s {8080}
 	insert rule $family t c meta l4proto tcp tcp dport @s accept
-	replace rule $family t c handle 2 meta l4proto tcp tcp dport {9090, 8080}
+	add rule $family t c meta l4proto tcp tcp dport {9090, 8080}
 	add map $family t m {type inet_service:verdict;}
 	add element $family t m {10080:drop}
 	insert rule $family t c meta l4proto tcp tcp dport vmap @m
-- 
2.11.0

[PATCH nft,v2 3/5] rule: skip cache population from do_command_monitor()

[Pablo Neira Ayuso]

nft_evaluate() already populates the cache before running the monitor
command. Remove this code.

Fixes: 7df42800cf89 ("src: single cache_update() call to build cache before evaluation")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
v2: move patch from 4/5 to 3/5.

 src/rule.c | 32 --------------------------------
 1 file changed, 32 deletions(-)

diff --git a/src/rule.c b/src/rule.c
index 4407b0b0ceaa..bcd1c0bf73e8 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -2427,8 +2427,6 @@ static bool need_cache(const struct cmd *cmd)
 
 static int do_command_monitor(struct netlink_ctx *ctx, struct cmd *cmd)
 {
-	struct table *t;
-	struct set *s;
 	struct netlink_mon_handler monhandler = {
 		.monitor_flags	= cmd->monitor->flags,
 		.format		= cmd->monitor->format,
@@ -2442,36 +2440,6 @@ static int do_command_monitor(struct netlink_ctx *ctx, struct cmd *cmd)
 		monhandler.format = NFTNL_OUTPUT_JSON;
 
 	monhandler.cache_needed = need_cache(cmd);
-	if (monhandler.cache_needed) {
-		struct rule *rule, *nrule;
-		struct chain *chain;
-		int ret;
-
-		list_for_each_entry(t, &ctx->nft->cache.list, list) {
-			list_for_each_entry(s, &t->sets, list)
-				s->init = set_expr_alloc(&cmd->location, s);
-
-			if (!(cmd->monitor->flags & (1 << NFT_MSG_TRACE)))
-				continue;
-
-			/* When tracing we'd like to translate the rule handle
-			 * we receive in the trace messages to the actual rule
-			 * struct to print that out.  Populate rule cache now.
-			 */
-			ret = netlink_list_table(ctx, &t->handle);
-
-			if (ret != 0)
-				/* Shouldn't happen and doesn't break things
-				 * too badly
-				 */
-				continue;
-
-			list_for_each_entry_safe(rule, nrule, &ctx->list, list) {
-				chain = chain_lookup(t, &rule->handle);
-				list_move_tail(&rule->list, &chain->rules);
-			}
-		}
-	}
 
 	return netlink_monitor(&monhandler, ctx->nft->nf_sock);
 }
-- 
2.11.0

[PATCH nft,v2 4/5] netlink: remove netlink_list_table()

[Pablo Neira Ayuso]

Remove this wrapper, call netlink_list_rules() instead.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
v2: move patch from 5/5 to 4/5.

 include/netlink.h | 2 +-
 src/netlink.c     | 7 +------
 src/rule.c        | 2 +-
 3 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/include/netlink.h b/include/netlink.h
index 0c08b1abbf6a..279723f33d31 100644
--- a/include/netlink.h
+++ b/include/netlink.h
@@ -104,6 +104,7 @@ extern struct expr *netlink_alloc_data(const struct location *loc,
 				       const struct nft_data_delinearize *nld,
 				       enum nft_registers dreg);
 
+extern int netlink_list_rules(struct netlink_ctx *ctx, const struct handle *h);
 extern void netlink_linearize_rule(struct netlink_ctx *ctx,
 				   struct nftnl_rule *nlr,
 				   const struct rule *rule);
@@ -115,7 +116,6 @@ extern struct chain *netlink_delinearize_chain(struct netlink_ctx *ctx,
 					       const struct nftnl_chain *nlc);
 
 extern int netlink_list_tables(struct netlink_ctx *ctx, const struct handle *h);
-extern int netlink_list_table(struct netlink_ctx *ctx, const struct handle *h);
 extern struct table *netlink_delinearize_table(struct netlink_ctx *ctx,
 					       const struct nftnl_table *nlt);
 
diff --git a/src/netlink.c b/src/netlink.c
index a6d81b4f6424..24d8f03ae4be 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -333,7 +333,7 @@ static int list_rule_cb(struct nftnl_rule *nlr, void *arg)
 	return 0;
 }
 
-static int netlink_list_rules(struct netlink_ctx *ctx, const struct handle *h)
+int netlink_list_rules(struct netlink_ctx *ctx, const struct handle *h)
 {
 	struct nftnl_rule_list *rule_cache;
 
@@ -485,11 +485,6 @@ int netlink_list_tables(struct netlink_ctx *ctx, const struct handle *h)
 	return 0;
 }
 
-int netlink_list_table(struct netlink_ctx *ctx, const struct handle *h)
-{
-	return netlink_list_rules(ctx, h);
-}
-
 enum nft_data_types dtype_map_to_kernel(const struct datatype *dtype)
 {
 	switch (dtype->type) {
diff --git a/src/rule.c b/src/rule.c
index bcd1c0bf73e8..ed199889702f 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -191,7 +191,7 @@ static int cache_init_objects(struct netlink_ctx *ctx, enum cmd_ops cmd)
 		if (cmd != CMD_LIST)
 			continue;
 
-		ret = netlink_list_table(ctx, &table->handle);
+		ret = netlink_list_rules(ctx, &table->handle);
 		list_for_each_entry_safe(rule, nrule, &ctx->list, list) {
 			chain = chain_lookup(table, &rule->handle);
 			list_move_tail(&rule->list, &chain->rules);
-- 
2.11.0

[PATCH nft,v2 5/5] src: add cache level flags

[Pablo Neira Ayuso]

The score approach based on command type is confusing.

This patch introduces cache level flags, each flag specifies what kind
of object type is needed. These flags are set on/off depending on the
list of commands coming in this batch.

cache_is_complete() now checks if the cache contains the objects that
are needed through these new flags.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
v2: use 'enum cache_level_bits' as proposed by Phil Sutter.

 include/Makefile.am |   1 +
 include/cache.h     |  35 +++++++++++++++++
 include/nftables.h  |   2 +-
 include/rule.h      |   3 +-
 src/cache.c         |  87 ++++++++++++++++++++++--------------------
 src/evaluate.c      |   1 +
 src/libnftables.c   |   6 +--
 src/rule.c          | 106 +++++++++++++++++++++++-----------------------------
 8 files changed, 135 insertions(+), 106 deletions(-)
 create mode 100644 include/cache.h

diff --git a/include/Makefile.am b/include/Makefile.am
index b1f4fcf29015..2d77a768acb0 100644
--- a/include/Makefile.am
+++ b/include/Makefile.am
@@ -2,6 +2,7 @@ SUBDIRS =		linux		\
 			nftables
 
 noinst_HEADERS = 	cli.h		\
+			cache.h		\
 			datatype.h	\
 			expression.h	\
 			fib.h		\
diff --git a/include/cache.h b/include/cache.h
new file mode 100644
index 000000000000..d3502a8a6039
--- /dev/null
+++ b/include/cache.h
@@ -0,0 +1,35 @@
+#ifndef _NFT_CACHE_H_
+#define _NFT_CACHE_H_
+
+enum cache_level_bits {
+	NFT_CACHE_TABLE_BIT	= (1 << 0),
+	NFT_CACHE_CHAIN_BIT	= (1 << 1),
+	NFT_CACHE_SET_BIT	= (1 << 2),
+	NFT_CACHE_FLOWTABLE_BIT	= (1 << 3),
+	NFT_CACHE_OBJECT_BIT	= (1 << 4),
+	NFT_CACHE_SETELEM_BIT	= (1 << 5),
+	NFT_CACHE_RULE_BIT	= (1 << 6),
+	__NFT_CACHE_MAX_BIT	= (1 << 7),
+};
+
+enum cache_level_flags {
+	NFT_CACHE_EMPTY		= 0,
+	NFT_CACHE_TABLE		= NFT_CACHE_TABLE_BIT,
+	NFT_CACHE_CHAIN		= NFT_CACHE_TABLE_BIT |
+				  NFT_CACHE_CHAIN_BIT,
+	NFT_CACHE_SET		= NFT_CACHE_TABLE_BIT |
+				  NFT_CACHE_SET_BIT,
+	NFT_CACHE_FLOWTABLE	= NFT_CACHE_TABLE_BIT |
+				  NFT_CACHE_FLOWTABLE_BIT,
+	NFT_CACHE_OBJECT	= NFT_CACHE_TABLE_BIT |
+				  NFT_CACHE_OBJECT_BIT,
+	NFT_CACHE_SETELEM	= NFT_CACHE_TABLE_BIT |
+				  NFT_CACHE_SET_BIT |
+				  NFT_CACHE_SETELEM_BIT,
+	NFT_CACHE_RULE		= NFT_CACHE_TABLE_BIT |
+				  NFT_CACHE_CHAIN_BIT |
+				  NFT_CACHE_RULE_BIT,
+	NFT_CACHE_FULL		= __NFT_CACHE_MAX_BIT - 1,
+};
+
+#endif /* _NFT_CACHE_H_ */
diff --git a/include/nftables.h b/include/nftables.h
index b7c78572da77..ed446e2d16cf 100644
--- a/include/nftables.h
+++ b/include/nftables.h
@@ -81,7 +81,7 @@ struct nft_cache {
 	uint32_t		genid;
 	struct list_head	list;
 	uint32_t		seqnum;
-	uint32_t		cmd;
+	uint32_t		flags;
 };
 
 struct mnl_socket;
diff --git a/include/rule.h b/include/rule.h
index 299485ffeeaa..aefb24d95163 100644
--- a/include/rule.h
+++ b/include/rule.h
@@ -462,7 +462,6 @@ enum cmd_ops {
 	CMD_EXPORT,
 	CMD_MONITOR,
 	CMD_DESCRIBE,
-	__CMD_FLUSH_RULESET,
 };
 
 /**
@@ -636,7 +635,7 @@ extern struct error_record *rule_postprocess(struct rule *rule);
 struct netlink_ctx;
 extern int do_command(struct netlink_ctx *ctx, struct cmd *cmd);
 
-extern int cache_evaluate(struct nft_ctx *nft, struct list_head *cmds);
+extern unsigned int cache_evaluate(struct nft_ctx *nft, struct list_head *cmds);
 extern int cache_update(struct nft_ctx *ctx, enum cmd_ops cmd,
 			struct list_head *msgs);
 extern void cache_flush(struct nft_ctx *ctx, struct list_head *msgs);
diff --git a/src/cache.c b/src/cache.c
index d7153f6f6b8f..f9b8080e0b3c 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -11,112 +11,117 @@
 #include <rule.h>
 #include <erec.h>
 #include <utils.h>
+#include <cache.h>
 
-static unsigned int evaluate_cache_add(struct cmd *cmd)
+static unsigned int evaluate_cache_add(struct cmd *cmd, unsigned int flags)
 {
-	unsigned int completeness = CMD_INVALID;
-
 	switch (cmd->obj) {
 	case CMD_OBJ_SETELEM:
-	case CMD_OBJ_SET:
-	case CMD_OBJ_CHAIN:
-	case CMD_OBJ_FLOWTABLE:
-		completeness = cmd->op;
+		flags |= NFT_CACHE_SETELEM;
 		break;
 	case CMD_OBJ_RULE:
-		if (cmd->handle.index.id)
-			completeness = CMD_LIST;
+		if (cmd->handle.index.id ||
+		    cmd->handle.position.id ||
+		    cmd->handle.handle.id)
+			flags |= NFT_CACHE_RULE;
 		break;
 	default:
 		break;
 	}
 
-	return completeness;
+	return flags;
 }
 
-static unsigned int evaluate_cache_del(struct cmd *cmd)
+static unsigned int evaluate_cache_del(struct cmd *cmd, unsigned int flags)
 {
-	unsigned int completeness = CMD_INVALID;
-
 	switch (cmd->obj) {
 	case CMD_OBJ_SETELEM:
-		completeness = cmd->op;
+		flags |= NFT_CACHE_SETELEM;
 		break;
 	default:
 		break;
 	}
 
-	return completeness;
+	return flags;
 }
 
-static unsigned int evaluate_cache_flush(struct cmd *cmd)
+static unsigned int evaluate_cache_flush(struct cmd *cmd, unsigned int flags)
 {
-	unsigned int completeness = CMD_INVALID;
-
 	switch (cmd->obj) {
-	case CMD_OBJ_RULESET:
-		completeness = __CMD_FLUSH_RULESET;
-		break;
 	case CMD_OBJ_SET:
 	case CMD_OBJ_MAP:
 	case CMD_OBJ_METER:
-		completeness = cmd->op;
+		flags |= NFT_CACHE_SET;
 		break;
+	case CMD_OBJ_RULESET:
 	default:
+		flags = NFT_CACHE_EMPTY;
 		break;
 	}
 
-	return completeness;
+	return flags;
 }
 
-static unsigned int evaluate_cache_rename(struct cmd *cmd)
+static unsigned int evaluate_cache_rename(struct cmd *cmd, unsigned int flags)
 {
-	unsigned int completeness = CMD_INVALID;
-
 	switch (cmd->obj) {
 	case CMD_OBJ_CHAIN:
-		completeness = cmd->op;
+		flags |= NFT_CACHE_CHAIN;
 		break;
 	default:
 		break;
 	}
 
-	return completeness;
+	return flags;
 }
 
-int cache_evaluate(struct nft_ctx *nft, struct list_head *cmds)
+unsigned int cache_evaluate(struct nft_ctx *nft, struct list_head *cmds)
 {
-	unsigned int echo_completeness = CMD_INVALID;
-	unsigned int completeness = CMD_INVALID;
+	unsigned int flags = NFT_CACHE_EMPTY;
 	struct cmd *cmd;
 
 	list_for_each_entry(cmd, cmds, list) {
 		switch (cmd->op) {
 		case CMD_ADD:
 		case CMD_INSERT:
-		case CMD_REPLACE:
-			if (nft_output_echo(&nft->output))
-				echo_completeness = cmd->op;
-
+			if (nft_output_echo(&nft->output)) {
+				flags = NFT_CACHE_FULL;
+				break;
+			}
+
+			flags |= NFT_CACHE_TABLE |
+				 NFT_CACHE_CHAIN |
+				 NFT_CACHE_SET |
+				 NFT_CACHE_FLOWTABLE |
+				 NFT_CACHE_OBJECT;
 			/* Fall through */
 		case CMD_CREATE:
-			completeness = evaluate_cache_add(cmd);
+			flags = evaluate_cache_add(cmd, flags);
+			break;
+		case CMD_REPLACE:
+			flags = NFT_CACHE_FULL;
 			break;
 		case CMD_DELETE:
-			completeness = evaluate_cache_del(cmd);
+			flags |= NFT_CACHE_TABLE |
+				 NFT_CACHE_CHAIN |
+				 NFT_CACHE_SET |
+				 NFT_CACHE_FLOWTABLE |
+				 NFT_CACHE_OBJECT;
+
+			flags = evaluate_cache_del(cmd, flags);
 			break;
 		case CMD_GET:
 		case CMD_LIST:
 		case CMD_RESET:
 		case CMD_EXPORT:
 		case CMD_MONITOR:
-			completeness = cmd->op;
+			flags |= NFT_CACHE_FULL;
 			break;
 		case CMD_FLUSH:
-			completeness = evaluate_cache_flush(cmd);
+			flags = evaluate_cache_flush(cmd, flags);
 			break;
 		case CMD_RENAME:
-			completeness = evaluate_cache_rename(cmd);
+			flags = evaluate_cache_rename(cmd, flags);
 			break;
 		case CMD_DESCRIBE:
 		case CMD_IMPORT:
@@ -126,5 +131,5 @@ int cache_evaluate(struct nft_ctx *nft, struct list_head *cmds)
 		}
 	}
 
-	return max(completeness, echo_completeness);
+	return flags;
 }
diff --git a/src/evaluate.c b/src/evaluate.c
index 73a4be339ce1..ff0888d0c784 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -29,6 +29,7 @@
 #include <netlink.h>
 #include <time.h>
 #include <rule.h>
+#include <cache.h>
 #include <erec.h>
 #include <gmputil.h>
 #include <utils.h>
diff --git a/src/libnftables.c b/src/libnftables.c
index abd133bee127..dccb8ab4da42 100644
--- a/src/libnftables.c
+++ b/src/libnftables.c
@@ -381,11 +381,11 @@ static int nft_parse_bison_filename(struct nft_ctx *nft, const char *filename,
 static int nft_evaluate(struct nft_ctx *nft, struct list_head *msgs,
 			struct list_head *cmds)
 {
-	unsigned int completeness;
+	unsigned int flags;
 	struct cmd *cmd;
 
-	completeness = cache_evaluate(nft, cmds);
-	if (cache_update(nft, completeness, msgs) < 0)
+	flags = cache_evaluate(nft, cmds);
+	if (cache_update(nft, flags, msgs) < 0)
 		return -1;
 
 	list_for_each_entry(cmd, cmds, list) {
diff --git a/src/rule.c b/src/rule.c
index ed199889702f..f60374abcfbc 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -24,6 +24,7 @@
 #include <mnl.h>
 #include <misspell.h>
 #include <json.h>
+#include <cache.h>
 
 #include <libnftnl/common.h>
 #include <libnftnl/ruleset.h>
@@ -147,97 +148,85 @@ static int cache_init_tables(struct netlink_ctx *ctx, struct handle *h,
 	return 0;
 }
 
-static int cache_init_objects(struct netlink_ctx *ctx, enum cmd_ops cmd)
+static int cache_init_objects(struct netlink_ctx *ctx, unsigned int flags)
 {
+	struct rule *rule, *nrule;
 	struct table *table;
 	struct chain *chain;
-	struct rule *rule, *nrule;
 	struct set *set;
 	int ret;
 
 	list_for_each_entry(table, &ctx->nft->cache.list, list) {
-		ret = netlink_list_sets(ctx, &table->handle);
-		list_splice_tail_init(&ctx->list, &table->sets);
-
-		if (ret < 0)
-			return -1;
-
-		list_for_each_entry(set, &table->sets, list) {
-			ret = netlink_list_setelems(ctx, &set->handle, set);
+		if (flags & NFT_CACHE_SET_BIT) {
+			ret = netlink_list_sets(ctx, &table->handle);
+			list_splice_tail_init(&ctx->list, &table->sets);
 			if (ret < 0)
 				return -1;
 		}
-
-		ret = netlink_list_chains(ctx, &table->handle);
-		if (ret < 0)
-			return -1;
-		list_splice_tail_init(&ctx->list, &table->chains);
-
-		ret = netlink_list_flowtables(ctx, &table->handle);
-		if (ret < 0)
-			return -1;
-		list_splice_tail_init(&ctx->list, &table->flowtables);
-
-		if (cmd != CMD_RESET) {
+		if (flags & NFT_CACHE_SETELEM_BIT) {
+			list_for_each_entry(set, &table->sets, list) {
+				ret = netlink_list_setelems(ctx, &set->handle,
+							    set);
+				if (ret < 0)
+					return -1;
+			}
+		}
+		if (flags & NFT_CACHE_CHAIN_BIT) {
+			ret = netlink_list_chains(ctx, &table->handle);
+			if (ret < 0)
+				return -1;
+			list_splice_tail_init(&ctx->list, &table->chains);
+		}
+		if (flags & NFT_CACHE_FLOWTABLE_BIT) {
+			ret = netlink_list_flowtables(ctx, &table->handle);
+			if (ret < 0)
+				return -1;
+			list_splice_tail_init(&ctx->list, &table->flowtables);
+		}
+		if (flags & NFT_CACHE_OBJECT_BIT) {
 			ret = netlink_list_objs(ctx, &table->handle);
 			if (ret < 0)
 				return -1;
 			list_splice_tail_init(&ctx->list, &table->objs);
 		}
 
-		/* Skip caching other objects to speed up things: We only need
-		 * a full cache when listing the existing ruleset.
-		 */
-		if (cmd != CMD_LIST)
-			continue;
-
-		ret = netlink_list_rules(ctx, &table->handle);
-		list_for_each_entry_safe(rule, nrule, &ctx->list, list) {
-			chain = chain_lookup(table, &rule->handle);
-			list_move_tail(&rule->list, &chain->rules);
+		if (flags & NFT_CACHE_RULE_BIT) {
+			ret = netlink_list_rules(ctx, &table->handle);
+			list_for_each_entry_safe(rule, nrule, &ctx->list, list) {
+				chain = chain_lookup(table, &rule->handle);
+				list_move_tail(&rule->list, &chain->rules);
+			}
+			if (ret < 0)
+				return -1;
 		}
-
-		if (ret < 0)
-			return -1;
 	}
 	return 0;
 }
 
-static int cache_init(struct netlink_ctx *ctx, enum cmd_ops cmd)
+static int cache_init(struct netlink_ctx *ctx, unsigned int flags)
 {
 	struct handle handle = {
 		.family = NFPROTO_UNSPEC,
 	};
 	int ret;
 
-	if (cmd == __CMD_FLUSH_RULESET)
+	if (flags == NFT_CACHE_EMPTY)
 		return 0;
 
+	/* assume NFT_CACHE_TABLE is always set. */
 	ret = cache_init_tables(ctx, &handle, &ctx->nft->cache);
 	if (ret < 0)
 		return ret;
-	ret = cache_init_objects(ctx, cmd);
+	ret = cache_init_objects(ctx, flags);
 	if (ret < 0)
 		return ret;
 
 	return 0;
 }
 
-/* Return a "score" of how complete local cache will be if
- * cache_init_objects() ran for given cmd. Higher value
- * means more complete. */
-static int cache_completeness(enum cmd_ops cmd)
+bool cache_is_complete(struct nft_cache *cache, unsigned int flags)
 {
-	if (cmd == CMD_LIST)
-		return 3;
-	if (cmd != CMD_RESET)
-		return 2;
-	return 1;
-}
-
-bool cache_is_complete(struct nft_cache *cache, enum cmd_ops cmd)
-{
-	return cache_completeness(cache->cmd) >= cache_completeness(cmd);
+	return (cache->flags & flags) == flags;
 }
 
 static bool cache_is_updated(struct nft_cache *cache, uint16_t genid)
@@ -245,7 +234,7 @@ static bool cache_is_updated(struct nft_cache *cache, uint16_t genid)
 	return genid && genid == cache->genid;
 }
 
-int cache_update(struct nft_ctx *nft, enum cmd_ops cmd, struct list_head *msgs)
+int cache_update(struct nft_ctx *nft, unsigned int flags, struct list_head *msgs)
 {
 	struct netlink_ctx ctx = {
 		.list		= LIST_HEAD_INIT(ctx.list),
@@ -259,14 +248,14 @@ int cache_update(struct nft_ctx *nft, enum cmd_ops cmd, struct list_head *msgs)
 replay:
 	ctx.seqnum = cache->seqnum++;
 	genid = mnl_genid_get(&ctx);
-	if (cache_is_complete(cache, cmd) &&
+	if (cache_is_complete(cache, flags) &&
 	    cache_is_updated(cache, genid))
 		return 0;
 
 	if (cache->genid)
 		cache_release(cache);
 
-	ret = cache_init(&ctx, cmd);
+	ret = cache_init(&ctx, flags);
 	if (ret < 0) {
 		cache_release(cache);
 		if (errno == EINTR) {
@@ -283,7 +272,7 @@ replay:
 	}
 
 	cache->genid = genid;
-	cache->cmd = cmd;
+	cache->flags = flags;
 	return 0;
 }
 
@@ -308,15 +297,14 @@ void cache_flush(struct nft_ctx *nft, struct list_head *msgs)
 
 	__cache_flush(&cache->list);
 	cache->genid = mnl_genid_get(&ctx);
-	cache->cmd = CMD_LIST;
+	cache->flags = NFT_CACHE_FULL;
 }
 
 void cache_release(struct nft_cache *cache)
 {
 	__cache_flush(&cache->list);
 	cache->genid = 0;
-	cache->cmd = CMD_INVALID;
-
+	cache->flags = NFT_CACHE_EMPTY;
 }
 
 /* internal ID to uniquely identify a set in the batch */
-- 
2.11.0

Re: [PATCH nft,v2 2/5] tests: shell: cannot use handle for non-existing rule in kernel

[Phil Sutter]

Hey Pablo!

On Mon, Jun 17, 2019 at 07:18:39PM +0200, Pablo Neira Ayuso wrote:
> This test invokes the 'replace rule ... handle 2' command. However,
> there are no rules in the kernel, therefore it always fails.

I found the cause for why this stopped working: You forgot to adjust
rule_evaluate(), what you need is something like this:

diff --git a/src/evaluate.c b/src/evaluate.c
index ff0888d0c7842..f17bebe4a5f22 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -3295,7 +3295,7 @@ static int rule_evaluate(struct eval_ctx *ctx, struct rule *rule,
        }
 
        /* add rules to cache only if it is complete enough to contain them */
-       if (!cache_is_complete(&ctx->nft->cache, CMD_LIST))
+       if (!(ctx->nft->cache.flags & NFT_CACHE_RULE))
                return 0;
 
        return rule_cache_update(ctx, op);

Then handle guessing works again. :)

Cheers, Phil

Re: [PATCH nft,v2 2/5] tests: shell: cannot use handle for non-existing rule in kernel

[Pablo Neira Ayuso]

On Mon, Jun 17, 2019 at 07:26:53PM +0200, Phil Sutter wrote:
> Hey Pablo!
> 
> On Mon, Jun 17, 2019 at 07:18:39PM +0200, Pablo Neira Ayuso wrote:
> > This test invokes the 'replace rule ... handle 2' command. However,
> > there are no rules in the kernel, therefore it always fails.
> 
> I found the cause for why this stopped working: You forgot to adjust
> rule_evaluate(), what you need is something like this:
> 
> diff --git a/src/evaluate.c b/src/evaluate.c
> index ff0888d0c7842..f17bebe4a5f22 100644
> --- a/src/evaluate.c
> +++ b/src/evaluate.c
> @@ -3295,7 +3295,7 @@ static int rule_evaluate(struct eval_ctx *ctx, struct rule *rule,
>         }
>  
>         /* add rules to cache only if it is complete enough to contain them */
> -       if (!cache_is_complete(&ctx->nft->cache, CMD_LIST))
> +       if (!(ctx->nft->cache.flags & NFT_CACHE_RULE))
>                 return 0;

Thanks! I'll fix this an send a new version.

Re: [PATCH nft,v2 2/5] tests: shell: cannot use handle for non-existing rule in kernel

[Pablo Neira Ayuso]

On Mon, Jun 17, 2019 at 07:34:42PM +0200, Pablo Neira Ayuso wrote:
> On Mon, Jun 17, 2019 at 07:26:53PM +0200, Phil Sutter wrote:
> > Hey Pablo!
> > 
> > On Mon, Jun 17, 2019 at 07:18:39PM +0200, Pablo Neira Ayuso wrote:
> > > This test invokes the 'replace rule ... handle 2' command. However,
> > > there are no rules in the kernel, therefore it always fails.
> > 
> > I found the cause for why this stopped working: You forgot to adjust
> > rule_evaluate(), what you need is something like this:
> > 
> > diff --git a/src/evaluate.c b/src/evaluate.c
> > index ff0888d0c7842..f17bebe4a5f22 100644
> > --- a/src/evaluate.c
> > +++ b/src/evaluate.c
> > @@ -3295,7 +3295,7 @@ static int rule_evaluate(struct eval_ctx *ctx, struct rule *rule,
> >         }
> >  
> >         /* add rules to cache only if it is complete enough to contain them */
> > -       if (!cache_is_complete(&ctx->nft->cache, CMD_LIST))
> > +       if (!(ctx->nft->cache.flags & NFT_CACHE_RULE))
> >                 return 0;
> 
> Thanks! I'll fix this an send a new version.

If you're fine with this patchset, I propose to rewrite this commit log to:

    tests: shell: cannot use handle for non-existing rule in kernel

    Do not guess handle for an unexisting rule.

    Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

Thanks!