[nf_tables PATCH 0/5] Extended NFT_MSG_DELTABLE

[nf_tables PATCH 0/5] Extended NFT_MSG_DELTABLE

[Arturo Borrero Gonzalez]

The following series implements some changes to the NFT_MSG_DELTABLE
API call.
The main change is the ability to flush the entire ruleset (also per AF).
We are no longing returning EBUSY.

This way, clients of the API can wipe the ruleset and add a new one in just
one transaction/batch.

In the first patches, there are a lot of code factorization and renaming.
The last patch is the change to NFT_MSG_DELTABLE itself.

Comments are welcome.
---

Arturo Borrero Gonzalez (5):
      netfilter: nf_tables: refactor rule deletion helper
      netfilter: nf_tables: add helper to unregister chain hooks
      netfilter: nf_tables: rename nf_table_delrule_by_chain()
      netfilter: nf_tables: add helpers to schedule objects deletion
      netfilter: nf_tables: extend NFT_MSG_DELTABLE to support flushing the ruleset


 net/netfilter/nf_tables_api.c |  464 ++++++++++++++++++++++++++---------------
 1 file changed, 291 insertions(+), 173 deletions(-)

-- 
Arturo Borrero Gonzalez

[nf_tables PATCH 1/5] netfilter: nf_tables: refactor rule deletion helper

[Arturo Borrero Gonzalez]

This helper function always schedule the rule to be removed in the following
transaction.
In follow-up patches, it is interesting to handle separately the logic of rule
activation/disactivation from the transaction mechanism.

So, this patch simply splits the original nf_tables_delrule_one() in two
functions, allowing further control.

While at it, for the sake of homigeneize the function naming scheme, let's
rename nf_tables_delrule_one() to nft_delrule().

Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
---
 net/netfilter/nf_tables_api.c |   26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 624e083..76c2c45 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1838,12 +1838,10 @@ err1:
 }
 
 static int
-nf_tables_delrule_one(struct nft_ctx *ctx, struct nft_rule *rule)
+nf_tables_delrule_disactivate(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 (nft_trans_rule_add(ctx, NFT_MSG_DELRULE, rule) == NULL)
-			return -ENOMEM;
 		nft_rule_disactivate_next(ctx->net, rule);
 		ctx->chain->use--;
 		return 0;
@@ -1851,13 +1849,31 @@ nf_tables_delrule_one(struct nft_ctx *ctx, struct nft_rule *rule)
 	return -ENOENT;
 }
 
+static int nft_delrule(struct nft_ctx *ctx, struct nft_rule *rule)
+{
+	struct nft_trans *trans;
+	int err;
+
+	trans = nft_trans_rule_add(ctx, NFT_MSG_DELRULE, rule);
+	if (trans == NULL)
+		return -ENOMEM;
+
+	err = nf_tables_delrule_disactivate(ctx, rule);
+	if (err < 0) {
+		nft_trans_destroy(trans);
+		return err;
+	}
+
+	return 0;
+}
+
 static int nf_table_delrule_by_chain(struct nft_ctx *ctx)
 {
 	struct nft_rule *rule;
 	int err;
 
 	list_for_each_entry(rule, &ctx->chain->rules, list) {
-		err = nf_tables_delrule_one(ctx, rule);
+		err = nft_delrule(ctx, rule);
 		if (err < 0)
 			return err;
 	}
@@ -1902,7 +1918,7 @@ static int nf_tables_delrule(struct sock *nlsk, struct sk_buff *skb,
 			if (IS_ERR(rule))
 				return PTR_ERR(rule);
 
-			err = nf_tables_delrule_one(&ctx, rule);
+			err = nft_delrule(&ctx, rule);
 		} else {
 			err = nf_table_delrule_by_chain(&ctx);
 		}

[nf_tables PATCH 2/5] netfilter: nf_tables: add helper to unregister chain hooks

[Arturo Borrero Gonzalez]

This patch adds a helper function to unregister chain hooks in the chain
deletion path. Basically, a code factorization.

The new function is useful in follow-up patches.

Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
---
 net/netfilter/nf_tables_api.c |   31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 76c2c45..df024dd 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -127,6 +127,15 @@ static void nft_trans_destroy(struct nft_trans *trans)
 	kfree(trans);
 }
 
+static void nf_tables_unregister_hooks(const struct nft_table *table,
+				       const struct nft_chain *chain,
+				       unsigned int hook_nops)
+{
+	if (!(table->flags & NFT_TABLE_F_DORMANT) &&
+	    chain->flags & NFT_BASE_CHAIN)
+		nf_unregister_hooks(nft_base_chain(chain)->ops, hook_nops);
+}
+
 /*
  * Tables
  */
@@ -1133,11 +1142,7 @@ static int nf_tables_newchain(struct sock *nlsk, struct sk_buff *skb,
 	list_add_tail(&chain->list, &table->chains);
 	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);
-	}
+	nf_tables_unregister_hooks(table, chain, afi->nops);
 err1:
 	nf_tables_chain_destroy(chain);
 	return err;
@@ -3396,11 +3401,9 @@ static int nf_tables_commit(struct sk_buff *skb)
 			break;
 		case NFT_MSG_DELCHAIN:
 			nf_tables_chain_notify(&trans->ctx, NFT_MSG_DELCHAIN);
-			if (!(trans->ctx.table->flags & NFT_TABLE_F_DORMANT) &&
-			    trans->ctx.chain->flags & NFT_BASE_CHAIN) {
-				nf_unregister_hooks(nft_base_chain(trans->ctx.chain)->ops,
-						    trans->ctx.afi->nops);
-			}
+			nf_tables_unregister_hooks(trans->ctx.table,
+						   trans->ctx.chain,
+						   trans->ctx.afi->nops);
 			break;
 		case NFT_MSG_NEWRULE:
 			nft_rule_clear(trans->ctx.net, nft_trans_rule(trans));
@@ -3519,11 +3522,9 @@ static int nf_tables_abort(struct sk_buff *skb)
 			} else {
 				trans->ctx.table->use--;
 				list_del(&trans->ctx.chain->list);
-				if (!(trans->ctx.table->flags & NFT_TABLE_F_DORMANT) &&
-				    trans->ctx.chain->flags & NFT_BASE_CHAIN) {
-					nf_unregister_hooks(nft_base_chain(trans->ctx.chain)->ops,
-							    trans->ctx.afi->nops);
-				}
+				nf_tables_unregister_hooks(trans->ctx.table,
+							   trans->ctx.chain,
+							   trans->ctx.afi->nops);
 			}
 			break;
 		case NFT_MSG_DELCHAIN:

[nf_tables PATCH 3/5] netfilter: nf_tables: rename nf_table_delrule_by_chain()

[Arturo Borrero Gonzalez]

For the sake of homogenize the function naming scheme, let's rename
nf_table_delrule_by_chain() to nft_delrule_by_chain().

Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
---
 net/netfilter/nf_tables_api.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index df024dd..3ff48bc 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1872,7 +1872,7 @@ static int nft_delrule(struct nft_ctx *ctx, struct nft_rule *rule)
 	return 0;
 }
 
-static int nf_table_delrule_by_chain(struct nft_ctx *ctx)
+static int nft_delrule_by_chain(struct nft_ctx *ctx)
 {
 	struct nft_rule *rule;
 	int err;
@@ -1925,12 +1925,12 @@ static int nf_tables_delrule(struct sock *nlsk, struct sk_buff *skb,
 
 			err = nft_delrule(&ctx, rule);
 		} else {
-			err = nf_table_delrule_by_chain(&ctx);
+			err = nft_delrule_by_chain(&ctx);
 		}
 	} else {
 		list_for_each_entry(chain, &table->chains, list) {
 			ctx.chain = chain;
-			err = nf_table_delrule_by_chain(&ctx);
+			err = nft_delrule_by_chain(&ctx);
 			if (err < 0)
 				break;
 		}

[nf_tables PATCH 4/5] netfilter: nf_tables: add helpers to schedule objects deletion

[Arturo Borrero Gonzalez]

This patch refactor the code to schedule objects deletion.

They are useful in follow-up patches.

In order to be able to use these new helper functions in all the code,
they are placed in the top of the file, with all the dependant functions
and symbols.

Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
---
 net/netfilter/nf_tables_api.c |  361 ++++++++++++++++++++++-------------------
 1 file changed, 193 insertions(+), 168 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 3ff48bc..d954eed 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -136,6 +136,195 @@ static void nf_tables_unregister_hooks(const struct nft_table *table,
 		nf_unregister_hooks(nft_base_chain(chain)->ops, hook_nops);
 }
 
+/* Internal table flags */
+#define NFT_TABLE_INACTIVE	(1 << 15)
+
+static int nft_trans_table_add(struct nft_ctx *ctx, int msg_type)
+{
+	struct nft_trans *trans;
+
+	trans = nft_trans_alloc(ctx, msg_type, sizeof(struct nft_trans_table));
+	if (trans == NULL)
+		return -ENOMEM;
+
+	if (msg_type == NFT_MSG_NEWTABLE)
+		ctx->table->flags |= NFT_TABLE_INACTIVE;
+
+	list_add_tail(&trans->list, &ctx->net->nft.commit_list);
+	return 0;
+}
+
+static int nft_deltable(struct nft_ctx *ctx)
+{
+	int err;
+
+	err = nft_trans_table_add(ctx, NFT_MSG_DELTABLE);
+	if (err < 0)
+		return err;
+
+	list_del(&ctx->table->list);
+	return err;
+}
+
+static int nft_trans_chain_add(struct nft_ctx *ctx, int msg_type)
+{
+	struct nft_trans *trans;
+
+	trans = nft_trans_alloc(ctx, msg_type, sizeof(struct nft_trans_chain));
+	if (trans == NULL)
+		return -ENOMEM;
+
+	if (msg_type == NFT_MSG_NEWCHAIN)
+		ctx->chain->flags |= NFT_CHAIN_INACTIVE;
+
+	list_add_tail(&trans->list, &ctx->net->nft.commit_list);
+	return 0;
+}
+
+static int nft_delchain(struct nft_ctx *ctx)
+{
+	int err;
+
+	err = nft_trans_chain_add(ctx, NFT_MSG_DELCHAIN);
+	if (err < 0)
+		return err;
+
+	ctx->table->use--;
+	list_del(&ctx->chain->list);
+
+	return err;
+}
+
+static inline bool
+nft_rule_is_active(struct net *net, const struct nft_rule *rule)
+{
+	return (rule->genmask & (1 << net->nft.gencursor)) == 0;
+}
+
+static inline int gencursor_next(struct net *net)
+{
+	return net->nft.gencursor+1 == 1 ? 1 : 0;
+}
+
+static inline int
+nft_rule_is_active_next(struct net *net, const struct nft_rule *rule)
+{
+	return (rule->genmask & (1 << gencursor_next(net))) == 0;
+}
+
+static inline void
+nft_rule_activate_next(struct net *net, struct nft_rule *rule)
+{
+	/* Now inactive, will be active in the future */
+	rule->genmask = (1 << net->nft.gencursor);
+}
+
+static inline void
+nft_rule_disactivate_next(struct net *net, struct nft_rule *rule)
+{
+	rule->genmask = (1 << gencursor_next(net));
+}
+
+static inline void nft_rule_clear(struct net *net, struct nft_rule *rule)
+{
+	rule->genmask = 0;
+}
+
+static int
+nf_tables_delrule_disactivate(struct nft_ctx *ctx, struct nft_rule *rule)
+{
+	/* You cannot delete the same rule twice */
+	if (nft_rule_is_active_next(ctx->net, rule)) {
+		nft_rule_disactivate_next(ctx->net, rule);
+		ctx->chain->use--;
+		return 0;
+	}
+	return -ENOENT;
+}
+
+static struct nft_trans *nft_trans_rule_add(struct nft_ctx *ctx, int msg_type,
+					    struct nft_rule *rule)
+{
+	struct nft_trans *trans;
+
+	trans = nft_trans_alloc(ctx, msg_type, sizeof(struct nft_trans_rule));
+	if (trans == NULL)
+		return NULL;
+
+	nft_trans_rule(trans) = rule;
+	list_add_tail(&trans->list, &ctx->net->nft.commit_list);
+
+	return trans;
+}
+
+static int nft_delrule(struct nft_ctx *ctx, struct nft_rule *rule)
+{
+	struct nft_trans *trans;
+	int err;
+
+	trans = nft_trans_rule_add(ctx, NFT_MSG_DELRULE, rule);
+	if (trans == NULL)
+		return -ENOMEM;
+
+	err = nf_tables_delrule_disactivate(ctx, rule);
+	if (err < 0) {
+		nft_trans_destroy(trans);
+		return err;
+	}
+
+	return 0;
+}
+
+static int nft_delrule_by_chain(struct nft_ctx *ctx)
+{
+	struct nft_rule *rule;
+	int err;
+
+	list_for_each_entry(rule, &ctx->chain->rules, list) {
+		err = nft_delrule(ctx, rule);
+		if (err < 0)
+			return err;
+	}
+	return 0;
+}
+
+/* Internal set flag */
+#define NFT_SET_INACTIVE	(1 << 15)
+
+static int nft_trans_set_add(struct nft_ctx *ctx, int msg_type,
+			     struct nft_set *set)
+{
+	struct nft_trans *trans;
+
+	trans = nft_trans_alloc(ctx, msg_type, sizeof(struct nft_trans_set));
+	if (trans == NULL)
+		return -ENOMEM;
+
+	if (msg_type == NFT_MSG_NEWSET && ctx->nla[NFTA_SET_ID] != NULL) {
+		nft_trans_set_id(trans) =
+			ntohl(nla_get_be32(ctx->nla[NFTA_SET_ID]));
+		set->flags |= NFT_SET_INACTIVE;
+	}
+	nft_trans_set(trans) = set;
+	list_add_tail(&trans->list, &ctx->net->nft.commit_list);
+
+	return 0;
+}
+
+static int nft_delset(struct nft_ctx *ctx, struct nft_set *set)
+{
+	int err;
+
+	err = nft_trans_set_add(ctx, NFT_MSG_DELSET, set);
+	if (err < 0)
+		return err;
+
+	list_del(&set->list);
+	ctx->table->use--;
+
+	return err;
+}
+
 /*
  * Tables
  */
@@ -312,9 +501,6 @@ done:
 	return skb->len;
 }
 
