Could anyone give me a hint, how I can furher proceed to find the error?
Thanks.
Please try following patch :
[PATCH] ip_gre: dont increase dev->needed_headroom on a live device
It seems ip_gre is able to change dev->needed_headroom on the fly.
Its is not legal unfortunately and triggers a BUG in raw_sendmsg()
skb = sock_alloc_send_skb(sk, ... + LL_ALLOCATED_SPACE(rt->dst.dev)
< another cpu change dev->needed_headromm (making it bigger)
...
skb_reserve(skb, LL_RESERVED_SPACE(rt->dst.dev));
We end with LL_RESERVED_SPACE() being bigger than LL_ALLOCATED_SPACE()
-> we crash later because skb head is exhausted.
Bug introduced in commit 243aad83 in 2.6.34 (ip_gre: include route
header_len in max_headroom calculation)
Reported-by: Elmar Vonlanthen <redacted>
Signed-off-by: Eric Dumazet <redacted>
CC: Timo Teräs <redacted>
CC: Herbert Xu <herbert@gondor.apana.org.au>
---
From: Elmar Vonlanthen <hidden> Date: 2011-10-17 07:16:27
2011/10/14 Eric Dumazet [off-list ref]:
quoted hunk
Please try following patch :
[PATCH] ip_gre: dont increase dev->needed_headroom on a live device
It seems ip_gre is able to change dev->needed_headroom on the fly.
Its is not legal unfortunately and triggers a BUG in raw_sendmsg()
skb = sock_alloc_send_skb(sk, ... + LL_ALLOCATED_SPACE(rt->dst.dev)
< another cpu change dev->needed_headromm (making it bigger)
...
skb_reserve(skb, LL_RESERVED_SPACE(rt->dst.dev));
We end with LL_RESERVED_SPACE() being bigger than LL_ALLOCATED_SPACE()
-> we crash later because skb head is exhausted.
Bug introduced in commit 243aad83 in 2.6.34 (ip_gre: include route
header_len in max_headroom calculation)
Reported-by: Elmar Vonlanthen <redacted>
Signed-off-by: Eric Dumazet <redacted>
CC: Timo Teräs <redacted>
CC: Herbert Xu <herbert@gondor.apana.org.au>
---
Hello
I tried this patch and I was not able anymore to reproduce the kernel
oops. So the patch solved the bug.
Thank you very much!
Would it be possible to add the patch to the long term kernel 2.6.35
as well? Because this is the one I use at the moment in production.
And sorry for posting to the wrong mailing list (linux-kernel).
Best regards
Elmar
From: Eric Dumazet <hidden> Date: 2011-10-18 02:30:39
Le lundi 17 octobre 2011 à 09:16 +0200, Elmar Vonlanthen a écrit :
2011/10/14 Eric Dumazet [off-list ref]:
quoted
Please try following patch :
[PATCH] ip_gre: dont increase dev->needed_headroom on a live device
It seems ip_gre is able to change dev->needed_headroom on the fly.
Its is not legal unfortunately and triggers a BUG in raw_sendmsg()
skb = sock_alloc_send_skb(sk, ... + LL_ALLOCATED_SPACE(rt->dst.dev)
< another cpu change dev->needed_headromm (making it bigger)
...
skb_reserve(skb, LL_RESERVED_SPACE(rt->dst.dev));
We end with LL_RESERVED_SPACE() being bigger than LL_ALLOCATED_SPACE()
-> we crash later because skb head is exhausted.
Bug introduced in commit 243aad83 in 2.6.34 (ip_gre: include route
header_len in max_headroom calculation)
Reported-by: Elmar Vonlanthen <redacted>
Signed-off-by: Eric Dumazet <redacted>
CC: Timo Teräs <redacted>
CC: Herbert Xu <herbert@gondor.apana.org.au>
---
Hello
I tried this patch and I was not able anymore to reproduce the kernel
oops. So the patch solved the bug.
Thank you very much!
Would it be possible to add the patch to the long term kernel 2.6.35
as well? Because this is the one I use at the moment in production.
Thanks for testing.
If David/Herbert/Timo agree, then patch should find its way into current
kernel, then to stable trees as well.
Thanks
From: Herbert Xu <hidden> Date: 2011-10-18 09:34:17
On Tue, Oct 18, 2011 at 04:30:32AM +0200, Eric Dumazet wrote:
If David/Herbert/Timo agree, then patch should find its way into current
kernel, then to stable trees as well.
Actually, I think we should instead fix the users of needed_headroom
to not read it twice which is causing problems here.
GRE tunnels by their nature do not have a fixed value for
needed_headroom. As the underlying routes change the necessary
headroom may need to be adjusted due to further encapsulation such
as IPsec.
Keeping it constant from tunnel creation may result in suboptimal
performance due to unnecessary header reallocations.
However, until we audit the stack to see if there are further
instances of double-readings such as the one causing the crash
here, I'm fine with your patch making it constant.
Once we're sure that all of the double-readings are gone we
can revert to a dynamic needed_headroom.
Thanks,
--
Email: Herbert Xu [off-list ref]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
From: Eric Dumazet <hidden> Date: 2011-10-18 10:01:17
Le mardi 18 octobre 2011 à 11:34 +0200, Herbert Xu a écrit :
On Tue, Oct 18, 2011 at 04:30:32AM +0200, Eric Dumazet wrote:
quoted
If David/Herbert/Timo agree, then patch should find its way into current
kernel, then to stable trees as well.
Actually, I think we should instead fix the users of needed_headroom
to not read it twice which is causing problems here.
GRE tunnels by their nature do not have a fixed value for
needed_headroom. As the underlying routes change the necessary
headroom may need to be adjusted due to further encapsulation such
as IPsec.
Keeping it constant from tunnel creation may result in suboptimal
performance due to unnecessary header reallocations.
However, until we audit the stack to see if there are further
instances of double-readings such as the one causing the crash
here, I'm fine with your patch making it constant.
Once we're sure that all of the double-readings are gone we
can revert to a dynamic needed_headroom.
Sure, we can work on this path for future kernels.
Adding an RCU protected structure to hold hard_header_len /
needed_headroom / needed_tailroom should be possible, but this adds yet
another pointer dereference...
Thanks !
From: Herbert Xu <hidden> Date: 2011-10-18 10:05:31
On Tue, Oct 18, 2011 at 12:01:33PM +0200, Eric Dumazet wrote:
Adding an RCU protected structure to hold hard_header_len /
needed_headroom / needed_tailroom should be possible, but this adds yet
another pointer dereference...
I don't think we need RCU here since the problem is simply that
we're using two different values for skb allocations and skb_reserve.
As long as we use one and the same value it should work. The value
will rarely be incorrect and when it is, automatic reallocation will
occur.
Cheers,
--
Email: Herbert Xu [off-list ref]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
From: Eric Dumazet <hidden> Date: 2011-10-18 10:23:26
Le mardi 18 octobre 2011 à 12:05 +0200, Herbert Xu a écrit :
On Tue, Oct 18, 2011 at 12:01:33PM +0200, Eric Dumazet wrote:
quoted
Adding an RCU protected structure to hold hard_header_len /
needed_headroom / needed_tailroom should be possible, but this adds yet
another pointer dereference...
I don't think we need RCU here since the problem is simply that
we're using two different values for skb allocations and skb_reserve.
As long as we use one and the same value it should work. The value
will rarely be incorrect and when it is, automatic reallocation will
occur.
You're right, if reallocations are OK in all paths.
We'll need to change LL_RESERVED_SPACE() / LL_RESERVED_SPACE_EXTRA() /
LL_ALLOCATED_SPACE() macros and provide the [read once] values, instead
of a [read once] pointer to values.
Thats a bit complex change, but doable.
From: Herbert Xu <hidden> Date: 2011-10-18 10:45:10
On Tue, Oct 18, 2011 at 12:23:43PM +0200, Eric Dumazet wrote:
You're right, if reallocations are OK in all paths.
If it wasn't OK then making needed_headroom constant won't work
anyway.
We'll need to change LL_RESERVED_SPACE() / LL_RESERVED_SPACE_EXTRA() /
LL_ALLOCATED_SPACE() macros and provide the [read once] values, instead
of a [read once] pointer to values.
I'm not sure what you mean here. I don't see any need to change
these macros. All we need is to save the value in a local variable:
hh_len = LL_RESERVED_SPACE(dev);
skb = alloc_skb(hh_len + len);
skb_reserve(skb, hh_len);
Cheers,
--
Email: Herbert Xu [off-list ref]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
From: Eric Dumazet <hidden> Date: 2011-10-18 11:37:41
Le mardi 18 octobre 2011 à 12:45 +0200, Herbert Xu a écrit :
On Tue, Oct 18, 2011 at 12:23:43PM +0200, Eric Dumazet wrote:
quoted
You're right, if reallocations are OK in all paths.
If it wasn't OK then making needed_headroom constant won't work
anyway.
quoted
We'll need to change LL_RESERVED_SPACE() / LL_RESERVED_SPACE_EXTRA() /
LL_ALLOCATED_SPACE() macros and provide the [read once] values, instead
of a [read once] pointer to values.
I'm not sure what you mean here. I don't see any need to change
these macros. All we need is to save the value in a local variable:
hh_len = LL_RESERVED_SPACE(dev);
skb = alloc_skb(hh_len + len);
skb_reserve(skb, hh_len);
Not really Herbert. Please read again my patch changelog.
In the bug we try to fix, we have :
skb = sock_alloc_send_skb(sk, ... + LL_ALLOCATED_SPACE(rt->dst.dev)
... < increase of dev->needed_headroom by another cpu/task >
skb_reserve(skb, LL_RESERVED_SPACE(rt->dst.dev));
skb_put() -> crash because we reserved too much space
So we really want LL_ALLOCATED_SPACE() and LL_RESERVED_SPACE() use the
same needed_headroom, or else you can have LL_RESERVED_SPACE() >
LL_ALLOCATED_SPACE().
There are several way to fix this, but this kind of code assumed the
dev->needed... values were consistent for the whole block.
From: Herbert Xu <hidden> Date: 2011-10-18 11:49:50
On Tue, Oct 18, 2011 at 01:37:58PM +0200, Eric Dumazet wrote:
In the bug we try to fix, we have :
skb = sock_alloc_send_skb(sk, ... + LL_ALLOCATED_SPACE(rt->dst.dev)
... < increase of dev->needed_headroom by another cpu/task >
skb_reserve(skb, LL_RESERVED_SPACE(rt->dst.dev));
OK, in that case one fix would be to replace LL_ALLOCATED_SPACE
with its two constiuents so that they may be stored in local
variables for later use.
hlen = LL_HEADROOM(skb);
tlen = LL_TAILROOM(skb);
skb_alloc_send_skb(sk, ... + LL_ALIGN(hlen + tlen));
skb_reserve(skb, LL_ALIGN(hlen));
Cheers,
--
Email: Herbert Xu [off-list ref]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
From: Eric Dumazet <hidden> Date: 2011-10-18 12:55:44
Le mardi 18 octobre 2011 à 13:49 +0200, Herbert Xu a écrit :
On Tue, Oct 18, 2011 at 01:37:58PM +0200, Eric Dumazet wrote:
quoted
In the bug we try to fix, we have :
skb = sock_alloc_send_skb(sk, ... + LL_ALLOCATED_SPACE(rt->dst.dev)
... < increase of dev->needed_headroom by another cpu/task >
skb_reserve(skb, LL_RESERVED_SPACE(rt->dst.dev));
OK, in that case one fix would be to replace LL_ALLOCATED_SPACE
with its two constiuents so that they may be stored in local
variables for later use.
hlen = LL_HEADROOM(skb);
tlen = LL_TAILROOM(skb);
skb_alloc_send_skb(sk, ... + LL_ALIGN(hlen + tlen));
skb_reserve(skb, LL_ALIGN(hlen));
Cheers,
I am ok by this way, but we might hit another similar problem elsewhere.
(igmp.c ip6_output, ...)
We effectively want to remove LL_ALLOCATED_SPACE() usage and obfuscate
code...
[PATCH] raw: allow dev->needed_headroom dynamic change
It seems ip_gre is able to change dev->needed_headroom on the fly.
It triggers a BUG in raw_sendmsg()
skb = sock_alloc_send_skb(sk, ... + LL_ALLOCATED_SPACE(rt->dst.dev)
< another cpu change dev->needed_headromm (making it bigger)
...
skb_reserve(skb, LL_RESERVED_SPACE(rt->dst.dev));
We end with LL_RESERVED_SPACE() being bigger than LL_ALLOCATED_SPACE()
-> we crash later because skb head is exhausted.
Bug introduced in commit 243aad83 in 2.6.34 (ip_gre: include route
header_len in max_headroom calculation)
Reported-by: Reported-by: Elmar Vonlanthen <redacted>
Signed-off-by: Eric Dumazet <redacted>
CC: Timo Teräs <redacted>
CC: Herbert Xu <herbert@gondor.apana.org.au>
---
include/linux/netdevice.h | 10 +++++++---
net/ipv4/raw.c | 9 +++++++--
net/ipv6/raw.c | 9 +++++++--
3 files changed, 21 insertions(+), 7 deletions(-)
From: Herbert Xu <hidden> Date: 2011-10-18 13:45:41
On Tue, Oct 18, 2011 at 02:56:00PM +0200, Eric Dumazet wrote:
I am ok by this way, but we might hit another similar problem elsewhere.
(igmp.c ip6_output, ...)
We effectively want to remove LL_ALLOCATED_SPACE() usage and obfuscate
code...
From: David Miller <davem@davemloft.net> Date: 2011-10-19 07:09:29
From: Herbert Xu <redacted>
Date: Tue, 18 Oct 2011 15:45:37 +0200
On Tue, Oct 18, 2011 at 02:56:00PM +0200, Eric Dumazet wrote:
quoted
I am ok by this way, but we might hit another similar problem elsewhere.
(igmp.c ip6_output, ...)
We effectively want to remove LL_ALLOCATED_SPACE() usage and obfuscate
code...
Here's another idea, provide a helper to do the skb allocation
and the skb_reserve in one go. That way this ugliness would only
need to be done once.
Someone please test this:
--------------------
net: Fix crashes on devices which dynamically change needed headroom.
One such device is IP_GRE.
The problem is that we evaluate the device characteristics twice, once
to determine the allocation size, and once to do the skb_reserve().
Combine these into one operation using a helper function.
With help from Eric Dumazet and Herbert Xu.
Reported-by: Reported-by: Elmar Vonlanthen <redacted>
Signed-off-by: David S. Miller <davem@davemloft.net>
From: Eric Dumazet <hidden> Date: 2011-10-19 07:18:41
Le mercredi 19 octobre 2011 à 03:09 -0400, David Miller a écrit :
From: Herbert Xu <redacted>
Date: Tue, 18 Oct 2011 15:45:37 +0200
quoted
On Tue, Oct 18, 2011 at 02:56:00PM +0200, Eric Dumazet wrote:
quoted
I am ok by this way, but we might hit another similar problem elsewhere.
(igmp.c ip6_output, ...)
We effectively want to remove LL_ALLOCATED_SPACE() usage and obfuscate
code...
Here's another idea, provide a helper to do the skb allocation
and the skb_reserve in one go. That way this ugliness would only
need to be done once.
Someone please test this:
--------------------
net: Fix crashes on devices which dynamically change needed headroom.
One such device is IP_GRE.
The problem is that we evaluate the device characteristics twice, once
to determine the allocation size, and once to do the skb_reserve().
Combine these into one operation using a helper function.
With help from Eric Dumazet and Herbert Xu.
Reported-by: Reported-by: Elmar Vonlanthen <redacted>
Signed-off-by: David S. Miller <davem@davemloft.net>
Seems fine (Maybe do the +15 in caller site ?), but we also have other
problematic cases, using alloc_skb() only...
From: Eric Dumazet <hidden> Date: 2011-10-19 07:52:41
Le mercredi 19 octobre 2011 à 03:30 -0400, David Miller a écrit :
From: Eric Dumazet <redacted>
Date: Wed, 19 Oct 2011 09:18:33 +0200
quoted
Seems fine (Maybe do the +15 in caller site ?), but we also have other
problematic cases, using alloc_skb() only...
Ok, which ones operate over these problematic GRE tunnels?
I was more thinking of general idea to allow any device to change its
needed headroom.
For GRE tunnels, I dont think IPv6 fragmentation could be relevant,
but maybe IGMP could trigger a problem ?
From: David Miller <davem@davemloft.net> Date: 2011-10-19 08:02:55
From: Eric Dumazet <redacted>
Date: Wed, 19 Oct 2011 09:52:19 +0200
For GRE tunnels, I dont think IPv6 fragmentation could be relevant,
but maybe IGMP could trigger a problem ?
Funny... icmpv6 uses ip6_append_data() which uses hh_len in a
local variable, which seems to suggest that it's immune to
this problem.
IPV4 side seems identical in this regard.
So, as far as I can see, my patch is sufficient to cover IP_GRE
reasonably in the 'net' tree.
Agreed?
If so, someone please test this thing :-)
From: Herbert Xu <hidden> Date: 2011-10-19 08:08:17
On Wed, Oct 19, 2011 at 03:30:52AM -0400, David Miller wrote:
From: Eric Dumazet <redacted>
Date: Wed, 19 Oct 2011 09:18:33 +0200
quoted
Seems fine (Maybe do the +15 in caller site ?), but we also have other
problematic cases, using alloc_skb() only...
Ok, which ones operate over these problematic GRE tunnels?
Potentially all of them since we now support Ethernet-over-GRE.
I think Eric's initial patch is probably the safest bet for rc10.
We can then work on the proper fix for the next release.
As to the latter, I've just done a grep over net and it seems that
all users of LL_ALLOCATED_SPACE fall into two cases, alloc_skb users
or sock_alloc_send_skb (including pskb) users.
For alloc_skb we could add a new helper. While for the other
case we could either create a new helper or just add an extra
dev argument that may be NULL for those that don't care about
LL_ALLOCATED_SPACE.
Cheers,
--
Email: Herbert Xu [off-list ref]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
From: David Miller <davem@davemloft.net> Date: 2011-10-20 09:31:06
From: Herbert Xu <redacted>
Date: Wed, 19 Oct 2011 10:08:07 +0200
I think Eric's initial patch is probably the safest bet for rc10.
We can then work on the proper fix for the next release.
There are two "initial patch", I wonder which one you mean.
There's his really first patch, which remoevs the lines in IP_GRE
which change dev->needed_headroom. I was under the impression we
were against doing that.
The other patch he posted duplicates the device attribute variable
caching in two functions.
My patch is just a tweak so that we only do this sequence in one
place, the new sock_alloc_send_skb_reserve() helper, instead of
in both the ipv4 and ipv6 RAW code.
So I'm a little confused what your suggestion for rc10 really
is :-)
From: Herbert Xu <hidden> Date: 2011-10-20 09:35:52
On Thu, Oct 20, 2011 at 05:30:50AM -0400, David Miller wrote:
So I'm a little confused what your suggestion for rc10 really
is :-)
I meant his first initial patch :)
While it is suboptimal in the sense that should the value of
needed_headroom increase we'll end up constantly reallocating
skbs, I believe that it is at least semantically correct.
In the time being I'll look more closely at all the users of
needed_headroom to see if there's anything we've missed.
Thanks,
--
Email: Herbert Xu [off-list ref]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
From: David Miller <davem@davemloft.net> Date: 2011-10-20 20:21:14
From: Herbert Xu <redacted>
Date: Thu, 20 Oct 2011 11:35:41 +0200
On Thu, Oct 20, 2011 at 05:30:50AM -0400, David Miller wrote:
quoted
So I'm a little confused what your suggestion for rc10 really
is :-)
I meant his first initial patch :)
While it is suboptimal in the sense that should the value of
needed_headroom increase we'll end up constantly reallocating
skbs, I believe that it is at least semantically correct.
Ok, I applied Eric's patch which removes the dynamic changing of the
needed_headroom in IP_GRE.
Thanks everyone!
From: Herbert Xu <hidden> Date: 2011-10-25 11:54:38
On Thu, Oct 20, 2011 at 11:35:41AM +0200, Herbert Xu wrote:
In the time being I'll look more closely at all the users of
needed_headroom to see if there's anything we've missed.
OK I've reviewed all the users of needed_headroom and I haven't
found anything other than the cases that we have enumerated.
One thing I noticed is that the macro LL_ALLOCATED_SPACE is
completely bogus and buggy. It applies the alignment to the
sum of headroom and tailroom. However, the alignment is then
applied separately to the headroom when reserving, meaning that
we may end up with insufficient tailroom.
So I'm going to get rid of LL_ALLOCATED_SPACE completely and
replace it with explicit references to the tailroom as it doesn't
need the alignment anyway (The headroom needs alignment since
we use it to ensure the head is aligned).
Cheers,
--
Email: Herbert Xu [off-list ref]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
From: David Miller <davem@davemloft.net> Date: 2011-10-26 03:13:05
From: Herbert Xu <redacted>
Date: Tue, 25 Oct 2011 13:54:25 +0200
So I'm going to get rid of LL_ALLOCATED_SPACE completely and
replace it with explicit references to the tailroom as it doesn't
need the alignment anyway (The headroom needs alignment since
we use it to ensure the head is aligned).
From: Herbert Xu <hidden> Date: 2011-11-18 12:18:44
On Tue, Oct 25, 2011 at 11:12:04PM -0400, David Miller wrote:
From: Herbert Xu <redacted>
Date: Tue, 25 Oct 2011 13:54:25 +0200
quoted
So I'm going to get rid of LL_ALLOCATED_SPACE completely and
replace it with explicit references to the tailroom as it doesn't
need the alignment anyway (The headroom needs alignment since
we use it to ensure the head is aligned).
From: Herbert Xu <hidden> Date: 2011-11-18 12:20:13
ipv4: Remove all uses of LL_ALLOCATED_SPACE
The macro LL_ALLOCATED_SPACE was ill-conceived. It applies the
alignment to the sum of needed_headroom and needed_tailroom. As
the amount that is then reserved for head room is needed_headroom
with alignment, this means that the tail room left may be too small.
This patch replaces all uses of LL_ALLOCATED_SPACE in net/ipv4
with the macro LL_RESERVED_SPACE and direct reference to
needed_tailroom.
This also fixes the problem with needed_headroom changing between
allocating the skb and reserving the head room.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
net/ipv4/arp.c | 6 ++++--
net/ipv4/igmp.c | 13 +++++++++----
net/ipv4/ipconfig.c | 6 ++++--
net/ipv4/raw.c | 7 +++++--
4 files changed, 22 insertions(+), 10 deletions(-)
From: Herbert Xu <hidden> Date: 2011-11-18 12:20:15
net: Remove LL_ALLOCATED_SPACE
The macro LL_ALLOCATED_SPACE was ill-conceived. It applies the
alignment to the sum of needed_headroom and needed_tailroom. As
the amount that is then reserved for head room is needed_headroom
with alignment, this means that the tail room left may be too small.
Now that all uses of LL_ALLOCATED_SPACE have been removed, this
patch finally removes the macro itself.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
include/linux/netdevice.h | 5 -----
1 file changed, 5 deletions(-)
From: Herbert Xu <hidden> Date: 2011-11-18 12:20:16
net: Remove all uses of LL_ALLOCATED_SPACE
The macro LL_ALLOCATED_SPACE was ill-conceived. It applies the
alignment to the sum of needed_headroom and needed_tailroom. As
the amount that is then reserved for head room is needed_headroom
with alignment, this means that the tail room left may be too small.
This patch replaces all uses of LL_ALLOCATED_SPACE with the macro
LL_RESERVED_SPACE and direct reference to needed_tailroom.
This also fixes the problem with needed_headroom changing between
allocating the skb and reserving the head room.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
net/core/netpoll.c | 6 ++++--
net/econet/af_econet.c | 7 +++++--
net/ieee802154/dgram.c | 7 +++++--
net/ieee802154/raw.c | 7 +++++--
net/packet/af_packet.c | 18 +++++++++++-------
5 files changed, 30 insertions(+), 15 deletions(-)
From: Herbert Xu <hidden> Date: 2011-11-18 12:21:16
ip_gre: Set needed_headroom dynamically again
Now that all needed_headroom users have been fixed up so that
we can safely increase needed_headroom, this patch restore the
dynamic update of needed_headroom.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
net/ipv4/ip_gre.c | 2 ++
1 file changed, 2 insertions(+)
From: Herbert Xu <hidden> Date: 2011-11-18 12:21:17
packet: Add needed_tailroom to packet_sendmsg_spkt
While auditing LL_ALLOCATED_SPACE I noticed that packet_sendmsg_spkt
did not include needed_tailroom when allocating an skb. This isn't
a fatal error as we should always tolerate inadequate tail room but
it isn't optimal.
This patch fixes that.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
net/packet/af_packet.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
@@ -1499,10 +1499,11 @@ retry:if(!skb){size_treserved=LL_RESERVED_SPACE(dev);+inttlen=dev->needed_tailroom;unsignedinthhlen=dev->header_ops?dev->hard_header_len:0;rcu_read_unlock();-skb=sock_wmalloc(sk,len+reserved,0,GFP_KERNEL);+skb=sock_wmalloc(sk,len+reserved+tlen,0,GFP_KERNEL);if(skb==NULL)return-ENOBUFS;/* FIXME: Save some space for broken drivers that write a hard
From: Herbert Xu <hidden> Date: 2011-11-18 12:21:19
ipv6: Remove all uses of LL_ALLOCATED_SPACE
The macro LL_ALLOCATED_SPACE was ill-conceived. It applies the
alignment to the sum of needed_headroom and needed_tailroom. As
the amount that is then reserved for head room is needed_headroom
with alignment, this means that the tail room left may be too small.
This patch replaces all uses of LL_ALLOCATED_SPACE in net/ipv6
with the macro LL_RESERVED_SPACE and direct reference to
needed_tailroom.
This also fixes the problem with needed_headroom changing between
allocating the skb and reserving the head room.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
net/ipv6/ip6_output.c | 8 ++++++--
net/ipv6/mcast.c | 12 ++++++++----
net/ipv6/ndisc.c | 13 +++++++++----
net/ipv6/raw.c | 6 ++++--
4 files changed, 27 insertions(+), 12 deletions(-)
@@ -815,7 +818,8 @@ slow_path:*Allocatebuffer.*/-if((frag=alloc_skb(len+hlen+sizeof(structfrag_hdr)+LL_ALLOCATED_SPACE(rt->dst.dev),GFP_ATOMIC))==NULL){+if((frag=alloc_skb(len+hlen+sizeof(structfrag_hdr)++hroom+troom,GFP_ATOMIC))==NULL){NETDEBUG(KERN_INFO"IPv6: frag: no memory for new fragment!\n");IP6_INC_STATS(net,ip6_dst_idev(skb_dst(skb)),IPSTATS_MIB_FRAGFAILS);
From: David Miller <davem@davemloft.net> Date: 2011-11-18 20:02:11
From: Herbert Xu <redacted>
Date: Fri, 18 Nov 2011 20:18:32 +0800
On Tue, Oct 25, 2011 at 11:12:04PM -0400, David Miller wrote:
quoted
From: Herbert Xu <redacted>
Date: Tue, 25 Oct 2011 13:54:25 +0200
quoted
So I'm going to get rid of LL_ALLOCATED_SPACE completely and
replace it with explicit references to the tailroom as it doesn't
need the alignment anyway (The headroom needs alignment since
we use it to ensure the head is aligned).
Ok.
Here are the patches that do this. I also picked up one spot
that should have used LL_ALLOCATED_SPACE but did not (see 2nd
last patch).
This all looks good to me, applied to net-next, thanks!