Skip to contentSkip to conversation

netfilter: nf_tables: add nf_tables_updchain()

PATCH In review Pablo Neira Ayuso 1 participant 2 messages Sep 3, 2017

Subsystems (2)
Networking netfilter
lore
Share & tools
lore

Pablo Neira Ayuso

author maintainer
#0 lore raw

PATCH 1/5 netfilter: nf_tables: add nf_tables_updchain()

nf_tables_newchain() is too large, wrap the chain update path in a function to make it more maintainable.

Signed-off-by Pablo Neira Ayuso <[email protected]>
---
 net/netfilter/nf_tables_api.c | 170 +++++++++++++++++++++++-------------------
 1 file changed, 92 insertions(+), 78 deletions(-)
net/netfilter/nf_tables_api.c +92 -78
index 149785ff1c7b..14695062a925 100644
@@ -1335,6 +1335,97 @@ static void nft_chain_release_hook(struct nft_chain_hook *hook)
 		dev_put(hook->dev);
 }
 
+static int nf_tables_updchain(struct nft_ctx *ctx, u8 genmask, u8 policy,
+			      bool create)
+{
+	const struct nlattr * const *nla = ctx->nla;
+	struct nft_table *table = ctx->table;
+	struct nft_chain *chain = ctx->chain;
+	struct nft_af_info *afi = ctx->afi;
+	struct nft_base_chain *basechain;
+	struct nft_stats *stats = NULL;
+	struct nft_chain_hook hook;
+	const struct nlattr *name;
+	struct nf_hook_ops *ops;
+	struct nft_trans *trans;
+	int err, i;
+
+	if (nla[NFTA_CHAIN_HOOK]) {
+		if (!nft_is_base_chain(chain))
+			return -EBUSY;
+
+		err = nft_chain_parse_hook(ctx->net, nla, ctx->afi, &hook,
+					   create);
+		if (err < 0)
+			return err;
+
+		basechain = nft_base_chain(chain);
+		if (basechain->type != hook.type) {
+			nft_chain_release_hook(&hook);
+			return -EBUSY;
+		}
+
+		for (i = 0; i < afi->nops; i++) {
+			ops = &basechain->ops[i];
+			if (ops->hooknum != hook.num ||
+			    ops->priority != hook.priority ||
+			    ops->dev != hook.dev) {
+				nft_chain_release_hook(&hook);
+				return -EBUSY;
+			}
+		}
+		nft_chain_release_hook(&hook);
+	}
+
+	if (nla[NFTA_CHAIN_HANDLE] &&
+	    nla[NFTA_CHAIN_NAME]) {
+		struct nft_chain *chain2;
+
+		chain2 = nf_tables_chain_lookup(table, nla[NFTA_CHAIN_NAME],
+						genmask);
+		if (IS_ERR(chain2))
+			return PTR_ERR(chain2);
+	}
+
+	if (nla[NFTA_CHAIN_COUNTERS]) {
+		if (!nft_is_base_chain(chain))
+			return -EOPNOTSUPP;
+
+		stats = nft_stats_alloc(nla[NFTA_CHAIN_COUNTERS]);
+		if (IS_ERR(stats))
+			return PTR_ERR(stats);
+	}
+
+	trans = nft_trans_alloc(ctx, NFT_MSG_NEWCHAIN,
+				sizeof(struct nft_trans_chain));
+	if (trans == NULL) {
+		free_percpu(stats);
+		return -ENOMEM;
+	}
+
+	nft_trans_chain_stats(trans) = stats;
+	nft_trans_chain_update(trans) = true;
+
+	if (nla[NFTA_CHAIN_POLICY])
+		nft_trans_chain_policy(trans) = policy;
+	else
+		nft_trans_chain_policy(trans) = -1;
+
+	name = nla[NFTA_CHAIN_NAME];
+	if (nla[NFTA_CHAIN_HANDLE] && name) {
+		nft_trans_chain_name(trans) =
+			nla_strdup(name, GFP_KERNEL);
+		if (!nft_trans_chain_name(trans)) {
+			kfree(trans);
+			free_percpu(stats);
+			return -ENOMEM;
+		}
+	}
+	list_add_tail(&trans->list, &ctx->net->nft.commit_list);
+
+	return 0;
+}
+
 static int nf_tables_newchain(struct net *net, struct sock *nlsk,
 			      struct sk_buff *skb, const struct nlmsghdr *nlh,
 			      const struct nlattr * const nla[],

@@ -1403,91 +1494,14 @@ static int nf_tables_newchain(struct net *net, struct sock *nlsk,
 	}
 
 	if (chain != NULL) {
-		struct nft_stats *stats = NULL;
-		struct nft_trans *trans;
-
 		if (nlh->nlmsg_flags & NLM_F_EXCL)
 			return -EEXIST;
 		if (nlh->nlmsg_flags & NLM_F_REPLACE)
 			return -EOPNOTSUPP;
 
-		if (nla[NFTA_CHAIN_HOOK]) {
-			struct nft_base_chain *basechain;
-			struct nft_chain_hook hook;
-			struct nf_hook_ops *ops;
-
-			if (!nft_is_base_chain(chain))
-				return -EBUSY;
-
-			err = nft_chain_parse_hook(net, nla, afi, &hook,
-						   create);
-			if (err < 0)
-				return err;
-
-			basechain = nft_base_chain(chain);
-			if (basechain->type != hook.type) {
-				nft_chain_release_hook(&hook);
-				return -EBUSY;
-			}
-
-			for (i = 0; i < afi->nops; i++) {
-				ops = &basechain->ops[i];
-				if (ops->hooknum != hook.num ||
-				    ops->priority != hook.priority ||
-				    ops->dev != hook.dev) {
-					nft_chain_release_hook(&hook);
-					return -EBUSY;
-				}
-			}
-			nft_chain_release_hook(&hook);
-		}
-
-		if (nla[NFTA_CHAIN_HANDLE] && name) {
-			struct nft_chain *chain2;
-
-			chain2 = nf_tables_chain_lookup(table,
-							nla[NFTA_CHAIN_NAME],
-							genmask);
-			if (IS_ERR(chain2))
-				return PTR_ERR(chain2);
-		}
-
-		if (nla[NFTA_CHAIN_COUNTERS]) {
-			if (!nft_is_base_chain(chain))
-				return -EOPNOTSUPP;
-
-			stats = nft_stats_alloc(nla[NFTA_CHAIN_COUNTERS]);
-			if (IS_ERR(stats))
-				return PTR_ERR(stats);
-		}
-
 		nft_ctx_init(&ctx, net, skb, nlh, afi, table, chain, nla);
-		trans = nft_trans_alloc(&ctx, NFT_MSG_NEWCHAIN,
-					sizeof(struct nft_trans_chain));
-		if (trans == NULL) {
-			free_percpu(stats);
-			return -ENOMEM;
-		}
-
-		nft_trans_chain_stats(trans) = stats;
-		nft_trans_chain_update(trans) = true;
 
-		if (nla[NFTA_CHAIN_POLICY])
-			nft_trans_chain_policy(trans) = policy;
-		else
-			nft_trans_chain_policy(trans) = -1;
-
-		if (nla[NFTA_CHAIN_HANDLE] && name) {
-			nft_trans_chain_name(trans) =
-				nla_strdup(name, GFP_KERNEL);
-			if (!nft_trans_chain_name(trans)) {
-				kfree(trans);
-				free_percpu(stats);
-				return -ENOMEM;
-			}
-		}
-		list_add_tail(&trans->list, &net->nft.commit_list);
-		return 0;
+		return nf_tables_updchain(&ctx, genmask, policy, create);
 	}
 
 	if (table->use == UINT_MAX)
Signature · 1 line
2.1.4
author maintainer
#1 lore raw

PATCH 2/5 netfilter: nf_tables: add nf_tables_addchain()

Wrap the chain addition path in a function to make it more maintainable.

Signed-off-by Pablo Neira Ayuso <[email protected]>
---
 net/netfilter/nf_tables_api.c | 199 ++++++++++++++++++++++--------------------
 1 file changed, 106 insertions(+), 93 deletions(-)
net/netfilter/nf_tables_api.c +106 -93
index 14695062a925..8b86acbb9770 100644
@@ -1335,6 +1335,106 @@ static void nft_chain_release_hook(struct nft_chain_hook *hook)
 		dev_put(hook->dev);
 }
 
