Re: [PATCH] netfilter: xt_hashlimit: Enhance the xt_hashlimit to avoid duplicated codes

Re: [PATCH] netfilter: xt_hashlimit: Enhance the xt_hashlimit to avoid duplicated codes

[Pablo Neira Ayuso]

On Tue, Oct 21, 2014 at 11:23:16PM +0800, Feng Gao wrote:
> Hi all,
> 
> Enhance the functions "dsthash_alloc_init" and "hashlimit_mt" in file
> "xt_hashlimit.c" to avoid two duplicated codes following:
> 
> -           dh->expires = now + msecs_to_jiffies(hinfo->cfg.expire);
> -           rateinfo_recalc(dh, now, hinfo->cfg.mode);
> 
> 
> The whole patch is following

The patch seems mangled by your MUA.

BTW, you can just Cc netfilter patches to
netfilter-devel@vger.kernel.org. No need to Cc that many people.

Re: [PATCH] netfilter: xt_hashlimit: Enhance the xt_hashlimit to avoid duplicated codes

[Pablo Neira Ayuso]

On Tue, Oct 21, 2014 at 11:23:16PM +0800, Feng Gao wrote:
> Hi all,
> 
> Enhance the functions "dsthash_alloc_init" and "hashlimit_mt" in file
> "xt_hashlimit.c" to avoid two duplicated codes following:
> 
> -           dh->expires = now + msecs_to_jiffies(hinfo->cfg.expire);
> -           rateinfo_recalc(dh, now, hinfo->cfg.mode);
> 
> 
> The whole patch is following

The patch seems mangled by your MUA.

BTW, you can just Cc netfilter patches to
netfilter-devel@vger.kernel.org. No need to Cc that many people.

Re: [PATCH] netfilter: xt_hashlimit: Enhance the xt_hashlimit to avoid duplicated codes

[Feng Gao]

[-- Attachment #1: Type: text/plain, Size: 767 bytes --]

Firstly, I am sorry about many cc lists, and they are gotten by the
script "scripts/get_maintainer.pl"

Now I send the patch as attachment, is it OK?

On Tue, Oct 21, 2014 at 11:31 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Tue, Oct 21, 2014 at 11:23:16PM +0800, Feng Gao wrote:
>> Hi all,
>>
>> Enhance the functions "dsthash_alloc_init" and "hashlimit_mt" in file
>> "xt_hashlimit.c" to avoid two duplicated codes following:
>>
>> -           dh->expires = now + msecs_to_jiffies(hinfo->cfg.expire);
>> -           rateinfo_recalc(dh, now, hinfo->cfg.mode);
>>
>>
>> The whole patch is following
>
> The patch seems mangled by your MUA.
>
> BTW, you can just Cc netfilter patches to
> netfilter-devel@vger.kernel.org. No need to Cc that many people.
>

[-- Attachment #2: 0001-netfilter-Enhance-the-xt_hashlimit-to-avoid-duplicat.patch --]
[-- Type: application/octet-stream, Size: 2498 bytes --]

From 1f08fac90e191f6bb8ba236d48deebe3acad51b4 Mon Sep 17 00:00:00 2001
From: fgao <gfree.wind@gmail.com>
Date: Tue, 21 Oct 2014 08:12:00 -0700
Subject: [PATCH 1/1] netfilter: Enhance the xt_hashlimit to avoid duplicated
 codes

Signed-off-by: fgao <gfree.wind@gmail.com>
---
 net/netfilter/xt_hashlimit.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)
 mode change 100644 => 100755 net/netfilter/xt_hashlimit.c

diff --git a/net/netfilter/xt_hashlimit.c b/net/netfilter/xt_hashlimit.c
old mode 100644
new mode 100755
index 05fbc2a..06ded15
--- a/net/netfilter/xt_hashlimit.c
+++ b/net/netfilter/xt_hashlimit.c
@@ -158,7 +158,7 @@ dsthash_find(const struct xt_hashlimit_htable *ht,
 /* allocate dsthash_ent, initialize dst, put in htable and lock it */
 static struct dsthash_ent *
 dsthash_alloc_init(struct xt_hashlimit_htable *ht,
-		   const struct dsthash_dst *dst, bool *race)
+		   const struct dsthash_dst *dst, bool *new_one)
 {
 	struct dsthash_ent *ent;
 
@@ -170,7 +170,6 @@ dsthash_alloc_init(struct xt_hashlimit_htable *ht,
 	ent = dsthash_find(ht, dst);
 	if (ent != NULL) {
 		spin_unlock(&ht->lock);
-		*race = true;
 		return ent;
 	}
 
@@ -196,6 +195,8 @@ dsthash_alloc_init(struct xt_hashlimit_htable *ht,
 		ht->count++;
 	}
 	spin_unlock(&ht->lock);
+	*new_one = true;
+	
 	return ent;
 }
 
@@ -610,7 +611,7 @@ hashlimit_mt(const struct sk_buff *skb, struct xt_action_param *par)
 	unsigned long now = jiffies;
 	struct dsthash_ent *dh;
 	struct dsthash_dst dst;
-	bool race = false;
+	bool new_one = false;
 	u32 cost;
 
 	if (hashlimit_init_dst(hinfo, &dst, skb, par->thoff) < 0)
@@ -619,18 +620,16 @@ hashlimit_mt(const struct sk_buff *skb, struct xt_action_param *par)
 	rcu_read_lock_bh();
 	dh = dsthash_find(hinfo, &dst);
 	if (dh == NULL) {
-		dh = dsthash_alloc_init(hinfo, &dst, &race);
+		dh = dsthash_alloc_init(hinfo, &dst, &new_one);
 		if (dh == NULL) {
 			rcu_read_unlock_bh();
 			goto hotdrop;
-		} else if (race) {
-			/* Already got an entry, update expiration timeout */
-			dh->expires = now + msecs_to_jiffies(hinfo->cfg.expire);
-			rateinfo_recalc(dh, now, hinfo->cfg.mode);
-		} else {
-			dh->expires = jiffies + msecs_to_jiffies(hinfo->cfg.expire);
-			rateinfo_init(dh, hinfo);
 		}
+	}
+
+	if (new_one) {
+		dh->expires = jiffies + msecs_to_jiffies(hinfo->cfg.expire);
+		rateinfo_init(dh, hinfo);
 	} else {
 		/* update expiration timeout */
 		dh->expires = now + msecs_to_jiffies(hinfo->cfg.expire);
-- 
1.9.1