net: Stop decapitating clones that have a head_frag
Subsystems (3)
PATCH v2 1/1 net: Stop decapitating clones that have a head_frag
This change is meant ot prevent stealing the skb->head to use as a page in the event that the skb->head was cloned. This allows the other clones to track each other via shinfo->dataref.
Without this we break down to two methods for tracking the reference count, one being dataref, the other being the page count. As a result it becomes difficult to track how many references there are to skb->head.
---
net/core/skbuff.c | 9 +++++---- net/ipv4/tcp_input.c | 9 ++------- 2 files changed, 7 insertions(+), 11 deletions(-)
net/core/skbuff.c +5 -4
@@ -1699,17 +1699,18 @@ static bool __skb_splice_bits(struct sk_buff *skb, struct pipe_inode_info *pipe, struct splice_pipe_desc *spd, struct sock *sk) { int seg; -bool head_is_linear = !skb->head_frag; +bool head_is_locked = !skb->head_frag ||skb_cloned (skb);/* map the linear part : -* If skb->head_frag is set, this 'linear' part is backed -* by a fragment, and we can avoid a copy. +* If skb->head_frag is set, this 'linear' part is backed by a +* fragment, and if the head is not shared with any clones then +* we can avoid a copy since we own the head portion of this page. */ if (__splice_segment (virt_to_page (skb->data), (unsigned long ) skb->data & (PAGE_SIZE -1 ),skb_headlen (skb), offset, len, skb, spd, - head_is_linear, + head_is_locked, sk, pipe))return true;
net/ipv4/tcp_input.c +2 -7
@@ -4589,7 +4589,7 @@ copyfrags: to->data_len += len; goto merge; } -if (from->head_frag) { +if (from->head_frag && !skb_cloned (from)) {struct page *page;unsigned int offset;
@@ -4599,12 +4599,7 @@ copyfrags: offset = from->data - (unsigned char *)page_address (page);skb_fill_page_desc (to,skb_shinfo (to)->nr_frags, page, offset,skb_headlen (from)); - -if (skb_cloned (from)) -get_page (page); -else - *fragstolen = true; - + *fragstolen = true; delta = len;/* we dont know real truesize... */ goto copyfrags; }
Quoted 22 lines · Alexander Duyck
This change is meant ot prevent stealing the
skb->headto use as a page in the event that theskb->headwas cloned. This allows the other clones to track each other viashinfo->dataref.Without this we break down to two methods for tracking the reference count, one being dataref, the other being the page count. As a result it becomes difficult to track how many references there are to
skb->head.Signed-off-by Alexander Duyck <[email protected]>Cc Eric Dumazet <[email protected]>Cc Jeff Kirsher <[email protected]>---
net/core/skbuff.c | 9 +++++---- net/ipv4/tcp_input.c | 9 ++------- 2 files changed, 7 insertions(+), 11 deletions(-)net/core/skbuff.c +0
-0
net/core/skbuff.c static bool __skb_splice_bits(struct sk_buff *skb, struct pipe_inode_info *pipe, · line 1703
Quoted 6 lines
net/core/skbuff.c +1
-1@@ -1699,17 +1699,18 @@ static bool __skb_splice_bits(struct sk_buff *skb, struct pipe_inode_info *pipe, struct splice_pipe_desc *spd, struct sock *sk) { int seg; -bool head_is_linear = !skb->head_frag; +bool head_is_locked = !skb->head_frag ||skb_cloned (skb);
Thanks
From: Eric Dumazet <[email protected]>
Date: Thu, 03 May 2012 06:33:58 +0200
Quoted 10 lines · Alexander Duyck
Quoted 9 lines
This change is meant ot prevent stealing the
skb->headto use as a page in the event that theskb->headwas cloned. This allows the other clones to track each other viashinfo->dataref.Without this we break down to two methods for tracking the reference count, one being dataref, the other being the page count. As a result it becomes difficult to track how many references there are to
skb->head.Signed-off-by Alexander Duyck <[email protected]>
...
Quoted 1 line
Acked-by Eric Dumazet <[email protected]>
Applied, thanks.
These tests of the form:
head_frag || cloned
and
head_frag && cloned
will probably crop up in other places. Therefore we should probably add a "skb_head_is_locked()" helper function to skbuff.h
Well, there are already two instances, which is candidate enough. :-)