+static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
+			      u8 policy, bool create)
+{
+	const struct nlattr * const *nla = ctx->nla;
+	struct nft_table *table = ctx->table;
+	struct nft_af_info *afi = ctx->afi;
+	struct nft_base_chain *basechain;
+	struct nft_stats __percpu *stats;
+	struct net *net = ctx->net;
+	struct nft_chain *chain;
+	unsigned int i;
+	int err;
+
+	if (table->use == UINT_MAX)
+		return -EOVERFLOW;
+
+	if (nla[NFTA_CHAIN_HOOK]) {
+		struct nft_chain_hook hook;
+		struct nf_hook_ops *ops;
+		nf_hookfn *hookfn;
+
+		err = nft_chain_parse_hook(net, nla, afi, &hook, create);
+		if (err < 0)
+			return err;
+
+		basechain = kzalloc(sizeof(*basechain), GFP_KERNEL);
+		if (basechain == NULL) {
+			nft_chain_release_hook(&hook);
+			return -ENOMEM;
+		}
+
+		if (hook.dev != NULL)
+			strncpy(basechain->dev_name, hook.dev->name, IFNAMSIZ);
+
+		if (nla[NFTA_CHAIN_COUNTERS]) {
+			stats = nft_stats_alloc(nla[NFTA_CHAIN_COUNTERS]);
+			if (IS_ERR(stats)) {
+				nft_chain_release_hook(&hook);
+				kfree(basechain);
+				return PTR_ERR(stats);
+			}
+			basechain->stats = stats;
+			static_branch_inc(&nft_counters_enabled);
+		}
+
+		hookfn = hook.type->hooks[hook.num];
+		basechain->type = hook.type;
+		chain = &basechain->chain;
+
+		for (i = 0; i < afi->nops; i++) {
+			ops = &basechain->ops[i];
+			ops->pf		= family;
+			ops->hooknum	= hook.num;
+			ops->priority	= hook.priority;
+			ops->priv	= chain;
+			ops->hook	= afi->hooks[ops->hooknum];
+			ops->dev	= hook.dev;
+			if (hookfn)
+				ops->hook = hookfn;
+			if (afi->hook_ops_init)
+				afi->hook_ops_init(ops, i);
+		}
+
+		chain->flags |= NFT_BASE_CHAIN;
+		basechain->policy = policy;
+	} else {
+		chain = kzalloc(sizeof(*chain), GFP_KERNEL);
+		if (chain == NULL)
+			return -ENOMEM;
+	}
+	INIT_LIST_HEAD(&chain->rules);
+	chain->handle = nf_tables_alloc_handle(table);
+	chain->table = table;
+	chain->name = nla_strdup(nla[NFTA_CHAIN_NAME], GFP_KERNEL);
+	if (!chain->name) {
+		err = -ENOMEM;
+		goto err1;
+	}
+
+	err = nf_tables_register_hooks(net, table, chain, afi->nops);
+	if (err < 0)
+		goto err1;
+
+	ctx->chain = chain;
+	err = nft_trans_chain_add(ctx, NFT_MSG_NEWCHAIN);
+	if (err < 0)
+		goto err2;
+
+	table->use++;
+	list_add_tail_rcu(&chain->list, &table->chains);
+
+	return 0;
+err2:
+	nf_tables_unregister_hooks(net, table, chain, afi->nops);
+err1:
+	nf_tables_chain_destroy(chain);
+
+	return err;
+}
+
 static int nf_tables_updchain(struct nft_ctx *ctx, u8 genmask, u8 policy,
 			      bool create)
 {

@@ -1433,19 +1533,15 @@ static int nf_tables_newchain(struct net *net, struct sock *nlsk,
 {
 	const struct nfgenmsg *nfmsg = nlmsg_data(nlh);
 	const struct nlattr * uninitialized_var(name);
+	u8 genmask = nft_genmask_next(net);
+	int family = nfmsg->nfgen_family;
 	struct nft_af_info *afi;
 	struct nft_table *table;
 	struct nft_chain *chain;
-	struct nft_base_chain *basechain = NULL;
-	u8 genmask = nft_genmask_next(net);
-	int family = nfmsg->nfgen_family;
 	u8 policy = NF_ACCEPT;
+	struct nft_ctx ctx;
 	u64 handle = 0;
-	unsigned int i;
-	struct nft_stats __percpu *stats;
-	int err;
 	bool create;
-	struct nft_ctx ctx;
 
 	create = nlh->nlmsg_flags & NLM_F_CREATE ? true : false;
 

@@ -1493,101 +1589,18 @@ static int nf_tables_newchain(struct net *net, struct sock *nlsk,
 		}
 	}
 
+	nft_ctx_init(&ctx, net, skb, nlh, afi, table, chain, nla);
+
 	if (chain != NULL) {
 		if (nlh->nlmsg_flags & NLM_F_EXCL)
 			return -EEXIST;
 		if (nlh->nlmsg_flags & NLM_F_REPLACE)
 			return -EOPNOTSUPP;
 
-		nft_ctx_init(&ctx, net, skb, nlh, afi, table, chain, nla);
-
 		return nf_tables_updchain(&ctx, genmask, policy, create);
 	}
 
-	if (table->use == UINT_MAX)
-		return -EOVERFLOW;
-
-	if (nla[NFTA_CHAIN_HOOK]) {
-		struct nft_chain_hook hook;
-		struct nf_hook_ops *ops;
-		nf_hookfn *hookfn;
-
-		err = nft_chain_parse_hook(net, nla, afi, &hook, create);
-		if (err < 0)
-			return err;
-
-		basechain = kzalloc(sizeof(*basechain), GFP_KERNEL);
-		if (basechain == NULL) {
-			nft_chain_release_hook(&hook);
-			return -ENOMEM;
-		}
-
-		if (hook.dev != NULL)
-			strncpy(basechain->dev_name, hook.dev->name, IFNAMSIZ);
-
-		if (nla[NFTA_CHAIN_COUNTERS]) {
-			stats = nft_stats_alloc(nla[NFTA_CHAIN_COUNTERS]);
-			if (IS_ERR(stats)) {
-				nft_chain_release_hook(&hook);
-				kfree(basechain);
-				return PTR_ERR(stats);
-			}
-			basechain->stats = stats;
-			static_branch_inc(&nft_counters_enabled);
-		}
-
-		hookfn = hook.type->hooks[hook.num];
-		basechain->type = hook.type;
-		chain = &basechain->chain;
-
-		for (i = 0; i < afi->nops; i++) {
-			ops = &basechain->ops[i];
-			ops->pf		= family;
-			ops->hooknum	= hook.num;
-			ops->priority	= hook.priority;
-			ops->priv	= chain;
-			ops->hook	= afi->hooks[ops->hooknum];
-			ops->dev	= hook.dev;
-			if (hookfn)
-				ops->hook = hookfn;
-			if (afi->hook_ops_init)
-				afi->hook_ops_init(ops, i);
-		}
-
-		chain->flags |= NFT_BASE_CHAIN;
-		basechain->policy = policy;
-	} else {
-		chain = kzalloc(sizeof(*chain), GFP_KERNEL);
-		if (chain == NULL)
-			return -ENOMEM;
-	}
-
-	INIT_LIST_HEAD(&chain->rules);
-	chain->handle = nf_tables_alloc_handle(table);
-	chain->table = table;
-	chain->name = nla_strdup(name, GFP_KERNEL);
-	if (!chain->name) {
-		err = -ENOMEM;
-		goto err1;
-	}
-
-	err = nf_tables_register_hooks(net, table, chain, afi->nops);
-	if (err < 0)
-		goto err1;
-
-	nft_ctx_init(&ctx, net, skb, nlh, afi, table, chain, nla);
-	err = nft_trans_chain_add(&ctx, NFT_MSG_NEWCHAIN);
-	if (err < 0)
-		goto err2;
-
-	table->use++;
-	list_add_tail_rcu(&chain->list, &table->chains);
-	return 0;
-err2:
-	nf_tables_unregister_hooks(net, table, chain, afi->nops);
-err1:
-	nf_tables_chain_destroy(chain);
-	return err;
+	return nf_tables_addchain(&ctx, family, genmask, policy, create);
 }
 
 static int nf_tables_delchain(struct net *net, struct sock *nlsk,
Signature · 1 line
2.1.4