>>> On 09.07.13 at 18:51, Wei Liu <wei.liu2@xxxxxxxxxx> wrote: > On Tue, Jul 09, 2013 at 07:52:31AM +0100, Jan Beulich wrote: >> >>> On 08.07.13 at 17:48, Wei Liu <wei.liu2@xxxxxxxxxx> wrote: >> > On Mon, Jul 08, 2013 at 03:20:26PM +0100, Jan Beulich wrote: >> >> @@ -1014,7 +1025,7 @@ err: >> >> >> >> skb_shinfo(skb)->frags[0].page_offset = rx->offset; >> >> skb_frag_size_set(&skb_shinfo(skb)->frags[0], rx->status); >> >> - skb->data_len = rx->status; >> >> + skb->len = skb->data_len = rx->status; >> > >> > This is not correct. You should not be needing this. Now you lose count >> > of SKB head len. Try to go without the above line and see if it makes a >> > difference? >> >> I don't follow - at this point, there's 0 bytes of head (this only >> changes with the first call to __pskb_pull_tail()). Hence ->len == >> ->data_len seems correct to me (and afaict pulling would do the >> wrong thing if I dropped that change). >> > > My bad, I suggested the wrong thing. :-( > > But I would prefer skb->len += skb->data_len. In the case that skb->len > == 0 it's the same as your line while skb->len is not zero it would also > do the right thing. I can do that, albeit I don't see how ->len could end up non-zero here. > As for the warning in skb_try_coalesce, I don't see any direct call to > it in netfront, I will need to think about it. It looks like it's really > something very deep in the stack. Yes, as the call stack provided by Dion proves. The question really is whether the patch somehow results in ->truesize to be incorrect, or whether - as Eric points out - this is "normal" for the sort of special SKBs here (having a rather small headlen). If what he says is applicable here, it may hint at the pulling we do still not being sufficient for the full TCP header to be in the linear part (which iirc is the main [if not the only purpose] of us doing the pull in the first place). Jan -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html