[PATCH 0/7] new transaction infrastructure for nf_tables

[PATCH 0/7] new transaction infrastructure for nf_tables

[Pablo Neira Ayuso]

Hi,

This patchset contains updates to the transaction infrastructure and a new
batch API to userspace to update tables, chains and sets. Basically, it
generalises the existing rule batching so we can also include sets, chains and
tables in one single batch. This helps to speed up updates since we save many
netlink messages between kernel and userspace and this also improves several
batch loading error cases that resulted in inconsistent configurations.

Still, this patchset doesn't address the abortion of chain policy/counter
updates and new set elements addition/removals. Basically, this means that we
don't have atomic set element updates yet, but that wasn't possible with the
former API either.

Pablo Neira Ayuso (7):
  netfilter: nf_tables: deconstify table and chain in context structure
  netfilter: nf_tables: generalise transaction infrastructure
  netfilter: nf_tables: relocate commit and abort routines in the source file
  netfilter: nf_tables: better encapsulation for the rule transaction code
  netfilter: nf_tables: move set handling to the transaction infrastructure
  netfilter: nf_tables: move chain handling to the transaction infrastructure
  netfilter: nf_tables: move table handling to the transaction infrastructure

 include/net/netfilter/nf_tables.h        |   31 +-
 include/uapi/linux/netfilter/nf_tables.h |    6 +
 net/netfilter/nf_tables_api.c            |  830 ++++++++++++++++++++++--------
 net/netfilter/nft_lookup.c               |   15 +-
 4 files changed, 656 insertions(+), 226 deletions(-)

-- 
1.7.10.4

[PATCH 1/7] netfilter: nf_tables: deconstify table and chain in context structure

[Pablo Neira Ayuso]

The new transaction infrastructure updates the family, table and chain
objects in the context structure, so let's deconstify them. While at it,
move the context structure initialization routine to the top of the
source file as it will be also used from the table and chain routines.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/net/netfilter/nf_tables.h |    6 ++--
 net/netfilter/nf_tables_api.c     |   58 ++++++++++++++++++-------------------
 2 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index e6bc14d..13cd713 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -83,9 +83,9 @@ struct nft_ctx {
 	struct net			*net;
 	const struct sk_buff		*skb;
 	const struct nlmsghdr		*nlh;
-	const struct nft_af_info	*afi;
-	const struct nft_table		*table;
-	const struct nft_chain		*chain;
+	struct nft_af_info		*afi;
+	struct nft_table		*table;
+	struct nft_chain		*chain;
 	const struct nlattr * const 	*nla;
 };
 
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 33045a5..48a5645 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -88,6 +88,23 @@ nf_tables_afinfo_lookup(struct net *net, int family, bool autoload)
 	return ERR_PTR(-EAFNOSUPPORT);
 }
 
+static void nft_ctx_init(struct nft_ctx *ctx,
+			 const struct sk_buff *skb,
+			 const struct nlmsghdr *nlh,
+			 struct nft_af_info *afi,
+			 struct nft_table *table,
+			 struct nft_chain *chain,
+			 const struct nlattr * const *nla)
+{
+	ctx->net   = sock_net(skb->sk);
+	ctx->skb   = skb;
+	ctx->nlh   = nlh;
+	ctx->afi   = afi;
+	ctx->table = table;
+	ctx->chain = chain;
+	ctx->nla   = nla;
+}
+
 /*
  * Tables
  */