-/* Internal table flags */
-#define NFT_TABLE_INACTIVE	(1 << 15)
-
 static int nf_tables_gettable(struct sock *nlsk, struct sk_buff *skb,
 			      const struct nlmsghdr *nlh,
 			      const struct nlattr * const nla[])
@@ -443,21 +629,6 @@ err:
 	return ret;
 }
 
-static int nft_trans_table_add(struct nft_ctx *ctx, int msg_type)
-{
-	struct nft_trans *trans;
-
-	trans = nft_trans_alloc(ctx, msg_type, sizeof(struct nft_trans_table));
-	if (trans == NULL)
-		return -ENOMEM;
-
-	if (msg_type == NFT_MSG_NEWTABLE)
-		ctx->table->flags |= NFT_TABLE_INACTIVE;
-
-	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[])
@@ -535,7 +706,7 @@ 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, err;
+	int family = nfmsg->nfgen_family;
 	struct nft_ctx ctx;
 
 	afi = nf_tables_afinfo_lookup(net, family, false);
@@ -551,12 +722,8 @@ static int nf_tables_deltable(struct sock *nlsk, struct sk_buff *skb,
 		return -EBUSY;
 
 	nft_ctx_init(&ctx, skb, nlh, afi, table, NULL, nla);
-	err = nft_trans_table_add(&ctx, NFT_MSG_DELTABLE);
-	if (err < 0)
-		return err;
 
-	list_del(&table->list);
-	return 0;
+	return nft_deltable(&ctx);
 }
 
 static void nf_tables_table_destroy(struct nft_ctx *ctx)
@@ -898,21 +1065,6 @@ static void nft_chain_stats_replace(struct nft_base_chain *chain,
 		rcu_assign_pointer(chain->stats, newstats);
 }
 
-static int nft_trans_chain_add(struct nft_ctx *ctx, int msg_type)
-{
-	struct nft_trans *trans;
-
-	trans = nft_trans_alloc(ctx, msg_type, sizeof(struct nft_trans_chain));
-	if (trans == NULL)
-		return -ENOMEM;
-
-	if (msg_type == NFT_MSG_NEWCHAIN)
-		ctx->chain->flags |= NFT_CHAIN_INACTIVE;
-
-	list_add_tail(&trans->list, &ctx->net->nft.commit_list);
-	return 0;
-}
-
 static void nf_tables_chain_destroy(struct nft_chain *chain)
 {
 	BUG_ON(chain->use > 0);
@@ -1159,7 +1311,6 @@ static int nf_tables_delchain(struct sock *nlsk, struct sk_buff *skb,
 	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))
