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

[nf_tables PATCH 1/6 v5] 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>
---
v2: no changes, resending the series.
v3: change 'disactivate' and use 'deactivate'. Requested by Patrick.
v4: no changes, resending the series because v3 series is invalid.
v5: no changes, resending the series.

 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 deeb95f..3664bab 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1868,12 +1868,10 @@ err1:
 }
 
 static int
-nf_tables_delrule_one(struct nft_ctx *ctx, struct nft_rule *rule)
+nf_tables_delrule_deactivate(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;
@@ -1881,13 +1879,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_deactivate(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;
 	}
@@ -1932,7 +1948,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);
 		}
-- 
1.7.10.4

[nft PATCH 6/6] src: add `flush ruleset'

[Arturo Borrero Gonzalez]

This patch adds the `flush ruleset' operation to nft.

The syntax is:
 % nft flush ruleset [family]

To flush all the ruleset (all families):
 % nft flush ruleset

To flush the ruleset of a given family:
 % nft flush ruleset ip
 % nft flush ruleset inet

This flush is a shortcut operation which deletes all rules, sets, tables
and chains.
It's possible since the modifications in the kernel to the NFT_MSG_DELTABLE
API call.

Users can benefit of this operation when doing an atomic replacement of the
entire ruleset, loading a file like this:

 =========
 flush ruleset
 table ip filter {
 	chain input {
		counter accept
	}
 }
 =========

Also, users who want to simply clean the ruleset for whatever reason can do it now
without having to iterate families/tables.

Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
---
 include/netlink.h |    4 ++++
 src/netlink.c     |   22 ++++++++++++++++++++++
 src/parser.y      |   30 +++++++++++++++++++++++++-----
 src/rule.c        |    2 ++
 src/scanner.l     |    1 +
 5 files changed, 54 insertions(+), 5 deletions(-)

diff --git a/include/netlink.h b/include/netlink.h
index d7d5c2d..f611452 100644
--- a/include/netlink.h
+++ b/include/netlink.h
@@ -144,6 +144,10 @@ extern int netlink_io_error(struct netlink_ctx *ctx,
 			    const struct location *loc, const char *fmt, ...);
 extern void netlink_open_error(void) __noreturn;
 
+extern int netlink_flush_ruleset(struct netlink_ctx *ctx,
+				 const struct handle *h,
+				 const struct location *loc);
+
 extern struct nft_ruleset *netlink_dump_ruleset(struct netlink_ctx *ctx,
 						const struct handle *h,
 						const struct location *loc);
diff --git a/src/netlink.c b/src/netlink.c
index 102f799..7d3e71f 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -1444,6 +1444,28 @@ int netlink_batch_send(struct list_head *err_list)
 	return mnl_batch_talk(nf_sock, err_list);
 }
 
+int netlink_flush_ruleset(struct netlink_ctx *ctx, const struct handle *h,
+			  const struct location *loc)
+{
+	int err;
+	struct nft_table *nlt;
+
+	if (!ctx->batch_supported) {
+		netlink_io_error(ctx, loc, "Operation not supported.");
+		return -1;
+	}
+
+	nlt = alloc_nft_table(h);
+	err = mnl_nft_table_batch_del(nf_sock, nlt, 0, ctx->seqnum);
+	nft_table_free(nlt);
+
+	if (err < 0)
+		netlink_io_error(ctx, loc, "Could not flush the ruleset: %s",
+				 strerror(errno));
+
+	return err;
+}
+
 struct nft_ruleset *netlink_dump_ruleset(struct netlink_ctx *ctx,
 					 const struct handle *h,
 					 const struct location *loc)
diff --git a/src/parser.y b/src/parser.y
index d7bc287..c6df10a 100644
--- a/src/parser.y
+++ b/src/parser.y
@@ -187,6 +187,7 @@ static int monitor_lookup_event(const char *event)
 %token ELEMENT			"element"
 %token MAP			"map"
 %token HANDLE			"handle"
