* Help with NULL pointer derefrence in net/ipv4/xfrm4_policy.c
From: Mark Larwill @ 2011-10-13 19:39 UTC (permalink / raw)
To: netdev
I'm an software developer working for a small firewall company. I am
running into a kernel NULL pointer deference bug (using an older
2.6.35.12 kernel), and noticed a patch submitted that seems like it
might fix my problem. I was hoping someone could help me out a bit. It
is not practical for me to simply upgrade to the latest Linux, rather
I must make a targeted fix.
First a little on my specific issue, then my questions. My problem is
in the file net/ipv4/xfrm4_policy.c in the function
xfrm4_dst_ifdown(), on this line: " if (xdst->u.rt.idev->dev == dev)
{" basically idev is all 0 so xdst->u.rt.idev->dev will crash.
But before I blindly apply the patch I was hoping someone could help
answer some questions on it so I don't make things worse by not
understanding everything. The patch I reference below removes all of
this idev stuff from the kernel. I can see that as long as the if
condition (mentioned above) is met the (xfrm4_dst_ifdown) code
basically goes through a list and assigns the idev structures loopback
devices, so my guess is that for some reason these idev devices are no
longer being used or never were used to begin with. Also similar
behavior is done in the generic ipv4_dst_ifdown() function in
net/ipv4/route.c.
My questions are:
Q1) Why was the idev stuff there to begin with if it was unnecessary?
/ What was it's original purpose?
Q2) Why did we have to assign the devices to the loopback device? (Was
it so no packets would go out during deletion?)
Q3) When, and in what context are the ipv4_dst_ops.ifdown, and
ops->ifdown function pointers called? (What are they doing and why?)
Q4) Between 2.6.35.12 and the patch that removes the idev stuff in
from 11 Nov 2010 in 2.6.38-rc8 are there any other things
removed/changed in between that made it possible to remove the idef
stuff?
Q5) All of these basically lead up to: Why is it safe to remove all of
the idev stuff?
Q6) Is there a safe way I can safely remove this idev stuff or at
least avoid the NULL pointer deference? (Just blindly returning if it
is NULL would be the immediate reaction without thinking about it, but
chances are that's wrong---otherwise why is xfrm4_dst_ifdown written
the way it is?
Any help is greatly appreciated, thanks very much!
--Mark Larwill
REFERENCES:
A)
This post here to kerneltrap.org which seems to describe the old behavior:
http://kerneltrap.org/mailarchive/linux-netdev/2007/9/27/324077
B) The main change I am referring to, and text below
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=72cdd1d971c0deb1619c5c339270570c43647a78
It seems idev field in struct rtable has no special purpose, but adding
extra atomic ops.
We hold refcounts on the device itself (using percpu data, so pretty
cheap in current kernel).
infiniband case is solved using dst.dev instead of idev->dev
Removal of this field means routing without route cache is now using
shared data, percpu data, and only potential contention is a pair of
atomic ops on struct neighbour per forwarded packet.
About 5% speedup on routing test.
^ permalink raw reply
* [PATCH] ehea: Change maintainer to me
From: Thadeu Lima de Souza Cascardo @ 2011-10-13 19:56 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, davem, Thadeu Lima de Souza Cascardo, Breno Leitao
Breno Leitao has passed the maintainership to me.
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
Cc: Breno Leitao <leitao@linux.vnet.ibm.com>
---
MAINTAINERS | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index aac56f9..c25e93a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2468,7 +2468,7 @@ S: Supported
F: drivers/infiniband/hw/ehca/
EHEA (IBM pSeries eHEA 10Gb ethernet adapter) DRIVER
-M: Breno Leitao <leitao@linux.vnet.ibm.com>
+M: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
L: netdev@vger.kernel.org
S: Maintained
F: drivers/net/ethernet/ibm/ehea/
--
1.7.4.4
^ permalink raw reply related
* Re: [PATCH v7 0/8] Request for inclusion: tcp memory buffers
From: David Miller @ 2011-10-13 20:00 UTC (permalink / raw)
To: glommer
Cc: linux-kernel, akpm, lizf, kamezawa.hiroyu, ebiederm, paul,
gthelen, netdev, linux-mm, kirill, avagin, devel
In-Reply-To: <1318511382-31051-1-git-send-email-glommer@parallels.com>
From: Glauber Costa <glommer@parallels.com>
Date: Thu, 13 Oct 2011 17:09:34 +0400
> This series was extensively reviewed over the past month, and after
> all major comments were merged, I feel it is ready for inclusion when
> the next merge window opens. Minor fixes will be provided if they
> prove to be necessary.
I'm not applying this.
You're turning inline increments and decrements of the existing memory
limits into indirect function calls.
That imposes a new non-trivial cost, in fast paths, even when people
do not use your feature.
Make this evaluate into exactly the same exact code stream we have
now when the memory cgroup feature is not in use, which will be the
majority of users.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] ehea: Change maintainer to me
From: Breno Leitao @ 2011-10-13 20:05 UTC (permalink / raw)
To: Thadeu Lima de Souza Cascardo; +Cc: netdev, linux-kernel, davem
In-Reply-To: <1318535779-18275-1-git-send-email-cascardo@linux.vnet.ibm.com>
On 10/13/2011 04:56 PM, Thadeu Lima de Souza Cascardo wrote:
> Breno Leitao has passed the maintainership to me.
>
> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
Acked-by: Breno Leitão <leitao@linux.vnet.ibm.com>
^ permalink raw reply
* Re: [PATCH net-next] net: more accurate skb truesize
From: David Miller @ 2011-10-13 20:05 UTC (permalink / raw)
To: eric.dumazet; +Cc: bhutchings, netdev, ak
In-Reply-To: <1318526934.2393.49.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 13 Oct 2011 19:28:54 +0200
> [PATCH V2 net-next] net: more accurate skb truesize
>
> skb truesize currently accounts for sk_buff struct and part of skb head.
> kmalloc() roundings are also ignored.
>
> Considering that skb_shared_info is larger than sk_buff, its time to
> take it into account for better memory accounting.
>
> This patch introduces SKB_TRUESIZE(X) macro to centralize various
> assumptions into a single place.
>
> At skb alloc phase, we put skb_shared_info struct at the exact end of
> skb head, to allow a better use of memory (lowering number of
> reallocations), since kmalloc() gives us power-of-two memory blocks.
>
> Unless SLUB/SLUB debug is active, both skb->head and skb_shared_info are
> aligned to cache lines, as before.
>
> Note: This patch might trigger performance regressions because of
> misconfigured protocol stacks, hitting per socket or global memory
> limits that were previously not reached. But its a necessary step for a
> more accurate memory accounting.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH v7 0/8] Request for inclusion: tcp memory buffers
From: Glauber Costa @ 2011-10-13 20:05 UTC (permalink / raw)
To: David Miller
Cc: linux-kernel, akpm, lizf, kamezawa.hiroyu, ebiederm, paul,
gthelen, netdev, linux-mm, kirill, avagin, devel
In-Reply-To: <20111013.160031.605700447623532119.davem@davemloft.net>
On 10/14/2011 12:00 AM, David Miller wrote:
> From: Glauber Costa<glommer@parallels.com>
> Date: Thu, 13 Oct 2011 17:09:34 +0400
>
>> This series was extensively reviewed over the past month, and after
>> all major comments were merged, I feel it is ready for inclusion when
>> the next merge window opens. Minor fixes will be provided if they
>> prove to be necessary.
>
> I'm not applying this.
Thank you for letting me now about your view of this that early.
> You're turning inline increments and decrements of the existing memory
> limits into indirect function calls.
Yes, indeed.
> That imposes a new non-trivial cost, in fast paths, even when people
> do not use your feature.
Well, there is a cost, but all past submissions included round trip
benchmarks.
In none of them I could see any significant slowdown.
> Make this evaluate into exactly the same exact code stream we have
> now when the memory cgroup feature is not in use, which will be the
> majority of users.
What exactly do you mean by "not in use" ? Not compiled in or not
actively being exercised ? If you mean the later, I appreciate tips on
how to achieve it.
Also, I kind of dispute the affirmation that !cgroup will encompass
the majority of users, since cgroups is being enabled by default by
most vendors. All systemd based systems use it extensively, for instance.
^ permalink raw reply
* Re: [PATCH net-next] be2net: fix truesize errors
From: David Miller @ 2011-10-13 20:06 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, sathya.perla, subbu.seetharaman, ajit.khaparde
In-Reply-To: <1318523553.2393.34.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 13 Oct 2011 18:32:33 +0200
> Le jeudi 13 octobre 2011 à 18:31 +0200, Eric Dumazet a écrit :
>> Fix skb truesize underestimations of this driver.
>>
>> Each frag truesize is exactly rx_frag_size bytes. (2048 bytes per
>> default)
>>
>> A driver should not use "sizeof(struct sk_buff)" at all.
>>
>> Signed-off-by: Eric Dumazet <eric.dumazet>
>
> Oh well, garbled Signed-off-by, sorry !
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
:-) Applied.
^ permalink raw reply
* Re: [PATCH net-next] bnx2: fix skb truesize underestimation
From: David Miller @ 2011-10-13 20:06 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, mchan
In-Reply-To: <1318528219.2393.52.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 13 Oct 2011 19:50:19 +0200
> bnx2 allocates a full page per fragment. We must account PAGE_SIZE
> increments on skb->truesize, not the actual frag length.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] e1000: fix skb truesize underestimation
From: David Miller @ 2011-10-13 20:06 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, jeffrey.t.kirsher
In-Reply-To: <1318528422.2393.55.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 13 Oct 2011 19:53:42 +0200
> e1000 allocates a full page per skb fragment. We must account PAGE_SIZE
> increments on skb->truesize, not the actual frag length.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] ixgbe: fix skb truesize underestimation
From: David Miller @ 2011-10-13 20:06 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, jeffrey.t.kirsher
In-Reply-To: <1318528781.2393.59.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 13 Oct 2011 19:59:41 +0200
> ixgbe allocates half a page per skb fragment. We must account
> PAGE_SIZE/2 increments on skb->truesize, not the actual frag length.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] igb: fix skb truesize underestimation
From: David Miller @ 2011-10-13 20:06 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, jeffrey.t.kirsher
In-Reply-To: <1318528601.2393.57.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 13 Oct 2011 19:56:41 +0200
> e1000 allocates half a page per skb fragment. We must account
> PAGE_SIZE/2 increments on skb->truesize, not the actual frag length.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] e1000e: fix skb truesize underestimation
From: David Miller @ 2011-10-13 20:06 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, jeffrey.t.kirsher
In-Reply-To: <1318529016.2393.62.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 13 Oct 2011 20:03:36 +0200
> e1000e allocates a page per skb fragment. We must account
> PAGE_SIZE increments on skb->truesize, not the actual frag length.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH v7 0/8] Request for inclusion: tcp memory buffers
From: David Miller @ 2011-10-13 20:08 UTC (permalink / raw)
To: glommer
Cc: linux-kernel, akpm, lizf, kamezawa.hiroyu, ebiederm, paul,
gthelen, netdev, linux-mm, kirill, avagin, devel
In-Reply-To: <4E9744A6.5010101@parallels.com>
From: Glauber Costa <glommer@parallels.com>
Date: Fri, 14 Oct 2011 00:05:58 +0400
> On 10/14/2011 12:00 AM, David Miller wrote:
>> That imposes a new non-trivial cost, in fast paths, even when people
>> do not use your feature.
> Well, there is a cost, but all past submissions included round trip
> benchmarks.
> In none of them I could see any significant slowdown.
Did you try millions of sockets doing all kinds of different accesses?
Did you check the nanosecond latency of operations over loopback so
that the real cost of you change can be isolated and thus measured
properly?
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v7 0/8] Request for inclusion: tcp memory buffers
From: David Miller @ 2011-10-13 20:12 UTC (permalink / raw)
To: glommer
Cc: linux-kernel, akpm, lizf, kamezawa.hiroyu, ebiederm, paul,
gthelen, netdev, linux-mm, kirill, avagin, devel
In-Reply-To: <4E9744A6.5010101@parallels.com>
From: Glauber Costa <glommer@parallels.com>
Date: Fri, 14 Oct 2011 00:05:58 +0400
> Also, I kind of dispute the affirmation that !cgroup will encompass
> the majority of users, since cgroups is being enabled by default by
> most vendors. All systemd based systems use it extensively, for
> instance.
I will definitely advise people against this, since the cost of having
this on by default is absolutely non-trivial.
People keep asking every few releases "where the heck has my performance
gone" and it's because of creeping features like this. This socket
cgroup feature is a prime example of where that kind of stuff comes
from.
I really get irritated when people go "oh, it's just one indirect
function call" and "oh, it's just one more pointer in struct sock"
We work really hard to _remove_ elements from structures and make them
smaller, and to remove expensive operations from the fast paths.
It might take someone weeks if not months to find a way to make a
patch which compensates for the extra overhead your patches are adding.
And I don't think you fully appreciate that.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v7 0/8] Request for inclusion: tcp memory buffers
From: Glauber Costa @ 2011-10-13 20:14 UTC (permalink / raw)
To: David Miller
Cc: linux-kernel, akpm, lizf, kamezawa.hiroyu, ebiederm, paul,
gthelen, netdev, linux-mm, kirill, avagin, devel
In-Reply-To: <20111013.161221.1969725742975317077.davem@davemloft.net>
On 10/14/2011 12:12 AM, David Miller wrote:
> From: Glauber Costa<glommer@parallels.com>
> Date: Fri, 14 Oct 2011 00:05:58 +0400
>
>> Also, I kind of dispute the affirmation that !cgroup will encompass
>> the majority of users, since cgroups is being enabled by default by
>> most vendors. All systemd based systems use it extensively, for
>> instance.
>
> I will definitely advise people against this, since the cost of having
> this on by default is absolutely non-trivial.
>
> People keep asking every few releases "where the heck has my performance
> gone" and it's because of creeping features like this. This socket
> cgroup feature is a prime example of where that kind of stuff comes
> from.
>
> I really get irritated when people go "oh, it's just one indirect
> function call" and "oh, it's just one more pointer in struct sock"
>
> We work really hard to _remove_ elements from structures and make them
> smaller, and to remove expensive operations from the fast paths.
>
> It might take someone weeks if not months to find a way to make a
> patch which compensates for the extra overhead your patches are adding.
>
> And I don't think you fully appreciate that.
Let's focus on this:
Are you happy, or at least willing to accept, an approach that keep
things as they were with cgroups *compiled out*, or were you referring
to not in use == compiled in, but with no users?
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v7 0/8] Request for inclusion: tcp memory buffers
From: David Miller @ 2011-10-13 20:16 UTC (permalink / raw)
To: glommer
Cc: linux-kernel, akpm, lizf, kamezawa.hiroyu, ebiederm, paul,
gthelen, netdev, linux-mm, kirill, avagin, devel
In-Reply-To: <4E9744A6.5010101@parallels.com>
From: Glauber Costa <glommer@parallels.com>
Date: Fri, 14 Oct 2011 00:05:58 +0400
> Thank you for letting me now about your view of this that early.
I depend upon my colleagues to assist me in the large task that is reviewing
the enormous number of networking patches that get submitted.
Unfortunately, none of them got a chance to review this patch set
seriously, since I know most of them (especially Eric Dumazet) would
balk at the overhead you're proposing to add to our stack, just as I
did.
This is the reality of the situation, and I'm sorry to tell you that
snippy retorts when someone does take the time out to review your work
won't help at all.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] MAINTAINERS: can: the mailinglist moved to vger.kernel.org
From: Oliver Hartkopp @ 2011-10-13 20:17 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: linux-can, netdev
In-Reply-To: <1318506157-10329-1-git-send-email-mkl@pengutronix.de>
On 10/13/11 13:42, Marc Kleine-Budde wrote:
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
> ---
> MAINTAINERS | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index aac56f9..5008b08 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1671,7 +1671,7 @@ CAN NETWORK LAYER
> M: Oliver Hartkopp <socketcan@hartkopp.net>
> M: Oliver Hartkopp <oliver.hartkopp@volkswagen.de>
> M: Urs Thuermann <urs.thuermann@volkswagen.de>
> -L: socketcan-core@lists.berlios.de (subscribers-only)
> +L: linux-can@vger.kernel.org
> L: netdev@vger.kernel.org
> W: http://developer.berlios.de/projects/socketcan/
> S: Maintained
> @@ -1683,7 +1683,7 @@ F: include/linux/can/raw.h
>
> CAN NETWORK DRIVERS
> M: Wolfgang Grandegger <wg@grandegger.com>
> -L: socketcan-core@lists.berlios.de (subscribers-only)
> +L: linux-can@vger.kernel.org
> L: netdev@vger.kernel.org
> W: http://developer.berlios.de/projects/socketcan/
> S: Maintained
^ permalink raw reply
* Re: [PATCH v7 0/8] Request for inclusion: tcp memory buffers
From: David Miller @ 2011-10-13 20:18 UTC (permalink / raw)
To: glommer
Cc: linux-kernel, akpm, lizf, kamezawa.hiroyu, ebiederm, paul,
gthelen, netdev, linux-mm, kirill, avagin, devel
In-Reply-To: <4E9746B0.7030603@parallels.com>
From: Glauber Costa <glommer@parallels.com>
Date: Fri, 14 Oct 2011 00:14:40 +0400
> Are you happy, or at least willing to accept, an approach that keep
> things as they were with cgroups *compiled out*, or were you referring
> to not in use == compiled in, but with no users?
To me these are the same exact thing, because %99 of users will be running
a kernel with every feature turned on in the Kconfig.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v7 0/8] Request for inclusion: tcp memory buffers
From: Glauber Costa @ 2011-10-13 20:23 UTC (permalink / raw)
To: David Miller
Cc: linux-kernel, akpm, lizf, kamezawa.hiroyu, ebiederm, paul,
gthelen, netdev, linux-mm, kirill, avagin, devel
In-Reply-To: <20111013.161608.1413756673453885746.davem@davemloft.net>
On 10/14/2011 12:16 AM, David Miller wrote:
> From: Glauber Costa<glommer@parallels.com>
> Date: Fri, 14 Oct 2011 00:05:58 +0400
>
>> Thank you for letting me now about your view of this that early.
>
> I depend upon my colleagues to assist me in the large task that is reviewing
> the enormous number of networking patches that get submitted.
>
> Unfortunately, none of them got a chance to review this patch set
> seriously, since I know most of them (especially Eric Dumazet) would
> balk at the overhead you're proposing to add to our stack, just as I
> did.
>
> This is the reality of the situation, and I'm sorry to tell you that
> snippy retorts when someone does take the time out to review your work
> won't help at all.
I understand that and appreciate your time.
I'll try to come up with something that addresses this problem in the
next submission.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH net-next] net: more accurate skb truesize
From: Andi Kleen @ 2011-10-13 20:33 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev
In-Reply-To: <1318519581.2393.18.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
On Thu, Oct 13, 2011 at 05:26:21PM +0200, Eric Dumazet wrote:
> skb truesize currently accounts for sk_buff struct and part of skb head.
>
> Considering that skb_shared_info is larger than sk_buff, its time to
> take it into account for better memory accounting.
>
> This patch introduces SKB_TRUESIZE(X) macro to centralize various
> assumptions into a single place.
It's still quite inaccurate, especially for the kmalloced data area if it's not
paged. It would be better to ask slab how much memory was really
allocated. But at least this could be done more easily now with the new
macro, so it's definitely a step in the right direction.
-Andi
^ permalink raw reply
* Re: [net-next 1/5] stmmac: add CHAINED descriptor mode support (V2)
From: David Miller @ 2011-10-13 20:39 UTC (permalink / raw)
To: peppe.cavallaro; +Cc: netdev, rayagond
In-Reply-To: <1318426688-9419-2-git-send-email-peppe.cavallaro@st.com>
From: Giuseppe CAVALLARO <peppe.cavallaro@st.com>
Date: Wed, 12 Oct 2011 15:38:04 +0200
> +#if defined(CONFIG_STMMAC_RING)
> +
> +static unsigned int stmmac_jumbo_frm(struct stmmac_priv *priv,
> + struct sk_buff *skb, int csum_insertion)
> +{
This is not exactly what I meant.
In your original patch, two or three line snippets of code were conditionalized.
That's what I wanted you to do here. Keep as much common code around as possible
in the driver *.c file, but the small 2 or 3 line conditional parts are implemented
in very small well contained inline functions implemented in a header file.
These small, 2 or 3 line, inline functions are where the ifdefs go.
I didn't mean to replicate all of the functions, in their entirety, into some
header file.
You might was well put the entire driver into a header file, then you can add
all the ifdefs you want :-)
^ permalink raw reply
* Re: [PATCH net-next] net: more accurate skb truesize
From: Eric Dumazet @ 2011-10-13 20:51 UTC (permalink / raw)
To: Andi Kleen; +Cc: David Miller, netdev
In-Reply-To: <20111013203352.GA5707@tassilo.jf.intel.com>
Le jeudi 13 octobre 2011 à 13:33 -0700, Andi Kleen a écrit :
> On Thu, Oct 13, 2011 at 05:26:21PM +0200, Eric Dumazet wrote:
> > skb truesize currently accounts for sk_buff struct and part of skb head.
> >
> > Considering that skb_shared_info is larger than sk_buff, its time to
> > take it into account for better memory accounting.
> >
> > This patch introduces SKB_TRUESIZE(X) macro to centralize various
> > assumptions into a single place.
>
> It's still quite inaccurate, especially for the kmalloced data area if it's not
> paged. It would be better to ask slab how much memory was really
> allocated. But at least this could be done more easily now with the new
> macro, so it's definitely a step in the right direction.
Note : in skb_alloc() function, SKB_TRUESIZE(size) delivers the exact
value : I do the ksize(data) call to ask how many byte kmalloc()
provided me.
So skb->truesize is quite accurate (unless KMEMCHECK or other debug
stuff is used of course)
For the SKB_TRUESIZE() macro, we dont want to do a dummy call to
kmalloc()/kfree(), since its basically used to roughly set a queue
limit.
Thanks !
^ permalink raw reply
* [PATCH net-next] sky2: fix skb truesize underestimation
From: Eric Dumazet @ 2011-10-13 21:11 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Stephen Hemminger
sky2 allocates a page per skb fragment. We must account
PAGE_SIZE increments on skb->truesize, not the actual frag length.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
CC: Stephen Hemminger <shemminger@vyatta.com>
---
drivers/net/ethernet/marvell/sky2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/marvell/sky2.c b/drivers/net/ethernet/marvell/sky2.c
index 6895e3b..9263490 100644
--- a/drivers/net/ethernet/marvell/sky2.c
+++ b/drivers/net/ethernet/marvell/sky2.c
@@ -2486,7 +2486,7 @@ static void skb_put_frags(struct sk_buff *skb, unsigned int hdr_space,
frag->size = size;
skb->data_len += size;
- skb->truesize += size;
+ skb->truesize += PAGE_SIZE;
skb->len += size;
length -= size;
}
^ permalink raw reply related
* Re: [PATCH net-next] sky2: fix skb truesize underestimation
From: David Miller @ 2011-10-13 21:13 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, shemminger
In-Reply-To: <1318540290.2533.22.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 13 Oct 2011 23:11:30 +0200
> sky2 allocates a page per skb fragment. We must account
> PAGE_SIZE increments on skb->truesize, not the actual frag length.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied, thanks Eric.
^ permalink raw reply
* [PATCH net-next] ftmac100: fix skb truesize underestimation
From: Eric Dumazet @ 2011-10-13 21:20 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Po-Yu Chuang
ftmac100 allocates a page per skb fragment. We must account
PAGE_SIZE increments on skb->truesize, not the actual frag length.
If frame is under 64 bytes, page is freed, so increase truesize only for
bigger frames.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
CC: Po-Yu Chuang <ratbert@faraday-tech.com>
---
drivers/net/ethernet/faraday/ftmac100.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/faraday/ftmac100.c b/drivers/net/ethernet/faraday/ftmac100.c
index 9bd7746..a127cb2 100644
--- a/drivers/net/ethernet/faraday/ftmac100.c
+++ b/drivers/net/ethernet/faraday/ftmac100.c
@@ -439,7 +439,10 @@ static bool ftmac100_rx_packet(struct ftmac100 *priv, int *processed)
skb_fill_page_desc(skb, 0, page, 0, length);
skb->len += length;
skb->data_len += length;
- skb->truesize += length;
+
+ /* page might be freed in __pskb_pull_tail() */
+ if (length > 64)
+ skb->truesize += PAGE_SIZE;
__pskb_pull_tail(skb, min(length, 64));
ftmac100_alloc_rx_page(priv, rxdes, GFP_ATOMIC);
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox