| Message ID | 20150204163343.GA857@haze |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On Wed, 2015-02-04 at 17:33 +0100, Erik Hugne wrote: > skb_try_coalesce should bail out for a number of reasons, if the target skb is > cloned, doesn't have enough room, or if either source or target skb have > fraglists. > > However, it seems that the skb_has_frag_list check is done too late, and a small > skb may be copied into the tailroom of the head, even if it has a fraglist. > > Wouldn't this be more correct? > > diff --git a/net/core/skbuff.c b/net/core/skbuff.c > index 56db472..8d02140 100644 > --- a/net/core/skbuff.c > +++ b/net/core/skbuff.c > @@ -4056,6 +4056,9 @@ bool skb_try_coalesce(struct sk_buff *to, struct sk_buff *from, > if (skb_cloned(to)) > return false; > > + if (skb_has_frag_list(to) || skb_has_frag_list(from)) > + return false; > + > if (len <= skb_tailroom(to)) { > if (len) > BUG_ON(skb_copy_bits(from, 0, skb_put(to, len), len)); > @@ -4063,9 +4066,6 @@ bool skb_try_coalesce(struct sk_buff *to, struct sk_buff *from, > 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; > Patch is not needed. If to had a fraglist, skb_tailroom(to) would be 0 -- 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 56db472..8d02140 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -4056,6 +4056,9 @@ bool skb_try_coalesce(struct sk_buff *to, struct sk_buff *from, if (skb_cloned(to)) return false; + if (skb_has_frag_list(to) || skb_has_frag_list(from)) + return false; + if (len <= skb_tailroom(to)) { if (len) BUG_ON(skb_copy_bits(from, 0, skb_put(to, len), len)); @@ -4063,9 +4066,6 @@ bool skb_try_coalesce(struct sk_buff *to, struct sk_buff *from, 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;