[PATCH nft,v4 00/16] cache consolidation

[PATCH nft,v4 00/16] cache consolidation

[Pablo Neira Ayuso]

Hi,

This is another round of the patchset to consolidate the nft cache. The idea
consists of creating a cache of tables that is populated with chains, rules,
sets and elements before parsing/evaluation.

This comes with several advantages:

1) We can now keep the ruleset file in a linear list fashion. We can also apply
   incremental set declaration updates in a file in an atomic fashion, eg.

	-o-FILE:nft-ruleset-o-
	add table filter
	add chain filter input { type filter hook input priority 0; }
	add set filter blacklist { type ipv4_addr; }
	add element filter blacklist { 4.4.4.10 }
	-o-EOF-o-

2) We have a single point to create a consistent cache, thus, we can handle
   EINTR and validate generation counter to make sure we operate with a ruleset
   that is up-to-date.

3) We can provide better error reporting from the evaluation step, eg.

   # nft add element filter blacklist { 1.1.1.1 }
   <cmdline>:1:1-36: Error: Could not process rule: Table 'filter' does not exist
   add element filter blacklist { 1.1.1.1 }
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

   instead of:

   # nft add element filter blacklist { 1.1.1.1 }
   <cmdline>:1:1-36: Error: Could not process rule: No such file or directory
   add element filter blacklist { 1.1.1.1 }
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

   In follow up patches, it should be possible to reduce the number of object
   lookups by attaching the corresponding object to struct cmd, so we don't need
   to look it up again from the final command execution step.

4) We can later on use the cache to perform ruleset transformations as Patrick
   already suggested.

I will keep testing this here a bit more, then if no objections, I'll push this
to master.

Thanks.

Pablo Neira Ayuso (16):
  src: consolidate table cache
  src: add cmd_evaluate_list()
  rule: add reference counter to the table object
  src: add table declaration to cache
  src: consolidate set cache
  src: add set declaration to cache
  src: early allocation of the set ID
  segtree: pass element expression as parameter to set_to_intervals()
  rule: use netlink_add_setelems() when creating literal sets
  rule: fix use of intervals in set declarations
  rule: add chain reference counter
  src: consolidate chain cache
  evaluate: add cmd_evaluate_rename()
  src: add chain declarations to cache
  rule: consolidate rule cache
  src: consolidate set element cache

 include/expression.h |    3 +-
 include/rule.h       |    9 ++
 src/evaluate.c       |  142 +++++++++++++++++-------
 src/main.c           |   30 +++++-
 src/netlink.c        |    4 -
 src/rule.c           |  294 ++++++++++++++++++++++++++------------------------
 src/segtree.c        |   15 +--
 7 files changed, 300 insertions(+), 197 deletions(-)

-- 
1.7.10.4

[PATCH nft,v4 01/16] src: consolidate table cache

[Pablo Neira Ayuso]

This patch introduces the table cache that is populated before parsing and
evaluation. As a result, there is a single call to netlink_list_tables().

Follow up patches use this new table cache to store other objects that are
contained by the tables.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/rule.h |    3 +++
 src/main.c     |   30 +++++++++++++++++++++++---
 src/rule.c     |   64 ++++++++++++++++++++++++++++++++++++--------------------
 3 files changed, 71 insertions(+), 26 deletions(-)

diff --git a/include/rule.h b/include/rule.h
index 8ec7f91..b6aabda 100644
--- a/include/rule.h
+++ b/include/rule.h
@@ -63,6 +63,9 @@ extern void symbol_bind(struct scope *scope, const char *identifier,
 extern struct symbol *symbol_lookup(const struct scope *scope,
 				    const char *identifier);
 
+extern int cache_init(struct list_head *msgs);
+extern void cache_fini(void);
+
 enum table_flags {
 	TABLE_F_DORMANT		= (1 << 0),
 };
diff --git a/src/main.c b/src/main.c
index bfe589a..651bdbd 100644
--- a/src/main.c
+++ b/src/main.c
@@ -182,7 +182,6 @@ static int nft_netlink(struct parser_state *state, struct list_head *msgs)
 	bool batch_supported = netlink_batch_supported();
 	int ret = 0;
 
-	netlink_genid_get();
 	mnl_batch_init();
 
 	batch_seqnum = mnl_batch_begin();
@@ -224,25 +223,50 @@ out:
 	return ret;
 }
 
+static int nft_cache_init(struct list_head *msgs)
+{
+	netlink_genid_get();
+	return cache_init(msgs);
+}
+
+static void nft_cache_fini(void)
+{
+	cache_fini();
+}
+
 int nft_run(void *scanner, struct parser_state *state, struct list_head *msgs)
 {
 	struct cmd *cmd, *next;
 	int ret;
 
+retry:
+	ret = nft_cache_init(msgs);
+	if (ret < 0) {
+		nft_cache_fini();
+		if (errno == EINTR) {
+			netlink_restart();
+			goto retry;
+		}
+		return -1;
+	}
+
 	ret = nft_parse(scanner, state);
 	if (ret != 0 || state->nerrs > 0)
-		return -1;
-retry:
+		goto err1;
+
 	ret = nft_netlink(state, msgs);
 	if (ret < 0 && errno == EINTR) {
+		nft_cache_fini();
 		netlink_restart();
 		goto retry;
 	}
 
+err1:
 	list_for_each_entry_safe(cmd, next, &state->cmds, list) {
 		list_del(&cmd->list);
 		cmd_free(cmd);
 	}
+	nft_cache_fini();
 
 	return ret;
 }
diff --git a/src/rule.c b/src/rule.c
index c7a4ef8..3813039 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -53,6 +53,39 @@ void handle_merge(struct handle *dst, const struct handle *src)
 		dst->comment = xstrdup(src->comment);
 }
 
+static LIST_HEAD(table_list);
+
+int cache_init(struct list_head *msgs)
+{
+	struct handle handle = {
+		.family = NFPROTO_UNSPEC,
+	};
+	struct netlink_ctx ctx;
+	int ret;
+
+	memset(&ctx, 0, sizeof(ctx));
+	init_list_head(&ctx.list);
+	ctx.msgs = msgs;
+
+	ret = netlink_list_tables(&ctx, &handle, &internal_location);
+	if (ret < 0)
+		return -1;
+
+	list_splice_tail_init(&ctx.list, &table_list);
+
+	return 0;
+}
+
+void cache_fini(void)
+{
+	struct table *table, *next;
+
+	list_for_each_entry_safe(table, next, &table_list, list) {
+		list_del(&table->list);
+		table_free(table);
+	}
+}
+
 struct set *set_alloc(const struct location *loc)
 {
 	struct set *set;
@@ -516,8 +549,6 @@ void table_free(struct table *table)
 	xfree(table);
 }
 
