diff mbox series

[nf-next,1/2] netfilter: nf_conncount: Refactor nf_conncount

Message ID 1519856382-40212-1-git-send-email-yihung.wei@gmail.com
State Changes Requested
Delegated to: Pablo Neira
Headers show
Series [nf-next,1/2] netfilter: nf_conncount: Refactor nf_conncount | expand

Commit Message

Yi-Hung Wei Feb. 28, 2018, 10:19 p.m. UTC 
This patch contains two parts.

1. Remove parameter 'family' in nf_conncount_count() and count_tree().
Before commit 625c556118f3 ("netfilter: connlimit: split xt_connlimit
into front and backend"), 'family' was used to determine the type
of nf_inet_addr, but the parameter is not useful after that commit.

2. Move nf_ct_netns_get/put() to the user of nf_conncount.
Since nf_conncount now supports general keys, if the key is not related
to a particular NFPROTO_*, then it is not necessary to do
nf_ct_netns_get/put() in nf_conncount.

Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com>
---
 include/net/netfilter/nf_conntrack_count.h |  6 ++----
 net/netfilter/nf_conncount.c               | 19 ++++---------------
 net/netfilter/xt_connlimit.c               | 16 ++++++++++++----
 3 files changed, 18 insertions(+), 23 deletions(-)

Comments

Florian Westphal March 1, 2018, 8:07 a.m. UTC | #1 
Yi-Hung Wei <yihung.wei@gmail.com> wrote:
> This patch contains two parts.
> 
> 1. Remove parameter 'family' in nf_conncount_count() and count_tree().
> Before commit 625c556118f3 ("netfilter: connlimit: split xt_connlimit
> into front and backend"), 'family' was used to determine the type
> of nf_inet_addr, but the parameter is not useful after that commit.

Right, its useless now, lets remove it.

> 2. Move nf_ct_netns_get/put() to the user of nf_conncount.
> Since nf_conncount now supports general keys, if the key is not related
> to a particular NFPROTO_*, then it is not necessary to do
> nf_ct_netns_get/put() in nf_conncount.

I wonder if this is correct.

conncount relies on all entries being backed by a conntrack entry so
it can expire those that are no longer around.
--
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
Yi-Hung Wei March 1, 2018, 7:28 p.m. UTC | #2 
On Thu, Mar 1, 2018 at 12:07 AM, Florian Westphal <fw@strlen.de> wrote:
>> 2. Move nf_ct_netns_get/put() to the user of nf_conncount.
>> Since nf_conncount now supports general keys, if the key is not related
>> to a particular NFPROTO_*, then it is not necessary to do
>> nf_ct_netns_get/put() in nf_conncount.
>
> I wonder if this is correct.
>
> conncount relies on all entries being backed by a conntrack entry so
> it can expire those that are no longer around.

Thanks for the comment.

You are right.  I will drop the second part of this patch in v2.

Thanks,

-Yi-Hung
--
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
diff mbox series

Patch

diff --git a/include/net/netfilter/nf_conntrack_count.h b/include/net/netfilter/nf_conntrack_count.h
index adf8db44cf86..c4f33f762ceb 100644
--- a/include/net/netfilter/nf_conntrack_count.h
+++ b/include/net/netfilter/nf_conntrack_count.h
@@ -3,15 +3,13 @@ 
 
 struct nf_conncount_data;
 
-struct nf_conncount_data *nf_conncount_init(struct net *net, unsigned int family,
+struct nf_conncount_data *nf_conncount_init(struct net *net,
 					    unsigned int keylen);
-void nf_conncount_destroy(struct net *net, unsigned int family,
-			  struct nf_conncount_data *data);
+void nf_conncount_destroy(struct nf_conncount_data *data);
 
 unsigned int nf_conncount_count(struct net *net,
 				struct nf_conncount_data *data,
 				const u32 *key,
-				unsigned int family,
 				const struct nf_conntrack_tuple *tuple,
 				const struct nf_conntrack_zone *zone);
 #endif
diff --git a/net/netfilter/nf_conncount.c b/net/netfilter/nf_conncount.c
index 6d65389e308f..91b13142631e 100644
--- a/net/netfilter/nf_conncount.c
+++ b/net/netfilter/nf_conncount.c
@@ -158,7 +158,6 @@  static void tree_nodes_free(struct rb_root *root,
 static unsigned int
 count_tree(struct net *net, struct rb_root *root,
 	   const u32 *key, u8 keylen,
-	   u8 family,
 	   const struct nf_conntrack_tuple *tuple,
 	   const struct nf_conntrack_zone *zone)
 {
@@ -246,7 +245,6 @@  count_tree(struct net *net, struct rb_root *root,
 unsigned int nf_conncount_count(struct net *net,
 				struct nf_conncount_data *data,
 				const u32 *key,
-				unsigned int family,
 				const struct nf_conntrack_tuple *tuple,
 				const struct nf_conntrack_zone *zone)
 {
@@ -259,7 +257,7 @@  unsigned int nf_conncount_count(struct net *net,
 
 	spin_lock_bh(&nf_conncount_locks[hash % CONNCOUNT_LOCK_SLOTS]);
 
-	count = count_tree(net, root, key, data->keylen, family, tuple, zone);
+	count = count_tree(net, root, key, data->keylen, tuple, zone);
 
 	spin_unlock_bh(&nf_conncount_locks[hash % CONNCOUNT_LOCK_SLOTS]);
 
@@ -267,11 +265,11 @@  unsigned int nf_conncount_count(struct net *net,
 }
 EXPORT_SYMBOL_GPL(nf_conncount_count);
 
-struct nf_conncount_data *nf_conncount_init(struct net *net, unsigned int family,
+struct nf_conncount_data *nf_conncount_init(struct net *net,
 					    unsigned int keylen)
 {
 	struct nf_conncount_data *data;
-	int ret, i;
+	int i;
 
 	if (keylen % sizeof(u32) ||
 	    keylen / sizeof(u32) > MAX_KEYLEN ||
@@ -284,12 +282,6 @@  struct nf_conncount_data *nf_conncount_init(struct net *net, unsigned int family
 	if (!data)
 		return ERR_PTR(-ENOMEM);
 
-	ret = nf_ct_netns_get(net, family);
-	if (ret < 0) {
-		kfree(data);
-		return ERR_PTR(ret);
-	}
-
 	for (i = 0; i < ARRAY_SIZE(data->root); ++i)
 		data->root[i] = RB_ROOT;
 
@@ -318,13 +310,10 @@  static void destroy_tree(struct rb_root *r)
 	}
 }
 
-void nf_conncount_destroy(struct net *net, unsigned int family,
-			  struct nf_conncount_data *data)
+void nf_conncount_destroy(struct nf_conncount_data *data)
 {
 	unsigned int i;
 
-	nf_ct_netns_put(net, family);
-
 	for (i = 0; i < ARRAY_SIZE(data->root); ++i)
 		destroy_tree(&data->root[i]);
 
diff --git a/net/netfilter/xt_connlimit.c b/net/netfilter/xt_connlimit.c
index b1b17b9353e1..805e23155253 100644
--- a/net/netfilter/xt_connlimit.c
+++ b/net/netfilter/xt_connlimit.c
@@ -67,8 +67,8 @@  connlimit_mt(const struct sk_buff *skb, struct xt_action_param *par)
 		key[1] = zone->id;
 	}
 
-	connections = nf_conncount_count(net, info->data, key,
-					 xt_family(par), tuple_ptr, zone);
+	connections = nf_conncount_count(net, info->data, key, tuple_ptr,
+					 zone);
 	if (connections == 0)
 		/* kmalloc failed, drop it entirely */
 		goto hotdrop;
@@ -84,6 +84,7 @@  static int connlimit_mt_check(const struct xt_mtchk_param *par)
 {
 	struct xt_connlimit_info *info = par->matchinfo;
 	unsigned int keylen;
+	int ret;
 
 	keylen = sizeof(u32);
 	if (par->family == NFPROTO_IPV6)
@@ -92,10 +93,16 @@  static int connlimit_mt_check(const struct xt_mtchk_param *par)
 		keylen += sizeof(struct in_addr);
 
 	/* init private data */
-	info->data = nf_conncount_init(par->net, par->family, keylen);
+	info->data = nf_conncount_init(par->net, keylen);
 	if (IS_ERR(info->data))
 		return PTR_ERR(info->data);
 
+	ret = nf_ct_netns_get(par->net, par->family);
+	if (ret < 0) {
+		nf_conncount_destroy(info->data);
+		return ret;
+	}
+
 	return 0;
 }
 
@@ -103,7 +110,8 @@  static void connlimit_mt_destroy(const struct xt_mtdtor_param *par)
 {
 	const struct xt_connlimit_info *info = par->matchinfo;
 
-	nf_conncount_destroy(par->net, par->family, info->data);
+	nf_ct_netns_put(par->net, par->family);
+	nf_conncount_destroy(info->data);
 }
 
 static struct xt_match connlimit_mt_reg __read_mostly = {