[IPSEC]: Don't warn if high-order hash resize fails

[IPSEC]: Don't warn if high-order hash resize fails

[Herbert Xu]

Hi Dave:

I just got this when rekeying my VPN (2.6.21.1):

events/0: page allocation failure. order:7, mode:0xd0
 [<c0146235>] __alloc_pages+0x1e5/0x2e0
 [<c0146356>] __get_free_pages+0x26/0x50
 [<c02a01ef>] xfrm_hash_alloc+0x2f/0x80
 [<c029a947>] xfrm_bydst_resize+0x37/0xc0
 [<c029aa70>] xfrm_hash_resize+0x0/0x80
 [<c029aae7>] xfrm_hash_resize+0x77/0x80
 [<c012a8ea>] run_workqueue+0x8a/0x190
 [<c012ab17>] worker_thread+0x127/0x160
 [<c0117490>] default_wake_function+0x0/0x10
 [<c01174d7>] __wake_up_common+0x37/0x70
 [<c0117490>] default_wake_function+0x0/0x10
 [<c012a9f0>] worker_thread+0x0/0x160
 [<c012e044>] kthread+0xb4/0xc0
 [<c012df90>] kthread+0x0/0xc0
 [<c0105287>] kernel_thread_helper+0x7/0x10
 =======================

Isn't an order-7 allocation (half a meg) a bit over-the-top for that hash,
especially since I've got a grand total of 4 SAs :)

Anyway, we shouldn't warn about these allocations since they're expected
to fail after the box has been up for a while.  Also, should we try
vmalloc when it does fail?

[IPSEC]: Don't warn if high-order hash resize fails

Multi-page allocations are always likely to fail.  Since such failures
are expected and non-critical in xfrm_hash_alloc, we shouldn't warn about
them.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
diff --git a/net/xfrm/xfrm_hash.c b/net/xfrm/xfrm_hash.c
index 37643bb..55ab579 100644
--- a/net/xfrm/xfrm_hash.c
+++ b/net/xfrm/xfrm_hash.c
@@ -22,7 +22,8 @@ struct hlist_head *xfrm_hash_alloc(unsigned int sz)
 		n = __vmalloc(sz, GFP_KERNEL, PAGE_KERNEL);
 	else
 		n = (struct hlist_head *)
-			__get_free_pages(GFP_KERNEL, get_order(sz));
+			__get_free_pages(GFP_KERNEL | __GFP_NOWARN,
+					 get_order(sz));
 
 	if (n)
 		memset(n, 0, sz);

Re: [IPSEC]: Don't warn if high-order hash resize fails

[David Miller]

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Mon, 14 May 2007 15:48:37 +1000

> Hi Dave:
> 
> I just got this when rekeying my VPN (2.6.21.1):
> 
> events/0: page allocation failure. order:7, mode:0xd0
>  [<c0146235>] __alloc_pages+0x1e5/0x2e0
>  [<c0146356>] __get_free_pages+0x26/0x50
>  [<c02a01ef>] xfrm_hash_alloc+0x2f/0x80
>  [<c029a947>] xfrm_bydst_resize+0x37/0xc0
>  [<c029aa70>] xfrm_hash_resize+0x0/0x80
>  [<c029aae7>] xfrm_hash_resize+0x77/0x80
>  [<c012a8ea>] run_workqueue+0x8a/0x190
>  [<c012ab17>] worker_thread+0x127/0x160
>  [<c0117490>] default_wake_function+0x0/0x10
>  [<c01174d7>] __wake_up_common+0x37/0x70
>  [<c0117490>] default_wake_function+0x0/0x10
>  [<c012a9f0>] worker_thread+0x0/0x160
>  [<c012e044>] kthread+0xb4/0xc0
>  [<c012df90>] kthread+0x0/0xc0
>  [<c0105287>] kernel_thread_helper+0x7/0x10
>  =======================
> 
> Isn't an order-7 allocation (half a meg) a bit over-the-top for that hash,
> especially since I've got a grand total of 4 SAs :)
> 
> Anyway, we shouldn't warn about these allocations since they're expected
> to fail after the box has been up for a while.  Also, should we try
> vmalloc when it does fail?

