[PATCH nft,v3 0/7] cache consolidation

[PATCH nft,v3 0/7] cache consolidation

[Pablo Neira Ayuso]

This is v3 of the cache consolidation patchset.

In this round, I have patch 2/7 to use the scope so nft knows if this is a
table declaration that contains a full block of further declarations or not.
I have also fixed several problems I found while testing from the previous
round.

More info at:
http://marc.info/?l=netfilter-devel&m=143560012416115&w=2

Pablo Neira Ayuso (7):
  src: consolidate table cache
  src: add table declaration to cache
  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       |    6 ++
 src/evaluate.c       |   49 ++++---------
 src/main.c           |   37 +++++++++-
 src/netlink.c        |   16 +----
 src/parser_bison.y   |   10 ++-
 src/rule.c           |  191 ++++++++++++++++++++++++++++++++------------------
 src/segtree.c        |   15 ++--
 8 files changed, 198 insertions(+), 129 deletions(-)

-- 
1.7.10.4

[PATCH nft,v3 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     |   29 +++++++++++++++++++--
 src/netlink.c  |   12 +++------
 src/rule.c     |   78 ++++++++++++++++++++++++++++++++++----------------------
 4 files changed, 81 insertions(+), 41 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..5df0eda 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);
@@ -847,8 +885,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)
@@ -878,25 +914,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;
@@ -913,23 +943,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 +1017,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 +1036,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 +1059,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 +1073,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,v3 2/7] src: add table declaration to cache

[Pablo Neira Ayuso]

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().

Table objects that are declared with no table block to the cache are now
available through table_lookup().

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

diff --git a/src/evaluate.c b/src/evaluate.c
index d99b38f..576509c 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1910,8 +1910,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);
diff --git a/src/parser_bison.y b/src/parser_bison.y
index 5c4e272..5f2fb1d 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -654,7 +654,10 @@ base_cmd		:	/* empty */	add_cmd		{ $$ = $1; }
 
 add_cmd			:	TABLE		table_spec
 			{
-				$$ = cmd_alloc(CMD_ADD, CMD_OBJ_TABLE, &$2, &@$, NULL);
+				struct table *table = table_alloc();
+
+				handle_merge(&table->handle, &$2);
+				$$ = cmd_alloc(CMD_ADD, CMD_OBJ_TABLE, &$2, &@$, table);
 			}
 			|	TABLE		table_spec	table_block_alloc
 						'{'	table_block	'}'
@@ -705,7 +708,10 @@ add_cmd			:	TABLE		table_spec
 
 create_cmd		:	TABLE		table_spec
 			{
-				$$ = cmd_alloc(CMD_CREATE, CMD_OBJ_TABLE, &$2, &@$, NULL);
+				struct table *table = table_alloc();
+
+				handle_merge(&table->handle, &$2);
+				$$ = cmd_alloc(CMD_CREATE, CMD_OBJ_TABLE, &$2, &@$, table);
 			}
 			|	TABLE		table_spec	table_block_alloc
 						'{'	table_block	'}'
diff --git a/src/rule.c b/src/rule.c
index 5df0eda..98bb1eb 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -759,7 +759,7 @@ static int do_add_table(struct netlink_ctx *ctx, const struct handle *h,
 
 	if (netlink_add_table(ctx, h, loc, table, excl) < 0)
 		return -1;
-	if (table != NULL) {
+	if (table->scope.parent != NULL) {
 		list_for_each_entry(chain, &table->chains, list) {
 			if (netlink_add_chain(ctx, &chain->handle,
 					      &chain->location, chain,
-- 
1.7.10.4

[PATCH nft,v3 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 so they can be referenced from
this batch.

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 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 |   47 +++++++++----------------------
 src/main.c     |   10 ++++++-
 src/rule.c     |   86 ++++++++++++++++++++++++++++++++------------------------
 4 files changed, 76 insertions(+), 70 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 576509c..28ddf12 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;
 
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 98bb1eb..838bd7e 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 = {
@@ -835,15 +871,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;
 }
@@ -960,15 +992,15 @@ 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(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);
 			}
-			set_print(set);
 		}
 		return 0;
 	case CMD_OBJ_SET:
@@ -1037,10 +1069,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:
@@ -1055,24 +1084,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,v3 4/7] 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    |    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 838bd7e..7491ee7 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 nft,v3 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 7491ee7..623e464 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -783,7 +783,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,v3 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 623e464..bc9042f 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -785,7 +785,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,v3 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 bc9042f..8c9e8e8 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -769,10 +769,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