All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4 -nftables v2] nf_tables atomic rule-set update
@ 2013-09-17 10:43 Pablo Neira Ayuso
  2013-09-17 10:43 ` [PATCH -nftables v2 1/4] netfilter: nf_tables: get rid of per rule list_head for commits Pablo Neira Ayuso
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2013-09-17 10:43 UTC (permalink / raw
  To: netfilter-devel

This patchset introduces improvements for the atomic rule update
infrastructure, main changes are:

* Get rid of the extra struct list_head per rule as discussed.
  With this patch, a temporary object is allocated to store the
  rule update information.

* The commit and abort loops have been also simplified. Basically,
  there is a single list per net namespace that contains pending
  rule updates.

* A new begin message to explicitly enter the transaction mode,
  The end message indicates that commit need to happen. If not
  specified, the pending updates are aborted.

* Remove the commit flag per rule, thus, all rule updates are
  transactional.

These changes requires userspace updates, they will be posted soon.

Pablo Neira Ayuso (4):
  netfilter: nf_tables: get rid of per rule list_head for commits
  netfilter: nf_tables: use per netns commit list
  netfilter: nfnetlink: add batch support and use it from nf_tables
  netfilter: nf_tables: all rule updates are transactional

 include/linux/netfilter/nfnetlink.h      |    2 +
 include/net/netfilter/nf_tables.h        |   23 +++-
 include/net/netns/nftables.h             |    1 +
 include/uapi/linux/netfilter/nf_tables.h |    7 -
 include/uapi/linux/netfilter/nfnetlink.h |    4 +
 net/netfilter/nf_tables_api.c            |  213 +++++++++++-------------------
 net/netfilter/nfnetlink.c                |  171 +++++++++++++++++++++++-
 7 files changed, 272 insertions(+), 149 deletions(-)

-- 
1.7.10.4


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH -nftables v2 1/4] netfilter: nf_tables: get rid of per rule list_head for commits
  2013-09-17 10:43 [PATCH 0/4 -nftables v2] nf_tables atomic rule-set update Pablo Neira Ayuso
@ 2013-09-17 10:43 ` Pablo Neira Ayuso
  2013-09-17 10:43 ` [PATCH -nftables v2 2/4] netfilter: nf_tables: use per netns commit list Pablo Neira Ayuso
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2013-09-17 10:43 UTC (permalink / raw
  To: netfilter-devel

Get rid of the extra struct list_head per rule. With this patch, a
temporary object is allocated to store the rule update information.

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

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 215edf5..fe08cf4 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -321,7 +321,6 @@ static inline void *nft_expr_priv(const struct nft_expr *expr)
  *	struct nft_rule - nf_tables rule
  *
  *	@list: used internally
- *	@dirty_list: this rule needs an update after new generation
  *	@rcu_head: used internally for rcu
  *	@handle: rule handle
  *	@genmask: generation mask
@@ -330,7 +329,6 @@ static inline void *nft_expr_priv(const struct nft_expr *expr)
  */
 struct nft_rule {
 	struct list_head		list;
-	struct list_head		dirty_list;
 	struct rcu_head			rcu_head;
 	u64				handle:46,
 					genmask:2,
@@ -339,6 +337,19 @@ struct nft_rule {
 		__attribute__((aligned(__alignof__(struct nft_expr))));
 };
 
+/**
+ *	struct nft_rule_trans - nf_tables rule update in transaction
+ *
+ *	@list: used internally
+ *	@rule: rule that needs to be updated
+ *	@family: family expressesed as AF_*
+ */
+struct nft_rule_trans {
+	struct list_head		list;
+	struct nft_rule			*rule;
+	u8				family;
+};
+
 static inline struct nft_expr *nft_expr_first(const struct nft_rule *rule)
 {
 	return (struct nft_expr *)&rule->data[0];
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index c5d0129..f099d0d 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1537,6 +1537,22 @@ static void nf_tables_rule_destroy(struct nft_rule *rule)
 
 static struct nft_expr_info *info;
 
+static int nf_tables_trans_add(struct nft_rule *rule, const struct nft_ctx *ctx)
+{
+	struct nft_rule_trans *rupd;
+	struct nft_chain *chain = (struct nft_chain *)ctx->chain;
+
+	rupd = kmalloc(sizeof(struct nft_rule_trans), GFP_KERNEL);
+	if (rupd == NULL)
+	       return -ENOMEM;
+
+	rupd->rule = rule;
+	rupd->family = ctx->afi->family;
+	list_add(&rupd->list, &chain->dirty_rules);
+
+	return 0;
+}
+
 static int nf_tables_newrule(struct sock *nlsk, struct sk_buff *skb,
 			     const struct nlmsghdr *nlh,
 			     const struct nlattr * const nla[])
@@ -1664,7 +1680,9 @@ static int nf_tables_newrule(struct sock *nlsk, struct sk_buff *skb,
 	}
 
 	if (flags & NFT_RULE_F_COMMIT)
-		list_add(&rule->dirty_list, &chain->dirty_rules);
+		err = nf_tables_trans_add(rule, &ctx);
+		if (err < 0)
+			goto err2;
 	else {
 		nf_tables_rule_notify(skb, nlh, table, chain, rule,
 				      NFT_MSG_NEWRULE,
@@ -1683,14 +1701,14 @@ err1:
 	return err;
 }
 
-static void
+static int
 nf_tables_delrule_one(struct nft_ctx *ctx, struct nft_rule *rule, u32 flags)
 {
-	if (flags & NFT_RULE_F_COMMIT) {
-		struct nft_chain *chain = (struct nft_chain *)ctx->chain;
+	int err = 0;
 
+	if (flags & NFT_RULE_F_COMMIT) {
 		nft_rule_disactivate_next(ctx->net, rule);
-		list_add(&rule->dirty_list, &chain->dirty_rules);
+		err = nf_tables_trans_add(rule, ctx);
 	} else {
 		list_del_rcu(&rule->list);
 		nf_tables_rule_notify(ctx->skb, ctx->nlh, ctx->table,
@@ -1698,6 +1716,7 @@ nf_tables_delrule_one(struct nft_ctx *ctx, struct nft_rule *rule, u32 flags)
 				      0, ctx->afi->family);
 		nf_tables_rule_destroy(rule);
 	}
+	return err;
 }
 
 static int nf_tables_delrule(struct sock *nlsk, struct sk_buff *skb,
@@ -1710,7 +1729,7 @@ static int nf_tables_delrule(struct sock *nlsk, struct sk_buff *skb,
 	const struct nft_table *table;
 	struct nft_chain *chain;
 	struct nft_rule *rule, *tmp;
-	int family = nfmsg->nfgen_family;
+	int family = nfmsg->nfgen_family, err = 0;
 	struct nft_ctx ctx;
 	u32 flags = 0;
 
@@ -1740,14 +1759,17 @@ static int nf_tables_delrule(struct sock *nlsk, struct sk_buff *skb,
 		if (IS_ERR(rule))
 			return PTR_ERR(rule);
 
-		nf_tables_delrule_one(&ctx, rule, flags);
+		err = nf_tables_delrule_one(&ctx, rule, flags);
 	} else {
 		/* Remove all rules in this chain */
-		list_for_each_entry_safe(rule, tmp, &chain->rules, list)
-			nf_tables_delrule_one(&ctx, rule, flags);
+		list_for_each_entry_safe(rule, tmp, &chain->rules, list) {
+			err = nf_tables_delrule_one(&ctx, rule, flags);
+			if (err < 0)
+				break;
+		}
 	}
 
-	return 0;
+	return err;
 }
 
 static int nf_tables_commit(struct sock *nlsk, struct sk_buff *skb,
@@ -1759,8 +1781,7 @@ static int nf_tables_commit(struct sock *nlsk, struct sk_buff *skb,
 	struct net *net = sock_net(skb->sk);
 	struct nft_table *table;
 	struct nft_chain *chain;
-	struct nft_rule *rule, *tmp;
-	int family = nfmsg->nfgen_family;
+	struct nft_rule_trans *rupd, *tmp;
 	bool create;
 
 	create = nlh->nlmsg_flags & NLM_F_CREATE ? true : false;
@@ -1782,31 +1803,33 @@ static int nf_tables_commit(struct sock *nlsk, struct sk_buff *skb,
 
 	list_for_each_entry(table, &afi->tables, list) {
 		list_for_each_entry(chain, &table->chains, list) {
-			list_for_each_entry_safe(rule, tmp, &chain->dirty_rules, dirty_list) {
+			list_for_each_entry_safe(rupd, tmp, &chain->dirty_rules, list) {
 				/* Delete this rule from the dirty list */
-				list_del(&rule->dirty_list);
+				list_del(&rupd->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, rule)) {
-					nft_rule_clear(net, rule);
+				if (nft_rule_is_active(net, rupd->rule)) {
+					nft_rule_clear(net, rupd->rule);
 					nf_tables_rule_notify(skb, nlh, table,
-							      chain, rule,
+							      chain, rupd->rule,
 							      NFT_MSG_NEWRULE,
 							      0,
-							      nfmsg->nfgen_family);
+							      rupd->family);
+					kfree(rupd);
 					continue;
 				}
 
 				/* This rule is in the past, get rid of it */
-				list_del_rcu(&rule->list);
+				list_del_rcu(&rupd->rule->list);
 				nf_tables_rule_notify(skb, nlh, table, chain,
-						      rule, NFT_MSG_DELRULE, 0,
-						      family);
-				nf_tables_rule_destroy(rule);
+						      rupd->rule, NFT_MSG_DELRULE, 0,
+						      rupd->family);
+				nf_tables_rule_destroy(rupd->rule);
+				kfree(rupd);
 			}
 		}
 	}
@@ -1823,7 +1846,7 @@ static int nf_tables_abort(struct sock *nlsk, struct sk_buff *skb,
 	struct net *net = sock_net(skb->sk);
 	struct nft_table *table;
 	struct nft_chain *chain;
-	struct nft_rule *rule, *tmp;
+	struct nft_rule_trans *rupd, *tmp;
 	bool create;
 
 	create = nlh->nlmsg_flags & NLM_F_CREATE ? true : false;
@@ -1834,18 +1857,20 @@ static int nf_tables_abort(struct sock *nlsk, struct sk_buff *skb,
 
 	list_for_each_entry(table, &afi->tables, list) {
 		list_for_each_entry(chain, &table->chains, list) {
-			list_for_each_entry_safe(rule, tmp, &chain->dirty_rules, dirty_list) {
+			list_for_each_entry_safe(rupd, tmp, &chain->dirty_rules, list) {
 				/* Delete all rules from the dirty list */
-				list_del(&rule->dirty_list);
+				list_del(&rupd->list);
 
-				if (!nft_rule_is_active_next(net, rule)) {
-					nft_rule_clear(net, rule);
+				if (!nft_rule_is_active_next(net, rupd->rule)) {
+					nft_rule_clear(net, rupd->rule);
+					kfree(rupd);
 					continue;
 				}
 
 				/* This rule is inactive, get rid of it */
-				list_del_rcu(&rule->list);
-				nf_tables_rule_destroy(rule);
+				list_del_rcu(&rupd->rule->list);
+				nf_tables_rule_destroy(rupd->rule);
+				kfree(rupd);
 			}
 		}
 	}
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH -nftables v2 2/4] netfilter: nf_tables: use per netns commit list
  2013-09-17 10:43 [PATCH 0/4 -nftables v2] nf_tables atomic rule-set update Pablo Neira Ayuso
  2013-09-17 10:43 ` [PATCH -nftables v2 1/4] netfilter: nf_tables: get rid of per rule list_head for commits Pablo Neira Ayuso
