Re: [Xen-devel] [PATCH] xen-netfront: pull on receive skb may need to happen earlier

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 2013-07-10 at 07:58 +0100, Jan Beulich wrote:

> 
> 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).

I have not closely followed the thread (it seems it did not began on
netdev so I do not have the first messages), but if we cook an skb with
no tailroom (headroom is irrelevant), and IP stack or TCP stack has to
pull more bytes from the frags to skb->head, then skb->head must be
reallocated to get more headroom before pulling the headers.

This operation is _expensive_, as it involves copy of struct
skb_shared_info and eventually page refcounts games if original skb
was cloned (sniffer)

We have a one time WARN_ON_ONCE() in skb_try_coalesce(),
to detect such non optimal behavior, but not in the fast path, only in
the part used in TCP coalescing.

So the right thing would be to cook skbs so that there is enough
tailroom.

But there are 256 bytes of tailroom, it should be enough unless some
huge amount of headers are used.

If you believe it's fine and very unlikely, then ignore the warning,
because using 512 bytes of tailroom for all skbs only to cope with very
unlikely reallocations is not worth it.



--
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




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]