@@ -1180,13 +1331,8 @@ static int nf_tables_delchain(struct sock *nlsk, struct sk_buff *skb,
 		return -EBUSY;
 
 	nft_ctx_init(&ctx, skb, nlh, afi, table, chain, nla);
-	err = nft_trans_chain_add(&ctx, NFT_MSG_DELCHAIN);
-	if (err < 0)
-		return err;
 
-	table->use--;
-	list_del(&chain->list);
-	return 0;
+	return nft_delchain(&ctx);
 }
 
 /*
@@ -1508,41 +1654,6 @@ err:
 	return err;
 }
 
-static inline bool
-nft_rule_is_active(struct net *net, const struct nft_rule *rule)
-{
-	return (rule->genmask & (1 << net->nft.gencursor)) == 0;
-}
-
-static inline int gencursor_next(struct net *net)
-{
-	return net->nft.gencursor+1 == 1 ? 1 : 0;
-}
-
-static inline int
-nft_rule_is_active_next(struct net *net, const struct nft_rule *rule)
-{
-	return (rule->genmask & (1 << gencursor_next(net))) == 0;
-}
-
-static inline void
-nft_rule_activate_next(struct net *net, struct nft_rule *rule)
-{
-	/* Now inactive, will be active in the future */
-	rule->genmask = (1 << net->nft.gencursor);
-}
-
-static inline void
-nft_rule_disactivate_next(struct net *net, struct nft_rule *rule)
-{
-	rule->genmask = (1 << gencursor_next(net));
-}
-
-static inline void nft_rule_clear(struct net *net, struct nft_rule *rule)
-{
-	rule->genmask = 0;
-}
-
 static int nf_tables_dump_rules(struct sk_buff *skb,
 				struct netlink_callback *cb)
 {
@@ -1667,21 +1778,6 @@ static void nf_tables_rule_destroy(const struct nft_ctx *ctx,
 	kfree(rule);
 }
 
-static struct nft_trans *nft_trans_rule_add(struct nft_ctx *ctx, int msg_type,
-					    struct nft_rule *rule)
-{
-	struct nft_trans *trans;
-
-	trans = nft_trans_alloc(ctx, msg_type, sizeof(struct nft_trans_rule));
-	if (trans == NULL)
-		return NULL;
-
-	nft_trans_rule(trans) = rule;
-	list_add_tail(&trans->list, &ctx->net->nft.commit_list);
-
-	return trans;
-}
-
 #define NFT_RULE_MAXEXPRS	128
 
 static struct nft_expr_info *info;
@@ -1842,49 +1938,6 @@ err1:
 	return err;
 }
 
-static int
-nf_tables_delrule_disactivate(struct nft_ctx *ctx, struct nft_rule *rule)
-{
-	/* You cannot delete the same rule twice */
-	if (nft_rule_is_active_next(ctx->net, rule)) {
-		nft_rule_disactivate_next(ctx->net, rule);
-		ctx->chain->use--;
-		return 0;
-	}
-	return -ENOENT;
-}
-
-static int nft_delrule(struct nft_ctx *ctx, struct nft_rule *rule)
-{
-	struct nft_trans *trans;
-	int err;
-
-	trans = nft_trans_rule_add(ctx, NFT_MSG_DELRULE, rule);
-	if (trans == NULL)
-		return -ENOMEM;
-
-	err = nf_tables_delrule_disactivate(ctx, rule);
-	if (err < 0) {
-		nft_trans_destroy(trans);
-		return err;
-	}
-
-	return 0;
-}
-
-static int nft_delrule_by_chain(struct nft_ctx *ctx)
-{
-	struct nft_rule *rule;
-	int err;
-
-	list_for_each_entry(rule, &ctx->chain->rules, list) {
-		err = nft_delrule(ctx, rule);
-		if (err < 0)
-			return err;
-	}
-	return 0;
-}
-
 static int nf_tables_delrule(struct sock *nlsk, struct sk_buff *skb,
 			     const struct nlmsghdr *nlh,
 			     const struct nlattr * const nla[])
@@ -2385,8 +2438,6 @@ static int nf_tables_dump_sets(struct sk_buff *skb, struct netlink_callback *cb)
 	return ret;
 }
 
-#define NFT_SET_INACTIVE	(1 << 15)	/* Internal set flag */
-
 static int nf_tables_getset(struct sock *nlsk, struct sk_buff *skb,
 			    const struct nlmsghdr *nlh,
 			    const struct nlattr * const nla[])