@ 2013-09-17 10:43 ` Pablo Neira Ayuso
  2013-09-17 10:43 ` [PATCH -nftables v2 3/4] netfilter: nfnetlink: add batch support and use it from nf_tables Pablo Neira Ayuso
  2013-09-17 10:43 ` [PATCH -nftables v2 4/4] netfilter: nf_tables: all rule updates are transactional Pablo Neira Ayuso
  3 siblings, 0 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2013-09-17 10:43 UTC (permalink / raw
  To: netfilter-devel

Instead of one list per chain.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/net/netfilter/nf_tables.h |    6 +-
 include/net/netns/nftables.h      |    1 +
 net/netfilter/nf_tables_api.c     |  109 +++++++++++++------------------------
 3 files changed, 44 insertions(+), 72 deletions(-)

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index fe08cf4..6b644c2 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -342,11 +342,15 @@ struct nft_rule {
  *
  *	@list: used internally
  *	@rule: rule that needs to be updated
+ *	@chain: chain that this rule belongs to
+ *	@table: table for which this chain applies
  *	@family: family expressesed as AF_*
  */
 struct nft_rule_trans {
 	struct list_head		list;
 	struct nft_rule			*rule;
+	const struct nft_chain		*chain;
+	const struct nft_table		*table;
 	u8				family;
 };
 
@@ -383,7 +387,6 @@ enum nft_chain_flags {
  *	struct nft_chain - nf_tables chain
  *
  *	@rules: list of rules in the chain
- *	@dirty_rules: rules that need an update after next generation
  *	@list: used internally
  *	@rcu_head: used internally
  *	@net: net namespace that this chain belongs to
@@ -396,7 +399,6 @@ enum nft_chain_flags {
  */
 struct nft_chain {
 	struct list_head		rules;
-	struct list_head		dirty_rules;
 	struct list_head		list;
 	struct rcu_head			rcu_head;
 	struct net			*net;
diff --git a/include/net/netns/nftables.h b/include/net/netns/nftables.h
index 9b35901..15d056d 100644
--- a/include/net/netns/nftables.h
+++ b/include/net/netns/nftables.h
@@ -7,6 +7,7 @@ struct nft_af_info;
 
 struct netns_nftables {
 	struct list_head	af_info;
+	struct list_head	commit_list;
 	struct nft_af_info	*ipv4;
 	struct nft_af_info	*ipv6;
 	struct nft_af_info	*arp;
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index f099d0d..8c78d36 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -976,7 +976,6 @@ static int nf_tables_newchain(struct sock *nlsk, struct sk_buff *skb,
 	}
 
 	INIT_LIST_HEAD(&chain->rules);
-	INIT_LIST_HEAD(&chain->dirty_rules);
 	chain->handle = nf_tables_alloc_handle(table);
 	chain->net = net;
 	chain->table = table;
@@ -1540,15 +1539,16 @@ static struct nft_expr_info *info;
 static int nf_tables_trans_add(struct nft_rule *rule, const struct nft_ctx *ctx)
 {
 	struct nft_rule_trans *rupd;
-	struct nft_chain *chain = (struct nft_chain *)ctx->chain;
 
 	rupd = kmalloc(sizeof(struct nft_rule_trans), GFP_KERNEL);
 	if (rupd == NULL)
 	       return -ENOMEM;
 
+	rupd->chain = ctx->chain;
+	rupd->table = ctx->table;
 	rupd->rule = rule;
 	rupd->family = ctx->afi->family;
-	list_add(&rupd->list, &chain->dirty_rules);
+	list_add(&rupd->list, &ctx->net->nft.commit_list);
 
 	return 0;
 }
@@ -1776,19 +1776,8 @@ static int nf_tables_commit(struct sock *nlsk, struct sk_buff *skb,
 			    const struct nlmsghdr *nlh,
 			    const struct nlattr * const nla[])
 {
-	const struct nfgenmsg *nfmsg = nlmsg_data(nlh);
-	const struct nft_af_info *afi;
 	struct net *net = sock_net(skb->sk);
-	struct nft_table *table;
-	struct nft_chain *chain;
 	struct nft_rule_trans *rupd, *tmp;
-	bool create;
-
-	create = nlh->nlmsg_flags & NLM_F_CREATE ? true : false;
-
-	afi = nf_tables_afinfo_lookup(net, nfmsg->nfgen_family, create);
-	if (IS_ERR(afi))
-		return PTR_ERR(afi);
 
 	/* Bump generation counter, invalidate any dump in progress */
 	net->nft.genctr++;
@@ -1801,37 +1790,31 @@ static int nf_tables_commit(struct sock *nlsk, struct sk_buff *skb,
 	 */
 	synchronize_rcu();
 
-	list_for_each_entry(table, &afi->tables, list) {
-		list_for_each_entry(chain, &table->chains, list) {
-			list_for_each_entry_safe(rupd, tmp, &chain->dirty_rules, list) {
-				/* Delete this rule from the dirty list */
-				list_del(&rupd->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, nlh, table,
-							      chain, rupd->rule,
-							      NFT_MSG_NEWRULE,
-							      0,
-							      rupd->family);
-					kfree(rupd);
-					continue;
-				}
+	list_for_each_entry_safe(rupd, tmp, &net->nft.commit_list, list) {
+		/* Delete this rule from the dirty list */
+		list_del(&rupd->list);
 
-				/* This rule is in the past, get rid of it */
-				list_del_rcu(&rupd->rule->list);
-				nf_tables_rule_notify(skb, nlh, table, chain,
-						      rupd->rule, NFT_MSG_DELRULE, 0,
-						      rupd->family);
-				nf_tables_rule_destroy(rupd->rule);
-				kfree(rupd);
-			}
+		/* 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, nlh, rupd->table,
+					      rupd->chain, rupd->rule,
+					      NFT_MSG_NEWRULE, 0,
+					      rupd->family);
+			kfree(rupd);
+			continue;
 		}
+
+		/* This rule is in the past, get rid of it */
+		list_del_rcu(&rupd->rule->list);
+		nf_tables_rule_notify(skb, nlh, rupd->table, rupd->chain,
+				      rupd->rule, NFT_MSG_DELRULE, 0,
+				      rupd->family);
+		nf_tables_rule_destroy(rupd->rule);
+		kfree(rupd);
 	}
 
 	return 0;
@@ -1841,38 +1824,23 @@ static int nf_tables_abort(struct sock *nlsk, struct sk_buff *skb,
 			   const struct nlmsghdr *nlh,
 			   const struct nlattr * const nla[])
 {
-	const struct nfgenmsg *nfmsg = nlmsg_data(nlh);
-	const struct nft_af_info *afi;
 	struct net *net = sock_net(skb->sk);
-	struct nft_table *table;
-	struct nft_chain *chain;
 	struct nft_rule_trans *rupd, *tmp;
-	bool create;
-
-	create = nlh->nlmsg_flags & NLM_F_CREATE ? true : false;
-
-	afi = nf_tables_afinfo_lookup(net, nfmsg->nfgen_family, create);
-	if (IS_ERR(afi))
-		return PTR_ERR(afi);
 
-	list_for_each_entry(table, &afi->tables, list) {
-		list_for_each_entry(chain, &table->chains, list) {
-			list_for_each_entry_safe(rupd, tmp, &chain->dirty_rules, list) {
-				/* Delete all rules from the dirty list */
-				list_del(&rupd->list);
-
-				if (!nft_rule_is_active_next(net, rupd->rule)) {
-					nft_rule_clear(net, rupd->rule);
-					kfree(rupd);
-					continue;
-				}
+	list_for_each_entry_safe(rupd, tmp, &net->nft.commit_list, list) {
+		/* Delete all rules from the dirty list */
+		list_del(&rupd->list);
 
-				/* This rule is inactive, get rid of it */
-				list_del_rcu(&rupd->rule->list);
-				nf_tables_rule_destroy(rupd->rule);
-				kfree(rupd);
-			}
+		if (!nft_rule_is_active_next(net, rupd->rule)) {
+			nft_rule_clear(net, rupd->rule);
+			kfree(rupd);
+			continue;
 		}
+
+		/* This rule is inactive, get rid of it */
+		list_del_rcu(&rupd->rule->list);
+		nf_tables_rule_destroy(rupd->rule);
+		kfree(rupd);
 	}
 	return 0;
 }
@@ -3224,6 +3192,7 @@ EXPORT_SYMBOL_GPL(nft_data_dump);
 static int nf_tables_init_net(struct net *net)
 {
 	INIT_LIST_HEAD(&net->nft.af_info);
+	INIT_LIST_HEAD(&net->nft.commit_list);
 	return 0;
 }
 
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH -nftables v2 3/4] netfilter: nfnetlink: add batch support and use it from nf_tables
  2013-09-17 10:43 [PATCH 0/4 -nftables v2] nf_tables atomic rule-set update Pablo Neira Ayuso
  2013-09-17 10:43 ` [PATCH -nftables v2 1/4] netfilter: nf_tables: get rid of per rule list_head for commits Pablo Neira Ayuso
  2013-09-17 10:43 ` [PATCH -nftables v2 2/4] netfilter: nf_tables: use per netns commit list Pablo Neira Ayuso
@ 2013-09-17 10:43 ` Pablo Neira Ayuso
  2013-09-17 10:43 ` [PATCH -nftables v2 4/4] netfilter: nf_tables: all rule updates are transactional Pablo Neira Ayuso
  3 siblings, 0 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2013-09-17 10:43 UTC (permalink / raw
  To: netfilter-devel

This patch adds batch support for nfnetlink, this allows you
to atomically handle a batch of netlink messages. This is used
by nf_tables to improve the existing atomic rule-set update
approach.

Two new nfnetlink control messages are added for this purpose,
they are:

* NFNL_MSG_BATCH_BEGIN, that indicates the beginning of a batch,
  the nfgenmsg->res_id indicates the nfnetlink subsystem ID.

* NFNL_MSG_BATCH_END, that results in the invocation of the
  ss->commit callback function. If not specified or an error
  ocurred in the batch, the ss->abort function is invoked
  instead.

This allows us to get rid of the NFT_MSG_COMMIT and NFT_MSG_ABORT
nf_tables message types.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/linux/netfilter/nfnetlink.h      |    2 +
 include/net/netfilter/nf_tables.h        |    2 +
 include/uapi/linux/netfilter/nf_tables.h |    2 -
 include/uapi/linux/netfilter/nfnetlink.h |    4 +
 net/netfilter/nf_tables_api.c            |   25 ++---
 net/netfilter/nfnetlink.c                |  171 +++++++++++++++++++++++++++++-
 6 files changed, 182 insertions(+), 24 deletions(-)

diff --git a/include/linux/netfilter/nfnetlink.h b/include/linux/netfilter/nfnetlink.h
index cadb740..0aab055 100644
--- a/include/linux/netfilter/nfnetlink.h
+++ b/include/linux/netfilter/nfnetlink.h
@@ -23,6 +23,8 @@ struct nfnetlink_subsystem {
 	__u8 subsys_id;			/* nfnetlink subsystem ID */
 	__u8 cb_count;			/* number of callbacks */
 	const struct nfnl_callback *cb;	/* callback for individual types */
+	int (*commit)(struct sk_buff *skb);
+	int (*abort)(struct sk_buff *skb);
 };
 
 extern int nfnetlink_subsys_register(const struct nfnetlink_subsystem *n);
diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 6b644c2..54c4a5c 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -344,6 +344,7 @@ struct nft_rule {
  *	@rule: rule that needs to be updated
  *	@chain: chain that this rule belongs to
  *	@table: table for which this chain applies
+ *	@nlh: netlink header of the message that contain this update
  *	@family: family expressesed as AF_*
  */
 struct nft_rule_trans {
@@ -351,6 +352,7 @@ struct nft_rule_trans {
 	struct nft_rule			*rule;
 	const struct nft_chain		*chain;
 	const struct nft_table		*table;
+	const struct nlmsghdr		*nlh;
 	u8				family;
 };
 
diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index d9bf8ea..cbb5c75 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -48,8 +48,6 @@ enum nf_tables_msg_types {
 	NFT_MSG_NEWSETELEM,
 	NFT_MSG_GETSETELEM,
 	NFT_MSG_DELSETELEM,
-	NFT_MSG_COMMIT,
-	NFT_MSG_ABORT,
 	NFT_MSG_MAX,
 };
 
diff --git a/include/uapi/linux/netfilter/nfnetlink.h b/include/uapi/linux/netfilter/nfnetlink.h
index 2889594..596ddd4 100644
--- a/include/uapi/linux/netfilter/nfnetlink.h
+++ b/include/uapi/linux/netfilter/nfnetlink.h
@@ -57,4 +57,8 @@ struct nfgenmsg {
 #define NFNL_SUBSYS_NFT_COMPAT		11
 #define NFNL_SUBSYS_COUNT		12
 
+/* Reserved control nfnetlink messages */
+#define NFNL_MSG_BATCH_BEGIN		NLMSG_MIN_TYPE
+#define NFNL_MSG_BATCH_END		NLMSG_MIN_TYPE+1
+
 #endif /* _UAPI_NFNETLINK_H */
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 8c78d36..ec2a566 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1548,6 +1548,7 @@ static int nf_tables_trans_add(struct nft_rule *rule, const struct nft_ctx *ctx)
 	rupd->table = ctx->table;
 	rupd->rule = rule;
 	rupd->family = ctx->afi->family;
+	rupd->nlh = ctx->nlh;
 	list_add(&rupd->list, &ctx->net->nft.commit_list);
 
 	return 0;
@@ -1772,9 +1773,7 @@ static int nf_tables_delrule(struct sock *nlsk, struct sk_buff *skb,
 	return err;
 }
 
-static int nf_tables_commit(struct sock *nlsk, struct sk_buff *skb,
-			    const struct nlmsghdr *nlh,
-			    const struct nlattr * const nla[])
+static int nf_tables_commit(struct sk_buff *skb)
 {
 	struct net *net = sock_net(skb->sk);
 	struct nft_rule_trans *rupd, *tmp;
@@ -1800,7 +1799,7 @@ static int nf_tables_commit(struct sock *nlsk, struct sk_buff *skb,
 		 */
 		if (nft_rule_is_active(net, rupd->rule)) {
 			nft_rule_clear(net, rupd->rule);
-			nf_tables_rule_notify(skb, nlh, rupd->table,
+			nf_tables_rule_notify(skb, rupd->nlh, rupd->table,
 					      rupd->chain, rupd->rule,
 					      NFT_MSG_NEWRULE, 0,
 					      rupd->family);
@@ -1810,7 +1809,7 @@ static int nf_tables_commit(struct sock *nlsk, struct sk_buff *skb,
 
 		/* This rule is in the past, get rid of it */
 		list_del_rcu(&rupd->rule->list);
-		nf_tables_rule_notify(skb, nlh, rupd->table, rupd->chain,
+		nf_tables_rule_notify(skb, rupd->nlh, rupd->table, rupd->chain,
 				      rupd->rule, NFT_MSG_DELRULE, 0,
 				      rupd->family);
 		nf_tables_rule_destroy(rupd->rule);
@@ -1820,9 +1819,7 @@ static int nf_tables_commit(struct sock *nlsk, struct sk_buff *skb,
 	return 0;
 }
 
-static int nf_tables_abort(struct sock *nlsk, struct sk_buff *skb,
-			   const struct nlmsghdr *nlh,
-			   const struct nlattr * const nla[])
+static int nf_tables_abort(struct sk_buff *skb)
 {
 	struct net *net = sock_net(skb->sk);
 	struct nft_rule_trans *rupd, *tmp;
@@ -2804,16 +2801,6 @@ static const struct nfnl_callback nf_tables_cb[NFT_MSG_MAX] = {
 		.attr_count	= NFTA_SET_ELEM_LIST_MAX,
 		.policy		= nft_set_elem_list_policy,
 	},
-	[NFT_MSG_COMMIT] = {
-		.call		= nf_tables_commit,
-		.attr_count	= NFTA_TABLE_MAX,
-		.policy		= nft_rule_policy,
-	},
-	[NFT_MSG_ABORT] = {
-		.call		= nf_tables_abort,
-		.attr_count	= NFTA_TABLE_MAX,
-		.policy		= nft_rule_policy,
-	},
 };
 
 static const struct nfnetlink_subsystem nf_tables_subsys = {
@@ -2821,6 +2808,8 @@ static const struct nfnetlink_subsystem nf_tables_subsys = {
 	.subsys_id	= NFNL_SUBSYS_NFTABLES,
 	.cb_count	= NFT_MSG_MAX,
 	.cb		= nf_tables_cb,
+	.commit		= nf_tables_commit,
+	.abort		= nf_tables_abort,
 };
 
 /*
diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c
index 572d87d..8dde792 100644
--- a/net/netfilter/nfnetlink.c
+++ b/net/netfilter/nfnetlink.c
@@ -147,9 +147,6 @@ static int nfnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 	const struct nfnetlink_subsystem *ss;
 	int type, err;
 
-	if (!ns_capable(net->user_ns, CAP_NET_ADMIN))
-		return -EPERM;
-
 	/* All the messages must at least contain nfgenmsg */
 	if (nlmsg_len(nlh) < sizeof(struct nfgenmsg))
 		return 0;
@@ -217,9 +214,175 @@ replay:
 	}
 }
 
+static void nfnetlink_rcv_batch(struct sk_buff *skb, struct nlmsghdr *nlh,
+				u_int16_t subsys_id)
+{
+	struct sk_buff *nskb, *oskb = skb;
+	struct net *net = sock_net(skb->sk);
+	const struct nfnetlink_subsystem *ss;
+	const struct nfnl_callback *nc;
+	bool success = true, done = false;
+	int err;
+
+	if (subsys_id >= NFNL_SUBSYS_COUNT)
+		return netlink_ack(skb, nlh, -EINVAL);
+replay:
+	nskb = netlink_skb_clone(oskb, GFP_KERNEL);
+	if (!nskb)
+		return netlink_ack(oskb, nlh, -ENOMEM);
+
+	nskb->sk = oskb->sk;
+	skb = nskb;
+
+	nfnl_lock(subsys_id);
+	ss = rcu_dereference_protected(table[subsys_id].subsys,
+				       lockdep_is_held(&table[subsys_id].mutex));
+	if (!ss) {
+#ifdef CONFIG_MODULES
+		nfnl_unlock(subsys_id);
+		request_module("nfnetlink-subsys-%d", subsys_id);
+		nfnl_lock(subsys_id);
+		ss = rcu_dereference_protected(table[subsys_id].subsys,
+					       lockdep_is_held(&table[subsys_id].mutex));
+		if (!ss)
+#endif
+		{
+			nfnl_unlock(subsys_id);
+			kfree_skb(nskb);
+			return netlink_ack(skb, nlh, -EOPNOTSUPP);
+		}
+	}
+
+	if (!ss->commit || !ss->abort) {
+		nfnl_unlock(subsys_id);
+		kfree_skb(nskb);
+		return netlink_ack(skb, nlh, -EOPNOTSUPP);
+	}
+
+	while (skb->len >= nlmsg_total_size(0)) {
+		int msglen, type;
+
+		nlh = nlmsg_hdr(skb);
+		err = 0;
+
+		if (nlh->nlmsg_len < NLMSG_HDRLEN) {
+			err = -EINVAL;
+			goto ack;
+		}
+
+		/* Only requests are handled by the kernel */
+		if (!(nlh->nlmsg_flags & NLM_F_REQUEST)) {
+			err = -EINVAL;
+			goto ack;
+		}
+
+		type = nlh->nlmsg_type;
+		if (type == NFNL_MSG_BATCH_END) {
+			done = true;
+			goto done;
+		} else if (type < NLMSG_MIN_TYPE) {
+			err = -EINVAL;
+			goto ack;
+		}
+
+		/* We only accept a batch with messages for the same
+		 * subsystem.
+		 */
+		if (NFNL_SUBSYS_ID(type) != subsys_id) {
+			err = -EINVAL;
+			goto ack;
+		}
+
+		nc = nfnetlink_find_client(type, ss);
+		if (!nc) {
+			err = -EINVAL;
+			goto ack;
+		}
+
+		{
+			int min_len = nlmsg_total_size(sizeof(struct nfgenmsg));
+			u_int8_t cb_id = NFNL_MSG_TYPE(nlh->nlmsg_type);
+			struct nlattr *cda[ss->cb[cb_id].attr_count + 1];
+			struct nlattr *attr = (void *)nlh + min_len;
+			int attrlen = nlh->nlmsg_len - min_len;
+
+			err = nla_parse(cda, ss->cb[cb_id].attr_count,
+					attr, attrlen, ss->cb[cb_id].policy);
+			if (err < 0)
+				goto ack;
+
+			if (nc->call) {
+				err = nc->call(net->nfnl, skb, nlh,
+					       (const struct nlattr **)cda);
+			}
+
+			/* The lock was released to autoload some module, we
+			 * have to abort and start from scratch using the
+			 * original skb.
+			 */
+			if (err == -EAGAIN) {
+				ss->abort(skb);
+				nfnl_unlock(subsys_id);
+				kfree_skb(nskb);
+				goto replay;
+			}
+		}
+ack:
+		if (nlh->nlmsg_flags & NLM_F_ACK || err) {
+			/* We don't stop processing the batch on errors, thus,
+			 * userspace gets all the errors that the batch
+			 * triggers.
+			 */
+			netlink_ack(skb, nlh, err);
+			if (err)
+				success = false;
+		}
+
+		msglen = NLMSG_ALIGN(nlh->nlmsg_len);
+		if (msglen > skb->len)
+			msglen = skb->len;
+		skb_pull(skb, msglen);
+	}
+done:
+	if (success && done)
+		ss->commit(skb);
+	else
+		ss->abort(skb);
+
+	nfnl_unlock(subsys_id);
+	kfree_skb(nskb);
+}
+
 static void nfnetlink_rcv(struct sk_buff *skb)
 {
-	netlink_rcv_skb(skb, &nfnetlink_rcv_msg);
+	struct nlmsghdr *nlh = nlmsg_hdr(skb);
+	struct net *net = sock_net(skb->sk);
+	int msglen;
+
+	if (!ns_capable(net->user_ns, CAP_NET_ADMIN))
+		return netlink_ack(skb, nlh, -EPERM);
+
+	if (nlh->nlmsg_len < NLMSG_HDRLEN ||
+	    skb->len < nlh->nlmsg_len)
+		return;
+
+	if (nlh->nlmsg_type == NFNL_MSG_BATCH_BEGIN) {
+		struct nfgenmsg *nfgenmsg;
+
+		msglen = NLMSG_ALIGN(nlh->nlmsg_len);
+		if (msglen > skb->len)
+			msglen = skb->len;
+
+		if (nlh->nlmsg_len < NLMSG_HDRLEN ||
+		    skb->len < NLMSG_HDRLEN + sizeof(struct nfgenmsg))
+			return;
+
+		nfgenmsg = nlmsg_data(nlh);
+		skb_pull(skb, msglen);
+		nfnetlink_rcv_batch(skb, nlh, nfgenmsg->res_id);
+	} else {
+		netlink_rcv_skb(skb, &nfnetlink_rcv_msg);
+	}
 }
 
 #ifdef CONFIG_MODULES
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH -nftables v2 4/4] netfilter: nf_tables: all rule updates are transactional
  2013-09-17 10:43 [PATCH 0/4 -nftables v2] nf_tables atomic rule-set update Pablo Neira Ayuso
                   ` (2 preceding siblings ...)
  2013-09-17 10:43 ` [PATCH -nftables v2 3/4] netfilter: nfnetlink: add batch support and use it from nf_tables Pablo Neira Ayuso
@ 2013-09-17 10:43 ` Pablo Neira Ayuso
  3 siblings, 0 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2013-09-17 10:43 UTC (permalink / raw
  To: netfilter-devel

This patch makes all rule updates transactional, this simplifies
the ruleset update logic. Suggested by Patrick McHardy.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/uapi/linux/netfilter/nf_tables.h |    5 ---
 net/netfilter/nf_tables_api.c            |   62 ++++++------------------------
 2 files changed, 12 insertions(+), 55 deletions(-)

diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index cbb5c75..b8cd62f 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -120,11 +120,6 @@ enum nft_chain_attributes {
 };
 #define NFTA_CHAIN_MAX		(__NFTA_CHAIN_MAX - 1)
 
-enum {
-	NFT_RULE_F_COMMIT       = (1 << 0),
-	NFT_RULE_F_MASK         = NFT_RULE_F_COMMIT,
-};
-
 /**
  * enum nft_rule_attributes - nf_tables rule netlink attributes
  *
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index ec2a566..c02a698 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1570,7 +1570,6 @@ static int nf_tables_newrule(struct sock *nlsk, struct sk_buff *skb,
 	unsigned int size, i, n;
 	int err, rem;
 	bool create;
-	u32 flags = 0;
 	u64 handle, pos_handle;
 
 	create = nlh->nlmsg_flags & NLM_F_CREATE ? true : false;
@@ -1639,14 +1638,7 @@ static int nf_tables_newrule(struct sock *nlsk, struct sk_buff *skb,
 	if (rule == NULL)
 		goto err1;
 
-	if (nla[NFTA_RULE_FLAGS]) {
-		flags = ntohl(nla_get_be32(nla[NFTA_RULE_FLAGS]));
-		if (flags & ~NFT_RULE_F_MASK)
-			return -EINVAL;
-
-		if (flags & NFT_RULE_F_COMMIT)
-			nft_rule_activate_next(net, rule);
-	}
+	nft_rule_activate_next(net, rule);
 
 	rule->handle = handle;
 	rule->dlen   = size;
@@ -1661,13 +1653,8 @@ static int nf_tables_newrule(struct sock *nlsk, struct sk_buff *skb,
 	}
 
 	if (nlh->nlmsg_flags & NLM_F_REPLACE) {
-		if (flags & NFT_RULE_F_COMMIT) {
-			nft_rule_disactivate_next(net, old_rule);
-			list_add_tail_rcu(&rule->list, &chain->rules);
-		} else {
-			list_replace_rcu(&old_rule->list, &rule->list);
-			nf_tables_rule_destroy(old_rule);
-		}
+		nft_rule_disactivate_next(net, old_rule);
+		list_add_tail_rcu(&rule->list, &chain->rules);
 	} else if (nlh->nlmsg_flags & NLM_F_APPEND)
 		if (old_rule)
 			list_add_rcu(&rule->list, &old_rule->list);
@@ -1680,16 +1667,10 @@ static int nf_tables_newrule(struct sock *nlsk, struct sk_buff *skb,
 			list_add_rcu(&rule->list, &chain->rules);
 	}
 
-	if (flags & NFT_RULE_F_COMMIT)
-		err = nf_tables_trans_add(rule, &ctx);
-		if (err < 0)
-			goto err2;
-	else {
-		nf_tables_rule_notify(skb, nlh, table, chain, rule,
-				      NFT_MSG_NEWRULE,
-				      nlh->nlmsg_flags & (NLM_F_APPEND | NLM_F_REPLACE),
-				      nfmsg->nfgen_family);
-	}
+	err = nf_tables_trans_add(rule, &ctx);
+	if (err < 0)
+		goto err2;
+
 	return 0;
 
 err2:
@@ -1703,21 +1684,10 @@ err1:
 }
 
 static int
-nf_tables_delrule_one(struct nft_ctx *ctx, struct nft_rule *rule, u32 flags)
+nf_tables_delrule_one(struct nft_ctx *ctx, struct nft_rule *rule)
 {
-	int err = 0;
-
-	if (flags & NFT_RULE_F_COMMIT) {
-		nft_rule_disactivate_next(ctx->net, rule);
-		err = nf_tables_trans_add(rule, ctx);
-	} else {
-		list_del_rcu(&rule->list);
-		nf_tables_rule_notify(ctx->skb, ctx->nlh, ctx->table,
-				      ctx->chain, rule, NFT_MSG_DELRULE,
-				      0, ctx->afi->family);
-		nf_tables_rule_destroy(rule);
-	}
-	return err;
+	nft_rule_disactivate_next(ctx->net, rule);
+	return nf_tables_trans_add(rule, ctx);
 }
 
 static int nf_tables_delrule(struct sock *nlsk, struct sk_buff *skb,
@@ -1732,7 +1702,6 @@ static int nf_tables_delrule(struct sock *nlsk, struct sk_buff *skb,
 	struct nft_rule *rule, *tmp;
 	int family = nfmsg->nfgen_family, err = 0;
 	struct nft_ctx ctx;
-	u32 flags = 0;
 
 	afi = nf_tables_afinfo_lookup(net, family, false);
 	if (IS_ERR(afi))
@@ -1748,23 +1717,16 @@ static int nf_tables_delrule(struct sock *nlsk, struct sk_buff *skb,
 
 	nft_ctx_init(&ctx, skb, nlh, afi, table, chain, nla);
 
-	if (nla[NFTA_RULE_FLAGS]) {
-		flags = ntohl(nla_get_be32(nla[NFTA_RULE_FLAGS]));
-
-		if (flags & ~NFT_RULE_F_MASK)
-			return -EINVAL;
-	}
-
 	if (nla[NFTA_RULE_HANDLE]) {
 		rule = nf_tables_rule_lookup(chain, nla[NFTA_RULE_HANDLE]);
 		if (IS_ERR(rule))
 			return PTR_ERR(rule);
 
-		err = nf_tables_delrule_one(&ctx, rule, flags);
+		err = nf_tables_delrule_one(&ctx, rule);
 	} else {
 		/* Remove all rules in this chain */
 		list_for_each_entry_safe(rule, tmp, &chain->rules, list) {
-			err = nf_tables_delrule_one(&ctx, rule, flags);
+			err = nf_tables_delrule_one(&ctx, rule);
 			if (err < 0)
 				break;
 		}
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2013-09-17 10:43 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-17 10:43 [PATCH 0/4 -nftables v2] nf_tables atomic rule-set update Pablo Neira Ayuso
2013-09-17 10:43 ` [PATCH -nftables v2 1/4] netfilter: nf_tables: get rid of per rule list_head for commits Pablo Neira Ayuso
2013-09-17 10:43 ` [PATCH -nftables v2 2/4] netfilter: nf_tables: use per netns commit list Pablo Neira Ayuso
2013-09-17 10:43 ` [PATCH -nftables v2 3/4] netfilter: nfnetlink: add batch support and use it from nf_tables Pablo Neira Ayuso
2013-09-17 10:43 ` [PATCH -nftables v2 4/4] netfilter: nf_tables: all rule updates are transactional Pablo Neira Ayuso

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.