[PATCH net-next] skbuff: update sk truesize in pskb_expand_head

[PATCH net-next] skbuff: update sk truesize in pskb_expand_head

[roy.qing.li]

when pskb_expand_head reallocates header of &sk_buff, the sk
truesize should be updated simultaneously

Signed-off-by: RongQing.Li <roy.qing.li@gmail.com>
---
 net/core/skbuff.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 387703f..48e0be9 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -920,6 +920,7 @@ int pskb_expand_head(struct sk_buff *skb, int nhead, int ntail,
 	}
 	off = (data + nhead) - skb->head;
 
+	skb->truesize += (size - (skb_end_pointer(skb) - skb->head));
 	skb->head     = data;
 adjust_others:
 	skb->data    += off;
-- 
1.7.1

Re: [PATCH net-next] skbuff: update sk truesize in pskb_expand_head

[David Miller]

From: roy.qing.li@gmail.com
Date: Fri, 14 Oct 2011 15:39:30 +0800

> when pskb_expand_head reallocates header of &sk_buff, the sk
> truesize should be updated simultaneously
> 
> Signed-off-by: RongQing.Li <roy.qing.li@gmail.com>

I know you did not test this patch at all.

You can't modify the truesize because packets passed to this routine
are often attached to a socket, thus if you change the truesize the
socket memory accouting will be adjusted incorrectly later when the
SKB is freed up.

Most SKB modifying functions have to operate with this restriction.

Re: [PATCH net-next] skbuff: update sk truesize in pskb_expand_head

[RongQing Li]

2011/10/14 David Miller <davem@davemloft.net>:
> From: roy.qing.li@gmail.com
> Date: Fri, 14 Oct 2011 15:39:30 +0800
>
>> when pskb_expand_head reallocates header of &sk_buff, the sk
>> truesize should be updated simultaneously
>>
>> Signed-off-by: RongQing.Li <roy.qing.li@gmail.com>
>
> I know you did not test this patch at all.
>
> You can't modify the truesize because packets passed to this routine
> are often attached to a socket, thus if you change the truesize the
> socket memory accouting will be adjusted incorrectly later when the
> SKB is freed up.
>
> Most SKB modifying functions have to operate with this restriction.
>
>

I am sorry I do not know how to test it in fact.

But I believe the adjustment is right here
About incorrectly impact on socket, I am consider it, but I still no
idea about it.

Thanks.

Re: [PATCH net-next] skbuff: update sk truesize in pskb_expand_head

[David Miller]

From: RongQing Li <roy.qing.li@gmail.com>
Date: Sat, 15 Oct 2011 11:52:22 +0800

> About incorrectly impact on socket, I am consider it, but I still no
> idea about it.

Sockets attach themselves to packets, and assign a destructor.

This destructor atomically decrements the receive or send buffer
space used, and amount decremented is skb->truesize.

Therefore if you change skb->truesize on such an SKB, the wrong
amount will be decremented when the destructor is called.

This is a very fundamental aspec of SKB handling, perhaps you should
familiarize yourself with SKBs a little bit more before modifying code
which manages them.

Re: [PATCH net-next] skbuff: update sk truesize in pskb_expand_head

[Eric Dumazet]

Le vendredi 14 octobre 2011 à 15:39 +0800, roy.qing.li@gmail.com a
écrit :
> when pskb_expand_head reallocates header of &sk_buff, the sk
> truesize should be updated simultaneously
> 
> Signed-off-by: RongQing.Li <roy.qing.li@gmail.com>
> ---
>  net/core/skbuff.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 387703f..48e0be9 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -920,6 +920,7 @@ int pskb_expand_head(struct sk_buff *skb, int nhead, int ntail,
>  	}
>  	off = (data + nhead) - skb->head;
>  
> +	skb->truesize += (size - (skb_end_pointer(skb) - skb->head));
>  	skb->head     = data;
>  adjust_others:
>  	skb->data    += off;

I dont believe this is needed or complete patch.

Callers that need an updated truesize do the adjustement.

Re: [PATCH net-next] skbuff: update sk truesize in pskb_expand_head

[RongQing Li]

2011/10/14 Eric Dumazet <eric.dumazet@gmail.com>:
> Le vendredi 14 octobre 2011 à 15:39 +0800, roy.qing.li@gmail.com a
> écrit :
>
> I dont believe this is needed or complete patch.
>
> Callers that need an updated truesize do the adjustement.
>

If we think the size which needs to adjust is small, we can ignore it,
I accept it.

if Let caller to adjust it, I think the caller is hard to get the adjustment.

Do you think the below adjustment is needed?

void skb_add_rx_frag(struct sk_buff *skb, int i, struct page *page, int off,
		int size)
{
	skb_fill_page_desc(skb, i, page, off, size);
	skb->len += size;
	skb->data_len += size;
-	skb->truesize += size;
+	skb->truesize += PAGE_SIZE;
}

Thanks
-Qing