+%token RULESET			"ruleset"
 
 %token INET			"inet"
 
@@ -395,11 +396,11 @@ static int monitor_lookup_event(const char *event)
 %type <cmd>			base_cmd add_cmd create_cmd insert_cmd delete_cmd list_cmd flush_cmd rename_cmd export_cmd monitor_cmd
 %destructor { cmd_free($$); }	base_cmd add_cmd create_cmd insert_cmd delete_cmd list_cmd flush_cmd rename_cmd export_cmd monitor_cmd
 
-%type <handle>			table_spec tables_spec chain_spec chain_identifier ruleid_spec
-%destructor { handle_free(&$$); } table_spec tables_spec chain_spec chain_identifier ruleid_spec
+%type <handle>			table_spec tables_spec chain_spec chain_identifier ruleid_spec ruleset_spec
+%destructor { handle_free(&$$); } table_spec tables_spec chain_spec chain_identifier ruleid_spec ruleset_spec
 %type <handle>			set_spec set_identifier
 %destructor { handle_free(&$$); } set_spec set_identifier
-%type <val>			handle_spec family_spec position_spec
+%type <val>			handle_spec family_spec family_spec_explicit position_spec
 
 %type <table>			table_block_alloc table_block
 %destructor { close_scope(state); table_free($$); }	table_block_alloc
@@ -773,6 +774,10 @@ flush_cmd		:	TABLE		table_spec
 			{
 				$$ = cmd_alloc(CMD_FLUSH, CMD_OBJ_SET, &$2, &@$, NULL);
 			}
+			|	RULESET		ruleset_spec
+			{
+				$$ = cmd_alloc(CMD_FLUSH, CMD_OBJ_RULESET, &$2, &@$, NULL);
+			}
 			;
 
 rename_cmd		:	CHAIN		chain_spec	identifier
@@ -1162,8 +1167,11 @@ string			:	STRING
 			|	QUOTED_STRING
 			;
 
-family_spec		:	/* empty */	{ $$ = NFPROTO_IPV4; }
-			|	IP		{ $$ = NFPROTO_IPV4; }
+family_spec		:	/* empty */		{ $$ = NFPROTO_IPV4; }
+			|	family_spec_explicit
+			;
+
+family_spec_explicit	:	IP		{ $$ = NFPROTO_IPV4; }
 			|	IP6		{ $$ = NFPROTO_IPV6; }
 			|	INET		{ $$ = NFPROTO_INET; }
 			|	ARP		{ $$ = NFPROTO_ARP; }
@@ -1252,6 +1260,18 @@ comment_spec		:	/* empty */
 			}
 			;
 
+ruleset_spec		:	/* empty */
+			{
+				memset(&$$, 0, sizeof($$));
+				$$.family	= NFPROTO_UNSPEC;
+			}
+			|	family_spec_explicit
+			{
+				memset(&$$, 0, sizeof($$));
+				$$.family	= $1;
+			}
+			;
+
 rule			:	stmt_list	comment_spec
 			{
 				struct stmt *i;
diff --git a/src/rule.c b/src/rule.c
index 1e54526..cb2a228 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -820,6 +820,8 @@ static int do_command_flush(struct netlink_ctx *ctx, struct cmd *cmd)
 		return netlink_flush_table(ctx, &cmd->handle, &cmd->location);
 	case CMD_OBJ_CHAIN:
 		return netlink_flush_chain(ctx, &cmd->handle, &cmd->location);
+	case CMD_OBJ_RULESET:
+		return netlink_flush_ruleset(ctx, &cmd->handle, &cmd->location);
 	default:
 		BUG("invalid command object type %u\n", cmd->obj);
 	}
diff --git a/src/scanner.l b/src/scanner.l
index b7a00b4..f45c61c 100644
--- a/src/scanner.l
+++ b/src/scanner.l
@@ -240,6 +240,7 @@ addrstring	({macaddr}|{ip4addr}|{ip6addr})
 "element"		{ return ELEMENT; }
 "map"			{ return MAP; }
 "handle"		{ return HANDLE; }