Thanks I'll add the warning fix, but there must be some counter
bug if it tried to grow to order-7, that really shouldn't
be happening.

Re: [IPSEC]: Don't warn if high-order hash resize fails

[Herbert Xu]

On Sun, May 13, 2007 at 11:36:35PM -0700, David Miller wrote:
> 
> Thanks I'll add the warning fix, but there must be some counter
> bug if it tried to grow to order-7, that really shouldn't
> be happening.

Good point.  A bit of dd-ing shows that I've got a negative value
in xfrm_policy_count[0] (0xfffffff6).

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [IPSEC]: Don't warn if high-order hash resize fails

[Herbert Xu]

On Mon, May 14, 2007 at 05:07:33PM +1000, Herbert Xu wrote:
> 
> Good point.  A bit of dd-ing shows that I've got a negative value
> in xfrm_policy_count[0] (0xfffffff6).

In fact the other two directions are bogus too, OUT says 10 and FWD
says 5.  The actual values are IN == 2, OUT == 2, FWD == 1.  The
socket policy counts are correct though.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [IPSEC]: Don't warn if high-order hash resize fails

[Herbert Xu]

On Mon, May 14, 2007 at 05:26:57PM +1000, Herbert Xu wrote:
> On Mon, May 14, 2007 at 05:07:33PM +1000, Herbert Xu wrote:
> > 
> > Good point.  A bit of dd-ing shows that I've got a negative value
> > in xfrm_policy_count[0] (0xfffffff6).
> 
> In fact the other two directions are bogus too, OUT says 10 and FWD
> says 5.  The actual values are IN == 2, OUT == 2, FWD == 1.  The
> socket policy counts are correct though.

In fact that's a clue.  If you add them all up they come out at the
same value.  So we're getting the directions mixed up...

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [IPSEC]: Don't warn if high-order hash resize fails

[Herbert Xu]

On Mon, May 14, 2007 at 05:27:55PM +1000, Herbert Xu wrote:
> 
> In fact that's a clue.  If you add them all up they come out at the
> same value.  So we're getting the directions mixed up...

Found the problem.  It was my own fault in a way :)

[IPSEC]: Check validity of direction in xfrm_policy_byid

The function xfrm_policy_byid takes a dir argument but finds the policy
using the index instead.  We only use the dir argument to update the
policy count for that direction.  Since the user can supply any value
for dir, this can corrupt our policy count.

I know this is the problem because a few days ago I was deleting
policies by hand using indicies and accidentally typed in the wrong
direction.  It still deleted the policy and at the time I thought
that was cool.  In retrospect it isn't such a good idea :)

I decided against letting it delete the policy anyway just in case
we ever remove the connection between indicies and direction.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 95271e8..d0882e5 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -796,6 +796,10 @@ struct xfrm_policy *xfrm_policy_byid(u8 type, int dir, u32 id, int delete,
 	struct hlist_head *chain;
 	struct hlist_node *entry;
 
+	*err = -ENOENT;
+	if (xfrm_policy_id2dir(id) != dir)
+		return NULL;
+
 	*err = 0;
 	write_lock_bh(&xfrm_policy_lock);
 	chain = xfrm_policy_byidx + idx_hash(id);

Re: [IPSEC]: Don't warn if high-order hash resize fails

[David Miller]

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Mon, 14 May 2007 17:39:41 +1000

> [IPSEC]: Check validity of direction in xfrm_policy_byid
> 
> The function xfrm_policy_byid takes a dir argument but finds the policy
> using the index instead.  We only use the dir argument to update the
> policy count for that direction.  Since the user can supply any value
> for dir, this can corrupt our policy count.
> 
> I know this is the problem because a few days ago I was deleting
> policies by hand using indicies and accidentally typed in the wrong
> direction.  It still deleted the policy and at the time I thought
> that was cool.  In retrospect it isn't such a good idea :)
> 
> I decided against letting it delete the policy anyway just in case
> we ever remove the connection between indicies and direction.
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Good spotting, patch applied, thanks!