@@ -2451,26 +2502,6 @@ static int nf_tables_set_desc_parse(const struct nft_ctx *ctx,
 	return 0;
 }
 
-static int nft_trans_set_add(struct nft_ctx *ctx, int msg_type,
-			     struct nft_set *set)
-{
-	struct nft_trans *trans;
-
-	trans = nft_trans_alloc(ctx, msg_type, sizeof(struct nft_trans_set));
-	if (trans == NULL)
-		return -ENOMEM;
-
-	if (msg_type == NFT_MSG_NEWSET && ctx->nla[NFTA_SET_ID] != NULL) {
-		nft_trans_set_id(trans) =
-			ntohl(nla_get_be32(ctx->nla[NFTA_SET_ID]));
-		set->flags |= NFT_SET_INACTIVE;
-	}
-	nft_trans_set(trans) = set;
-	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[])
@@ -2664,13 +2695,7 @@ static int nf_tables_delset(struct sock *nlsk, struct sk_buff *skb,
 	if (!list_empty(&set->bindings))
 		return -EBUSY;
 
-	err = nft_trans_set_add(&ctx, NFT_MSG_DELSET, set);
-	if (err < 0)
-		return err;
-
-	list_del(&set->list);
-	ctx.table->use--;
-	return 0;
+	return nft_delset(&ctx, set);
 }
 
 static int nf_tables_bind_check_setelem(const struct nft_ctx *ctx,

[nf_tables PATCH 5/5] netfilter: nf_tables: extend NFT_MSG_DELTABLE to support flushing the ruleset

[Arturo Borrero Gonzalez]

This patch extend the NFT_MSG_DELTABLE call to support flushing the entire
ruleset.

The options now are:
 * No family speficied, no table specified: flush all the ruleset.
 * Family specified, no table specified: flush all tables in the AF.
 * Family specified, table specified: flush the given table.

Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
---
 net/netfilter/nf_tables_api.c |   84 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 80 insertions(+), 4 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index d954eed..917bbc2 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -698,6 +698,72 @@ static int nf_tables_newtable(struct sock *nlsk, struct sk_buff *skb,
 	return 0;
 }
 
+static int nft_flush_table(struct nft_ctx *ctx)
+{
+	int err;
+	struct nft_chain *chain, *nc;
+	struct nft_set *set, *ns;
+
+	list_for_each_entry_safe(chain, nc, &ctx->table->chains, list) {
+		ctx->chain = chain;
+
+		err = nft_delrule_by_chain(ctx);
+		if (err < 0)
+			goto out;
+
+		err = nft_delchain(ctx);
+		if (err < 0)
+			goto out;
+	}
+
+	list_for_each_entry_safe(set, ns, &ctx->table->sets, list) {
+		if (set->flags & NFT_SET_ANONYMOUS)
+			continue;
+
+		err = nft_delset(ctx, set);
+		if (err < 0)
+			goto out;
+	}
+
+	err = nft_deltable(ctx);
+out:
+	return err;
+}
+
+static int nft_flush_family(struct nft_ctx *ctx)
+{
+	int err = 0;
+	struct nft_table *table, *nt;
+
+	list_for_each_entry_safe(table, nt, &ctx->afi->tables, list) {
+		ctx->table = table;
+
+		err = nft_flush_table(ctx);
+		if (err < 0)
+			goto out;
+	}
+
+out:
+	return err;
+}
+
+static int nft_flush_ruleset(struct nft_ctx *ctx)
+{
+	int err = 0;
+	struct nft_af_info *afi;
+
+	list_for_each_entry(afi, &ctx->net->nft.af_info, list) {
+		ctx->afi = afi;
+
+		err = nft_flush_family(ctx);
+		if (err < 0)
+			goto out;
+	}
+
+out:
+	return err;
+}
+
 static int nf_tables_deltable(struct sock *nlsk, struct sk_buff *skb,
 			      const struct nlmsghdr *nlh,
 			      const struct nlattr * const nla[])
@@ -709,21 +775,31 @@ static int nf_tables_deltable(struct sock *nlsk, struct sk_buff *skb,
 	int family = nfmsg->nfgen_family;
 	struct nft_ctx ctx;
 
+	if (family == NFPROTO_UNSPEC) {
+		if (nla[NFTA_TABLE_NAME] != NULL)
+			return -EINVAL;
+
+		nft_ctx_init(&ctx, skb, nlh, NULL, NULL, NULL, nla);
+		return nft_flush_ruleset(&ctx);
+	}
+
 	afi = nf_tables_afinfo_lookup(net, family, false);
 	if (IS_ERR(afi))
 		return PTR_ERR(afi);
 
+	if (nla[NFTA_TABLE_NAME] == NULL) {
+		nft_ctx_init(&ctx, skb, nlh, afi, NULL, NULL, nla);
+		return nft_flush_family(&ctx);
+	}
+
 	table = nf_tables_table_lookup(afi, nla[NFTA_TABLE_NAME]);
 	if (IS_ERR(table))
 		return PTR_ERR(table);
 	if (table->flags & NFT_TABLE_INACTIVE)
 		return -ENOENT;
-	if (table->use > 0)
-		return -EBUSY;
 
 	nft_ctx_init(&ctx, skb, nlh, afi, table, NULL, nla);
-
-	return nft_deltable(&ctx);
+	return nft_flush_table(&ctx);
 }
 
 static void nf_tables_table_destroy(struct nft_ctx *ctx)

Re: [nf_tables PATCH 5/5] netfilter: nf_tables: extend NFT_MSG_DELTABLE to support flushing the ruleset

[Pablo Neira Ayuso]

On Tue, Aug 26, 2014 at 11:57:01AM +0200, Arturo Borrero Gonzalez wrote:
> This patch extend the NFT_MSG_DELTABLE call to support flushing the entire
> ruleset.
> 
> The options now are:
>  * No family speficied, no table specified: flush all the ruleset.
>  * Family specified, no table specified: flush all tables in the AF.
>  * Family specified, table specified: flush the given table.
> 
> Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
> ---
>  net/netfilter/nf_tables_api.c |   84 +++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 80 insertions(+), 4 deletions(-)
> 
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index d954eed..917bbc2 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -698,6 +698,72 @@ static int nf_tables_newtable(struct sock *nlsk, struct sk_buff *skb,
>  	return 0;
>  }
>  
> +static int nft_flush_table(struct nft_ctx *ctx)
> +{
> +	int err;
> +	struct nft_chain *chain, *nc;
> +	struct nft_set *set, *ns;
> +
> +	list_for_each_entry_safe(chain, nc, &ctx->table->chains, list) {
> +		ctx->chain = chain;
> +
> +		err = nft_delrule_by_chain(ctx);
> +		if (err < 0)
> +			goto out;
> +
> +		err = nft_delchain(ctx);
> +		if (err < 0)
> +			goto out;
> +	}
> +
> +	list_for_each_entry_safe(set, ns, &ctx->table->sets, list) {
> +		if (set->flags & NFT_SET_ANONYMOUS)
> +			continue;
> +
> +		err = nft_delset(ctx, set);
> +		if (err < 0)
> +			goto out;
> +	}
> +
> +	err = nft_deltable(ctx);
> +out:
> +	return err;
> +}
> +
> +static int nft_flush_family(struct nft_ctx *ctx)
> +{
> +	int err = 0;
> +	struct nft_table *table, *nt;
> +
> +	list_for_each_entry_safe(table, nt, &ctx->afi->tables, list) {
> +		ctx->table = table;
> +
> +		err = nft_flush_table(ctx);
> +		if (err < 0)
> +			goto out;
> +	}
> +
> +out:
> +	return err;
> +}
> +
> +static int nft_flush_ruleset(struct nft_ctx *ctx)
> +{
> +	int err = 0;
> +	struct nft_af_info *afi;
> +
> +	list_for_each_entry(afi, &ctx->net->nft.af_info, list) {
> +		ctx->afi = afi;
> +
> +		err = nft_flush_family(ctx);
> +		if (err < 0)
> +			goto out;
> +	}
> +
> +out:
> +	return err;
> +}
> +
>  static int nf_tables_deltable(struct sock *nlsk, struct sk_buff *skb,
>  			      const struct nlmsghdr *nlh,
>  			      const struct nlattr * const nla[])
> @@ -709,21 +775,31 @@ static int nf_tables_deltable(struct sock *nlsk, struct sk_buff *skb,
>  	int family = nfmsg->nfgen_family;
>  	struct nft_ctx ctx;
>  
> +	if (family == NFPROTO_UNSPEC) {
> +		if (nla[NFTA_TABLE_NAME] != NULL)
> +			return -EINVAL;

We should implement this too. I think you can replace
nft_flush_ruleset and nft_flush_family by something more generic like:

static int nf_tables_flush(struct net *net, ...)
{
        ...

        list_for_each_entry(afi, &net->nft.af_info, list) {
                 if (family != AF_UNSPEC && afi->family != family)
                        continue;

                 list_for_each_entry_safe(table, next, afi->tables, list) {
                        if (nla[NFTA_TABLE_NAME] &&
                            nla_strcmp(nla[NFTA_TABLE_NAME], table->name) != 0)
                                continue;

                        err = nft_flush_table(table);
                        ...
                 }
        }
        return err;
}

You can also skip the nft_ctx_init with this since it's quite empty
and I guess my proposed nf_tables_flush should take around three
parameters.

> +
> +		nft_ctx_init(&ctx, skb, nlh, NULL, NULL, NULL, nla);
> +		return nft_flush_ruleset(&ctx);
> +	}
> +
>  	afi = nf_tables_afinfo_lookup(net, family, false);
>  	if (IS_ERR(afi))
>  		return PTR_ERR(afi);
>  
> +	if (nla[NFTA_TABLE_NAME] == NULL) {
> +		nft_ctx_init(&ctx, skb, nlh, afi, NULL, NULL, nla);
> +		return nft_flush_family(&ctx);
> +	}
> +
>  	table = nf_tables_table_lookup(afi, nla[NFTA_TABLE_NAME]);
>  	if (IS_ERR(table))
>  		return PTR_ERR(table);
>  	if (table->flags & NFT_TABLE_INACTIVE)
>  		return -ENOENT;
> -	if (table->use > 0)
> -		return -EBUSY;
>  
>  	nft_ctx_init(&ctx, skb, nlh, afi, table, NULL, nla);
> -
> -	return nft_deltable(&ctx);
> +	return nft_flush_table(&ctx);
>  }
>  
>  static void nf_tables_table_destroy(struct nft_ctx *ctx)
> 

Re: [nf_tables PATCH 5/5] netfilter: nf_tables: extend NFT_MSG_DELTABLE to support flushing the ruleset

[Arturo Borrero Gonzalez]

On 26 August 2014 12:53, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Tue, Aug 26, 2014 at 11:57:01AM +0200, Arturo Borrero Gonzalez wrote:
>> This patch extend the NFT_MSG_DELTABLE call to support flushing the entire
>> ruleset.
>>
>> The options now are:
>>  * No family speficied, no table specified: flush all the ruleset.
>>  * Family specified, no table specified: flush all tables in the AF.
>>  * Family specified, table specified: flush the given table.
>>
[...]
>> @@ -709,21 +775,31 @@ static int nf_tables_deltable(struct sock *nlsk, struct sk_buff *skb,
>>       int family = nfmsg->nfgen_family;
>>       struct nft_ctx ctx;
>>
>> +     if (family == NFPROTO_UNSPEC) {
>> +             if (nla[NFTA_TABLE_NAME] != NULL)
>> +                     return -EINVAL;
>
> We should implement this too. I think you can replace
> nft_flush_ruleset and nft_flush_family by something more generic like:
>
> static int nf_tables_flush(struct net *net, ...)
> {
>         ...
>
>         list_for_each_entry(afi, &net->nft.af_info, list) {
>                  if (family != AF_UNSPEC && afi->family != family)
>                         continue;
>
>                  list_for_each_entry_safe(table, next, afi->tables, list) {
>                         if (nla[NFTA_TABLE_NAME] &&
>                             nla_strcmp(nla[NFTA_TABLE_NAME], table->name) != 0)
>                                 continue;
>
>                         err = nft_flush_table(table);
>                         ...
>                  }
>         }
>         return err;
> }
>

Ok. I agree.

> You can also skip the nft_ctx_init with this since it's quite empty
> and I guess my proposed nf_tables_flush should take around three
> parameters.
>

All functions take that nft_ctx and fill the needed data for the inner
and later functions (trans allocation, trans commit/abort, event
notifications..). I didn't find a cleaner way to do it, and the
nft_ctx_init() doesn't hurt.

So, I think we still need to use the nft_ctx even with that small
data, don't you?
-- 
Arturo Borrero González
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html