-static LIST_HEAD(table_list);
-
 void table_add_hash(struct table *table)
 {
 	list_add_tail(&table->list, &table_list);
@@ -845,8 +876,6 @@ static int do_list_table(struct netlink_ctx *ctx, struct cmd *cmd,
 	struct rule *rule, *nrule;
 	struct chain *chain;
 
-	if (netlink_get_table(ctx, &cmd->handle, &cmd->location, table) < 0)
-		goto err;
 	if (do_list_sets(ctx, &cmd->location, table) < 0)
 		goto err;
 	if (netlink_list_chains(ctx, &cmd->handle, &cmd->location) < 0)
@@ -876,25 +905,19 @@ err:
 
 static int do_list_ruleset(struct netlink_ctx *ctx, struct cmd *cmd)
 {
-	struct table *table, *next;
-	LIST_HEAD(tables);
-
-	if (netlink_list_tables(ctx, &cmd->handle, &cmd->location) < 0)
-		return -1;
-
-	list_splice_tail_init(&ctx->list, &tables);
+	unsigned int family = cmd->handle.family;
+	struct table *table;
 
-	list_for_each_entry_safe(table, next, &tables, list) {
-		table_add_hash(table);
+	list_for_each_entry(table, &table_list, list) {
+		if (family != NFPROTO_UNSPEC &&
+		    table->handle.family != family)
+			continue;
 
 		cmd->handle.family = table->handle.family;
 		cmd->handle.table = xstrdup(table->handle.table);
 
 		if (do_list_table(ctx, cmd, table) < 0)
 			return -1;
-
-		list_del(&table->list);
-		table_free(table);
 	}
 
 	return 0;
@@ -1012,7 +1035,7 @@ static int do_command_rename(struct netlink_ctx *ctx, struct cmd *cmd)
 
 static int do_command_monitor(struct netlink_ctx *ctx, struct cmd *cmd)
 {
-	struct table *t, *nt;
+	struct table *t;
 	struct set *s, *ns;
 	struct netlink_ctx set_ctx;
 	LIST_HEAD(msgs);
@@ -1035,10 +1058,7 @@ static int do_command_monitor(struct netlink_ctx *ctx, struct cmd *cmd)
 		init_list_head(&msgs);
 		set_ctx.msgs = &msgs;
 
-		if (netlink_list_tables(ctx, &cmd->handle, &cmd->location) < 0)
-			return -1;
-
-		list_for_each_entry_safe(t, nt, &ctx->list, list) {
+		list_for_each_entry(t, &table_list, list) {
 			set_handle.family = t->handle.family;
 			set_handle.table = t->handle.table;
 
@@ -1052,8 +1072,6 @@ static int do_command_monitor(struct netlink_ctx *ctx, struct cmd *cmd)
 				s->init = set_expr_alloc(&cmd->location);
 				set_add_hash(s, t);
 			}
-
-			table_add_hash(t);
 		}
 	}
 
-- 
1.7.10.4

[PATCH nft,v4 02/16] src: add cmd_evaluate_list()

[Pablo Neira Ayuso]

This function validates that the table that we want to list already exists by
looking up from the new table cache.

This also adds cmd_error() to display an error from the evaluation step, when
the objects that the rule indicates do not exist.

We can now simplify the later handling at do_command_list() since we're now
sure that the table exists at that stage.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/evaluate.c |   21 +++++++++++++++++++++
 src/rule.c     |    9 +--------
 2 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/src/evaluate.c b/src/evaluate.c
index d99b38f..c6c6038 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -62,6 +62,8 @@ static int __fmtstring(4, 5) __stmt_binary_error(struct eval_ctx *ctx,
 	__stmt_binary_error(ctx, &(s1)->location, NULL, fmt, ## args)
 #define monitor_error(ctx, s1, fmt, args...) \
 	__stmt_binary_error(ctx, &(s1)->location, NULL, fmt, ## args)
+#define cmd_error(ctx, fmt, args...) \
+	__stmt_binary_error(ctx, &(ctx->cmd)->location, NULL, fmt, ## args)
 
 static int __fmtstring(3, 4) set_error(struct eval_ctx *ctx,
 				       const struct set *set,
@@ -1933,6 +1935,24 @@ static int cmd_evaluate_delete(struct eval_ctx *ctx, struct cmd *cmd)
 	}
 }
 
+static int cmd_evaluate_list(struct eval_ctx *ctx, struct cmd *cmd)
+{
+	switch (cmd->obj) {
+	case CMD_OBJ_TABLE:
+	case CMD_OBJ_CHAIN:
+	case CMD_OBJ_SETS:
+	case CMD_OBJ_SET:
+		if (table_lookup(&cmd->handle) == NULL)
+			return cmd_error(ctx, "Could not process rule: Table '%s' does not exist",
+					 cmd->handle.table);
+		return 0;
+	case CMD_OBJ_RULESET:
+		return 0;
+	default:
+		BUG("invalid command object type %u\n", cmd->obj);
+	}
+}
+
 enum {
 	CMD_MONITOR_EVENT_ANY,
 	CMD_MONITOR_EVENT_NEW,
@@ -2018,6 +2038,7 @@ int cmd_evaluate(struct eval_ctx *ctx, struct cmd *cmd)
 	case CMD_DELETE:
 		return cmd_evaluate_delete(ctx, cmd);
 	case CMD_LIST:
+		return cmd_evaluate_list(ctx, cmd);
 	case CMD_FLUSH:
 	case CMD_RENAME:
 	case CMD_EXPORT:
diff --git a/src/rule.c b/src/rule.c
index 3813039..4ae32b8 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -940,15 +940,8 @@ static int do_command_list(struct netlink_ctx *ctx, struct cmd *cmd)
 	struct table *table = NULL;
 	struct set *set;
 
-	/* No need to allocate the table object when listing all tables */
-	if (cmd->handle.table != NULL) {
+	if (cmd->handle.table != NULL)
 		table = table_lookup(&cmd->handle);
-		if (table == NULL) {
-			table = table_alloc();
-			handle_merge(&table->handle, &cmd->handle);
-			table_add_hash(table);
-		}
-	}
 
 	switch (cmd->obj) {
 	case CMD_OBJ_TABLE:
-- 
1.7.10.4

[PATCH nft,v4 03/16] rule: add reference counter to the table object

[Pablo Neira Ayuso]

We may hold multiple references to table objects in follow up patches when
adding object declarations to the cache.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/rule.h |    3 +++
 src/rule.c     |   10 ++++++++++
 2 files changed, 13 insertions(+)

diff --git a/include/rule.h b/include/rule.h
index b6aabda..03d7d45 100644
--- a/include/rule.h
+++ b/include/rule.h
@@ -79,6 +79,7 @@ enum table_flags {
  * @chains:	chains contained in the table
  * @sets:	sets contained in the table
  * @flags:	table flags
+ * @refcnt:	table reference counter
  */
 struct table {
 	struct list_head	list;
@@ -88,9 +89,11 @@ struct table {
 	struct list_head	chains;
 	struct list_head	sets;
 	enum table_flags 	flags;
+	unsigned int		refcnt;
 };
 
 extern struct table *table_alloc(void);
+extern struct table *table_get(struct table *table);
 extern void table_free(struct table *table);
 extern void table_add_hash(struct table *table);
 extern struct table *table_lookup(const struct handle *h);
diff --git a/src/rule.c b/src/rule.c
index 4ae32b8..67bce43 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -535,6 +535,8 @@ struct table *table_alloc(void)
 	init_list_head(&table->chains);
 	init_list_head(&table->sets);
 	init_list_head(&table->scope.symbols);
+	table->refcnt = 1;
+
 	return table;
 }
 
@@ -542,6 +544,8 @@ void table_free(struct table *table)
 {
 	struct chain *chain, *next;
 
+	if (--table->refcnt > 0)
+		return;
 	list_for_each_entry_safe(chain, next, &table->chains, list)
 		chain_free(chain);
 	handle_free(&table->handle);
@@ -549,6 +553,12 @@ void table_free(struct table *table)
 	xfree(table);
 }
 
+struct table *table_get(struct table *table)
+{
+	table->refcnt++;
+	return table;
+}
+
 void table_add_hash(struct table *table)
 {
 	list_add_tail(&table->list, &table_list);
-- 
1.7.10.4

[PATCH nft,v4 04/16] src: add table declaration to cache

[Pablo Neira Ayuso]

Declared table objects are added to the cache, thus we can reference objects
that come in this batch, but that are not yet available in the kernel. This
happens from the evaluation step.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/evaluate.c |   15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/src/evaluate.c b/src/evaluate.c
index c6c6038..475eb16 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1881,6 +1881,19 @@ static int table_evaluate(struct eval_ctx *ctx, struct table *table)
 	struct chain *chain;
 	struct set *set;
 
+	if (table_lookup(&ctx->cmd->handle) == NULL) {
+		if (table == NULL) {
+			table = table_alloc();
+			handle_merge(&table->handle, &ctx->cmd->handle);
+			table_add_hash(table);
+		} else {
+			table_add_hash(table_get(table));
+		}
+	}
+
+	if (ctx->cmd->table == NULL)
+		return 0;
+
 	ctx->table = table;
 	list_for_each_entry(set, &table->sets, list) {
 		handle_merge(&set->handle, &table->handle);
@@ -1912,8 +1925,6 @@ static int cmd_evaluate_add(struct eval_ctx *ctx, struct cmd *cmd)
 			return 0;
 		return chain_evaluate(ctx, cmd->chain);
 	case CMD_OBJ_TABLE:
-		if (cmd->data == NULL)
-			return 0;
 		return table_evaluate(ctx, cmd->table);
 	default:
 		BUG("invalid command object type %u\n", cmd->obj);
-- 
1.7.10.4

[PATCH nft,v4 05/16] src: consolidate set cache

[Pablo Neira Ayuso]

This patch populates the table cache only once through netlink_list_sets()
before parsing and evaluation. As a result, there is a single call to
netlink_list_sets().

After this change, we can rid of get_set(). This function was fine by the time
we had no transaction support, but this doesn't work for set objects that are
declared in this batch, so inquiring the kernel doesn't help since they are not
yet available.

This also adds cmd_error() to display an error from the evaluation step, when
the objects that the rule indicates do not exist.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/evaluate.c |   53 ++++++++++-------------------
 src/rule.c     |  103 ++++++++++++++++++++++++++++----------------------------
 2 files changed, 70 insertions(+), 86 deletions(-)

diff --git a/src/evaluate.c b/src/evaluate.c
index 475eb16..963622f 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -109,37 +109,6 @@ static struct expr *implicit_set_declaration(struct eval_ctx *ctx,
 	return set_ref_expr_alloc(&expr->location, set);
 }
 
-// FIXME
-#include <netlink.h>
-static struct set *get_set(struct eval_ctx *ctx, const struct handle *h,
-			   const char *identifier)
-{
-	struct netlink_ctx nctx = {
-		.msgs = ctx->msgs,
-	};
-	struct handle handle;
-	struct set *set;
-	int err;
-
-	if (ctx->table != NULL) {
-		set = set_lookup(ctx->table, identifier);
-		if (set != NULL)
-			return set;
-	}
-
-	init_list_head(&nctx.list);
-
-	memset(&handle, 0, sizeof(handle));
-	handle_merge(&handle, h);
-	handle.set = xstrdup(identifier);
-	err = netlink_get_set(&nctx, &handle, &internal_location);
-	handle_free(&handle);
-
-	if (err < 0)
-		return NULL;
-	return list_first_entry(&nctx.list, struct set, list);
-}
-
 static enum ops byteorder_conversion_op(struct expr *expr,
 					enum byteorder byteorder)
 {
@@ -192,6 +161,7 @@ static int expr_evaluate_symbol(struct eval_ctx *ctx, struct expr **expr)
 {
 	struct error_record *erec;
 	struct symbol *sym;
+	struct table *table;
 	struct set *set;
 	struct expr *new;
 
@@ -213,9 +183,15 @@ static int expr_evaluate_symbol(struct eval_ctx *ctx, struct expr **expr)
 		new = expr_clone(sym->expr);
 		break;
 	case SYMBOL_SET:
-		set = get_set(ctx, &ctx->cmd->handle, (*expr)->identifier);
+		table = table_lookup(&ctx->cmd->handle);
+		if (table == NULL)
+			return cmd_error(ctx, "Could not process rule: Table '%s' does not exist",
+					 ctx->cmd->handle.table);
+
+		set = set_lookup(table, (*expr)->identifier);
 		if (set == NULL)
-			return -1;
+			return cmd_error(ctx, "Could not process rule: Set '%s' does not exist",
+					 (*expr)->identifier);
 		new = set_ref_expr_alloc(&(*expr)->location, set);
 		break;
 	}
@@ -1737,11 +1713,18 @@ int stmt_evaluate(struct eval_ctx *ctx, struct stmt *stmt)
 
 static int setelem_evaluate(struct eval_ctx *ctx, struct expr **expr)
 {
+	struct table *table;
 	struct set *set;
 
-	set = get_set(ctx, &ctx->cmd->handle, ctx->cmd->handle.set);
+	table = table_lookup(&ctx->cmd->handle);
+	if (table == NULL)
+		return cmd_error(ctx, "Could not process rule: Table '%s' does not exist",
+				 ctx->cmd->handle.table);
+
+	set = set_lookup(table, ctx->cmd->handle.set);
 	if (set == NULL)
-		return -1;
+		return cmd_error(ctx, "Could not process rule: Set '%s' does not exist",
+				 ctx->cmd->handle.set);
 
 	ctx->set = set;
 	expr_set_context(&ctx->ectx, set->keytype, set->keylen);
diff --git a/src/rule.c b/src/rule.c
index 67bce43..8f0bc7e 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -61,6 +61,7 @@ int cache_init(struct list_head *msgs)
 		.family = NFPROTO_UNSPEC,
 	};
 	struct netlink_ctx ctx;
+	struct table *table;
 	int ret;
 
 	memset(&ctx, 0, sizeof(ctx));
@@ -73,6 +74,15 @@ int cache_init(struct list_head *msgs)
 
 	list_splice_tail_init(&ctx.list, &table_list);
 
+	list_for_each_entry(table, &table_list, list) {
+		ret = netlink_list_sets(&ctx, &table->handle,
+					&internal_location);
+		list_splice_tail_init(&ctx.list, &table->sets);
+
+		if (ret < 0)
+			return -1;
+	}
+
 	return 0;
 }
 
@@ -543,11 +553,14 @@ struct table *table_alloc(void)
 void table_free(struct table *table)
 {
 	struct chain *chain, *next;
+	struct set *set, *nset;
 
 	if (--table->refcnt > 0)
 		return;
 	list_for_each_entry_safe(chain, next, &table->chains, list)
 		chain_free(chain);
+	list_for_each_entry_safe(set, nset, &table->sets, list)
+		set_free(set);
 	handle_free(&table->handle);
 	scope_release(&table->scope);
 	xfree(table);
@@ -836,15 +849,11 @@ static int do_command_delete(struct netlink_ctx *ctx, struct cmd *cmd)
 static int do_list_sets(struct netlink_ctx *ctx, const struct location *loc,
 			struct table *table)
 {
-	struct set *set, *nset;
-
-	if (netlink_list_sets(ctx, &table->handle, loc) < 0)
-		return -1;
+	struct set *set;
 
-	list_for_each_entry_safe(set, nset, &ctx->list, list) {
+	list_for_each_entry(set, &table->sets, list) {
 		if (netlink_get_setelems(ctx, &set->handle, loc, set) < 0)
 			return -1;
-		list_move_tail(&set->list, &table->sets);
 	}
 	return 0;
 }
@@ -913,6 +922,23 @@ err:
 	return -1;
 }
 
+static int do_list_sets_global(struct netlink_ctx *ctx, struct cmd *cmd)
+{
+	struct table *table;
+	struct set *set;
+
+	list_for_each_entry(table, &table_list, list) {
+		list_for_each_entry(set, &table->sets, list) {
+			if (netlink_get_setelems(ctx, &set->handle,
+						 &cmd->location, set) < 0)
+				return -1;
+
+			set_print(set);
+		}
+	}
+	return 0;
+}
+
 static int do_list_ruleset(struct netlink_ctx *ctx, struct cmd *cmd)
 {
 	unsigned int family = cmd->handle.family;
@@ -945,10 +971,25 @@ static int do_list_tables(void)
 	return 0;
 }
 
+static int do_list_set(struct netlink_ctx *ctx, struct cmd *cmd,
+		       struct table *table)
+{
+	struct set *set;
+
+	set = set_lookup(table, cmd->handle.set);
+	if (set == NULL)
+		return -1;
+
+	if (netlink_get_setelems(ctx, &cmd->handle, &cmd->location, set) < 0)
+		return -1;
+
+	set_print(set);
+	return 0;
+}
+
 static int do_command_list(struct netlink_ctx *ctx, struct cmd *cmd)
 {
 	struct table *table = NULL;
-	struct set *set;
 
 	if (cmd->handle.table != NULL)
 		table = table_lookup(&cmd->handle);
@@ -961,28 +1002,9 @@ static int do_command_list(struct netlink_ctx *ctx, struct cmd *cmd)
 	case CMD_OBJ_CHAIN:
 		return do_list_table(ctx, cmd, table);
 	case CMD_OBJ_SETS:
-		if (netlink_list_sets(ctx, &cmd->handle, &cmd->location) < 0)
-			return -1;
-
-		list_for_each_entry(set, &ctx->list, list){
-			if (netlink_get_setelems(ctx, &set->handle,
-						 &cmd->location, set) < 0) {
-				return -1;
-			}
-			set_print(set);
-		}
-		return 0;
+		return do_list_sets_global(ctx, cmd);
 	case CMD_OBJ_SET:
-		if (netlink_get_set(ctx, &cmd->handle, &cmd->location) < 0)
-			goto err;
-		list_for_each_entry(set, &ctx->list, list) {
-			if (netlink_get_setelems(ctx, &cmd->handle,
-						 &cmd->location, set) < 0) {
-				goto err;
-			}
-			set_print(set);
-		}
-		return 0;
+		return do_list_set(ctx, cmd, table);
 	case CMD_OBJ_RULESET:
 		return do_list_ruleset(ctx, cmd);
 	default:
@@ -990,9 +1012,6 @@ static int do_command_list(struct netlink_ctx *ctx, struct cmd *cmd)
 	}
 
 	return 0;
-err:
-	table_cleanup(table);
-	return -1;
 }
 
 static int do_command_flush(struct netlink_ctx *ctx, struct cmd *cmd)
@@ -1039,10 +1058,7 @@ static int do_command_rename(struct netlink_ctx *ctx, struct cmd *cmd)
 static int do_command_monitor(struct netlink_ctx *ctx, struct cmd *cmd)
 {
 	struct table *t;
-	struct set *s, *ns;
-	struct netlink_ctx set_ctx;
-	LIST_HEAD(msgs);
-	struct handle set_handle;
+	struct set *s;
 	struct netlink_mon_handler monhandler;
 
 	/* cache only needed if monitoring:
@@ -1057,24 +1073,9 @@ static int do_command_monitor(struct netlink_ctx *ctx, struct cmd *cmd)
 		monhandler.cache_needed = false;
 
 	if (monhandler.cache_needed) {
-		memset(&set_ctx, 0, sizeof(set_ctx));
-		init_list_head(&msgs);
-		set_ctx.msgs = &msgs;
-
 		list_for_each_entry(t, &table_list, list) {
-			set_handle.family = t->handle.family;
-			set_handle.table = t->handle.table;
-
-			init_list_head(&set_ctx.list);
-
-			if (netlink_list_sets(&set_ctx, &set_handle,
-					      &cmd->location) < 0)
-				return -1;
-
-			list_for_each_entry_safe(s, ns, &set_ctx.list, list) {
+			list_for_each_entry(s, &t->sets, list)
 				s->init = set_expr_alloc(&cmd->location);
-				set_add_hash(s, t);
-			}
 		}
 	}
 
-- 
1.7.10.4

[PATCH nft,v4 06/16] src: add set declaration to cache

[Pablo Neira Ayuso]

This patch adds set objects to the cache if they don't exist in the kernel, so
they can be referenced from this batch. This occurs from the evaluation step.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/evaluate.c |    9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/src/evaluate.c b/src/evaluate.c
index 963622f..91c4989 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1736,8 +1736,17 @@ static int setelem_evaluate(struct eval_ctx *ctx, struct expr **expr)
 
 static int set_evaluate(struct eval_ctx *ctx, struct set *set)
 {
+	struct table *table;
 	const char *type;
 
+	table = table_lookup(&ctx->cmd->handle);
+	if (table == NULL)
+		return cmd_error(ctx, "Could not process rule: Table '%s' does not exist",
+				 ctx->cmd->handle.table);
+
+	if (set_lookup(table, set->handle.set) == NULL)
+		set_add_hash(set_get(set), table);
+
 	type = set->flags & SET_F_MAP ? "map" : "set";
 
 	if (set->keytype == NULL)
-- 
1.7.10.4

[PATCH nft,v4 07/16] src: early allocation of the set ID

[Pablo Neira Ayuso]

By when the set is created, so element in the batch use this set ID as
reference.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/netlink.c |    4 ----
 src/rule.c    |    4 ++++
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/netlink.c b/src/netlink.c
index 8ede8e6..0fb7b63 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -1121,9 +1121,6 @@ static int netlink_add_set_compat(struct netlink_ctx *ctx,
 	return err;
 }
 
-/* internal ID to uniquely identify a set in the batch */
-static uint32_t set_id;
-
 static int netlink_add_set_batch(struct netlink_ctx *ctx,
 				 const struct handle *h, struct set *set)
 {
@@ -1147,7 +1144,6 @@ static int netlink_add_set_batch(struct netlink_ctx *ctx,
 	if (set->gc_int)
 		nft_set_attr_set_u32(nls, NFT_SET_ATTR_GC_INTERVAL, set->gc_int);
 
-	set->handle.set_id = ++set_id;
 	nft_set_attr_set_u32(nls, NFT_SET_ATTR_ID, set->handle.set_id);
 
 	if (!(set->flags & (SET_F_CONSTANT))) {
diff --git a/src/rule.c b/src/rule.c
index 8f0bc7e..eefa5f1 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -96,12 +96,16 @@ void cache_fini(void)
 	}
 }
 
+/* internal ID to uniquely identify a set in the batch */
+static uint32_t set_id;
+
 struct set *set_alloc(const struct location *loc)
 {
 	struct set *set;
 
 	set = xzalloc(sizeof(*set));
 	set->refcnt = 1;
+	set->handle.set_id = ++set_id;
 	if (loc != NULL)
 		set->location = *loc;
 	return set;
-- 
1.7.10.4

[PATCH nft,v4 08/16] segtree: pass element expression as parameter to set_to_intervals()

[Pablo Neira Ayuso]

So we can reuse this code from set declarations.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/expression.h |    3 ++-
 src/rule.c           |    2 +-
 src/segtree.c        |   15 ++++++++-------
 3 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/include/expression.h b/include/expression.h
index 010cb95..bc17762 100644
--- a/include/expression.h
+++ b/include/expression.h
@@ -362,7 +362,8 @@ extern struct expr *concat_expr_alloc(const struct location *loc);
 extern struct expr *list_expr_alloc(const struct location *loc);
 
 extern struct expr *set_expr_alloc(const struct location *loc);
-extern int set_to_intervals(struct list_head *msgs, struct set *set);
+extern int set_to_intervals(struct list_head *msgs, struct set *set,
+			    struct expr *init);
 
 extern struct expr *mapping_expr_alloc(const struct location *loc,
 				       struct expr *from, struct expr *to);
diff --git a/src/rule.c b/src/rule.c
index eefa5f1..2b624c2 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -760,7 +760,7 @@ static int do_add_set(struct netlink_ctx *ctx, const struct handle *h,
 		return -1;
 	if (set->init != NULL) {
 		if (set->flags & SET_F_INTERVAL &&
-		    set_to_intervals(ctx->msgs, set) < 0)
+		    set_to_intervals(ctx->msgs, set, set->init) < 0)
 			return -1;
 		if (do_add_setelems(ctx, &set->handle, set->init) < 0)
 			return -1;
diff --git a/src/segtree.c b/src/segtree.c
index 060951c..429d35e 100644
--- a/src/segtree.c
+++ b/src/segtree.c
@@ -64,11 +64,12 @@ struct elementary_interval {
 	struct expr			*expr;
 };
 
-static void seg_tree_init(struct seg_tree *tree, const struct set *set)
+static void seg_tree_init(struct seg_tree *tree, const struct set *set,
+			  struct expr *init)
 {
 	struct expr *first;
 
-	first = list_entry(set->init->expressions.next, struct expr, list);
+	first = list_entry(init->expressions.next, struct expr, list);
 	tree->root	= RB_ROOT;
 	tree->keytype	= set->keytype;
 	tree->keylen	= set->keylen;
@@ -431,14 +432,14 @@ static void set_insert_interval(struct expr *set, struct seg_tree *tree,
 	compound_expr_add(set, expr);
 }
 
-int set_to_intervals(struct list_head *errs, struct set *set)
+int set_to_intervals(struct list_head *errs, struct set *set, struct expr *init)
 {
 	struct elementary_interval *ei, *next;
 	struct seg_tree tree;
 	LIST_HEAD(list);
 
-	seg_tree_init(&tree, set);
-	if (set_to_segtree(errs, set->init, &tree) < 0)
+	seg_tree_init(&tree, set, init);
+	if (set_to_segtree(errs, init, &tree) < 0)
 		return -1;
 	segtree_linearize(&list, &tree);
 
@@ -448,12 +449,12 @@ int set_to_intervals(struct list_head *errs, struct set *set)
 				     2 * tree.keylen / BITS_PER_BYTE, ei->left,
 				     2 * tree.keylen / BITS_PER_BYTE, ei->right);
 		}
-		set_insert_interval(set->init, &tree, ei);
+		set_insert_interval(init, &tree, ei);
 		ei_destroy(ei);
 	}
 
 	if (segtree_debug()) {
-		expr_print(set->init);
+		expr_print(init);
 		pr_gmp_debug("\n");
 	}
 	return 0;
-- 
1.7.10.4

[PATCH nft,v4 09/16] rule: use netlink_add_setelems() when creating literal sets

[Pablo Neira Ayuso]

Thus, do_add_setelems() is only used for set declarations. This comes in
preparation to the follow up patch, to avoid resending back to userspace the
list of existing elements.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/rule.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/rule.c b/src/rule.c
index 2b624c2..2b663f5 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -762,7 +762,7 @@ static int do_add_set(struct netlink_ctx *ctx, const struct handle *h,
 		if (set->flags & SET_F_INTERVAL &&
 		    set_to_intervals(ctx->msgs, set, set->init) < 0)
 			return -1;
-		if (do_add_setelems(ctx, &set->handle, set->init) < 0)
+		if (netlink_add_setelems(ctx, &set->handle, set->init) < 0)
 			return -1;
 	}
 	return 0;
-- 
1.7.10.4

[PATCH nft,v4 10/16] rule: fix use of intervals in set declarations

[Pablo Neira Ayuso]

 # nft add table test
 # nft add set test myset { type ipv4_addr\; flags interval\; }
 # nft add element test myset { 1.2.3.0/24 }

Then the listing shows:

 set myset2 {
         type ipv4_addr
         flags interval
         elements = { 1.2.3.0/24}
 }

This patch relies on the table and set caches.

Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=994
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/rule.c |   18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/src/rule.c b/src/rule.c
index 2b663f5..0077f8e 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -746,10 +746,26 @@ static int do_add_chain(struct netlink_ctx *ctx, const struct handle *h,
 }
 
 static int do_add_setelems(struct netlink_ctx *ctx, const struct handle *h,
-			   const struct expr *expr)
+			   struct expr *expr)
 {
+	struct table *table;
+	struct set *set;
+
+	table = table_lookup(h);
+	if (table == NULL)
+		return -1;
+
+	set = set_lookup(table, h->set);
+	if (set == NULL)
+		return -1;
+
+	if (set->flags & SET_F_INTERVAL &&
+	    set_to_intervals(ctx->msgs, set, expr) < 0)
+		return -1;
+
 	if (netlink_add_setelems(ctx, h, expr) < 0)
 		return -1;
+
 	return 0;
 }
 
-- 
1.7.10.4

[PATCH nft,v4 11/16] rule: add chain reference counter

[Pablo Neira Ayuso]

When adding declared chains to the cache, we may hold more than one single
reference from struct cmd and the cache.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/rule.h |    3 +++
 src/rule.c     |    9 +++++++++
 2 files changed, 12 insertions(+)

diff --git a/include/rule.h b/include/rule.h
index 03d7d45..6cf401d 100644
--- a/include/rule.h
+++ b/include/rule.h
@@ -113,6 +113,7 @@ enum chain_flags {
  * @list:	list node in table list
  * @handle:	chain handle
  * @location:	location the chain was defined at
+ * @refcnt:	reference counter
  * @flags:	chain flags
  * @hookstr:	unified and human readable hook name (base chains)
  * @hooknum:	hook number (base chains)
@@ -125,6 +126,7 @@ struct chain {
 	struct list_head	list;
 	struct handle		handle;
 	struct location		location;
+	unsigned int		refcnt;
 	uint32_t		flags;
 	const char		*hookstr;
 	unsigned int		hooknum;
@@ -138,6 +140,7 @@ struct chain {
 extern const char *chain_type_name_lookup(const char *name);
 extern const char *chain_hookname_lookup(const char *name);
 extern struct chain *chain_alloc(const char *name);
+extern struct chain *chain_get(struct chain *chain);
 extern void chain_free(struct chain *chain);
 extern void chain_add_hash(struct chain *chain, struct table *table);
 extern struct chain *chain_lookup(const struct table *table,
diff --git a/src/rule.c b/src/rule.c
index 0077f8e..88b1834 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -400,6 +400,7 @@ struct chain *chain_alloc(const char *name)
 	struct chain *chain;
 
 	chain = xzalloc(sizeof(*chain));
+	chain->refcnt = 1;
 	init_list_head(&chain->rules);
 	init_list_head(&chain->scope.symbols);
 	if (name != NULL)
@@ -409,10 +410,18 @@ struct chain *chain_alloc(const char *name)
 	return chain;
 }
 
+struct chain *chain_get(struct chain *chain)
+{
+	chain->refcnt++;
+	return chain;
+}
+
 void chain_free(struct chain *chain)
 {
 	struct rule *rule, *next;
 
+	if (--chain->refcnt > 0)
+		return;
 	list_for_each_entry_safe(rule, next, &chain->rules, list)
 		rule_free(rule);
 	handle_free(&chain->handle);
-- 
1.7.10.4

[PATCH nft,v4 12/16] src: consolidate chain cache

[Pablo Neira Ayuso]

Dump the list of existing chains and populate the cache from the initialization
step.

This also include some extra code to validate that the table and chain exists
from the evaluation step.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/rule.c |   51 ++++++++++-----------------------------------------
 1 file changed, 10 insertions(+), 41 deletions(-)

diff --git a/src/rule.c b/src/rule.c
index 88b1834..e005ce7 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -81,6 +81,13 @@ int cache_init(struct list_head *msgs)
 
 		if (ret < 0)
 			return -1;
+
+		ret = netlink_list_chains(&ctx, &table->handle,
+					  &internal_location);
+		list_splice_tail_init(&ctx.list, &table->chains);
+
+		if (ret < 0)
+			return -1;
 	}
 
 	return 0;
@@ -902,22 +909,6 @@ static int do_command_export(struct netlink_ctx *ctx, struct cmd *cmd)
 	return 0;
 }
 
-static void table_cleanup(struct table *table)
-{
-	struct chain *chain, *nchain;
-	struct set *set, *nset;
-
-	list_for_each_entry_safe(chain, nchain, &table->chains, list) {
-		list_del(&chain->list);
-		chain_free(chain);
-	}
-
-	list_for_each_entry_safe(set, nset, &table->sets, list) {
-		list_del(&set->list);
-		set_free(set);
-	}
-}
-
 static int do_list_table(struct netlink_ctx *ctx, struct cmd *cmd,
 			 struct table *table)
 {
@@ -925,30 +916,16 @@ static int do_list_table(struct netlink_ctx *ctx, struct cmd *cmd,
 	struct chain *chain;
 
 	if (do_list_sets(ctx, &cmd->location, table) < 0)
-		goto err;
-	if (netlink_list_chains(ctx, &cmd->handle, &cmd->location) < 0)
-		goto err;
-	list_splice_tail_init(&ctx->list, &table->chains);
+		return -1;
 	if (netlink_list_table(ctx, &cmd->handle, &cmd->location) < 0)
-		goto err;
+		return -1;
 
 	list_for_each_entry_safe(rule, nrule, &ctx->list, list) {
-		table = table_lookup(&rule->handle);
 		chain = chain_lookup(table, &rule->handle);
-		if (chain == NULL) {
-			chain = chain_alloc(rule->handle.chain);
-			chain_add_hash(chain, table);
-		}
-
 		list_move_tail(&rule->list, &chain->rules);
 	}
-
 	table_print(table);
-	table_cleanup(table);
 	return 0;
-err:
-	table_cleanup(table);
-	return -1;
 }
 
 static int do_list_sets_global(struct netlink_ctx *ctx, struct cmd *cmd)
@@ -1062,18 +1039,10 @@ static int do_command_rename(struct netlink_ctx *ctx, struct cmd *cmd)
 {
 	struct table *table;
 	struct chain *chain;
-	int err;
-
-	table = table_alloc();
-	handle_merge(&table->handle, &cmd->handle);
-	table_add_hash(table);
 
 	switch (cmd->obj) {
 	case CMD_OBJ_CHAIN:
-		err = netlink_get_chain(ctx, &cmd->handle, &cmd->location);
-		if (err < 0)
-			return err;
-		list_splice_tail_init(&ctx->list, &table->chains);
+		table = table_lookup(&cmd->handle);
 		chain = chain_lookup(table, &cmd->handle);
 
 		return netlink_rename_chain(ctx, &chain->handle, &cmd->location,
-- 
1.7.10.4

[PATCH nft,v4 13/16] evaluate: add cmd_evaluate_rename()

[Pablo Neira Ayuso]

Make sure the table and the original chain that we want to rename already exist.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/evaluate.c |   24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/src/evaluate.c b/src/evaluate.c
index 91c4989..022e1ff 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1956,6 +1956,28 @@ static int cmd_evaluate_list(struct eval_ctx *ctx, struct cmd *cmd)
 	}
 }
 
+static int cmd_evaluate_rename(struct eval_ctx *ctx, struct cmd *cmd)
+{
+	struct table *table;
+	struct chain *chain;
+
+	switch (cmd->obj) {
+	case CMD_OBJ_CHAIN:
+		table = table_lookup(&ctx->cmd->handle);
+		if (table == NULL) {
+			return cmd_error(ctx, "Table '%s' does not exist",
+					 ctx->cmd->handle.table);
+		}
+		chain = chain_lookup(table, &ctx->cmd->handle);
+		if (chain == NULL)
+			return cmd_error(ctx, "Chain '%s' does not exist",
+					 ctx->cmd->handle.chain);
+		return 0;
+	default:
+		BUG("invalid command object type %u\n", cmd->obj);
+	}
+}
+
 enum {
 	CMD_MONITOR_EVENT_ANY,
 	CMD_MONITOR_EVENT_NEW,
@@ -2043,7 +2065,9 @@ int cmd_evaluate(struct eval_ctx *ctx, struct cmd *cmd)
 	case CMD_LIST:
 		return cmd_evaluate_list(ctx, cmd);
 	case CMD_FLUSH:
+		return 0;
 	case CMD_RENAME:
+		return cmd_evaluate_rename(ctx, cmd);
 	case CMD_EXPORT:
 	case CMD_DESCRIBE:
 		return 0;
-- 
1.7.10.4

[PATCH nft,v4 14/16] src: add chain declarations to cache

[Pablo Neira Ayuso]

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/evaluate.c |   20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/src/evaluate.c b/src/evaluate.c
index 022e1ff..d101f8a 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1850,8 +1850,26 @@ static uint32_t str2hooknum(uint32_t family, const char *hook)
 
 static int chain_evaluate(struct eval_ctx *ctx, struct chain *chain)
 {
+	struct table *table;
 	struct rule *rule;
 
+	table = table_lookup(&ctx->cmd->handle);
+	if (table == NULL)
+		return cmd_error(ctx, "Table '%s' does not exist",
+				 ctx->cmd->handle.table);
+
+	if (chain == NULL) {
+		if (chain_lookup(table, &ctx->cmd->handle) == NULL) {
+			chain = chain_alloc(NULL);
+			handle_merge(&chain->handle, &ctx->cmd->handle);
+			chain_add_hash(chain, table);
+		}
+		return 0;
+	} else {
+		if (chain_lookup(table, &chain->handle) == NULL)
+			chain_add_hash(chain_get(chain), table);
+	}
+
 	if (chain->flags & CHAIN_F_BASECHAIN) {
 		chain->hooknum = str2hooknum(chain->handle.family,
 					     chain->hookstr);
@@ -1913,8 +1931,6 @@ static int cmd_evaluate_add(struct eval_ctx *ctx, struct cmd *cmd)
 		handle_merge(&cmd->rule->handle, &cmd->handle);
 		return rule_evaluate(ctx, cmd->rule);
 	case CMD_OBJ_CHAIN:
-		if (cmd->data == NULL)
-			return 0;
 		return chain_evaluate(ctx, cmd->chain);
 	case CMD_OBJ_TABLE:
 		return table_evaluate(ctx, cmd->table);
-- 
1.7.10.4

[PATCH nft,v4 15/16] rule: consolidate rule cache

[Pablo Neira Ayuso]

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/rule.c |   22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/src/rule.c b/src/rule.c
index e005ce7..4a207e2 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -60,8 +60,10 @@ int cache_init(struct list_head *msgs)
 	struct handle handle = {
 		.family = NFPROTO_UNSPEC,
 	};
+	struct rule *rule, *nrule;
 	struct netlink_ctx ctx;
 	struct table *table;
+	struct chain *chain;
 	int ret;
 
 	memset(&ctx, 0, sizeof(ctx));
@@ -88,6 +90,16 @@ int cache_init(struct list_head *msgs)
 
 		if (ret < 0)
 			return -1;
+
+		ret = netlink_list_table(&ctx, &table->handle,
+					 &internal_location);
+		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;
 	}
 
 	return 0;
@@ -912,18 +924,8 @@ static int do_command_export(struct netlink_ctx *ctx, struct cmd *cmd)
 static int do_list_table(struct netlink_ctx *ctx, struct cmd *cmd,
 			 struct table *table)
 {
-	struct rule *rule, *nrule;
-	struct chain *chain;
-
 	if (do_list_sets(ctx, &cmd->location, table) < 0)
 		return -1;
-	if (netlink_list_table(ctx, &cmd->handle, &cmd->location) < 0)
-		return -1;
-
-	list_for_each_entry_safe(rule, nrule, &ctx->list, list) {
-		chain = chain_lookup(table, &rule->handle);
-		list_move_tail(&rule->list, &chain->rules);
-	}
 	table_print(table);
 	return 0;
 }
-- 
1.7.10.4

[PATCH nft,v4 16/16] src: consolidate set element cache

[Pablo Neira Ayuso]

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/rule.c |   36 +++++++++++-------------------------
 1 file changed, 11 insertions(+), 25 deletions(-)

diff --git a/src/rule.c b/src/rule.c
index 4a207e2..cc927b3 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -64,6 +64,7 @@ int cache_init(struct list_head *msgs)
 	struct netlink_ctx ctx;
 	struct table *table;
 	struct chain *chain;
+	struct set *set;
 	int ret;
 
 	memset(&ctx, 0, sizeof(ctx));
@@ -84,6 +85,13 @@ int cache_init(struct list_head *msgs)
 		if (ret < 0)
 			return -1;
 
+		list_for_each_entry(set, &table->sets, list) {
+			ret = netlink_get_setelems(&ctx, &set->handle,
+						   &internal_location, set);
+			if (ret < 0)
+				return -1;
+		}
+
 		ret = netlink_list_chains(&ctx, &table->handle,
 					  &internal_location);
 		list_splice_tail_init(&ctx.list, &table->chains);
@@ -894,18 +902,6 @@ static int do_command_delete(struct netlink_ctx *ctx, struct cmd *cmd)
 	}
 }
 
-static int do_list_sets(struct netlink_ctx *ctx, const struct location *loc,
-			struct table *table)
-{
-	struct set *set;
-
-	list_for_each_entry(set, &table->sets, list) {
-		if (netlink_get_setelems(ctx, &set->handle, loc, set) < 0)
-			return -1;
-	}
-	return 0;
-}
-
 static int do_command_export(struct netlink_ctx *ctx, struct cmd *cmd)
 {
 	struct nft_ruleset *rs = netlink_dump_ruleset(ctx, &cmd->handle,
@@ -924,25 +920,18 @@ static int do_command_export(struct netlink_ctx *ctx, struct cmd *cmd)
 static int do_list_table(struct netlink_ctx *ctx, struct cmd *cmd,
 			 struct table *table)
 {
-	if (do_list_sets(ctx, &cmd->location, table) < 0)
-		return -1;
 	table_print(table);
 	return 0;
 }
 
-static int do_list_sets_global(struct netlink_ctx *ctx, struct cmd *cmd)
+static int do_list_sets(struct netlink_ctx *ctx, struct cmd *cmd)
 {
 	struct table *table;
 	struct set *set;
 
 	list_for_each_entry(table, &table_list, list) {
-		list_for_each_entry(set, &table->sets, list) {
-			if (netlink_get_setelems(ctx, &set->handle,
-						 &cmd->location, set) < 0)
-				return -1;
-
+		list_for_each_entry(set, &table->sets, list)
 			set_print(set);
-		}
 	}
 	return 0;
 }
@@ -988,9 +977,6 @@ static int do_list_set(struct netlink_ctx *ctx, struct cmd *cmd,
 	if (set == NULL)
 		return -1;
 
-	if (netlink_get_setelems(ctx, &cmd->handle, &cmd->location, set) < 0)
-		return -1;
-
 	set_print(set);
 	return 0;
 }
@@ -1010,7 +996,7 @@ static int do_command_list(struct netlink_ctx *ctx, struct cmd *cmd)
 	case CMD_OBJ_CHAIN:
 		return do_list_table(ctx, cmd, table);
 	case CMD_OBJ_SETS:
-		return do_list_sets_global(ctx, cmd);
+		return do_list_sets(ctx, cmd);
 	case CMD_OBJ_SET:
 		return do_list_set(ctx, cmd, table);
 	case CMD_OBJ_RULESET:
-- 
1.7.10.4