[INET]: Fix truesize setting in ip_append_data

[INET]: Fix truesize setting in ip_append_data

[Herbert Xu]

Hi Dave:

[INET]: Fix truesize setting in ip_append_data

As it is ip_append_data only counts page fragments to the skb that
allocated it.  As such it means that the first skb gets hit with a
4K charge even though it might have only used a fraction of it while
all subsequent skb's that use the same page gets away with no charge
at all.

This bug was exposed by the UDP accounting patch.

This patch also fixes a similar bug in ip_append_page spotted by you :)

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/ipv4/ip_output.c b/net/ipv4/ip_output.c
index fd99fbd..df1eff1 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -1016,7 +1016,6 @@ alloc_new_skb:
 
 				skb_fill_page_desc(skb, i, page, 0, 0);
 				frag = &skb_shinfo(skb)->frags[i];
-				skb->truesize += PAGE_SIZE;
 				atomic_add(PAGE_SIZE, &sk->sk_wmem_alloc);
 			} else {
 				err = -EMSGSIZE;
@@ -1030,6 +1029,7 @@ alloc_new_skb:
 			frag->size += copy;
 			skb->len += copy;
 			skb->data_len += copy;
+			skb->truesize += copy;
 		}
 		offset += copy;
 		length -= copy;
@@ -1172,6 +1172,7 @@ ssize_t	ip_append_page(struct sock *sk, struct page *page,
 
 		skb->len += len;
 		skb->data_len += len;
+		skb->truesize += len;
 		offset += len;
 		size -= len;
 	}
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 6338a9c..bb6ba1e 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -1316,7 +1316,6 @@ alloc_new_skb:
 
 				skb_fill_page_desc(skb, i, page, 0, 0);
 				frag = &skb_shinfo(skb)->frags[i];
-				skb->truesize += PAGE_SIZE;
 				atomic_add(PAGE_SIZE, &sk->sk_wmem_alloc);
 			} else {
 				err = -EMSGSIZE;
@@ -1330,6 +1329,7 @@ alloc_new_skb:
 			frag->size += copy;
 			skb->len += copy;
 			skb->data_len += copy;
+			skb->truesize += copy;
 		}
 		offset += copy;
 		length -= copy;

Re: [INET]: Fix truesize setting in ip_append_data

[David Miller]

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Wed, 23 Jan 2008 14:39:42 +1100

> [INET]: Fix truesize setting in ip_append_data
> 
> As it is ip_append_data only counts page fragments to the skb that
> allocated it.  As such it means that the first skb gets hit with a
> 4K charge even though it might have only used a fraction of it while
> all subsequent skb's that use the same page gets away with no charge
> at all.
> 
> This bug was exposed by the UDP accounting patch.
> 
> This patch also fixes a similar bug in ip_append_page spotted by you :)
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

I added this to net-2.6, thanks Herbert!

I already checked in the ip_append_page() fix to net-2.6 so
I simply removed that hunk when adding your fix.

Thanks again!

Re: [INET]: Fix truesize setting in ip_append_data

[Herbert Xu]

On Tue, Jan 22, 2008 at 09:47:19PM -0800, David Miller wrote:
> 
> I already checked in the ip_append_page() fix to net-2.6 so
> I simply removed that hunk when adding your fix.
> 
> Thanks again!

Thank you!

I just received a message from Takahiro Yasui that my bug fix is buggy
too :) So here is a fix on top of that.

[INET]: Change sk_wmem_alloc together with truesize

Whenever truesize gets updated on output we need to update sk_wmem_alloc
too because when the packet gets freed the truesize will be subtracted
from it.  This patch fixes the out-of-place sk_wmem_alloc in ip_append_data
and ip_append_page.

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/ipv4/ip_output.c b/net/ipv4/ip_output.c
index df1eff1..bc9e575 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -1016,7 +1016,6 @@ alloc_new_skb:
 
 				skb_fill_page_desc(skb, i, page, 0, 0);
 				frag = &skb_shinfo(skb)->frags[i];
-				atomic_add(PAGE_SIZE, &sk->sk_wmem_alloc);
 			} else {
 				err = -EMSGSIZE;
 				goto error;
@@ -1030,6 +1029,7 @@ alloc_new_skb:
 			skb->len += copy;
 			skb->data_len += copy;
 			skb->truesize += copy;
+			atomic_add(copy, &sk->sk_wmem_alloc);
 		}
 		offset += copy;
 		length -= copy;
@@ -1173,6 +1173,7 @@ ssize_t	ip_append_page(struct sock *sk, struct page *page,
 		skb->len += len;
 		skb->data_len += len;
 		skb->truesize += len;
+		atomic_add(len, &sk->sk_wmem_alloc);
 		offset += len;
 		size -= len;
 	}
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index bb6ba1e..3bef30e 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -1316,7 +1316,6 @@ alloc_new_skb:
 
 				skb_fill_page_desc(skb, i, page, 0, 0);
 				frag = &skb_shinfo(skb)->frags[i];
-				atomic_add(PAGE_SIZE, &sk->sk_wmem_alloc);
 			} else {
 				err = -EMSGSIZE;
 				goto error;
@@ -1330,6 +1329,7 @@ alloc_new_skb:
 			skb->len += copy;
 			skb->data_len += copy;
 			skb->truesize += copy;
+			atomic_add(copy, &sk->sk_wmem_alloc);
 		}
 		offset += copy;
 		length -= copy;

Re: [INET]: Fix truesize setting in ip_append_data

[David Miller]

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Wed, 23 Jan 2008 17:41:31 +1100

> On Tue, Jan 22, 2008 at 09:47:19PM -0800, David Miller wrote:
> > 
> > I already checked in the ip_append_page() fix to net-2.6 so
> > I simply removed that hunk when adding your fix.
> > 
> > Thanks again!
> 
> Thank you!
> 
> I just received a message from Takahiro Yasui that my bug fix is buggy
> too :) So here is a fix on top of that.

I already correct this and respinned the net-2.6 tree