Message ID | 1348111182-6827-1-git-send-email-roy.qing.li@gmail.com |
---|---|
State | Rejected, archived |
Delegated to: | David Miller |
Headers | show |
On Thu, 2012-09-20 at 11:19 +0800, roy.qing.li@gmail.com wrote: > From: Li RongQing <roy.qing.li@gmail.com> > > Ensure that frags and frags_list of dst skb are empty if need to call > skb_copy_bits, or else it will break the data sequence. > > Cc: Eric Dumazet <edumazet@google.com> > Signed-off-by: Li RongQing <roy.qing.li@gmail.com> > --- > net/core/skbuff.c | 8 ++++---- > 1 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/net/core/skbuff.c b/net/core/skbuff.c > index fe00d12..f0b9446 100644 > --- a/net/core/skbuff.c > +++ b/net/core/skbuff.c > @@ -3455,15 +3455,15 @@ bool skb_try_coalesce(struct sk_buff *to, struct sk_buff *from, > if (skb_cloned(to)) > return false; > > - if (len <= skb_tailroom(to)) { > + if (skb_has_frag_list(to) || skb_has_frag_list(from)) > + return false; > + > + if (len <= skb_tailroom(to) && !skb_shinfo(to)->nr_frags) { > BUG_ON(skb_copy_bits(from, 0, skb_put(to, len), len)); > *delta_truesize = 0; > return true; > } > > - if (skb_has_frag_list(to) || skb_has_frag_list(from)) > - return false; > - > if (skb_headlen(from) != 0) { > struct page *page; > unsigned int offset; This is not needed at all. -- 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
>> unsigned int offset; > > This is not needed at all. > > I think the below modification maybe needed, if (len <= skb_tailroom(to) && !skb_shinfo(to)->nr_frags) { .. } First skb A is added to skb TO frags, since the len is larger than skb_tailroot(TO), but second len of skb B is less than skb_tailroot(To) which will call skb_copy_bits. Of cause, this kinds of cases maybe only exist on my mind. -Roy -- 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
On Thu, 2012-09-20 at 13:40 +0800, RongQing Li wrote: > >> unsigned int offset; > > > > This is not needed at all. > > > > > > I think the below modification maybe needed, > if (len <= skb_tailroom(to) && !skb_shinfo(to)->nr_frags) { > .. > } > > First skb A is added to skb TO frags, since the len is larger > than skb_tailroot(TO), but second len of skb B is less than > skb_tailroot(To) which will call skb_copy_bits. > > Of cause, this kinds of cases maybe only exist on my mind. > Did you read skb_tailroom(to) definition by any chance ? static inline int skb_tailroom(const struct sk_buff *skb) { return skb_is_nonlinear(skb) ? 0 : skb->end - skb->tail; } Current code is fine, because if @to is linear, its ... linear. -- 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
> Did you read skb_tailroom(to) definition by any chance ? > > static inline int skb_tailroom(const struct sk_buff *skb) > { > return skb_is_nonlinear(skb) ? 0 : skb->end - skb->tail; > } > > Current code is fine, because if @to is linear, its ... linear. > > > I am wrong, thanks. -- 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
On Thu, 2012-09-20 at 13:57 +0800, RongQing Li wrote: > > I am wrong, thanks. Thanks for taking a look at this stuff ;) -- 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 --git a/net/core/skbuff.c b/net/core/skbuff.c index fe00d12..f0b9446 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -3455,15 +3455,15 @@ bool skb_try_coalesce(struct sk_buff *to, struct sk_buff *from, if (skb_cloned(to)) return false; - if (len <= skb_tailroom(to)) { + if (skb_has_frag_list(to) || skb_has_frag_list(from)) + return false; + + if (len <= skb_tailroom(to) && !skb_shinfo(to)->nr_frags) { BUG_ON(skb_copy_bits(from, 0, skb_put(to, len), len)); *delta_truesize = 0; return true; } - if (skb_has_frag_list(to) || skb_has_frag_list(from)) - return false; - if (skb_headlen(from) != 0) { struct page *page; unsigned int offset;