[PATCH 0/7 nft] cache consolidation

[PATCH 0/7 nft] cache consolidation

[Pablo Neira Ayuso]

Hi Patrick,

This patchset creates two caches, one for tables and another for sets, that
contain the existing objects in the kernel.

Moreover, this also adds the declared objects that don't exists yet in the
kernel to the cache, so they can be referenced from a batch, eg.

-BEGIN of test.ruleset-
add table test
add chain test test
add set test myset { type ipv4_addr; }
add element test myset { 4.4.4.10 }
add element test myset { 4.4.4.11 }
add element test myset { 4.4.4.12 }
add element test myset { 4.4.4.13 }
add rule test test ip saddr @myset
-EOF-

 # nft -f test.ruleset

The idea is to use table_lookup() and set_lookup(), instead of inquiring the
kernel (which would fail since those objects don't exist yet there). The
example above now works and those updates are handled from the same
transaction.

This patch also includes the fix of intervals in set declarations by using
these caches as you suggested, now that we got rid of the get_set() function.

Let me know if you have any concern with these, thanks!

Pablo Neira Ayuso (6):
  src: always allocate table object with no table block
  src: consolidate set 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

 include/expression.h |    3 +-
 include/rule.h       |    3 +
 src/evaluate.c       |   60 ++++++++-----------
 src/main.c           |    6 ++
 src/netlink.c        |    4 --
 src/parser_bison.y   |    7 ++-
 src/rule.c           |  156 ++++++++++++++++++++++++++++++++------------------
 src/segtree.c        |   15 ++---
 8 files changed, 147 insertions(+), 107 deletions(-)

--
1.7.10.4

[PATCH 1/7] src: consolidate table cache

[Pablo Neira Ayuso]

This patch populates the table cache only once through netlink_list_tables()
from the initialization step. As a result, there is a single call to
netlink_list_tables().

Then, new table declarations are also added to this cache, thus follow up calls
to table_lookup() for tables that don't exist yet in the kernel will be found
in the cache.

Note that table_alloc() inserts the object in the cache and table_free()
removes the object from the cache and it releases it.

Table objects may be released from cmd_free() or after all commands in the file
have been processed.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/rule.h |    3 +++
 src/main.c     |    8 +++++-
 src/netlink.c  |   12 +++------
 src/rule.c     |   78 ++++++++++++++++++++++++++++++++++++--------------------
 4 files changed, 63 insertions(+), 38 deletions(-)

diff --git a/include/rule.h b/include/rule.h
index 5d44599..ae69a8d 100644
--- a/include/rule.h
+++ b/include/rule.h
@@ -89,6 +89,9 @@ struct table {
 
 extern struct table *table_alloc(void);
 extern void table_free(struct table *table);
+
+extern int table_init_hash(void);
+extern void table_fini_hash(void);
 extern void table_add_hash(struct table *table);
 extern struct table *table_lookup(const struct handle *h);
 
diff --git a/src/main.c b/src/main.c
index bfe589a..7d559df 100644
--- a/src/main.c
+++ b/src/main.c
@@ -229,9 +229,14 @@ int nft_run(void *scanner, struct parser_state *state, struct list_head *msgs)
 	struct cmd *cmd, *next;
 	int ret;
 
+	if (table_init_hash() < 0)
+		return -1;
+
 	ret = nft_parse(scanner, state);
-	if (ret != 0 || state->nerrs > 0)
+	if (ret != 0 || state->nerrs > 0) {
+		table_fini_hash();
 		return -1;
+	}
 retry:
 	ret = nft_netlink(state, msgs);
 	if (ret < 0 && errno == EINTR) {
@@ -243,6 +248,7 @@ retry:
 		list_del(&cmd->list);
 		cmd_free(cmd);
 	}
+	table_fini_hash();
 
 	return ret;
 }
diff --git a/src/netlink.c b/src/netlink.c
index 429eed4..4b57aab 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -926,10 +926,8 @@ static struct table *netlink_delinearize_table(struct netlink_ctx *ctx,
 static int list_table_cb(struct nft_table *nlt, void *arg)
 {
 	struct netlink_ctx *ctx = arg;
-	struct table *table;
 
-	table = netlink_delinearize_table(ctx, nlt);
-	list_add_tail(&table->list, &ctx->list);
+	netlink_delinearize_table(ctx, nlt);
 
 	return 0;
 }
@@ -972,7 +970,7 @@ int netlink_get_table(struct netlink_ctx *ctx, const struct handle *h,
 
 	ntable = netlink_delinearize_table(ctx, nlt);
 	table->flags = ntable->flags;
-	xfree(ntable);
+	table_free(ntable);
 out:
 	nft_table_free(nlt);
 	return err;
@@ -1963,13 +1961,10 @@ static void netlink_events_cache_addtable(struct netlink_mon_handler *monh,
 					  const struct nlmsghdr *nlh)
 {
 	struct nft_table *nlt;
-	struct table *t;
 
 	nlt = netlink_table_alloc(nlh);
-	t = netlink_delinearize_table(monh->ctx, nlt);
+	netlink_delinearize_table(monh->ctx, nlt);
 	nft_table_free(nlt);
-
-	table_add_hash(t);
 }
 
 static void netlink_events_cache_deltable(struct netlink_mon_handler *monh,
@@ -1987,7 +1982,6 @@ static void netlink_events_cache_deltable(struct netlink_mon_handler *monh,
 	if (t == NULL)
 		goto out;
 
-	list_del(&t->list);
 	table_free(t);
 out:
 	nft_table_free(nlt);
diff --git a/src/rule.c b/src/rule.c
index b2090dd..cbc4931 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -20,6 +20,7 @@
 #include <rule.h>
 #include <utils.h>
 #include <netlink.h>
+#include <erec.h>
 
 #include <libnftnl/common.h>
 #include <libnftnl/ruleset.h>
@@ -504,6 +505,8 @@ struct table *table_alloc(void)
 	init_list_head(&table->chains);
 	init_list_head(&table->sets);
 	init_list_head(&table->scope.symbols);
+	table_add_hash(table);
+
 	return table;
 }
 
@@ -515,11 +518,49 @@ void table_free(struct table *table)
 		chain_free(chain);
 	handle_free(&table->handle);
 	scope_release(&table->scope);
+	list_del(&table->list);
 	xfree(table);
 }
 
 static LIST_HEAD(table_list);
 
+int table_init_hash(void)
+{
+	struct handle handle = {
+		.family = NFPROTO_UNSPEC,
+	};
+	struct netlink_ctx ctx;
+	LIST_HEAD(msgs);
+	int ret;
+
+retry:
+	netlink_genid_get();
+	memset(&ctx, 0, sizeof(ctx));
+	init_list_head(&ctx.list);
+	ctx.msgs = &msgs;
+
+	ret = netlink_list_tables(&ctx, &handle, &internal_location);
+	if (ret < 0) {
+		if (errno == EINTR)
+			goto retry;
+
+		erec_print_list(stdout, &msgs);
+		return ret;
+	}
+
+	list_splice_tail_init(&ctx.list, &table_list);
+
+	return 0;
+}
+
+void table_fini_hash(void)
+{
+	struct table *table, *next;
+
+	list_for_each_entry_safe(table, next, &table_list, list)
+		table_free(table);
+}
+
 void table_add_hash(struct table *table)
 {
 	list_add_tail(&table->list, &table_list);
@@ -878,25 +919,18 @@ 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);
+	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 (cmd->handle.family != NFPROTO_UNSPEC &&
+		    table->handle.family != cmd->handle.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;
@@ -913,23 +947,17 @@ static int do_command_list(struct netlink_ctx *ctx, struct cmd *cmd)
 		if (table == NULL) {
 			table = table_alloc();
 			handle_merge(&table->handle, &cmd->handle);
-			table_add_hash(table);
 		}
 	}
 
 	switch (cmd->obj) {
 	case CMD_OBJ_TABLE:
 		if (!cmd->handle.table) {
-			/* List all existing tables */
 			struct table *table;
 
-			if (netlink_list_tables(ctx, &cmd->handle,
-						&cmd->location) < 0)
-				return -1;
-
-			list_for_each_entry(table, &ctx->list, list) {
+			list_for_each_entry(table, &table_list, list)
 				printf("table %s\n", table->handle.table);
-			}
+
 			return 0;
 		}
 		return do_list_table(ctx, cmd, table);
@@ -993,7 +1021,6 @@ static int do_command_rename(struct netlink_ctx *ctx, struct cmd *cmd)
 
 	table = table_alloc();
 	handle_merge(&table->handle, &cmd->handle);
-	table_add_hash(table);
 
 	switch (cmd->obj) {
 	case CMD_OBJ_CHAIN:
@@ -1013,7 +1040,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);
@@ -1036,10 +1063,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;
 
@@ -1053,8 +1077,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 2/7] src: always allocate table object with no table block

[Pablo Neira Ayuso]

This adds the table object to the cache, so it can be reused in the linear list
way to express updates.

The CMD_OBJ_RULESET is used to know if the table block, that contains the
chain, set and rule declarations, is present.

This patch prepares the set cache consolidation to avoid that sets are added
twice, once from do_add_table() which iterates over the set list when cmd->data
is set, and then again from the do_add_set().

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/evaluate.c     |    3 +--
 src/parser_bison.y |    7 ++++---
 src/rule.c         |   44 ++++++++++++++++++++++++++++----------------
 3 files changed, 33 insertions(+), 21 deletions(-)

diff --git a/src/evaluate.c b/src/evaluate.c
index d99b38f..ac90162 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1910,8 +1910,7 @@ 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;
+	case CMD_OBJ_RULESET:
 		return table_evaluate(ctx, cmd->table);
 	default:
 		BUG("invalid command object type %u\n", cmd->obj);
diff --git a/src/parser_bison.y b/src/parser_bison.y
index 5c4e272..2b742a7 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -652,16 +652,17 @@ base_cmd		:	/* empty */	add_cmd		{ $$ = $1; }
 			|	DESCRIBE	describe_cmd	{ $$ = $2; }
 			;
 
-add_cmd			:	TABLE		table_spec
+add_cmd			:	TABLE		table_spec	table_block_alloc
 			{
-				$$ = cmd_alloc(CMD_ADD, CMD_OBJ_TABLE, &$2, &@$, NULL);
+				handle_merge(&$3->handle, &$2);
+				$$ = cmd_alloc(CMD_ADD, CMD_OBJ_TABLE, &$2, &@$, $3);
 			}
 			|	TABLE		table_spec	table_block_alloc
 						'{'	table_block	'}'
 			{
 				handle_merge(&$3->handle, &$2);
 				close_scope(state);
-				$$ = cmd_alloc(CMD_ADD, CMD_OBJ_TABLE, &$2, &@$, $5);
+				$$ = cmd_alloc(CMD_ADD, CMD_OBJ_RULESET, &$2, &@$, $5);
 			}
 			|	CHAIN		chain_spec
 			{
diff --git a/src/rule.c b/src/rule.c
index cbc4931..d250549 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -698,6 +698,10 @@ void cmd_free(struct cmd *cmd)
 		case CMD_OBJ_TABLE:
 			table_free(cmd->table);
 			break;
+		case CMD_OBJ_RULESET:
+			if (cmd->data != NULL)
+				table_free(cmd->table);
+			break;
 		case CMD_OBJ_EXPR:
 			expr_free(cmd->expr);
 			break;
@@ -757,27 +761,32 @@ static int do_add_table(struct netlink_ctx *ctx, const struct handle *h,
 			const struct location *loc, struct table *table,
 			bool excl)
 {
+	return netlink_add_table(ctx, h, loc, table, excl);
+}
+
+static int do_add_ruleset(struct netlink_ctx *ctx, const struct handle *h,
+			  const struct location *loc, struct table *table,
+			  bool excl)
+{
 	struct chain *chain;
 	struct set *set;
 
 	if (netlink_add_table(ctx, h, loc, table, excl) < 0)
 		return -1;
-	if (table != NULL) {
-		list_for_each_entry(chain, &table->chains, list) {
-			if (netlink_add_chain(ctx, &chain->handle,
-					      &chain->location, chain,
-					      excl) < 0)
-				return -1;
-		}
-		list_for_each_entry(set, &table->sets, list) {
-			handle_merge(&set->handle, &table->handle);
-			if (do_add_set(ctx, &set->handle, set) < 0)
-				return -1;
-		}
-		list_for_each_entry(chain, &table->chains, list) {
-			if (netlink_add_rule_list(ctx, h, &chain->rules) < 0)
-				return -1;
-		}
+
+	list_for_each_entry(chain, &table->chains, list) {
+		if (netlink_add_chain(ctx, &chain->handle, &chain->location,
+				      chain, excl) < 0)
+			return -1;
+	}
+	list_for_each_entry(set, &table->sets, list) {
+		handle_merge(&set->handle, &table->handle);
+		if (do_add_set(ctx, &set->handle, set) < 0)
+			return -1;
+	}
+	list_for_each_entry(chain, &table->chains, list) {
+		if (netlink_add_rule_list(ctx, h, &chain->rules) < 0)
+			return -1;
 	}
 	return 0;
 }
@@ -798,6 +807,9 @@ static int do_command_add(struct netlink_ctx *ctx, struct cmd *cmd, bool excl)
 		return do_add_set(ctx, &cmd->handle, cmd->set);
 	case CMD_OBJ_SETELEM:
 		return do_add_setelems(ctx, &cmd->handle, cmd->expr);
+	case CMD_OBJ_RULESET:
+		return do_add_ruleset(ctx, &cmd->handle, &cmd->location,
+				      cmd->table, excl);
 	default:
 		BUG("invalid command object type %u\n", cmd->obj);
 	}
-- 
1.7.10.4

[PATCH 3/7] src: consolidate set cache

[Pablo Neira Ayuso]

This patch populates the table cache only once through netlink_list_sets() from
the initialization step. As a result, there is a single call to
netlink_list_sets().

The set cache initialization happens once the table cache is ready. On the
other hand, declared sets are added to the cache to they can be referenced from
this batch.

After this changes, 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 consulting the kernel doesn't help since they are
not yet available.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/rule.h |    3 ++
 src/evaluate.c |   57 +++++++++++++++----------------------
 src/main.c     |    6 ++++
 src/rule.c     |   85 ++++++++++++++++++++++++++++++++------------------------
 4 files changed, 80 insertions(+), 71 deletions(-)

diff --git a/include/rule.h b/include/rule.h
index ae69a8d..46dace5 100644
--- a/include/rule.h
+++ b/include/rule.h
@@ -219,6 +219,9 @@ struct set {
 extern struct set *set_alloc(const struct location *loc);
 extern struct set *set_get(struct set *set);
 extern void set_free(struct set *set);
+
+int set_init_hash(void);
+void set_fini_hash(void);
 extern void set_add_hash(struct set *set, struct table *table);
 extern struct set *set_lookup(const struct table *table, const char *name);
 extern struct set *set_lookup_global(uint32_t family, const char *table,
diff --git a/src/evaluate.c b/src/evaluate.c
index ac90162..fb466ba 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -107,37 +107,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)
 {
@@ -190,6 +159,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;
 
@@ -211,7 +181,13 @@ 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 expr_error(ctx->msgs, *expr,
+					  "missing table '%s'",
+					  (*expr)->identifier);
+
+		set = set_lookup(table, (*expr)->identifier);
 		if (set == NULL)
 			return -1;
 		new = set_ref_expr_alloc(&(*expr)->location, set);
@@ -1735,9 +1711,14 @@ 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 -1;
+
+	set = set_lookup(table, ctx->cmd->handle.set);
 	if (set == NULL)
 		return -1;
 
@@ -1751,6 +1732,7 @@ 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;
 
 	type = set->flags & SET_F_MAP ? "map" : "set";
@@ -1775,7 +1757,7 @@ static int set_evaluate(struct eval_ctx *ctx, struct set *set)
 		set->flags |= SET_F_TIMEOUT;
 
 	if (!(set->flags & SET_F_MAP))
-		return 0;
+		goto out;
 
 	if (set->datatype == NULL)
 		return set_error(ctx, set, "map definition does not specify "
@@ -1786,6 +1768,13 @@ static int set_evaluate(struct eval_ctx *ctx, struct set *set)
 		return set_error(ctx, set, "unqualified mapping data type "
 				 "specified in map definition");
 
+out:
+	table = table_lookup(&set->handle);
+	if (table == NULL)
+		return set_error(ctx, set, "cannot find table");
+
+	set_add_hash(set, table);
+	set_get(set);
 	return 0;
 }
 
diff --git a/src/main.c b/src/main.c
index 7d559df..ec93877 100644
--- a/src/main.c
+++ b/src/main.c
@@ -232,6 +232,11 @@ int nft_run(void *scanner, struct parser_state *state, struct list_head *msgs)
 	if (table_init_hash() < 0)
 		return -1;
 
+	if (set_init_hash() < 0) {
+		table_fini_hash();
+		return -1;
+	}
+
 	ret = nft_parse(scanner, state);
 	if (ret != 0 || state->nerrs > 0) {
 		table_fini_hash();
@@ -248,6 +253,7 @@ retry:
 		list_del(&cmd->list);
 		cmd_free(cmd);
 	}
+	set_fini_hash();
 	table_fini_hash();
 
 	return ret;
diff --git a/src/rule.c b/src/rule.c
index d250549..1997b81 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -28,6 +28,8 @@
 #include <linux/netfilter.h>
 #include <linux/netfilter_arp.h>
 
+static LIST_HEAD(table_list);
+
 void handle_free(struct handle *h)
 {
 	xfree(h->table);
@@ -81,6 +83,47 @@ void set_free(struct set *set)
 	xfree(set);
 }
 
+int set_init_hash(void)
+{
+	struct netlink_ctx ctx;
+	struct table *table;
+	LIST_HEAD(msgs);
+	int ret;
+
+retry:
+	netlink_genid_get();
+	memset(&ctx, 0, sizeof(ctx));
+	init_list_head(&ctx.list);
+	ctx.msgs = &msgs;
+
+	list_for_each_entry(table, &table_list, list) {
+		ret = netlink_list_sets(&ctx, &table->handle,
+					&internal_location);
+		if (ret < 0) {
+			if (errno == EINTR) {
+				set_fini_hash();
+				goto retry;
+			}
+
+			erec_print_list(stdout, &msgs);
+			return ret;
+		}
+		list_splice_tail_init(&ctx.list, &table->sets);
+	}
+
+	return 0;
+}
+
+void set_fini_hash(void)
+{
+	struct set *set, *next;
+	struct table *table;
+
+	list_for_each_entry(table, &table_list, list)
+		list_for_each_entry_safe(set, next, &table->sets, list)
+			set_free(set);
+}
+
 void set_add_hash(struct set *set, struct table *table)
 {
 	list_add_tail(&set->list, &table->sets);
@@ -522,8 +565,6 @@ void table_free(struct table *table)
 	xfree(table);
 }
 
-static LIST_HEAD(table_list);
-
 int table_init_hash(void)
 {
 	struct handle handle = {
@@ -850,15 +891,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;
 }
@@ -976,16 +1013,8 @@ 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;
-			}
+		list_for_each_entry(set, &table->sets, list)
 			set_print(set);
-		}
 		return 0;
 	case CMD_OBJ_SET:
 		if (netlink_get_set(ctx, &cmd->handle, &cmd->location) < 0)
@@ -1053,10 +1082,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:
@@ -1071,24 +1097,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 4/7] src: early allocation of the set ID

[Pablo Neira Ayuso]

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

diff --git a/src/netlink.c b/src/netlink.c
index 4b57aab..1f28bf6 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -1117,9 +1117,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)
 {
@@ -1143,7 +1140,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 1997b81..1ec8c78 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -56,6 +56,9 @@ void handle_merge(struct handle *dst, const struct handle *src)
 		dst->comment = xstrdup(src->comment);
 }
 
+/* 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;
@@ -64,6 +67,8 @@ struct set *set_alloc(const struct location *loc)
 	set->refcnt = 1;
 	if (loc != NULL)
 		set->location = *loc;
+
+	set->handle.set_id = ++set_id;
 	return set;
 }
 
-- 
1.7.10.4

[PATCH 5/7] 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 1ec8c78..34685a9 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -795,7 +795,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 6/7] 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 34685a9..c7f2a4a 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -797,7 +797,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 7/7] 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 c7f2a4a..ac356ec 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -781,10 +781,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

Re: [PATCH 1/7] src: consolidate table cache

[Pablo Neira Ayuso]

[-- Attachment #1: Type: text/plain, Size: 101 bytes --]

I have a new round to revisit the generation ID checks, that were not
correct in the previous patch.

[-- Attachment #2: 0001-src-consolidate-table-cache.patch --]
[-- Type: text/x-diff, Size: 8166 bytes --]

>From d55709f87108645269ea478746cf98a63bb2e637 Mon Sep 17 00:00:00 2001
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Fri, 26 Jun 2015 11:33:22 +0200
Subject: [PATCH nft 1/7,v2] src: consolidate table cache

This patch populates the table cache only once through netlink_list_tables()
from the initialization step. As a result, there is a single call to
netlink_list_tables().

Then, new table declarations are also added to this cache, thus follow up calls
to table_lookup() for tables that don't exist yet in the kernel will be found
in the cache.

Note that table_alloc() inserts the object in the cache and table_free()
removes the object from the cache and it releases it.

Table objects may be released from cmd_free() or after all commands in the file
have been processed.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
v2: Rework v1 to fix generation ID checks.

 include/rule.h |    3 +++
 src/main.c     |   29 ++++++++++++++++++++--
 src/netlink.c  |   12 +++------
 src/rule.c     |   75 +++++++++++++++++++++++++++++++++++---------------------
 4 files changed, 80 insertions(+), 39 deletions(-)

diff --git a/include/rule.h b/include/rule.h
index 5d44599..ae69a8d 100644
--- a/include/rule.h
+++ b/include/rule.h
@@ -89,6 +89,9 @@ struct table {
 
 extern struct table *table_alloc(void);
 extern void table_free(struct table *table);
+
+extern int table_init_hash(void);
+extern void table_fini_hash(void);
 extern void table_add_hash(struct table *table);
 extern struct table *table_lookup(const struct handle *h);
 
diff --git a/src/main.c b/src/main.c
index bfe589a..a84f2f6 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,18 +223,43 @@ out:
 	return ret;
 }
 
+static int nft_cache_init(void)
+{
+	netlink_genid_get();
+
+	return table_init_hash();
+}
+
+static void nft_cache_fini(void)
+{
+	table_fini_hash();
+}
+
+static int nft_cache_restart(void)
+{
+	nft_cache_fini();
+	return nft_cache_init();
+}
+
 int nft_run(void *scanner, struct parser_state *state, struct list_head *msgs)
 {
 	struct cmd *cmd, *next;
 	int ret;
 
+	if (nft_cache_init() < 0)
+		return -1;
+
 	ret = nft_parse(scanner, state);
-	if (ret != 0 || state->nerrs > 0)
+	if (ret != 0 || state->nerrs > 0) {
+		nft_cache_fini();
 		return -1;
+	}
 retry:
 	ret = nft_netlink(state, msgs);
 	if (ret < 0 && errno == EINTR) {
 		netlink_restart();
+		if (nft_cache_restart() < 0)
+			return -1;
 		goto retry;
 	}
 
@@ -243,6 +267,7 @@ retry:
 		list_del(&cmd->list);
 		cmd_free(cmd);
 	}
+	nft_cache_fini();
 
 	return ret;
 }
diff --git a/src/netlink.c b/src/netlink.c
index 429eed4..4b57aab 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -926,10 +926,8 @@ static struct table *netlink_delinearize_table(struct netlink_ctx *ctx,
 static int list_table_cb(struct nft_table *nlt, void *arg)
 {
 	struct netlink_ctx *ctx = arg;
-	struct table *table;
 
-	table = netlink_delinearize_table(ctx, nlt);
-	list_add_tail(&table->list, &ctx->list);
+	netlink_delinearize_table(ctx, nlt);
 
 	return 0;
 }
@@ -972,7 +970,7 @@ int netlink_get_table(struct netlink_ctx *ctx, const struct handle *h,
 
 	ntable = netlink_delinearize_table(ctx, nlt);
 	table->flags = ntable->flags;
-	xfree(ntable);
+	table_free(ntable);
 out:
 	nft_table_free(nlt);
 	return err;
@@ -1963,13 +1961,10 @@ static void netlink_events_cache_addtable(struct netlink_mon_handler *monh,
 					  const struct nlmsghdr *nlh)
 {
 	struct nft_table *nlt;
-	struct table *t;
 
 	nlt = netlink_table_alloc(nlh);
-	t = netlink_delinearize_table(monh->ctx, nlt);
+	netlink_delinearize_table(monh->ctx, nlt);
 	nft_table_free(nlt);
-
-	table_add_hash(t);
 }
 
 static void netlink_events_cache_deltable(struct netlink_mon_handler *monh,
@@ -1987,7 +1982,6 @@ static void netlink_events_cache_deltable(struct netlink_mon_handler *monh,
 	if (t == NULL)
 		goto out;
 
-	list_del(&t->list);
 	table_free(t);
 out:
 	nft_table_free(nlt);
diff --git a/src/rule.c b/src/rule.c
index b2090dd..36b8a5e 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -20,6 +20,7 @@
 #include <rule.h>
 #include <utils.h>
 #include <netlink.h>
+#include <erec.h>
 
 #include <libnftnl/common.h>
 #include <libnftnl/ruleset.h>
@@ -504,6 +505,8 @@ struct table *table_alloc(void)
 	init_list_head(&table->chains);
 	init_list_head(&table->sets);
 	init_list_head(&table->scope.symbols);
+	table_add_hash(table);
+
 	return table;
 }
 
@@ -515,11 +518,46 @@ void table_free(struct table *table)
 		chain_free(chain);
 	handle_free(&table->handle);
 	scope_release(&table->scope);
+	list_del(&table->list);
 	xfree(table);
 }
 
 static LIST_HEAD(table_list);
 
+int table_init_hash(void)
+{
+	struct handle handle = {
+		.family = NFPROTO_UNSPEC,
+	};
+	struct netlink_ctx ctx;
+	LIST_HEAD(msgs);
+	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) {
+		if (errno != EINTR)
+			erec_print_list(stdout, &msgs);
+
+		return ret;
+	}
+
+	list_splice_tail_init(&ctx.list, &table_list);
+
+	return 0;
+}
+
+void table_fini_hash(void)
+{
+	struct table *table, *next;
+
+	list_for_each_entry_safe(table, next, &table_list, list)
+		table_free(table);
+}
+
 void table_add_hash(struct table *table)
 {
 	list_add_tail(&table->list, &table_list);
@@ -878,25 +916,18 @@ 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);
+	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 (cmd->handle.family != NFPROTO_UNSPEC &&
+		    table->handle.family != cmd->handle.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;
@@ -913,23 +944,17 @@ static int do_command_list(struct netlink_ctx *ctx, struct cmd *cmd)
 		if (table == NULL) {
 			table = table_alloc();
 			handle_merge(&table->handle, &cmd->handle);
-			table_add_hash(table);
 		}
 	}
 
 	switch (cmd->obj) {
 	case CMD_OBJ_TABLE:
 		if (!cmd->handle.table) {
-			/* List all existing tables */
 			struct table *table;
 
-			if (netlink_list_tables(ctx, &cmd->handle,
-						&cmd->location) < 0)
-				return -1;
-
-			list_for_each_entry(table, &ctx->list, list) {
+			list_for_each_entry(table, &table_list, list)
 				printf("table %s\n", table->handle.table);
-			}
+
 			return 0;
 		}
 		return do_list_table(ctx, cmd, table);
@@ -993,7 +1018,6 @@ static int do_command_rename(struct netlink_ctx *ctx, struct cmd *cmd)
 
 	table = table_alloc();
 	handle_merge(&table->handle, &cmd->handle);
-	table_add_hash(table);
 
 	switch (cmd->obj) {
 	case CMD_OBJ_CHAIN:
@@ -1013,7 +1037,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);
@@ -1036,10 +1060,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;
 
@@ -1053,8 +1074,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

Re: [PATCH 3/7] src: consolidate set cache

[Pablo Neira Ayuso]

[-- Attachment #1: Type: text/plain, Size: 47 bytes --]

Attaching a v2 to rework generation id checks.

[-- Attachment #2: 0003-src-consolidate-set-cache.patch --]
[-- Type: text/x-diff, Size: 8837 bytes --]

>From 403a192bb967d6dde3dbfe34ee3fa88f7aca4197 Mon Sep 17 00:00:00 2001
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Fri, 26 Jun 2015 11:33:22 +0200
Subject: [PATCH nft 3/7, v2] src: consolidate set cache

This patch populates the table cache only once through netlink_list_sets() from
the initialization step. As a result, there is a single call to
netlink_list_sets().

The set cache initialization happens once the table cache is ready. On the
other hand, declared sets are added to the cache so they can be referenced from
this batch.

After this changes, 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 consulting the kernel doesn't help since they are
not yet available.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
v2: Rework v1 to fix generation ID checks.

 include/rule.h |    3 +++
 src/evaluate.c |   57 ++++++++++++++++------------------------
 src/main.c     |   10 ++++++-
 src/rule.c     |   80 ++++++++++++++++++++++++++++++--------------------------
 4 files changed, 78 insertions(+), 72 deletions(-)

diff --git a/include/rule.h b/include/rule.h
index ae69a8d..46dace5 100644
--- a/include/rule.h
+++ b/include/rule.h
@@ -219,6 +219,9 @@ struct set {
 extern struct set *set_alloc(const struct location *loc);
 extern struct set *set_get(struct set *set);
 extern void set_free(struct set *set);
+
+int set_init_hash(void);
+void set_fini_hash(void);
 extern void set_add_hash(struct set *set, struct table *table);
 extern struct set *set_lookup(const struct table *table, const char *name);
 extern struct set *set_lookup_global(uint32_t family, const char *table,
diff --git a/src/evaluate.c b/src/evaluate.c
index ac90162..fb466ba 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -107,37 +107,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)
 {
@@ -190,6 +159,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;
 
@@ -211,7 +181,13 @@ 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 expr_error(ctx->msgs, *expr,
+					  "missing table '%s'",
+					  (*expr)->identifier);
+
+		set = set_lookup(table, (*expr)->identifier);
 		if (set == NULL)
 			return -1;
 		new = set_ref_expr_alloc(&(*expr)->location, set);
@@ -1735,9 +1711,14 @@ 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 -1;
+
+	set = set_lookup(table, ctx->cmd->handle.set);
 	if (set == NULL)
 		return -1;
 
@@ -1751,6 +1732,7 @@ 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;
 
 	type = set->flags & SET_F_MAP ? "map" : "set";
@@ -1775,7 +1757,7 @@ static int set_evaluate(struct eval_ctx *ctx, struct set *set)
 		set->flags |= SET_F_TIMEOUT;
 
 	if (!(set->flags & SET_F_MAP))
-		return 0;
+		goto out;
 
 	if (set->datatype == NULL)
 		return set_error(ctx, set, "map definition does not specify "
@@ -1786,6 +1768,13 @@ static int set_evaluate(struct eval_ctx *ctx, struct set *set)
 		return set_error(ctx, set, "unqualified mapping data type "
 				 "specified in map definition");
 
+out:
+	table = table_lookup(&set->handle);
+	if (table == NULL)
+		return set_error(ctx, set, "cannot find table");
+
+	set_add_hash(set, table);
+	set_get(set);
 	return 0;
 }
 
diff --git a/src/main.c b/src/main.c
index a84f2f6..1013497 100644
--- a/src/main.c
+++ b/src/main.c
@@ -227,11 +227,19 @@ static int nft_cache_init(void)
 {
 	netlink_genid_get();
 
-	return table_init_hash();
+	if (table_init_hash() < 0)
+		return -1;
+
+	if (set_init_hash() < 0) {
+		table_fini_hash();
+		return -1;
+	}
+	return 0;
 }
 
 static void nft_cache_fini(void)
 {
+	set_fini_hash();
 	table_fini_hash();
 }
 
diff --git a/src/rule.c b/src/rule.c
index 1ae374e..2344270 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -28,6 +28,8 @@
 #include <linux/netfilter.h>
 #include <linux/netfilter_arp.h>
 
+static LIST_HEAD(table_list);
+
 void handle_free(struct handle *h)
 {
 	xfree(h->table);
@@ -81,6 +83,42 @@ void set_free(struct set *set)
 	xfree(set);
 }
 
+int set_init_hash(void)
+{
+	struct netlink_ctx ctx;
+	struct table *table;
+	LIST_HEAD(msgs);
+	int ret;
+
+	memset(&ctx, 0, sizeof(ctx));
+	init_list_head(&ctx.list);
+	ctx.msgs = &msgs;
+
+	list_for_each_entry(table, &table_list, list) {
+		ret = netlink_list_sets(&ctx, &table->handle,
+					&internal_location);
+		if (ret < 0) {
+			if (errno != EINTR)
+				erec_print_list(stdout, &msgs);
+
+			return ret;
+		}
+		list_splice_tail_init(&ctx.list, &table->sets);
+	}
+
+	return 0;
+}
+
+void set_fini_hash(void)
+{
+	struct set *set, *next;
+	struct table *table;
+
+	list_for_each_entry(table, &table_list, list)
+		list_for_each_entry_safe(set, next, &table->sets, list)
+			set_free(set);
+}
+
 void set_add_hash(struct set *set, struct table *table)
 {
 	list_add_tail(&set->list, &table->sets);
@@ -522,8 +560,6 @@ void table_free(struct table *table)
 	xfree(table);
 }
 
-static LIST_HEAD(table_list);
-
 int table_init_hash(void)
 {
 	struct handle handle = {
@@ -847,15 +883,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;
 }
@@ -973,16 +1005,8 @@ 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;
-			}
+		list_for_each_entry(set, &table->sets, list)
 			set_print(set);
-		}
 		return 0;
 	case CMD_OBJ_SET:
 		if (netlink_get_set(ctx, &cmd->handle, &cmd->location) < 0)
@@ -1050,10 +1074,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:
@@ -1068,24 +1089,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