+"ruleset"		{ return RULESET; }
 
 "accept"		{ return ACCEPT; }
 "drop"			{ return DROP; }
-- 
1.7.10.4

[nf_tables PATCH 2/6 v5] 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>
---
v2: rebased
v3: no changes, resending the series.
v4: no changes, resending the series because v3 series is invalid.
v5: no changes, resending the series.

 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 3664bab..90e3496 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
  */
@@ -1157,11 +1166,7 @@ static int nf_tables_newchain(struct sock *nlsk, struct sk_buff *skb,
 	list_add_tail_rcu(&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;
@@ -3368,11 +3373,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));
@@ -3495,11 +3498,9 @@ static int nf_tables_abort(struct sk_buff *skb)
 			} else {
 				trans->ctx.table->use--;
 				list_del_rcu(&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:
-- 
1.7.10.4

[nf_tables PATCH 3/6 v5] 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>
---
v2: no changes, resending the series.
v3: no changes, resending the series.
v4: no changes, resending the series, v3 series was invalid.
v5: no changes, resending the series.

 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 90e3496..eac4fab 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1902,7 +1902,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;
@@ -1955,12 +1955,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;
 		}
-- 
1.7.10.4

[nf_tables PATCH 4/6 v5] 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>
---
v2: rebased: now, nft_del*() functions use list_del_rcu().
v3: rebased on top of patch 1/5.
v4: The v3 version had several issues due to the rebase. Fixed in this v4.
v5: no changes, resending the series.

 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 eac4fab..8f9bcce 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_rcu(&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_rcu(&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_deactivate(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_deactivate(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_rcu(&set->list);
+	ctx->table->use--;
+
+	return err;
+}
+
 /*
  * Tables
  */
@@ -318,9 +507,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[])
@@ -452,21 +638,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[])
@@ -544,7 +715,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);
@@ -560,12 +731,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_rcu(&table->list);
-	return 0;
+	return nft_deltable(&ctx);
 }
 
 static void nf_tables_table_destroy(struct nft_ctx *ctx)
@@ -922,21 +1089,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);
@@ -1183,7 +1335,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))
@@ -1204,13 +1355,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_rcu(&chain->list);
-	return 0;
+	return nft_delchain(&ctx);
 }
 
 /*
@@ -1532,41 +1678,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)
 {
@@ -1692,21 +1803,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;
@@ -1872,49 +1968,6 @@ err1:
 	return err;
 }
 
-static int
-nf_tables_delrule_deactivate(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_deactivate(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[])
@@ -2343,8 +2396,6 @@ static int nf_tables_dump_sets_done(struct netlink_callback *cb)
 	return 0;
 }
 
-#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[])
@@ -2419,26 +2470,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[])
@@ -2632,13 +2663,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_rcu(&set->list);
-	ctx.table->use--;
-	return 0;
+	return nft_delset(&ctx, set);
 }
 
 static int nf_tables_bind_check_setelem(const struct nft_ctx *ctx,
-- 
1.7.10.4

[nf_tables PATCH 5/6 v5] 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>
---
v2: address comments by Pablo:
    * don't return EINVAL if called with AF_UNSPEC and a concrete table.
    * A more generic function, nft_flush()

v3: no changes, resending the series.
v4: no changes, resending the series because v3 series was invalid.
v5: address comment by Pablo: delete set if the list of bindings is empty.

 net/netfilter/nf_tables_api.c |   75 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 71 insertions(+), 4 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 8f9bcce..2520ba7 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -707,6 +707,69 @@ 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 &&
+		    !list_empty(&set->bindings))
+			continue;
+
+		err = nft_delset(ctx, set);
+		if (err < 0)
+			goto out;
+	}
+
+	err = nft_deltable(ctx);
+out:
+	return err;
+}
+
+static int nft_flush(struct nft_ctx *ctx, int family)
+{
+	struct nft_af_info *afi;
+	struct nft_table *table, *nt;
+	const struct nlattr * const *nla = ctx->nla;
+	int err = 0;
+
+	list_for_each_entry(afi, &ctx->net->nft.af_info, list) {
+		if (family != AF_UNSPEC && afi->family != family)
+			continue;
+
+		ctx->afi = afi;
+
+		list_for_each_entry_safe(table, nt, &afi->tables, list) {
+			if (nla[NFTA_TABLE_NAME] &&
+			    nla_strcmp(nla[NFTA_TABLE_NAME], table->name) != 0)
+				continue;
+
+			ctx->table = table;
+
+			err = nft_flush_table(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[])
@@ -718,6 +781,11 @@ static int nf_tables_deltable(struct sock *nlsk, struct sk_buff *skb,
 	int family = nfmsg->nfgen_family;
 	struct nft_ctx ctx;
 
+	nft_ctx_init(&ctx, skb, nlh, NULL, NULL, NULL, nla);
+	if (family == AF_UNSPEC || nla[NFTA_TABLE_NAME] == NULL)
+		return nft_flush(&ctx, family);
+
+
 	afi = nf_tables_afinfo_lookup(net, family, false);
 	if (IS_ERR(afi))
 		return PTR_ERR(afi);
@@ -727,12 +795,11 @@ static int nf_tables_deltable(struct sock *nlsk, struct sk_buff *skb,
 		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);
+	ctx.afi = afi;
+	ctx.table = table;
 
-	return nft_deltable(&ctx);
+	return nft_flush_table(&ctx);
 }
 
 static void nf_tables_table_destroy(struct nft_ctx *ctx)
-- 
1.7.10.4

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

[Patrick McHardy]

On Tue, Sep 02, 2014 at 04:42:26PM +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>
> ---
> v2: address comments by Pablo:
>     * don't return EINVAL if called with AF_UNSPEC and a concrete table.
>     * A more generic function, nft_flush()
> 
> v3: no changes, resending the series.
> v4: no changes, resending the series because v3 series was invalid.
> v5: address comment by Pablo: delete set if the list of bindings is empty.

> +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 &&
> +		    !list_empty(&set->bindings))
> +			continue;

So we're removing anonymous sets iff the bindings are empty. I feel I'm
missing something:

- how could we possibly still have bindings after the table has been flushed?
- if that were possible, why wouldn't it also apply to non-anonymous sets?

If I'm not mistaken we should be able to unconditionally delete all sets
once the ruleset has been flushed.

> +		err = nft_delset(ctx, set);
> +		if (err < 0)
> +			goto out;
> +	}

Re: [nf_tables PATCH 4/6 v5] netfilter: nf_tables: add helpers to schedule objects deletion

[Patrick McHardy]

On Tue, Sep 02, 2014 at 04:42:25PM +0200, Arturo Borrero Gonzalez wrote:
> 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>
> ---
> v2: rebased: now, nft_del*() functions use list_del_rcu().
> v3: rebased on top of patch 1/5.
> v4: The v3 version had several issues due to the rebase. Fixed in this v4.
> v5: no changes, resending the series.


> +static inline void
> +nft_rule_disactivate_next(struct net *net, struct nft_rule *rule)
> +{
> +	rule->genmask = (1 << gencursor_next(net));
> +}

Since you're touching this anyway please get rid of the disactivate here as
well and use deactivate consistently.

> +static int
> +nf_tables_delrule_deactivate(struct nft_ctx *ctx, struct nft_rule *rule)
> +{

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

[Pablo Neira Ayuso]

On Tue, Sep 02, 2014 at 04:12:41PM +0100, Patrick McHardy wrote:
> On Tue, Sep 02, 2014 at 04:42:26PM +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>
> > ---
> > v2: address comments by Pablo:
> >     * don't return EINVAL if called with AF_UNSPEC and a concrete table.
> >     * A more generic function, nft_flush()
> > 
> > v3: no changes, resending the series.
> > v4: no changes, resending the series because v3 series was invalid.
> > v5: address comment by Pablo: delete set if the list of bindings is empty.
> 
> > +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 &&
> > +		    !list_empty(&set->bindings))
> > +			continue;
> 
> So we're removing anonymous sets iff the bindings are empty. I feel I'm
> missing something:
> 
> - how could we possibly still have bindings after the table has been flushed?

We remove break the binding until we reach the commit path. The table
is not actually flushed from nft_flush_table, instead the objects are
marked to be removed from the commit path.

The rule <-> set binding remains there until we reach the commit path.

> - if that were possible, why wouldn't it also apply to non-anonymous sets?

Non-anonymous sets need to be explicitly destroyed after the rule is
removed.

> If I'm not mistaken we should be able to unconditionally delete all sets
> once the ruleset has been flushed.

We have to keep bound-to-rule anonymous set until we reach the commit
path, packets may still be walking on it.

If we don't skip the removal of bound-to-rule anonymous sets, we'll
crash since we'll try to delete them twice in the commit path, once
from _DELRULE and again from _DELSET.

Re: [nf_tables PATCH 4/6 v5] netfilter: nf_tables: add helpers to schedule objects deletion

[Pablo Neira Ayuso]

On Tue, Sep 02, 2014 at 04:20:01PM +0100, Patrick McHardy wrote:
> On Tue, Sep 02, 2014 at 04:42:25PM +0200, Arturo Borrero Gonzalez wrote:
> > 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>
> > ---
> > v2: rebased: now, nft_del*() functions use list_del_rcu().
> > v3: rebased on top of patch 1/5.
> > v4: The v3 version had several issues due to the rebase. Fixed in this v4.
> > v5: no changes, resending the series.
> 
> 
> > +static inline void
> > +nft_rule_disactivate_next(struct net *net, struct nft_rule *rule)
> > +{
> > +	rule->genmask = (1 << gencursor_next(net));
> > +}
> 
> Since you're touching this anyway please get rid of the disactivate here as
> well and use deactivate consistently.

Arturo, I can fix that here. That word I made up myself (which looks
to Spanish...) was my fault after all :).

Re: [nf_tables PATCH 1/6 v5] netfilter: nf_tables: refactor rule deletion helper

[Pablo Neira Ayuso]

On Tue, Sep 02, 2014 at 04:42:21PM +0200, Arturo Borrero Gonzalez wrote:
> 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().

Applied, thanks.

Re: [nf_tables PATCH 2/6 v5] netfilter: nf_tables: add helper to unregister chain hooks

[Pablo Neira Ayuso]

On Tue, Sep 02, 2014 at 04:42:23PM +0200, Arturo Borrero Gonzalez wrote:
> 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.

Applied, thanks.

Re: [nf_tables PATCH 3/6 v5] netfilter: nf_tables: rename nf_table_delrule_by_chain()

[Pablo Neira Ayuso]

On Tue, Sep 02, 2014 at 04:42:24PM +0200, Arturo Borrero Gonzalez wrote:
> For the sake of homogenize the function naming scheme, let's rename
> nf_table_delrule_by_chain() to nft_delrule_by_chain().

Applied, thanks.

Re: [nf_tables PATCH 4/6 v5] netfilter: nf_tables: add helpers to schedule objects deletion

[Pablo Neira Ayuso]

On Tue, Sep 02, 2014 at 04:42:25PM +0200, Arturo Borrero Gonzalez wrote:
> 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.

Applied, thanks Arturo.

I have also renamed disactivate to deactivate as requested by Patrick.

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

[Pablo Neira Ayuso]

On Tue, Sep 02, 2014 at 04:42:26PM +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.

Also applied, thanks Arturo.