Patchwork ipv6: fix two typos in a comment in xfrm6_init()

login
register
mail settings
Submitter roy.qing.li@gmail.com
Date Nov. 8, 2012, 8:38 a.m.
Message ID <1352363888-14068-1-git-send-email-roy.qing.li@gmail.com>
Download mbox | patch
Permalink /patch/197805/
State Changes Requested
Delegated to: David Miller
Headers show

Comments

roy.qing.li@gmail.com - Nov. 8, 2012, 8:38 a.m.
From: Li RongQing <roy.qing.li@gmail.com>

Signed-off-by: Li RongQing <roy.qing.li@gmail.com>
---
 net/ipv6/xfrm6_policy.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
Steffen Klassert - Nov. 8, 2012, 2:19 p.m.
On Thu, Nov 08, 2012 at 04:38:08PM +0800, roy.qing.li@gmail.com wrote:
> From: Li RongQing <roy.qing.li@gmail.com>
> 
> Signed-off-by: Li RongQing <roy.qing.li@gmail.com>
> ---
>  net/ipv6/xfrm6_policy.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
> index f3ed8ca..93832e8 100644
> --- a/net/ipv6/xfrm6_policy.c
> +++ b/net/ipv6/xfrm6_policy.c
> @@ -332,10 +332,10 @@ int __init xfrm6_init(void)
>  	/*
>  	 * We need a good default value for the xfrm6 gc threshold.
>  	 * In ipv4 we set it to the route hash table size * 8, which
> -	 * is half the size of the maximaum route cache for ipv4.  It
> +	 * is half the size of the maximum route cache for ipv4.

The routing cache is removed, so this comment is obsolete. But it reminds
me that we set the gc threshold to ip_rt_max_size/2 in ipv4. With the
routing cache removal patch, ip_rt_max_size was set to INT_MAX. So the gc
starts to remove entries when a threshold of INT_MAX/2 is reached.

cat /proc/sys/net/ipv4/xfrm4_gc_thresh
1073741823

I guess this was not intentional.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - Nov. 8, 2012, 7:59 p.m.
From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Thu, 8 Nov 2012 15:19:36 +0100

> On Thu, Nov 08, 2012 at 04:38:08PM +0800, roy.qing.li@gmail.com wrote:
>> From: Li RongQing <roy.qing.li@gmail.com>
>> 
>> Signed-off-by: Li RongQing <roy.qing.li@gmail.com>
>> ---
>>  net/ipv6/xfrm6_policy.c |    4 ++--
>>  1 files changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
>> index f3ed8ca..93832e8 100644
>> --- a/net/ipv6/xfrm6_policy.c
>> +++ b/net/ipv6/xfrm6_policy.c
>> @@ -332,10 +332,10 @@ int __init xfrm6_init(void)
>>  	/*
>>  	 * We need a good default value for the xfrm6 gc threshold.
>>  	 * In ipv4 we set it to the route hash table size * 8, which
>> -	 * is half the size of the maximaum route cache for ipv4.  It
>> +	 * is half the size of the maximum route cache for ipv4.
> 
> The routing cache is removed, so this comment is obsolete. But it reminds
> me that we set the gc threshold to ip_rt_max_size/2 in ipv4. With the
> routing cache removal patch, ip_rt_max_size was set to INT_MAX. So the gc
> starts to remove entries when a threshold of INT_MAX/2 is reached.
> 
> cat /proc/sys/net/ipv4/xfrm4_gc_thresh
> 1073741823
> 
> I guess this was not intentional.

Do you mean for IPSEC routes?  For non-IPSEC routes on ipv4 there
is nothing to garbage collect.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - Nov. 9, 2012, 2:36 a.m.
From: roy.qing.li@gmail.com
Date: Thu,  8 Nov 2012 16:38:08 +0800

> From: Li RongQing <roy.qing.li@gmail.com>
> 
> Signed-off-by: Li RongQing <roy.qing.li@gmail.com>

As Steffen stated, this comment is largely obsolete.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
roy.qing.li@gmail.com - Nov. 9, 2012, 3:04 a.m.
2012/11/9 David Miller <davem@davemloft.net>:
> From: roy.qing.li@gmail.com
> Date: Thu,  8 Nov 2012 16:38:08 +0800
>
>> From: Li RongQing <roy.qing.li@gmail.com>
>>
>> Signed-off-by: Li RongQing <roy.qing.li@gmail.com>
>
> As Steffen stated, this comment is largely obsolete.

Thanks, I will resend new patch to remove this comments

-Roy
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Steffen Klassert - Nov. 9, 2012, 7:57 a.m.
On Thu, Nov 08, 2012 at 02:59:39PM -0500, David Miller wrote:
> From: Steffen Klassert <steffen.klassert@secunet.com>
> > 
> > The routing cache is removed, so this comment is obsolete. But it reminds
> > me that we set the gc threshold to ip_rt_max_size/2 in ipv4. With the
> > routing cache removal patch, ip_rt_max_size was set to INT_MAX. So the gc
> > starts to remove entries when a threshold of INT_MAX/2 is reached.
> > 
> > cat /proc/sys/net/ipv4/xfrm4_gc_thresh
> > 1073741823
> > 
> > I guess this was not intentional.
> 
> Do you mean for IPSEC routes?  For non-IPSEC routes on ipv4 there
> is nothing to garbage collect.

Yes, I mean IPsec routes. We still cache them at the flow cache
and at sockets, so we should do some garbage collecting.

We could either go back to a static threshold, as it was before

git commit a33bc5c15154c835aae26f16e6a3a7d9ad4acb45
xfrm: select sane defaults for xfrm[4|6] gc_thresh

or do the same as ipv6 does. I'll take care of this.

Btw. it seems to me that the flow cache has similar limitations
as the routing cache had. At least I was able to fill the flow
cache with a nmap scan from a remote entity. In practice, it's
hard to DOS the flow cache because we use a Jenkins hash with
a random initialization value, but this was the same with the
routing cache.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
index f3ed8ca..93832e8 100644
--- a/net/ipv6/xfrm6_policy.c
+++ b/net/ipv6/xfrm6_policy.c
@@ -332,10 +332,10 @@  int __init xfrm6_init(void)
 	/*
 	 * We need a good default value for the xfrm6 gc threshold.
 	 * In ipv4 we set it to the route hash table size * 8, which
-	 * is half the size of the maximaum route cache for ipv4.  It
+	 * is half the size of the maximum route cache for ipv4.  It
 	 * would be good to do the same thing for v6, except the table is
 	 * constructed differently here.  Here each table for a net namespace
-	 * can have FIB_TABLE_HASHSZ entries, so lets go with the same
+	 * can have FIB6_TABLE_HASHSZ entries, so lets go with the same
 	 * computation that we used for ipv4 here.  Also, lets keep the initial
 	 * gc_thresh to a minimum of 1024, since, the ipv6 route cache defaults
 	 * to that as a minimum as well