diff mbox

[net-next] net: Fix truesize accounting in skb_gro_receive()

Message ID 1336037601.3752.8.camel@edumazet-glaptop
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Eric Dumazet May 3, 2012, 9:33 a.m. UTC
From: Eric Dumazet <edumazet@google.com>

GRO is very optimistic in skb truesize estimates, only taking into
account the used part of fragments.

Be conservative, and use more precise computation, so that bloated GRO
skbs can be collapsed eventually.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Alexander Duyck <alexander.h.duyck@intel.com>
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 net/core/skbuff.c |   11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)



--
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

Comments

Duyck, Alexander H May 3, 2012, 10:48 a.m. UTC | #1
On 05/03/2012 02:33 AM, Eric Dumazet wrote:
> From: Eric Dumazet<edumazet@google.com>
>
> GRO is very optimistic in skb truesize estimates, only taking into
> account the used part of fragments.
>
> Be conservative, and use more precise computation, so that bloated GRO
> skbs can be collapsed eventually.
>
> Signed-off-by: Eric Dumazet<edumazet@google.com>
> Cc: Alexander Duyck<alexander.h.duyck@intel.com>
> Cc: Jeff Kirsher<jeffrey.t.kirsher@intel.com>
> ---
>   net/core/skbuff.c |   11 ++++++++---
>   1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 9e8caa0..e1f8bba 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -2871,6 +2871,7 @@ int skb_gro_receive(struct sk_buff **head, struct sk_buff *skb)
>   	unsigned int len = skb_gro_len(skb);
>   	unsigned int offset = skb_gro_offset(skb);
>   	unsigned int headlen = skb_headlen(skb);
> +	unsigned int delta_truesize;
>
>   	if (p->len + len>= 65536)
>   		return -E2BIG;
> @@ -2900,11 +2901,14 @@ int skb_gro_receive(struct sk_buff **head, struct sk_buff *skb)
>   		frag->page_offset += offset;
>   		skb_frag_size_sub(frag, offset);
>
> +		/* all fragments truesize : remove (head size + sk_buff) */
> +		delta_truesize = skb->truesize - SKB_TRUESIZE(skb_end_pointer(skb) - skb->head);
> +
>   		skb->truesize -= skb->data_len;
>   		skb->len -= skb->data_len;
>   		skb->data_len = 0;
>
> -		NAPI_GRO_CB(skb)->free = 1;
> +		NAPI_GRO_CB(skb)->free = NAPI_GRO_FREE;
>   		goto done;
>   	} else if (skb->head_frag) {
>   		int nr_frags = pinfo->nr_frags;
> @@ -2929,6 +2933,7 @@ int skb_gro_receive(struct sk_buff **head, struct sk_buff *skb)
>   		memcpy(frag + 1, skbinfo->frags, sizeof(*frag) * skbinfo->nr_frags);
>   		/* We dont need to clear skbinfo->nr_frags here */
>
> +		delta_truesize = skb->truesize - SKB_DATA_ALIGN(sizeof(struct sk_buff));
>   		NAPI_GRO_CB(skb)->free = NAPI_GRO_FREE_STOLEN_HEAD;
>   		goto done;
>   	} else if (skb_gro_len(p) != pinfo->gso_size)
> @@ -2971,7 +2976,7 @@ int skb_gro_receive(struct sk_buff **head, struct sk_buff *skb)
>   	p = nskb;
>
>   merge:
> -	p->truesize += skb->truesize - len;
> +	delta_truesize = skb->truesize;
>   	if (offset>  headlen) {
>   		unsigned int eat = offset - headlen;
>
> @@ -2991,7 +2996,7 @@ merge:
>   done:
>   	NAPI_GRO_CB(p)->count++;
>   	p->data_len += len;
> -	p->truesize += len;
> +	p->truesize += delta_truesize;
>   	p->len += len;
>
>   	NAPI_GRO_CB(skb)->same_flow = 1;
>
>
Couldn't sleep so I figured I would review some patches and maybe get a 
few more written before the sun came up.   I was actually thinking of 
trying to get to this before I logged in.  Looks like you have this one 
taken care of already so I will go take care of skb_head_is_locked.

Acked-by: Alexander Duyck <alexander.h.duyck@intel.com>
--
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 May 3, 2012, 5:22 p.m. UTC | #2
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 03 May 2012 11:33:21 +0200

> From: Eric Dumazet <edumazet@google.com>
> 
> GRO is very optimistic in skb truesize estimates, only taking into
> account the used part of fragments.
> 
> Be conservative, and use more precise computation, so that bloated GRO
> skbs can be collapsed eventually.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied.
--
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
diff mbox

Patch

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 9e8caa0..e1f8bba 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -2871,6 +2871,7 @@  int skb_gro_receive(struct sk_buff **head, struct sk_buff *skb)
 	unsigned int len = skb_gro_len(skb);
 	unsigned int offset = skb_gro_offset(skb);
 	unsigned int headlen = skb_headlen(skb);
+	unsigned int delta_truesize;
 
 	if (p->len + len >= 65536)
 		return -E2BIG;
@@ -2900,11 +2901,14 @@  int skb_gro_receive(struct sk_buff **head, struct sk_buff *skb)
 		frag->page_offset += offset;
 		skb_frag_size_sub(frag, offset);
 
+		/* all fragments truesize : remove (head size + sk_buff) */
+		delta_truesize = skb->truesize - SKB_TRUESIZE(skb_end_pointer(skb) - skb->head);
+
 		skb->truesize -= skb->data_len;
 		skb->len -= skb->data_len;
 		skb->data_len = 0;
 
-		NAPI_GRO_CB(skb)->free = 1;
+		NAPI_GRO_CB(skb)->free = NAPI_GRO_FREE;
 		goto done;
 	} else if (skb->head_frag) {
 		int nr_frags = pinfo->nr_frags;
@@ -2929,6 +2933,7 @@  int skb_gro_receive(struct sk_buff **head, struct sk_buff *skb)
 		memcpy(frag + 1, skbinfo->frags, sizeof(*frag) * skbinfo->nr_frags);
 		/* We dont need to clear skbinfo->nr_frags here */
 
+		delta_truesize = skb->truesize - SKB_DATA_ALIGN(sizeof(struct sk_buff));
 		NAPI_GRO_CB(skb)->free = NAPI_GRO_FREE_STOLEN_HEAD;
 		goto done;
 	} else if (skb_gro_len(p) != pinfo->gso_size)
@@ -2971,7 +2976,7 @@  int skb_gro_receive(struct sk_buff **head, struct sk_buff *skb)
 	p = nskb;
 
 merge:
-	p->truesize += skb->truesize - len;
+	delta_truesize = skb->truesize;
 	if (offset > headlen) {
 		unsigned int eat = offset - headlen;
 
@@ -2991,7 +2996,7 @@  merge:
 done:
 	NAPI_GRO_CB(p)->count++;
 	p->data_len += len;
-	p->truesize += len;
+	p->truesize += delta_truesize;
 	p->len += len;
 
 	NAPI_GRO_CB(skb)->same_flow = 1;