@@ -812,7 +829,7 @@ static int nf_tables_newchain(struct sock *nlsk, struct sk_buff *skb,
 {
 	const struct nfgenmsg *nfmsg = nlmsg_data(nlh);
 	const struct nlattr * uninitialized_var(name);
-	const struct nft_af_info *afi;
+	struct nft_af_info *afi;
 	struct nft_table *table;
 	struct nft_chain *chain;
 	struct nft_base_chain *basechain = NULL;
@@ -1024,7 +1041,7 @@ static int nf_tables_delchain(struct sock *nlsk, struct sk_buff *skb,
 			      const struct nlattr * const nla[])
 {
 	const struct nfgenmsg *nfmsg = nlmsg_data(nlh);
-	const struct nft_af_info *afi;
+	struct nft_af_info *afi;
 	struct nft_table *table;
 	struct nft_chain *chain;
 	struct net *net = sock_net(skb->sk);
@@ -1062,23 +1079,6 @@ static int nf_tables_delchain(struct sock *nlsk, struct sk_buff *skb,
 	return 0;
 }
 
-static void nft_ctx_init(struct nft_ctx *ctx,
-			 const struct sk_buff *skb,
-			 const struct nlmsghdr *nlh,
-			 const struct nft_af_info *afi,
-			 const struct nft_table *table,
-			 const struct nft_chain *chain,
-			 const struct nlattr * const *nla)
-{
-	ctx->net   = sock_net(skb->sk);
-	ctx->skb   = skb;
-	ctx->nlh   = nlh;
-	ctx->afi   = afi;
-	ctx->table = table;
-	ctx->chain = chain;
-	ctx->nla   = nla;
-}
-
 /*
  * Expressions
  */
@@ -1579,7 +1579,7 @@ static int nf_tables_newrule(struct sock *nlsk, struct sk_buff *skb,
 			     const struct nlattr * const nla[])
 {
 	const struct nfgenmsg *nfmsg = nlmsg_data(nlh);
-	const struct nft_af_info *afi;
+	struct nft_af_info *afi;
 	struct net *net = sock_net(skb->sk);
 	struct nft_table *table;
 	struct nft_chain *chain;
@@ -1760,9 +1760,9 @@ static int nf_tables_delrule(struct sock *nlsk, struct sk_buff *skb,
 			     const struct nlattr * const nla[])
 {
 	const struct nfgenmsg *nfmsg = nlmsg_data(nlh);
-	const struct nft_af_info *afi;
+	struct nft_af_info *afi;
 	struct net *net = sock_net(skb->sk);
-	const struct nft_table *table;
+	struct nft_table *table;
 	struct nft_chain *chain = NULL;
 	struct nft_rule *rule;
 	int family = nfmsg->nfgen_family, err = 0;
@@ -1961,8 +1961,8 @@ static int nft_ctx_init_from_setattr(struct nft_ctx *ctx,
 {
 	struct net *net = sock_net(skb->sk);
 	const struct nfgenmsg *nfmsg = nlmsg_data(nlh);
-	const struct nft_af_info *afi = NULL;
-	const struct nft_table *table = NULL;
+	struct nft_af_info *afi = NULL;
+	struct nft_table *table = NULL;
 
 	if (nfmsg->nfgen_family != NFPROTO_UNSPEC) {
 		afi = nf_tables_afinfo_lookup(net, nfmsg->nfgen_family, false);
@@ -2182,7 +2182,7 @@ static int nf_tables_dump_sets_all(struct nft_ctx *ctx, struct sk_buff *skb,
 {
 	const struct nft_set *set;
 	unsigned int idx, s_idx = cb->args[0];
-	const struct nft_af_info *afi;
+	struct nft_af_info *afi;
 	struct nft_table *table, *cur_table = (struct nft_table *)cb->args[2];
 	struct net *net = sock_net(skb->sk);
 	int cur_family = cb->args[3];
@@ -2310,7 +2310,7 @@ static int nf_tables_newset(struct sock *nlsk, struct sk_buff *skb,
 {
 	const struct nfgenmsg *nfmsg = nlmsg_data(nlh);
 	const struct nft_set_ops *ops;
-	const struct nft_af_info *afi;
+	struct nft_af_info *afi;
 	struct net *net = sock_net(skb->sk);
 	struct nft_table *table;
 	struct nft_set *set;
@@ -2559,8 +2559,8 @@ static int nft_ctx_init_from_elemattr(struct nft_ctx *ctx,
 				      const struct nlattr * const nla[])
 {
 	const struct nfgenmsg *nfmsg = nlmsg_data(nlh);
-	const struct nft_af_info *afi;
-	const struct nft_table *table;
+	struct nft_af_info *afi;
+	struct nft_table *table;
 	struct net *net = sock_net(skb->sk);
 
 	afi = nf_tables_afinfo_lookup(net, nfmsg->nfgen_family, false);
@@ -2785,7 +2785,7 @@ static int nft_add_set_elem(const struct nft_ctx *ctx, struct nft_set *set,
 			struct nft_ctx bind_ctx = {
 				.afi	= ctx->afi,
 				.table	= ctx->table,
-				.chain	= binding->chain,
+				.chain	= (struct nft_chain *)binding->chain,
 			};
 
 			err = nft_validate_data_load(&bind_ctx, dreg,
-- 
1.7.10.4

[PATCH 2/7] netfilter: nf_tables: generalise transaction infrastructure

[Pablo Neira Ayuso]

This patch generalises the existing rule transaction infrastructure
so it can be used to handle set, table and chain object transactions
as well.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/net/netfilter/nf_tables.h |   15 +++--
 net/netfilter/nf_tables_api.c     |  114 +++++++++++++++++++++----------------
 2 files changed, 75 insertions(+), 54 deletions(-)

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 13cd713..03940f3 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -340,17 +340,24 @@ struct nft_rule {
 		__attribute__((aligned(__alignof__(struct nft_expr))));
 };
 
+enum nft_trans_type {
+	NFT_TRANS_RULE,
+};
+
 /**
- *	struct nft_rule_trans - nf_tables rule update in transaction
+ *	struct nft_trans - nf_tables object update in transaction
  *
  *	@list: used internally
+ *	@type: object type
  *	@ctx: rule context
- *	@rule: rule that needs to be updated
  */
-struct nft_rule_trans {
+struct nft_trans {
 	struct list_head		list;
+	enum nft_trans_type		type;
 	struct nft_ctx			ctx;
-	struct nft_rule			*rule;
+	union {
+		struct nft_rule		*rule;
+	};
 };
 
 static inline struct nft_expr *nft_expr_first(const struct nft_rule *rule)
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 48a5645..203a6bd 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -105,6 +105,21 @@ static void nft_ctx_init(struct nft_ctx *ctx,
 	ctx->nla   = nla;
 }
 
+static struct nft_trans *nft_trans_alloc(struct nft_ctx *ctx,
+					 enum nft_trans_type type)
+{
+	struct nft_trans *trans;
+
+	trans = kzalloc(sizeof(struct nft_trans), GFP_KERNEL);
+	if (trans == NULL)
+		return NULL;
+
+	trans->type = type;
+	trans->ctx  = *ctx;
+
+	return trans;
+}
+
 /*
  * Tables
  */
@@ -1558,20 +1573,19 @@ static void nf_tables_rule_destroy(const struct nft_ctx *ctx,
 
 static struct nft_expr_info *info;
 
-static struct nft_rule_trans *
-nf_tables_trans_add(struct nft_ctx *ctx, struct nft_rule *rule)
+static struct nft_trans *
+nft_rule_trans_add(struct nft_ctx *ctx, struct nft_rule *rule)
 {
-	struct nft_rule_trans *rupd;
+	struct nft_trans *trans;
 
-	rupd = kmalloc(sizeof(struct nft_rule_trans), GFP_KERNEL);
-	if (rupd == NULL)
-	       return NULL;
+	trans = nft_trans_alloc(ctx, NFT_TRANS_RULE);
+	if (trans == NULL)
+		return NULL;
 
-	rupd->ctx = *ctx;
-	rupd->rule = rule;
-	list_add_tail(&rupd->list, &ctx->net->nft.commit_list);
+	trans->rule = rule;
+	list_add_tail(&trans->list, &ctx->net->nft.commit_list);
 
-	return rupd;
+	return trans;
 }
 
 static int nf_tables_newrule(struct sock *nlsk, struct sk_buff *skb,
@@ -1584,7 +1598,7 @@ static int nf_tables_newrule(struct sock *nlsk, struct sk_buff *skb,
 	struct nft_table *table;
 	struct nft_chain *chain;
 	struct nft_rule *rule, *old_rule = NULL;
-	struct nft_rule_trans *repl = NULL;
+	struct nft_trans *trans = NULL;
 	struct nft_expr *expr;
 	struct nft_ctx ctx;
 	struct nlattr *tmp;
@@ -1682,8 +1696,8 @@ static int nf_tables_newrule(struct sock *nlsk, struct sk_buff *skb,
 
 	if (nlh->nlmsg_flags & NLM_F_REPLACE) {
 		if (nft_rule_is_active_next(net, old_rule)) {
-			repl = nf_tables_trans_add(&ctx, old_rule);
-			if (repl == NULL) {
+			trans = nft_rule_trans_add(&ctx, old_rule);
+			if (trans == NULL) {
 				err = -ENOMEM;
 				goto err2;
 			}
@@ -1705,7 +1719,7 @@ static int nf_tables_newrule(struct sock *nlsk, struct sk_buff *skb,
 			list_add_rcu(&rule->list, &chain->rules);
 	}
 
-	if (nf_tables_trans_add(&ctx, rule) == NULL) {
+	if (nft_rule_trans_add(&ctx, rule) == NULL) {
 		err = -ENOMEM;
 		goto err3;
 	}
@@ -1713,11 +1727,11 @@ static int nf_tables_newrule(struct sock *nlsk, struct sk_buff *skb,
 
 err3:
 	list_del_rcu(&rule->list);
-	if (repl) {
-		list_del_rcu(&repl->rule->list);
-		list_del(&repl->list);
-		nft_rule_clear(net, repl->rule);
-		kfree(repl);
+	if (trans) {
+		list_del_rcu(&trans->rule->list);
+		list_del(&trans->list);
+		nft_rule_clear(net, trans->rule);
+		kfree(trans);
 	}
 err2:
 	nf_tables_rule_destroy(&ctx, rule);
@@ -1734,7 +1748,7 @@ nf_tables_delrule_one(struct nft_ctx *ctx, struct nft_rule *rule)
 {
 	/* You cannot delete the same rule twice */
 	if (nft_rule_is_active_next(ctx->net, rule)) {
-		if (nf_tables_trans_add(ctx, rule) == NULL)
+		if (nft_rule_trans_add(ctx, rule) == NULL)
 			return -ENOMEM;
 		nft_rule_disactivate_next(ctx->net, rule);
 		return 0;
@@ -1810,7 +1824,7 @@ static int nf_tables_delrule(struct sock *nlsk, struct sk_buff *skb,
 static int nf_tables_commit(struct sk_buff *skb)
 {
 	struct net *net = sock_net(skb->sk);
-	struct nft_rule_trans *rupd, *tmp;
+	struct nft_trans *trans, *next;
 
 	/* Bump generation counter, invalidate any dump in progress */
 	net->nft.genctr++;
@@ -1823,38 +1837,38 @@ static int nf_tables_commit(struct sk_buff *skb)
 	 */
 	synchronize_rcu();
 
-	list_for_each_entry_safe(rupd, tmp, &net->nft.commit_list, list) {
+	list_for_each_entry_safe(trans, next, &net->nft.commit_list, list) {
 		/* This rule was inactive in the past and just became active.
 		 * Clear the next bit of the genmask since its meaning has
 		 * changed, now it is the future.
 		 */
-		if (nft_rule_is_active(net, rupd->rule)) {
-			nft_rule_clear(net, rupd->rule);
-			nf_tables_rule_notify(skb, rupd->ctx.nlh,
-					      rupd->ctx.table, rupd->ctx.chain,
-					      rupd->rule, NFT_MSG_NEWRULE, 0,
-					      rupd->ctx.afi->family);
-			list_del(&rupd->list);
-			kfree(rupd);
+		if (nft_rule_is_active(net, trans->rule)) {
+			nft_rule_clear(net, trans->rule);
+			nf_tables_rule_notify(skb, trans->ctx.nlh,
+					      trans->ctx.table, trans->ctx.chain,
+					      trans->rule, NFT_MSG_NEWRULE, 0,
+					      trans->ctx.afi->family);
+			list_del(&trans->list);
+			kfree(trans);
 			continue;
 		}
 
 		/* This rule is in the past, get rid of it */
-		list_del_rcu(&rupd->rule->list);
-		nf_tables_rule_notify(skb, rupd->ctx.nlh,
-				      rupd->ctx.table, rupd->ctx.chain,
-				      rupd->rule, NFT_MSG_DELRULE, 0,
-				      rupd->ctx.afi->family);
+		list_del_rcu(&trans->rule->list);
+		nf_tables_rule_notify(skb, trans->ctx.nlh,
+				      trans->ctx.table, trans->ctx.chain,
+				      trans->rule, NFT_MSG_DELRULE, 0,
+				      trans->ctx.afi->family);
 	}
 
 	/* Make sure we don't see any packet traversing old rules */
 	synchronize_rcu();
 
 	/* Now we can safely release unused old rules */
-	list_for_each_entry_safe(rupd, tmp, &net->nft.commit_list, list) {
-		nf_tables_rule_destroy(&rupd->ctx, rupd->rule);
-		list_del(&rupd->list);
-		kfree(rupd);
+	list_for_each_entry_safe(trans, next, &net->nft.commit_list, list) {
+		nf_tables_rule_destroy(&trans->ctx, trans->rule);
+		list_del(&trans->list);
+		kfree(trans);
 	}
 
 	return 0;
@@ -1863,27 +1877,27 @@ static int nf_tables_commit(struct sk_buff *skb)
 static int nf_tables_abort(struct sk_buff *skb)
 {
 	struct net *net = sock_net(skb->sk);
-	struct nft_rule_trans *rupd, *tmp;
+	struct nft_trans *trans, *next;
 
-	list_for_each_entry_safe(rupd, tmp, &net->nft.commit_list, list) {
-		if (!nft_rule_is_active_next(net, rupd->rule)) {
-			nft_rule_clear(net, rupd->rule);
-			list_del(&rupd->list);
-			kfree(rupd);
+	list_for_each_entry_safe(trans, next, &net->nft.commit_list, list) {
+		if (!nft_rule_is_active_next(net, trans->rule)) {
+			nft_rule_clear(net, trans->rule);
+			list_del(&trans->list);
+			kfree(trans);
 			continue;
 		}
 
 		/* This rule is inactive, get rid of it */
-		list_del_rcu(&rupd->rule->list);
+		list_del_rcu(&trans->rule->list);
 	}
 
 	/* Make sure we don't see any packet accessing aborted rules */
 	synchronize_rcu();
 
-	list_for_each_entry_safe(rupd, tmp, &net->nft.commit_list, list) {
-		nf_tables_rule_destroy(&rupd->ctx, rupd->rule);
-		list_del(&rupd->list);
-		kfree(rupd);
+	list_for_each_entry_safe(trans, next, &net->nft.commit_list, list) {
+		nf_tables_rule_destroy(&trans->ctx, trans->rule);
+		list_del(&trans->list);
+		kfree(trans);
 	}
 
 	return 0;
-- 
1.7.10.4

[PATCH 3/7] netfilter: nf_tables: relocate commit and abort routines in the source file

[Pablo Neira Ayuso]

Move the commit and abort routines to the bottom of the source code
file. This change is required by the follow up patches that add the
set, chain and table transaction support.

This patch is just a cleanup to access several functions without
having to declare their prototypes.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_tables_api.c |  164 ++++++++++++++++++++---------------------
 1 file changed, 82 insertions(+), 82 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 203a6bd..9a0aad5 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1821,88 +1821,6 @@ static int nf_tables_delrule(struct sock *nlsk, struct sk_buff *skb,
 	return err;
 }
 
-static int nf_tables_commit(struct sk_buff *skb)
-{
-	struct net *net = sock_net(skb->sk);
-	struct nft_trans *trans, *next;
-
-	/* Bump generation counter, invalidate any dump in progress */
-	net->nft.genctr++;
-
-	/* A new generation has just started */
-	net->nft.gencursor = gencursor_next(net);
-
-	/* Make sure all packets have left the previous generation before
-	 * purging old rules.
-	 */
-	synchronize_rcu();
-
-	list_for_each_entry_safe(trans, next, &net->nft.commit_list, list) {
-		/* This rule was inactive in the past and just became active.
-		 * Clear the next bit of the genmask since its meaning has
-		 * changed, now it is the future.
-		 */
-		if (nft_rule_is_active(net, trans->rule)) {
-			nft_rule_clear(net, trans->rule);
-			nf_tables_rule_notify(skb, trans->ctx.nlh,
-					      trans->ctx.table, trans->ctx.chain,
-					      trans->rule, NFT_MSG_NEWRULE, 0,
-					      trans->ctx.afi->family);
-			list_del(&trans->list);
-			kfree(trans);
-			continue;
-		}
-
-		/* This rule is in the past, get rid of it */
-		list_del_rcu(&trans->rule->list);
-		nf_tables_rule_notify(skb, trans->ctx.nlh,
-				      trans->ctx.table, trans->ctx.chain,
-				      trans->rule, NFT_MSG_DELRULE, 0,
-				      trans->ctx.afi->family);
-	}
-
-	/* Make sure we don't see any packet traversing old rules */
-	synchronize_rcu();
-
-	/* Now we can safely release unused old rules */
-	list_for_each_entry_safe(trans, next, &net->nft.commit_list, list) {
-		nf_tables_rule_destroy(&trans->ctx, trans->rule);
-		list_del(&trans->list);
-		kfree(trans);
-	}
-
-	return 0;
-}
-
-static int nf_tables_abort(struct sk_buff *skb)
-{
-	struct net *net = sock_net(skb->sk);
-	struct nft_trans *trans, *next;
-
-	list_for_each_entry_safe(trans, next, &net->nft.commit_list, list) {
-		if (!nft_rule_is_active_next(net, trans->rule)) {
-			nft_rule_clear(net, trans->rule);
-			list_del(&trans->list);
-			kfree(trans);
-			continue;
-		}
-
-		/* This rule is inactive, get rid of it */
-		list_del_rcu(&trans->rule->list);
-	}
-
-	/* Make sure we don't see any packet accessing aborted rules */
-	synchronize_rcu();
-
-	list_for_each_entry_safe(trans, next, &net->nft.commit_list, list) {
-		nf_tables_rule_destroy(&trans->ctx, trans->rule);
-		list_del(&trans->list);
-		kfree(trans);
-	}
-
-	return 0;
-}
-
 /*
  * Sets
  */
@@ -2997,6 +2915,88 @@ static const struct nfnl_callback nf_tables_cb[NFT_MSG_MAX] = {
 	},
 };
 
+static int nf_tables_commit(struct sk_buff *skb)
+{
+	struct net *net = sock_net(skb->sk);
+	struct nft_trans *trans, *next;
+
+	/* Bump generation counter, invalidate any dump in progress */
+	net->nft.genctr++;
+
+	/* A new generation has just started */
+	net->nft.gencursor = gencursor_next(net);
+
+	/* Make sure all packets have left the previous generation before
+	 * purging old rules.
+	 */
+	synchronize_rcu();
+
+	list_for_each_entry_safe(trans, next, &net->nft.commit_list, list) {
+		/* This rule was inactive in the past and just became active.
+		 * Clear the next bit of the genmask since its meaning has
+		 * changed, now it is the future.
+		 */
+		if (nft_rule_is_active(net, trans->rule)) {
+			nft_rule_clear(net, trans->rule);
+			nf_tables_rule_notify(skb, trans->ctx.nlh,
+					      trans->ctx.table, trans->ctx.chain,
+					      trans->rule, NFT_MSG_NEWRULE, 0,
+					      trans->ctx.afi->family);
+			list_del(&trans->list);
+			kfree(trans);
+			continue;
+		}
+
+		/* This rule is in the past, get rid of it */
+		list_del_rcu(&trans->rule->list);
+		nf_tables_rule_notify(skb, trans->ctx.nlh,
+				      trans->ctx.table, trans->ctx.chain,
+				      trans->rule, NFT_MSG_DELRULE, 0,
+				      trans->ctx.afi->family);
+	}
+
+	/* Make sure we don't see any packet traversing old rules */
+	synchronize_rcu();
+
+	/* Now we can safely release unused old rules */
+	list_for_each_entry_safe(trans, next, &net->nft.commit_list, list) {
+		nf_tables_rule_destroy(&trans->ctx, trans->rule);
+		list_del(&trans->list);
+		kfree(trans);
+	}
+
+	return 0;
+}
+
+static int nf_tables_abort(struct sk_buff *skb)
+{
+	struct net *net = sock_net(skb->sk);
+	struct nft_trans *trans, *next;
+
+	list_for_each_entry_safe(trans, next, &net->nft.commit_list, list) {
+		if (!nft_rule_is_active_next(net, trans->rule)) {
+			nft_rule_clear(net, trans->rule);
+			list_del(&trans->list);
+			kfree(trans);
+			continue;
+		}
+
+		/* This rule is inactive, get rid of it */
+		list_del_rcu(&trans->rule->list);
+	}
+
+	/* Make sure we don't see any packet accessing aborted rules */
+	synchronize_rcu();
+
+	list_for_each_entry_safe(trans, next, &net->nft.commit_list, list) {
+		nf_tables_rule_destroy(&trans->ctx, trans->rule);
+		list_del(&trans->list);
+		kfree(trans);
+	}
+
+	return 0;
+}
+
 static const struct nfnetlink_subsystem nf_tables_subsys = {
 	.name		= "nf_tables",
 	.subsys_id	= NFNL_SUBSYS_NFTABLES,
-- 
1.7.10.4

[PATCH 4/7] netfilter: nf_tables: better encapsulation for the rule transaction code

[Pablo Neira Ayuso]

Another cleanup to accomodate set, chain and table transaction support.
Move the rule transaction code to several functions to avoid too large
commit and abort routines.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_tables_api.c |  102 ++++++++++++++++++++++++++++-------------
 1 file changed, 69 insertions(+), 33 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 9a0aad5..4d4d5fc 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -2915,6 +2915,32 @@ static const struct nfnl_callback nf_tables_cb[NFT_MSG_MAX] = {
 	},
 };
 
+static void nft_rule_commit_update(struct net *net, struct sk_buff *skb,
+				   struct nft_trans *trans)
+{
+	/* This rule was inactive in the past and just became active.
+	 * Clear the next bit of the genmask since its meaning has
+	 * changed, now it is the future.
+	 */
+	if (nft_rule_is_active(net, trans->rule)) {
+		nft_rule_clear(net, trans->rule);
+		nf_tables_rule_notify(skb, trans->ctx.nlh,
+				      trans->ctx.table, trans->ctx.chain,
+				      trans->rule, NFT_MSG_NEWRULE, 0,
+				      trans->ctx.afi->family);
+		list_del(&trans->list);
+		kfree(trans);
+		return;
+	}
+
+	/* This rule is in the past, get rid of it */
+	list_del_rcu(&trans->rule->list);
+	nf_tables_rule_notify(skb, trans->ctx.nlh,
+			      trans->ctx.table, trans->ctx.chain,
+			      trans->rule, NFT_MSG_DELRULE, 0,
+			      trans->ctx.afi->family);
+}
+
 static int nf_tables_commit(struct sk_buff *skb)
 {
 	struct net *net = sock_net(skb->sk);
@@ -2927,70 +2953,80 @@ static int nf_tables_commit(struct sk_buff *skb)
 	net->nft.gencursor = gencursor_next(net);
 
 	/* Make sure all packets have left the previous generation before
-	 * purging old rules.
+	 * purging old objects.
 	 */
 	synchronize_rcu();
 
 	list_for_each_entry_safe(trans, next, &net->nft.commit_list, list) {
-		/* This rule was inactive in the past and just became active.
-		 * Clear the next bit of the genmask since its meaning has
-		 * changed, now it is the future.
-		 */
-		if (nft_rule_is_active(net, trans->rule)) {
-			nft_rule_clear(net, trans->rule);
-			nf_tables_rule_notify(skb, trans->ctx.nlh,
-					      trans->ctx.table, trans->ctx.chain,
-					      trans->rule, NFT_MSG_NEWRULE, 0,
-					      trans->ctx.afi->family);
-			list_del(&trans->list);
-			kfree(trans);
-			continue;
+		switch (trans->type) {
+		case NFT_TRANS_RULE:
+			nft_rule_commit_update(net, skb, trans);
+			break;
 		}
-
-		/* This rule is in the past, get rid of it */
-		list_del_rcu(&trans->rule->list);
-		nf_tables_rule_notify(skb, trans->ctx.nlh,
-				      trans->ctx.table, trans->ctx.chain,
-				      trans->rule, NFT_MSG_DELRULE, 0,
-				      trans->ctx.afi->family);
 	}
 
-	/* Make sure we don't see any packet traversing old rules */
+	/* Make sure we don't see any packet traversing old objects */
 	synchronize_rcu();
 
-	/* Now we can safely release unused old rules */
+	/* Now we can safely release unused old objects */
 	list_for_each_entry_safe(trans, next, &net->nft.commit_list, list) {
-		nf_tables_rule_destroy(&trans->ctx, trans->rule);
 		list_del(&trans->list);
+		switch (trans->type) {
+		case NFT_TRANS_RULE:
+			nf_tables_rule_destroy(&trans->ctx, trans->rule);
+			break;
+		}
 		kfree(trans);
 	}
 
 	return 0;
 }
 
+static void nft_rule_abort_undo(struct net *net, struct sk_buff *skb,
+				struct nft_trans *trans)
+{
+	/* This rule was scheduled for removal but this transaction has been
+	 * aborted, clear the rule generation bitmask and leave it in place.
+	 */
+	if (!nft_rule_is_active_next(net, trans->rule)) {
+		nft_rule_clear(net, trans->rule);
+		list_del(&trans->list);
+		kfree(trans);
+		return;
+	}
+
+	/* This rule was scheduled to be active in the next generation, but
+	 * this transaction was aborted, remove it from the list.
+	 */
+	list_del_rcu(&trans->rule->list);
+}
+
 static int nf_tables_abort(struct sk_buff *skb)
 {
 	struct net *net = sock_net(skb->sk);
 	struct nft_trans *trans, *next;
 
 	list_for_each_entry_safe(trans, next, &net->nft.commit_list, list) {
-		if (!nft_rule_is_active_next(net, trans->rule)) {
-			nft_rule_clear(net, trans->rule);
-			list_del(&trans->list);
-			kfree(trans);
-			continue;
+		switch (trans->type) {
+		case NFT_TRANS_RULE:
+			nft_rule_abort_undo(net, skb, trans);
+			break;
 		}
-
-		/* This rule is inactive, get rid of it */
-		list_del_rcu(&trans->rule->list);
 	}
 
 	/* Make sure we don't see any packet accessing aborted rules */
 	synchronize_rcu();
 
 	list_for_each_entry_safe(trans, next, &net->nft.commit_list, list) {
-		nf_tables_rule_destroy(&trans->ctx, trans->rule);
 		list_del(&trans->list);
+		switch (trans->type) {
+		case NFT_TRANS_RULE:
+			/* Release new rules, already removed from the list,
+			 * that we couldn't activate.
+			 */
+			nf_tables_rule_destroy(&trans->ctx, trans->rule);
+			break;
+		}
 		kfree(trans);
 	}
 
-- 
1.7.10.4

[PATCH 5/7] netfilter: nf_tables: move set handling to the transaction infrastructure

[Pablo Neira Ayuso]

This patch reworks the nf_tables API so set updates are moved into
the same batch that contains rule updates. This speeds up rule-set
updates we skip a dialog of four messages between kernel and
user-space (two on each direction).

 1) create the set and send netlink message to the kernel
 2) process the response from the kernel that contains the allocated name.
 3) add the set elements and send netlink message to the kernel.
 4) process the response from the kernel (to check for errors).

To:

 1) add the set to the batch.
 2) add the set elements to the batch.
 3) add the rule that points to the set.
 4) send batch to the kernel.

The idea is to allocate an internal set ID to the batch that can be
used when adding set elements and rules that refer to the set in the
batch.

Note that this patch doesn't add atomic set element updates, it just
helps to leave the set configuration in consistent state in case
that we fail to load the entire batch for some reason.

Backward compatibility has been only retained in userspace, this
means that new nft versions can talk to the kernel both in the new
and the old fashion.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/net/netfilter/nf_tables.h        |    7 ++
 include/uapi/linux/netfilter/nf_tables.h |    6 ++
 net/netfilter/nf_tables_api.c            |  130 +++++++++++++++++++++++++++---
 net/netfilter/nft_lookup.c               |   15 +++-
 4 files changed, 141 insertions(+), 17 deletions(-)

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 03940f3..02e990d 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -222,6 +222,8 @@ static inline void *nft_set_priv(const struct nft_set *set)
 
 struct nft_set *nf_tables_set_lookup(const struct nft_table *table,
 				     const struct nlattr *nla);
+struct nft_set *nf_tables_set_lookup_byid(const struct net *net,
+					  const struct nlattr *nla);
 
 /**
  *	struct nft_set_binding - nf_tables set binding
@@ -342,6 +344,7 @@ struct nft_rule {
 
 enum nft_trans_type {
 	NFT_TRANS_RULE,
+	NFT_TRANS_SET,
 };
 
 /**
@@ -357,6 +360,10 @@ struct nft_trans {
 	struct nft_ctx			ctx;
 	union {
 		struct nft_rule		*rule;
+		struct {
+			struct nft_set	*set;
+			u32		set_id;
+		};
 	};
 };
 
diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index c88ccbf..8427e17 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -221,6 +221,7 @@ enum nft_set_flags {
  * @NFTA_SET_KEY_LEN: key data length (NLA_U32)
  * @NFTA_SET_DATA_TYPE: mapping data type (NLA_U32)
  * @NFTA_SET_DATA_LEN: mapping data length (NLA_U32)
+ * @NFTA_SET_ID: uniquely identifies a set in a transaction (NLA_U32)
  */
 enum nft_set_attributes {
 	NFTA_SET_UNSPEC,
@@ -231,6 +232,7 @@ enum nft_set_attributes {
 	NFTA_SET_KEY_LEN,
 	NFTA_SET_DATA_TYPE,
 	NFTA_SET_DATA_LEN,
+	NFTA_SET_ID,
 	__NFTA_SET_MAX
 };
 #define NFTA_SET_MAX		(__NFTA_SET_MAX - 1)
@@ -266,12 +268,14 @@ enum nft_set_elem_attributes {
  * @NFTA_SET_ELEM_LIST_TABLE: table of the set to be changed (NLA_STRING)
  * @NFTA_SET_ELEM_LIST_SET: name of the set to be changed (NLA_STRING)
  * @NFTA_SET_ELEM_LIST_ELEMENTS: list of set elements (NLA_NESTED: nft_set_elem_attributes)
+ * @NFTA_SET_ELEM_LIST_SET_ID: uniquely identifies a set in a transaction (NLA_U32)
  */
 enum nft_set_elem_list_attributes {
 	NFTA_SET_ELEM_LIST_UNSPEC,
 	NFTA_SET_ELEM_LIST_TABLE,
 	NFTA_SET_ELEM_LIST_SET,
 	NFTA_SET_ELEM_LIST_ELEMENTS,
+	NFTA_SET_ELEM_LIST_SET_ID,
 	__NFTA_SET_ELEM_LIST_MAX
 };
 #define NFTA_SET_ELEM_LIST_MAX	(__NFTA_SET_ELEM_LIST_MAX - 1)
@@ -457,12 +461,14 @@ enum nft_cmp_attributes {
  * @NFTA_LOOKUP_SET: name of the set where to look for (NLA_STRING)
  * @NFTA_LOOKUP_SREG: source register of the data to look for (NLA_U32: nft_registers)
  * @NFTA_LOOKUP_DREG: destination register (NLA_U32: nft_registers)
+ * @NFTA_LOOKUP_SET_ID: uniquely identifies a set in a transaction (NLA_U32)
  */
 enum nft_lookup_attributes {
 	NFTA_LOOKUP_UNSPEC,
 	NFTA_LOOKUP_SET,
 	NFTA_LOOKUP_SREG,
 	NFTA_LOOKUP_DREG,
+	NFTA_LOOKUP_SET_ID,
 	__NFTA_LOOKUP_MAX
 };
 #define NFTA_LOOKUP_MAX		(__NFTA_LOOKUP_MAX - 1)
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 4d4d5fc..0b2fb07 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1884,6 +1884,7 @@ static const struct nla_policy nft_set_policy[NFTA_SET_MAX + 1] = {
 	[NFTA_SET_KEY_LEN]		= { .type = NLA_U32 },
 	[NFTA_SET_DATA_TYPE]		= { .type = NLA_U32 },
 	[NFTA_SET_DATA_LEN]		= { .type = NLA_U32 },
+	[NFTA_SET_ID]			= { .type = NLA_U32 },
 };
 
 static int nft_ctx_init_from_setattr(struct nft_ctx *ctx,
@@ -1930,6 +1931,22 @@ struct nft_set *nf_tables_set_lookup(const struct nft_table *table,
 	return ERR_PTR(-ENOENT);
 }
 
+struct nft_set *nf_tables_set_lookup_byid(const struct net *net,
+					  const struct nlattr *nla)
+{
+	struct nft_trans *trans;
+	u32 id = ntohl(nla_get_be32(nla));
+
+	list_for_each_entry(trans, &net->nft.commit_list, list) {
+		if (trans->type != NFT_TRANS_SET)
+			continue;
+
+		if (id == trans->set_id)
+			return trans->set;
+	}
+	return ERR_PTR(-ENOENT);
+}
+
 static int nf_tables_set_alloc_name(struct nft_ctx *ctx, struct nft_set *set,
 				    const char *name)
 {
@@ -2236,6 +2253,33 @@ err:
 	return err;
 }
 
+/* Internal set flags */
+#define __NFT_SET_DYING		(1 << 14)
+#define __NFT_SET_INACTIVE	(1 << 15)
+#define __NFT_SET_MASK		(__NFT_SET_DYING | __NFT_SET_INACTIVE)
+
+static int nft_set_trans_add(struct nft_ctx *ctx, struct nft_set *set)
+{
+	struct nft_trans *trans;
+
+	/* You cannot delete an existing set twice */
+	if (set->flags & __NFT_SET_DYING)
+		return -ENOENT;
+
+	trans = nft_trans_alloc(ctx, NFT_TRANS_SET);
+	if (trans == NULL)
+		return -ENOMEM;
+
+	trans->set = set;
+	if (ctx->nla[NFTA_SET_ID])
+		trans->set_id = ntohl(nla_get_be32(ctx->nla[NFTA_SET_ID]));
+
+	set->flags |= __NFT_SET_INACTIVE;
+	list_add_tail(&trans->list, &ctx->net->nft.commit_list);
+
+	return 0;
+}
+
 static int nf_tables_newset(struct sock *nlsk, struct sk_buff *skb,
 			    const struct nlmsghdr *nlh,
 			    const struct nlattr * const nla[])
@@ -2360,8 +2404,10 @@ static int nf_tables_newset(struct sock *nlsk, struct sk_buff *skb,
 	if (err < 0)
 		goto err2;
 
-	list_add_tail(&set->list, &table->sets);
-	nf_tables_set_notify(&ctx, set, NFT_MSG_NEWSET);
+	err = nft_set_trans_add(&ctx, set);
+	if (err < 0)
+		goto err2;
+
 	return 0;
 
 err2:
@@ -2371,16 +2417,20 @@ err1:
 	return err;
 }
 
-static void nf_tables_set_destroy(const struct nft_ctx *ctx, struct nft_set *set)
+static void nft_set_destroy(struct nft_set *set)
 {
-	list_del(&set->list);
-	nf_tables_set_notify(ctx, set, NFT_MSG_DELSET);
-
 	set->ops->destroy(set);
 	module_put(set->ops->owner);
 	kfree(set);
 }
 
+static void nf_tables_set_destroy(const struct nft_ctx *ctx, struct nft_set *set)
+{
+	list_del(&set->list);
+	nf_tables_set_notify(ctx, set, NFT_MSG_DELSET);
+	nft_set_destroy(set);
+}
+
 static int nf_tables_delset(struct sock *nlsk, struct sk_buff *skb,
 			    const struct nlmsghdr *nlh,
 			    const struct nlattr * const nla[])
@@ -2405,7 +2455,14 @@ static int nf_tables_delset(struct sock *nlsk, struct sk_buff *skb,
 	if (!list_empty(&set->bindings))
 		return -EBUSY;
 
-	nf_tables_set_destroy(&ctx, set);
+	set->flags |= __NFT_SET_DYING;
+
+	err = nft_set_trans_add(&ctx, set);
+	if (err < 0) {
+		set->flags &= ~__NFT_SET_DYING;
+		return err;
+	}
+
 	return 0;
 }
 
@@ -2465,7 +2522,8 @@ void nf_tables_unbind_set(const struct nft_ctx *ctx, struct nft_set *set,
 {
 	list_del(&binding->list);
 
-	if (list_empty(&set->bindings) && set->flags & NFT_SET_ANONYMOUS)
+	if (list_empty(&set->bindings) && set->flags & NFT_SET_ANONYMOUS &&
+	    !(set->flags & __NFT_SET_INACTIVE))
 		nf_tables_set_destroy(ctx, set);
 }
 
@@ -2483,6 +2541,7 @@ static const struct nla_policy nft_set_elem_list_policy[NFTA_SET_ELEM_LIST_MAX +
 	[NFTA_SET_ELEM_LIST_TABLE]	= { .type = NLA_STRING },
 	[NFTA_SET_ELEM_LIST_SET]	= { .type = NLA_STRING },
 	[NFTA_SET_ELEM_LIST_ELEMENTS]	= { .type = NLA_NESTED },
+	[NFTA_SET_ELEM_LIST_SET_ID]	= { .type = NLA_U32 },
 };
 
 static int nft_ctx_init_from_elemattr(struct nft_ctx *ctx,
@@ -2746,6 +2805,7 @@ static int nf_tables_newsetelem(struct sock *nlsk, struct sk_buff *skb,
 				const struct nlmsghdr *nlh,
 				const struct nlattr * const nla[])
 {
+	struct net *net = sock_net(skb->sk);
 	const struct nlattr *attr;
 	struct nft_set *set;
 	struct nft_ctx ctx;
@@ -2755,7 +2815,13 @@ static int nf_tables_newsetelem(struct sock *nlsk, struct sk_buff *skb,
 	if (err < 0)
 		return err;
 
-	set = nf_tables_set_lookup(ctx.table, nla[NFTA_SET_ELEM_LIST_SET]);
+	if (nla[NFTA_SET_ELEM_LIST_SET_ID]) {
+		set = nf_tables_set_lookup_byid(net,
+						nla[NFTA_SET_ELEM_LIST_SET_ID]);
+	} else {
+		set = nf_tables_set_lookup(ctx.table,
+					   nla[NFTA_SET_ELEM_LIST_SET]);
+	}
 	if (IS_ERR(set))
 		return PTR_ERR(set);
 	if (!list_empty(&set->bindings) && set->flags & NFT_SET_CONSTANT)
@@ -2884,7 +2950,7 @@ static const struct nfnl_callback nf_tables_cb[NFT_MSG_MAX] = {
 		.policy		= nft_rule_policy,
 	},
 	[NFT_MSG_NEWSET] = {
-		.call		= nf_tables_newset,
+		.call_batch	= nf_tables_newset,
 		.attr_count	= NFTA_SET_MAX,
 		.policy		= nft_set_policy,
 	},
@@ -2894,12 +2960,12 @@ static const struct nfnl_callback nf_tables_cb[NFT_MSG_MAX] = {
 		.policy		= nft_set_policy,
 	},
 	[NFT_MSG_DELSET] = {
-		.call		= nf_tables_delset,
+		.call_batch	= nf_tables_delset,
 		.attr_count	= NFTA_SET_MAX,
 		.policy		= nft_set_policy,
 	},
 	[NFT_MSG_NEWSETELEM] = {
-		.call		= nf_tables_newsetelem,
+		.call_batch	= nf_tables_newsetelem,
 		.attr_count	= NFTA_SET_ELEM_LIST_MAX,
 		.policy		= nft_set_elem_list_policy,
 	},
@@ -2909,7 +2975,7 @@ static const struct nfnl_callback nf_tables_cb[NFT_MSG_MAX] = {
 		.policy		= nft_set_elem_list_policy,
 	},
 	[NFT_MSG_DELSETELEM] = {
-		.call		= nf_tables_delsetelem,
+		.call_batch	= nf_tables_delsetelem,
 		.attr_count	= NFTA_SET_ELEM_LIST_MAX,
 		.policy		= nft_set_elem_list_policy,
 	},
@@ -2941,6 +3007,20 @@ static void nft_rule_commit_update(struct net *net, struct sk_buff *skb,
 			      trans->ctx.afi->family);
 }
 
+static void nft_set_commit_update(struct nft_trans *trans)
+{
+	if (trans->set->flags & __NFT_SET_DYING) {
+		list_del(&trans->set->list);
+		nf_tables_set_notify(&trans->ctx, trans->set, NFT_MSG_DELSET);
+	} else {
+		list_add_tail(&trans->set->list, &trans->ctx.table->sets);
+		trans->set->flags &= ~__NFT_SET_MASK;
+		nf_tables_set_notify(&trans->ctx, trans->set, NFT_MSG_NEWSET);
+		list_del(&trans->list);
+		kfree(trans);
+	}
+}
+
 static int nf_tables_commit(struct sk_buff *skb)
 {
 	struct net *net = sock_net(skb->sk);
@@ -2962,6 +3042,9 @@ static int nf_tables_commit(struct sk_buff *skb)
 		case NFT_TRANS_RULE:
 			nft_rule_commit_update(net, skb, trans);
 			break;
+		case NFT_TRANS_SET:
+			nft_set_commit_update(trans);
+			break;
 		}
 	}
 
@@ -2975,6 +3058,10 @@ static int nf_tables_commit(struct sk_buff *skb)
 		case NFT_TRANS_RULE:
 			nf_tables_rule_destroy(&trans->ctx, trans->rule);
 			break;
+		case NFT_TRANS_SET:
+			trans->set->flags &= ~__NFT_SET_MASK;
+			nft_set_destroy(trans->set);
+			break;
 		}
 		kfree(trans);
 	}
@@ -3001,6 +3088,18 @@ static void nft_rule_abort_undo(struct net *net, struct sk_buff *skb,
 	list_del_rcu(&trans->rule->list);
 }
 
+static void nft_set_abort(struct nft_trans *trans)
+{
+	/* This set was scheduled for removal, clear the dying flags to leave
+	 * it in place. Otherwise, it's a new one that we need to release as
+	 * the transaction was aborted.
+	 */
+	if (trans->set->flags & __NFT_SET_DYING)
+		trans->set->flags &= ~__NFT_SET_MASK;
+	else
+		nft_set_destroy(trans->set);
+}
+
 static int nf_tables_abort(struct sk_buff *skb)
 {
 	struct net *net = sock_net(skb->sk);
@@ -3011,6 +3110,8 @@ static int nf_tables_abort(struct sk_buff *skb)
 		case NFT_TRANS_RULE:
 			nft_rule_abort_undo(net, skb, trans);
 			break;
+		case NFT_TRANS_SET:
+			break;
 		}
 	}
 
@@ -3026,6 +3127,9 @@ static int nf_tables_abort(struct sk_buff *skb)
 			 */
 			nf_tables_rule_destroy(&trans->ctx, trans->rule);
 			break;
+		case NFT_TRANS_SET:
+			nft_set_abort(trans);
+			break;
 		}
 		kfree(trans);
 	}
diff --git a/net/netfilter/nft_lookup.c b/net/netfilter/nft_lookup.c
index 7fd2bea..201fbb5 100644
--- a/net/netfilter/nft_lookup.c
+++ b/net/netfilter/nft_lookup.c
@@ -51,13 +51,20 @@ static int nft_lookup_init(const struct nft_ctx *ctx,
 	struct nft_set *set;
 	int err;
 
-	if (tb[NFTA_LOOKUP_SET] == NULL ||
+	if ((tb[NFTA_LOOKUP_SET] == NULL && tb[NFTA_LOOKUP_SET_ID] == NULL) ||
 	    tb[NFTA_LOOKUP_SREG] == NULL)
 		return -EINVAL;
 
-	set = nf_tables_set_lookup(ctx->table, tb[NFTA_LOOKUP_SET]);
-	if (IS_ERR(set))
-		return PTR_ERR(set);
+	if (tb[NFTA_LOOKUP_SET_ID]) {
+		set = nf_tables_set_lookup_byid(ctx->net,
+						tb[NFTA_LOOKUP_SET_ID]);
+		if (IS_ERR(set))
+			return PTR_ERR(set);
+	} else {
+		set = nf_tables_set_lookup(ctx->table, tb[NFTA_LOOKUP_SET]);
+		if (IS_ERR(set))
+			return PTR_ERR(set);
+	}
 
 	priv->sreg = ntohl(nla_get_be32(tb[NFTA_LOOKUP_SREG]));
 	err = nft_validate_input_register(priv->sreg);
-- 
1.7.10.4

[PATCH 6/7] netfilter: nf_tables: move chain handling to the transaction infrastructure

[Pablo Neira Ayuso]

This patch speeds up rule-set updates and it helps to leave chains
in consistent state when processing a batch. Note this patch does
not introduce a way to revert chain updates, eg. counter or default
policy changes.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/net/netfilter/nf_tables.h |    2 +
 net/netfilter/nf_tables_api.c     |  199 ++++++++++++++++++++++++++++++-------
 2 files changed, 164 insertions(+), 37 deletions(-)

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 02e990d..c489ead 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -343,6 +343,7 @@ struct nft_rule {
 };
 
 enum nft_trans_type {
+	NFT_TRANS_CHAIN,
 	NFT_TRANS_RULE,
 	NFT_TRANS_SET,
 };
@@ -364,6 +365,7 @@ struct nft_trans {
 			struct nft_set	*set;
 			u32		set_id;
 		};
+		bool			update;
 	};
 };
 
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 0b2fb07..11b9d91 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -566,6 +566,27 @@ static struct nft_chain *nf_tables_chain_lookup(const struct nft_table *table,
 	return ERR_PTR(-ENOENT);
 }
 
+static struct nft_chain *
+nf_tables_chain_lookup_trans(struct net *net,
+			     const struct nft_table *table,
+			     const struct nlattr *nla)
+{
+	struct nft_trans *trans;
+
+	if (nla == NULL)
+		return ERR_PTR(-EINVAL);
+
+	list_for_each_entry(trans, &net->nft.commit_list, list) {
+		if (trans->type != NFT_TRANS_CHAIN)
+			continue;
+
+		if (!nla_strcmp(nla, trans->ctx.chain->name))
+			return trans->ctx.chain;
+	}
+
+	return ERR_PTR(-ENOENT);
+}
+
 static const struct nla_policy nft_chain_policy[NFTA_CHAIN_MAX + 1] = {
 	[NFTA_CHAIN_TABLE]	= { .type = NLA_STRING },
 	[NFTA_CHAIN_HANDLE]	= { .type = NLA_U64 },
@@ -838,6 +859,53 @@ nf_tables_counters(struct nft_base_chain *chain, const struct nlattr *attr)
 	return 0;
 }
 
+/* Internal chain flags */
+#define __NFT_CHAIN_DYING	(1 << 7)
+
+static int nft_chain_trans_add(struct nft_ctx *ctx, int msg_type)
+{
+	struct nft_trans *trans;
+
+	trans = nft_trans_alloc(ctx, NFT_TRANS_CHAIN);
+	if (trans == NULL)
+		return -ENOMEM;
+
+	if (msg_type == NFT_MSG_DELCHAIN) {
+		/* You cannot delete the same chain twice */
+		if (ctx->chain->flags & __NFT_CHAIN_DYING) {
+			kfree(trans);
+			return -ENOENT;
+		}
+		ctx->chain->flags |= __NFT_CHAIN_DYING;
+	}
+
+	list_add_tail(&trans->list, &ctx->net->nft.commit_list);
+	return 0;
+}
+
+static void nft_chain_destroy(struct nft_chain *chain)
+{
+	if (chain->flags & NFT_BASE_CHAIN) {
+		module_put(nft_base_chain(chain)->type->owner);
+		free_percpu(nft_base_chain(chain)->stats);
+		kfree(nft_base_chain(chain));
+	} else {
+		kfree(chain);
+	}
+}
+
+static void nf_tables_chain_destroy(struct nft_ctx *ctx)
+{
+	BUG_ON(ctx->chain->use > 0);
+
+	if (!(ctx->table->flags & NFT_TABLE_F_DORMANT) &&
+	    ctx->chain->flags & NFT_BASE_CHAIN) {
+		nf_unregister_hooks(nft_base_chain(ctx->chain)->ops,
+				    ctx->afi->nops);
+	}
+	nft_chain_destroy(ctx->chain);
+}
+
 static int nf_tables_newchain(struct sock *nlsk, struct sk_buff *skb,
 			      const struct nlmsghdr *nlh,
 			      const struct nlattr * const nla[])
@@ -856,6 +924,7 @@ static int nf_tables_newchain(struct sock *nlsk, struct sk_buff *skb,
 	unsigned int i;
 	int err;
 	bool create;
+	struct nft_ctx ctx;
 
 	create = nlh->nlmsg_flags & NLM_F_CREATE ? true : false;
 
@@ -901,6 +970,8 @@ static int nf_tables_newchain(struct sock *nlsk, struct sk_buff *skb,
 	}
 
 	if (chain != NULL) {
+		struct nft_trans *trans;
+
 		if (nlh->nlmsg_flags & NLM_F_EXCL)
 			return -EEXIST;
 		if (nlh->nlmsg_flags & NLM_F_REPLACE)
@@ -910,14 +981,23 @@ static int nf_tables_newchain(struct sock *nlsk, struct sk_buff *skb,
 		    !IS_ERR(nf_tables_chain_lookup(table, nla[NFTA_CHAIN_NAME])))
 			return -EEXIST;
 
+		nft_ctx_init(&ctx, skb, nlh, afi, table, chain, nla);
+		trans = nft_trans_alloc(&ctx, NFT_TRANS_CHAIN);
+		if (trans == NULL)
+			return -ENOMEM;
+
+		trans->update = true;
+
 		if (nla[NFTA_CHAIN_COUNTERS]) {
 			if (!(chain->flags & NFT_BASE_CHAIN))
 				return -EOPNOTSUPP;
 
 			err = nf_tables_counters(nft_base_chain(chain),
 						 nla[NFTA_CHAIN_COUNTERS]);
-			if (err < 0)
+			if (err < 0) {
+				kfree(trans);
 				return err;
+			}
 		}
 
 		if (nla[NFTA_CHAIN_POLICY])
@@ -926,7 +1006,8 @@ static int nf_tables_newchain(struct sock *nlsk, struct sk_buff *skb,
 		if (nla[NFTA_CHAIN_HANDLE] && name)
 			nla_strlcpy(chain->name, name, NFT_CHAIN_MAXNAMELEN);
 
-		goto notify;
+		list_add_tail(&trans->list, &net->nft.commit_list);
+		return 0;
 	}
 
 	if (table->use == UINT_MAX)
@@ -1024,31 +1105,43 @@ static int nf_tables_newchain(struct sock *nlsk, struct sk_buff *skb,
 	if (!(table->flags & NFT_TABLE_F_DORMANT) &&
 	    chain->flags & NFT_BASE_CHAIN) {
 		err = nf_register_hooks(nft_base_chain(chain)->ops, afi->nops);
-		if (err < 0) {
-			module_put(basechain->type->owner);
-			free_percpu(basechain->stats);
-			kfree(basechain);
-			return err;
-		}
+		if (err < 0)
+			goto err1;
 	}
-	list_add_tail(&chain->list, &table->chains);
-	table->use++;
-notify:
-	nf_tables_chain_notify(skb, nlh, table, chain, NFT_MSG_NEWCHAIN,
-			       family);
+
+	nft_ctx_init(&ctx, skb, nlh, afi, table, chain, nla);
+	err = nft_chain_trans_add(&ctx, NFT_MSG_NEWCHAIN);
+	if (err < 0)
+		goto err2;
+
 	return 0;
+err2:
+	if (!(table->flags & NFT_TABLE_F_DORMANT) &&
+	    chain->flags & NFT_BASE_CHAIN) {
+		nf_unregister_hooks(nft_base_chain(chain)->ops,
+				    afi->nops);
+	}
+err1:
+	nft_chain_destroy(chain);
+	return err;
 }
 
-static void nf_tables_chain_destroy(struct nft_chain *chain)
+static void nft_chain_add(struct nft_trans *trans)
 {
-	BUG_ON(chain->use > 0);
+	list_add_tail(&trans->ctx.chain->list, &trans->ctx.table->chains);
+	trans->ctx.table->use++;
+	nf_tables_chain_notify(trans->ctx.skb, trans->ctx.nlh,
+			       trans->ctx.table, trans->ctx.chain,
+			       NFT_MSG_NEWCHAIN,
+			       trans->ctx.afi->family);
+}
 
-	if (chain->flags & NFT_BASE_CHAIN) {
-		module_put(nft_base_chain(chain)->type->owner);
-		free_percpu(nft_base_chain(chain)->stats);
-		kfree(nft_base_chain(chain));
-	} else
-		kfree(chain);
+static void nft_chain_del(struct nft_ctx *ctx)
+{
+	list_del(&ctx->chain->list);
+	ctx->table->use--;
+	nf_tables_chain_notify(ctx->skb, ctx->nlh, ctx->table, ctx->chain,
+			       NFT_MSG_DELCHAIN, ctx->afi->family);
 }
 
 static int nf_tables_delchain(struct sock *nlsk, struct sk_buff *skb,
@@ -1061,6 +1154,8 @@ static int nf_tables_delchain(struct sock *nlsk, struct sk_buff *skb,
 	struct nft_chain *chain;
 	struct net *net = sock_net(skb->sk);
 	int family = nfmsg->nfgen_family;
+	struct nft_ctx ctx;
+	int err;
 
 	afi = nf_tables_afinfo_lookup(net, family, false);
 	if (IS_ERR(afi))
@@ -1077,20 +1172,11 @@ static int nf_tables_delchain(struct sock *nlsk, struct sk_buff *skb,
 	if (!list_empty(&chain->rules) || chain->use > 0)
 		return -EBUSY;
 
-	list_del(&chain->list);
-	table->use--;
-
-	if (!(table->flags & NFT_TABLE_F_DORMANT) &&
-	    chain->flags & NFT_BASE_CHAIN)
-		nf_unregister_hooks(nft_base_chain(chain)->ops, afi->nops);
-
-	nf_tables_chain_notify(skb, nlh, table, chain, NFT_MSG_DELCHAIN,
-			       family);
-
-	/* Make sure all rule references are gone before this is released */
-	synchronize_rcu();
+	nft_ctx_init(&ctx, skb, nlh, afi, table, chain, nla);
+	err = nft_chain_trans_add(&ctx, NFT_MSG_DELCHAIN);
+	if (err < 0)
+		return err;
 
-	nf_tables_chain_destroy(chain);
 	return 0;
 }
 
@@ -1617,7 +1703,7 @@ static int nf_tables_newrule(struct sock *nlsk, struct sk_buff *skb,
 	if (IS_ERR(table))
 		return PTR_ERR(table);
 
-	chain = nf_tables_chain_lookup(table, nla[NFTA_RULE_CHAIN]);
+	chain = nf_tables_chain_lookup_trans(net, table, nla[NFTA_RULE_CHAIN]);
 	if (IS_ERR(chain))
 		return PTR_ERR(chain);
 
@@ -2920,7 +3006,7 @@ static const struct nfnl_callback nf_tables_cb[NFT_MSG_MAX] = {
 		.policy		= nft_table_policy,
 	},
 	[NFT_MSG_NEWCHAIN] = {
-		.call		= nf_tables_newchain,
+		.call_batch	= nf_tables_newchain,
 		.attr_count	= NFTA_CHAIN_MAX,
 		.policy		= nft_chain_policy,
 	},
@@ -2930,7 +3016,7 @@ static const struct nfnl_callback nf_tables_cb[NFT_MSG_MAX] = {
 		.policy		= nft_chain_policy,
 	},
 	[NFT_MSG_DELCHAIN] = {
-		.call		= nf_tables_delchain,
+		.call_batch	= nf_tables_delchain,
 		.attr_count	= NFTA_CHAIN_MAX,
 		.policy		= nft_chain_policy,
 	},
@@ -2981,6 +3067,25 @@ static const struct nfnl_callback nf_tables_cb[NFT_MSG_MAX] = {
 	},
 };
 
+static void nft_chain_commit_update(struct nft_trans *trans)
+{
+	if (trans->ctx.chain->flags & __NFT_CHAIN_DYING) {
+		nft_chain_del(&trans->ctx);
+	} else {
+		list_del(&trans->list);
+		if (trans->update) {
+			nf_tables_chain_notify(trans->ctx.skb, trans->ctx.nlh,
+					       trans->ctx.table,
+					       trans->ctx.chain,
+					       NFT_MSG_NEWCHAIN,
+					       trans->ctx.afi->family);
+		} else {
+			nft_chain_add(trans);
+		}
+		kfree(trans);
+	}
+}
+
 static void nft_rule_commit_update(struct net *net, struct sk_buff *skb,
 				   struct nft_trans *trans)
 {
@@ -3039,6 +3144,9 @@ static int nf_tables_commit(struct sk_buff *skb)
 
 	list_for_each_entry_safe(trans, next, &net->nft.commit_list, list) {
 		switch (trans->type) {
+		case NFT_TRANS_CHAIN:
+			nft_chain_commit_update(trans);
+			break;
 		case NFT_TRANS_RULE:
 			nft_rule_commit_update(net, skb, trans);
 			break;
@@ -3055,6 +3163,10 @@ static int nf_tables_commit(struct sk_buff *skb)
 	list_for_each_entry_safe(trans, next, &net->nft.commit_list, list) {
 		list_del(&trans->list);
 		switch (trans->type) {
+		case NFT_TRANS_CHAIN:
+			trans->ctx.chain->flags &= ~__NFT_CHAIN_DYING;
+			nf_tables_chain_destroy(&trans->ctx);
+			break;
 		case NFT_TRANS_RULE:
 			nf_tables_rule_destroy(&trans->ctx, trans->rule);
 			break;
@@ -3088,6 +3200,14 @@ static void nft_rule_abort_undo(struct net *net, struct sk_buff *skb,
 	list_del_rcu(&trans->rule->list);
 }
 
+static void nft_chain_abort(struct nft_trans *trans)
+{
+	if (trans->ctx.chain->flags & __NFT_CHAIN_DYING)
+		trans->ctx.chain->flags &= ~__NFT_CHAIN_DYING;
+	else if (!trans->update)
+		nf_tables_chain_destroy(&trans->ctx);
+}
+
 static void nft_set_abort(struct nft_trans *trans)
 {
 	/* This set was scheduled for removal, clear the dying flags to leave
@@ -3107,6 +3227,8 @@ static int nf_tables_abort(struct sk_buff *skb)
 
 	list_for_each_entry_safe(trans, next, &net->nft.commit_list, list) {
 		switch (trans->type) {
+		case NFT_TRANS_CHAIN:
+			break;
 		case NFT_TRANS_RULE:
 			nft_rule_abort_undo(net, skb, trans);
 			break;
@@ -3121,6 +3243,9 @@ static int nf_tables_abort(struct sk_buff *skb)
 	list_for_each_entry_safe(trans, next, &net->nft.commit_list, list) {
 		list_del(&trans->list);
 		switch (trans->type) {
+		case NFT_TRANS_CHAIN:
+			nft_chain_abort(trans);
+			break;
 		case NFT_TRANS_RULE:
 			/* Release new rules, already removed from the list,
 			 * that we couldn't activate.
-- 
1.7.10.4

[PATCH 7/7] netfilter: nf_tables: move table handling to the transaction infrastructure

[Pablo Neira Ayuso]

This patch speeds up rule-set updates and it helps to leave the
table configuration in consistent state when processing a batch.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/net/netfilter/nf_tables.h |    1 +
 net/netfilter/nf_tables_api.c     |  189 ++++++++++++++++++++++++++++++-------
 2 files changed, 156 insertions(+), 34 deletions(-)

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index c489ead..39d5cc2 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -343,6 +343,7 @@ struct nft_rule {
 };
 
 enum nft_trans_type {
+	NFT_TRANS_TABLE = 0,
 	NFT_TRANS_CHAIN,
 	NFT_TRANS_RULE,
 	NFT_TRANS_SET,
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 11b9d91..917ba15 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -151,6 +151,26 @@ static struct nft_table *nf_tables_table_lookup(const struct nft_af_info *afi,
 	return ERR_PTR(-ENOENT);
 }
 
+static struct nft_table *
+nf_tables_table_lookup_trans(struct net *net, const struct nft_af_info *afi,
+			     const struct nlattr *nla)
+{
+	struct nft_trans *trans;
+
+	if (nla == NULL)
+		return ERR_PTR(-EINVAL);
+
+	list_for_each_entry(trans, &net->nft.commit_list, list) {
+		if (trans->type != NFT_TRANS_TABLE)
+			continue;
+
+		if (!nla_strcmp(nla, trans->ctx.table->name))
+			return trans->ctx.table;
+	}
+
+	return ERR_PTR(-ENOENT);
+}
+
 static inline u64 nf_tables_alloc_handle(struct nft_table *table)
 {
 	return ++table->hgenerator;
@@ -389,41 +409,66 @@ static int nf_tables_table_disable(const struct nft_af_info *afi,
 	return 0;
 }
 
-static int nf_tables_updtable(struct sock *nlsk, struct sk_buff *skb,
-			      const struct nlmsghdr *nlh,
-			      const struct nlattr * const nla[],
-			      struct nft_af_info *afi, struct nft_table *table)
+static int nf_tables_updtable(struct nft_ctx *ctx)
 {
-	const struct nfgenmsg *nfmsg = nlmsg_data(nlh);
-	int family = nfmsg->nfgen_family, ret = 0;
+	struct nft_trans *trans;
+	int ret = 0;
 
-	if (nla[NFTA_TABLE_FLAGS]) {
+	if (ctx->nla[NFTA_TABLE_FLAGS]) {
 		u32 flags;
 
-		flags = ntohl(nla_get_be32(nla[NFTA_TABLE_FLAGS]));
+		flags = ntohl(nla_get_be32(ctx->nla[NFTA_TABLE_FLAGS]));
 		if (flags & ~NFT_TABLE_F_DORMANT)
 			return -EINVAL;
 
 		if ((flags & NFT_TABLE_F_DORMANT) &&
-		    !(table->flags & NFT_TABLE_F_DORMANT)) {
-			ret = nf_tables_table_disable(afi, table);
+		    !(ctx->table->flags & NFT_TABLE_F_DORMANT)) {
+			ret = nf_tables_table_disable(ctx->afi, ctx->table);
 			if (ret >= 0)
-				table->flags |= NFT_TABLE_F_DORMANT;
+				ctx->table->flags |= NFT_TABLE_F_DORMANT;
 		} else if (!(flags & NFT_TABLE_F_DORMANT) &&
-			   table->flags & NFT_TABLE_F_DORMANT) {
-			ret = nf_tables_table_enable(afi, table);
+			   ctx->table->flags & NFT_TABLE_F_DORMANT) {
+			ret = nf_tables_table_enable(ctx->afi, ctx->table);
 			if (ret >= 0)
-				table->flags &= ~NFT_TABLE_F_DORMANT;
+				ctx->table->flags &= ~NFT_TABLE_F_DORMANT;
 		}
 		if (ret < 0)
 			goto err;
 	}
 
-	nf_tables_table_notify(skb, nlh, table, NFT_MSG_NEWTABLE, family);
+	trans = nft_trans_alloc(ctx, NFT_TRANS_TABLE);
+	if (trans == NULL)
+		return -ENOMEM;
+
+	trans->update = true;
+	list_add_tail(&trans->list, &ctx->net->nft.commit_list);
 err:
 	return ret;
 }
 
+/* Internal table flags */
+#define __NFT_TABLE_DYING	(1 << 15)
+
+static int nft_table_trans_add(struct nft_ctx *ctx, int msg_type)
+{
+	struct nft_trans *trans;
+
+	trans = nft_trans_alloc(ctx, NFT_TRANS_TABLE);
+	if (trans == NULL)
+		return -ENOMEM;
+
+	if (msg_type == NFT_MSG_DELTABLE) {
+		/* You cannot delete the same table twice */
+		if (ctx->table->flags & __NFT_TABLE_DYING)
+			return -ENOENT;
+
+		ctx->table->flags |= __NFT_TABLE_DYING;
+	}
+
+	list_add_tail(&trans->list, &ctx->net->nft.commit_list);
+	return 0;
+}
+
 static int nf_tables_newtable(struct sock *nlsk, struct sk_buff *skb,
 			      const struct nlmsghdr *nlh,
 			      const struct nlattr * const nla[])
@@ -435,6 +480,8 @@ static int nf_tables_newtable(struct sock *nlsk, struct sk_buff *skb,
 	struct net *net = sock_net(skb->sk);
 	int family = nfmsg->nfgen_family;
 	u32 flags = 0;
+	struct nft_ctx ctx;
+	int err;
 
 	afi = nf_tables_afinfo_lookup(net, family, true);
 	if (IS_ERR(afi))
@@ -453,7 +500,9 @@ static int nf_tables_newtable(struct sock *nlsk, struct sk_buff *skb,
 			return -EEXIST;
 		if (nlh->nlmsg_flags & NLM_F_REPLACE)
 			return -EOPNOTSUPP;
-		return nf_tables_updtable(nlsk, skb, nlh, nla, afi, table);
+
+		nft_ctx_init(&ctx, skb, nlh, afi, table, NULL, nla);
+		return nf_tables_updtable(&ctx);
 	}
 
 	if (nla[NFTA_TABLE_FLAGS]) {
@@ -476,11 +525,23 @@ static int nf_tables_newtable(struct sock *nlsk, struct sk_buff *skb,
 	INIT_LIST_HEAD(&table->sets);
 	table->flags = flags;
 
-	list_add_tail(&table->list, &afi->tables);
-	nf_tables_table_notify(skb, nlh, table, NFT_MSG_NEWTABLE, family);
+	nft_ctx_init(&ctx, skb, nlh, afi, table, NULL, nla);
+	err = nft_table_trans_add(&ctx, NFT_MSG_NEWTABLE);
+	if (err < 0) {
+		kfree(table);
+		module_put(afi->owner);
+		return err;
+	}
 	return 0;
 }
 
+static void nft_table_add(struct nft_ctx *ctx)
+{
+	list_add_tail(&ctx->table->list, &ctx->afi->tables);
+	nf_tables_table_notify(ctx->skb, ctx->nlh, ctx->table,
+			       NFT_MSG_NEWTABLE, ctx->afi->family);
+}
+
 static int nf_tables_deltable(struct sock *nlsk, struct sk_buff *skb,
 			      const struct nlmsghdr *nlh,
 			      const struct nlattr * const nla[])
@@ -489,7 +550,8 @@ static int nf_tables_deltable(struct sock *nlsk, struct sk_buff *skb,
 	struct nft_af_info *afi;
 	struct nft_table *table;
 	struct net *net = sock_net(skb->sk);
-	int family = nfmsg->nfgen_family;
+	int family = nfmsg->nfgen_family, err;
+	struct nft_ctx ctx;
 
 	afi = nf_tables_afinfo_lookup(net, family, false);
 	if (IS_ERR(afi))
@@ -502,13 +564,27 @@ static int nf_tables_deltable(struct sock *nlsk, struct sk_buff *skb,
 	if (!list_empty(&table->chains) || !list_empty(&table->sets))
 		return -EBUSY;
 
-	list_del(&table->list);
-	nf_tables_table_notify(skb, nlh, table, NFT_MSG_DELTABLE, family);
-	kfree(table);
-	module_put(afi->owner);
+	nft_ctx_init(&ctx, skb, nlh, afi, table, NULL, nla);
+	err = nft_table_trans_add(&ctx, NFT_MSG_DELTABLE);
+	if (err < 0)
+		return err;
+
 	return 0;
 }
 
+static void nf_tables_table_destroy(struct nft_ctx *ctx)
+{
+	kfree(ctx->table);
+	module_put(ctx->afi->owner);
+}
+
+static void nft_table_del(struct nft_ctx *ctx)
+{
+	list_del(&ctx->table->list);
+	nf_tables_table_notify(ctx->skb, ctx->nlh, ctx->table,
+			       NFT_MSG_DELTABLE, ctx->afi->family);
+}
+
 int nft_register_chain_type(const struct nf_chain_type *ctype)
 {
 	int err = 0;
@@ -932,7 +1008,7 @@ static int nf_tables_newchain(struct sock *nlsk, struct sk_buff *skb,
 	if (IS_ERR(afi))
 		return PTR_ERR(afi);
 
-	table = nf_tables_table_lookup(afi, nla[NFTA_CHAIN_TABLE]);
+	table = nf_tables_table_lookup_trans(net, afi, nla[NFTA_CHAIN_TABLE]);
 	if (IS_ERR(table))
 		return PTR_ERR(table);
 
@@ -1699,7 +1775,7 @@ static int nf_tables_newrule(struct sock *nlsk, struct sk_buff *skb,
 	if (IS_ERR(afi))
 		return PTR_ERR(afi);
 
-	table = nf_tables_table_lookup(afi, nla[NFTA_RULE_TABLE]);
+	table = nf_tables_table_lookup_trans(net, afi, nla[NFTA_RULE_TABLE]);
 	if (IS_ERR(table))
 		return PTR_ERR(table);
 
@@ -2436,7 +2512,7 @@ static int nf_tables_newset(struct sock *nlsk, struct sk_buff *skb,
 	if (IS_ERR(afi))
 		return PTR_ERR(afi);
 
-	table = nf_tables_table_lookup(afi, nla[NFTA_SET_TABLE]);
+	table = nf_tables_table_lookup_trans(net, afi, nla[NFTA_SET_TABLE]);
 	if (IS_ERR(table))
 		return PTR_ERR(table);
 
@@ -2633,7 +2709,8 @@ static const struct nla_policy nft_set_elem_list_policy[NFTA_SET_ELEM_LIST_MAX +
 static int nft_ctx_init_from_elemattr(struct nft_ctx *ctx,
 				      const struct sk_buff *skb,
 				      const struct nlmsghdr *nlh,
-				      const struct nlattr * const nla[])
+				      const struct nlattr * const nla[],
+				      bool trans)
 {
 	const struct nfgenmsg *nfmsg = nlmsg_data(nlh);
 	struct nft_af_info *afi;
@@ -2644,7 +2721,13 @@ static int nft_ctx_init_from_elemattr(struct nft_ctx *ctx,
 	if (IS_ERR(afi))
 		return PTR_ERR(afi);
 
-	table = nf_tables_table_lookup(afi, nla[NFTA_SET_ELEM_LIST_TABLE]);
+	if (trans) {
+		table = nf_tables_table_lookup_trans(net, afi,
+					       nla[NFTA_SET_ELEM_LIST_TABLE]);
+	} else {
+		table = nf_tables_table_lookup(afi,
+					       nla[NFTA_SET_ELEM_LIST_TABLE]);
+	}
 	if (IS_ERR(table))
 		return PTR_ERR(table);
 
@@ -2720,7 +2803,8 @@ static int nf_tables_dump_set(struct sk_buff *skb, struct netlink_callback *cb)
 	if (err < 0)
 		return err;
 
-	err = nft_ctx_init_from_elemattr(&ctx, cb->skb, cb->nlh, (void *)nla);
+	err = nft_ctx_init_from_elemattr(&ctx, cb->skb, cb->nlh, (void *)nla,
+					 false);
 	if (err < 0)
 		return err;
 
@@ -2783,7 +2867,7 @@ static int nf_tables_getsetelem(struct sock *nlsk, struct sk_buff *skb,
 	struct nft_ctx ctx;
 	int err;
 
-	err = nft_ctx_init_from_elemattr(&ctx, skb, nlh, nla);
+	err = nft_ctx_init_from_elemattr(&ctx, skb, nlh, nla, false);
 	if (err < 0)
 		return err;
 
@@ -2897,7 +2981,7 @@ static int nf_tables_newsetelem(struct sock *nlsk, struct sk_buff *skb,
 	struct nft_ctx ctx;
 	int rem, err;
 
-	err = nft_ctx_init_from_elemattr(&ctx, skb, nlh, nla);
+	err = nft_ctx_init_from_elemattr(&ctx, skb, nlh, nla, true);
 	if (err < 0)
 		return err;
 
@@ -2971,7 +3055,7 @@ static int nf_tables_delsetelem(struct sock *nlsk, struct sk_buff *skb,
 	struct nft_ctx ctx;
 	int rem, err;
 
-	err = nft_ctx_init_from_elemattr(&ctx, skb, nlh, nla);
+	err = nft_ctx_init_from_elemattr(&ctx, skb, nlh, nla, false);
 	if (err < 0)
 		return err;
 
@@ -2991,7 +3075,7 @@ static int nf_tables_delsetelem(struct sock *nlsk, struct sk_buff *skb,
 
 static const struct nfnl_callback nf_tables_cb[NFT_MSG_MAX] = {
 	[NFT_MSG_NEWTABLE] = {
-		.call		= nf_tables_newtable,
+		.call_batch	= nf_tables_newtable,
 		.attr_count	= NFTA_TABLE_MAX,
 		.policy		= nft_table_policy,
 	},
@@ -3001,7 +3085,7 @@ static const struct nfnl_callback nf_tables_cb[NFT_MSG_MAX] = {
 		.policy		= nft_table_policy,
 	},
 	[NFT_MSG_DELTABLE] = {
-		.call		= nf_tables_deltable,
+		.call_batch	= nf_tables_deltable,
 		.attr_count	= NFTA_TABLE_MAX,
 		.policy		= nft_table_policy,
 	},
@@ -3067,6 +3151,24 @@ static const struct nfnl_callback nf_tables_cb[NFT_MSG_MAX] = {
 	},
 };
 
+static void nft_table_commit_update(struct nft_trans *trans)
+{
+	if (trans->ctx.table->flags & __NFT_TABLE_DYING) {
+		nft_table_del(&trans->ctx);
+	} else {
+		list_del(&trans->list);
+		if (trans->update) {
+			nf_tables_table_notify(trans->ctx.skb, trans->ctx.nlh,
+					       trans->ctx.table,
+					       NFT_MSG_NEWTABLE,
+					       trans->ctx.afi->family);
+		} else {
+			nft_table_add(&trans->ctx);
+		}
+		kfree(trans);
+	}
+}
+
 static void nft_chain_commit_update(struct nft_trans *trans)
 {
 	if (trans->ctx.chain->flags & __NFT_CHAIN_DYING) {
@@ -3144,6 +3246,9 @@ static int nf_tables_commit(struct sk_buff *skb)
 
 	list_for_each_entry_safe(trans, next, &net->nft.commit_list, list) {
 		switch (trans->type) {
+		case NFT_TRANS_TABLE:
+			nft_table_commit_update(trans);
+			break;
 		case NFT_TRANS_CHAIN:
 			nft_chain_commit_update(trans);
 			break;
@@ -3163,6 +3268,10 @@ static int nf_tables_commit(struct sk_buff *skb)
 	list_for_each_entry_safe(trans, next, &net->nft.commit_list, list) {
 		list_del(&trans->list);
 		switch (trans->type) {
+		case NFT_TRANS_TABLE:
+			trans->ctx.table->flags &= ~__NFT_TABLE_DYING;
+			nf_tables_table_destroy(&trans->ctx);
+			break;
 		case NFT_TRANS_CHAIN:
 			trans->ctx.chain->flags &= ~__NFT_CHAIN_DYING;
 			nf_tables_chain_destroy(&trans->ctx);
@@ -3200,6 +3309,14 @@ static void nft_rule_abort_undo(struct net *net, struct sk_buff *skb,
 	list_del_rcu(&trans->rule->list);
 }
 
+static void nft_table_abort(struct nft_trans *trans)
+{
+	if (trans->ctx.table->flags & __NFT_TABLE_DYING)
+		trans->ctx.table->flags &= ~__NFT_TABLE_DYING;
+	else if (!trans->update)
+		nf_tables_table_destroy(&trans->ctx);
+}
+
 static void nft_chain_abort(struct nft_trans *trans)
 {
 	if (trans->ctx.chain->flags & __NFT_CHAIN_DYING)
@@ -3227,6 +3344,7 @@ static int nf_tables_abort(struct sk_buff *skb)
 
 	list_for_each_entry_safe(trans, next, &net->nft.commit_list, list) {
 		switch (trans->type) {
+		case NFT_TRANS_TABLE:
 		case NFT_TRANS_CHAIN:
 			break;
 		case NFT_TRANS_RULE:
@@ -3243,6 +3361,9 @@ static int nf_tables_abort(struct sk_buff *skb)
 	list_for_each_entry_safe(trans, next, &net->nft.commit_list, list) {
 		list_del(&trans->list);
 		switch (trans->type) {
+		case NFT_TRANS_TABLE:
+			nft_table_abort(trans);
+			break;
 		case NFT_TRANS_CHAIN:
 			nft_chain_abort(trans);
 			break;
-- 
1.7.10.4

Re: [PATCH 1/7] netfilter: nf_tables: deconstify table and chain in context structure

[Patrick McHardy]

On Thu, Mar 27, 2014 at 10:53:11PM +0100, Pablo Neira Ayuso wrote:
> The new transaction infrastructure updates the family, table and chain
> objects in the context structure, so let's deconstify them. While at it,
> move the context structure initialization routine to the top of the
> source file as it will be also used from the table and chain routines.

I would prefer to keep the consts for the context, but I'm fine with it
if we don't find a clean other way. Basically I'd suggest to use your
patch if we have functions that are called from both the transaction
code and non-transaction code, IOW need the nft_ctx structure.
Otherwise, if this stuff is only used by the transaction code, we could
add the individual members to the transaction as non-consts.

Re: [PATCH 2/7] netfilter: nf_tables: generalise transaction infrastructure

[Patrick McHardy]

On Thu, Mar 27, 2014 at 10:53:12PM +0100, Pablo Neira Ayuso wrote:
> This patch generalises the existing rule transaction infrastructure
> so it can be used to handle set, table and chain object transactions
> as well.

This looks very good to me.

Re: [PATCH 4/7] netfilter: nf_tables: better encapsulation for the rule transaction code

[Patrick McHardy]

On Thu, Mar 27, 2014 at 10:53:14PM +0100, Pablo Neira Ayuso wrote:
> Another cleanup to accomodate set, chain and table transaction support.
> Move the rule transaction code to several functions to avoid too large
> commit and abort routines.
> 

> +static void nft_rule_commit_update(struct net *net, struct sk_buff *skb,
> +				   struct nft_trans *trans)

Just wondering, why aren't we using the net and skb from the context?

> +static void nft_rule_abort_undo(struct net *net, struct sk_buff *skb,
> +				struct nft_trans *trans)

In this case neither net nor skb are used.

Re: [PATCH 1/7] netfilter: nf_tables: deconstify table and chain in context structure

[Pablo Neira Ayuso]

On Fri, Mar 28, 2014 at 12:42:29PM +0000, Patrick McHardy wrote:
> On Thu, Mar 27, 2014 at 10:53:11PM +0100, Pablo Neira Ayuso wrote:
> > The new transaction infrastructure updates the family, table and chain
> > objects in the context structure, so let's deconstify them. While at it,
> > move the context structure initialization routine to the top of the
> > source file as it will be also used from the table and chain routines.
> 
> I would prefer to keep the consts for the context, but I'm fine with it
> if we don't find a clean other way. Basically I'd suggest to use your
> patch if we have functions that are called from both the transaction
> code and non-transaction code, IOW need the nft_ctx structure.
> Otherwise, if this stuff is only used by the transaction code, we could
> add the individual members to the transaction as non-consts.

This is being used from both transaction and non-transaction code. I
initially casted this to non-const as a workaround to calm down gcc,
but at some point I needed the list handling to add and to remove
elements from the lists which was quite ugly. I liked those compile
time checkings that we were getting before this.

Another possibility is to add some nft_ctx_trans which would look very
similar to nft_ctx, but in some cases we would need to define both
nft_ctx and nft_ctx_trans, which doesn't look very nice either. So I
don't see a better way to make this at this moment.

Re: [PATCH 5/7] netfilter: nf_tables: move set handling to the transaction infrastructure

[Patrick McHardy]

On Thu, Mar 27, 2014 at 10:53:15PM +0100, Pablo Neira Ayuso wrote:
> This patch reworks the nf_tables API so set updates are moved into
> the same batch that contains rule updates. This speeds up rule-set
> updates we skip a dialog of four messages between kernel and
> user-space (two on each direction).
> 
>  1) create the set and send netlink message to the kernel
>  2) process the response from the kernel that contains the allocated name.
>  3) add the set elements and send netlink message to the kernel.
>  4) process the response from the kernel (to check for errors).
> 
> To:
> 
>  1) add the set to the batch.
>  2) add the set elements to the batch.
>  3) add the rule that points to the set.
>  4) send batch to the kernel.
> 
> The idea is to allocate an internal set ID to the batch that can be
> used when adding set elements and rules that refer to the set in the
> batch.
> 
> Note that this patch doesn't add atomic set element updates, it just
> helps to leave the set configuration in consistent state in case
> that we fail to load the entire batch for some reason.

Looks fine to me. However I'm wondering whether this couldn't be
simplified. Basically all we need is a way to detect sets contained
in the batch for abort/commit, which could be achieved using a single
flag. The sets don't have any direct impact on runtime and they're
not visible to userspace as long as we hold the nfnl. So all we need
to do on abort is kill all the sets with this flag, on commit we
clear the flag and send notifications.

> Backward compatibility has been only retained in userspace, this
> means that new nft versions can talk to the kernel both in the new
> and the old fashion.

And old nftables can't talk to new kernels?

Re: [PATCH 6/7] netfilter: nf_tables: move chain handling to the transaction infrastructure

[Patrick McHardy]

On Thu, Mar 27, 2014 at 10:53:16PM +0100, Pablo Neira Ayuso wrote:
> This patch speeds up rule-set updates and it helps to leave chains
> in consistent state when processing a batch. Note this patch does
> not introduce a way to revert chain updates, eg. counter or default
> policy changes.

Also it doesn't seem to handle chain renames. I think this is a conceptual
shortcoming, there's no way to implement updates properly by just storing
the chain in the transaction, we need to store the actual actions to be
performed.

Re: [PATCH 7/7] netfilter: nf_tables: move table handling to the transaction infrastructure

[Patrick McHardy]

On Thu, Mar 27, 2014 at 10:53:17PM +0100, Pablo Neira Ayuso wrote:
> This patch speeds up rule-set updates and it helps to leave the
> table configuration in consistent state when processing a batch.

Same things as for chain updates apply to this patch. I think we should
limit this to sets until the conceptual shortcomings have been resolved.