Re: [Xen-devel] [PATCH] xen-netfront: Fix handling packets on compound pages with skb_linearize

[Stefan Bader <stefan.bader@xxxxxxxxxxxxx>]

On 08.12.2014 12:31, David Vrabel wrote:
> On 08/12/14 11:21, Stefan Bader wrote:
>> On 08.12.2014 12:11, David Vrabel wrote:
>>> On 08/12/14 10:19, Luis Henriques wrote:
>>>> On Mon, Dec 01, 2014 at 09:55:24AM +0100, Stefan Bader wrote:
>>>>> On 11.08.2014 19:32, Zoltan Kiss wrote:
>>>>>> There is a long known problem with the netfront/netback interface: if 
>>>>>> the guest
>>>>>> tries to send a packet which constitues more than MAX_SKB_FRAGS + 1 ring 
>>>>>> slots,
>>>>>> it gets dropped. The reason is that netback maps these slots to a frag 
>>>>>> in the
>>>>>> frags array, which is limited by size. Having so many slots can occur 
>>>>>> since
>>>>>> compound pages were introduced, as the ring protocol slice them up into
>>>>>> individual (non-compound) page aligned slots. The theoretical worst case
>>>>>> scenario looks like this (note, skbs are limited to 64 Kb here):
>>>>>> linear buffer: at most PAGE_SIZE - 17 * 2 bytes, overlapping page 
>>>>>> boundary,
>>>>>> using 2 slots
>>>>>> first 15 frags: 1 + PAGE_SIZE + 1 bytes long, first and last bytes are 
>>>>>> at the
>>>>>> end and the beginning of a page, therefore they use 3 * 15 = 45 slots
>>>>>> last 2 frags: 1 + 1 bytes, overlapping page boundary, 2 * 2 = 4 slots
>>>>>> Although I don't think this 51 slots skb can really happen, we need a 
>>>>>> solution
>>>>>> which can deal with every scenario. In real life there is only a few 
>>>>>> slots
>>>>>> overdue, but usually it causes the TCP stream to be blocked, as the 
>>>>>> retry will
>>>>>> most likely have the same buffer layout.
>>>>>> This patch solves this problem by linearizing the packet. This is not the
>>>>>> fastest way, and it can fail much easier as it tries to allocate a big 
>>>>>> linear
>>>>>> area for the whole packet, but probably easier by an order of magnitude 
>>>>>> than
>>>>>> anything else. Probably this code path is not touched very frequently 
>>>>>> anyway.
>>>>>>
>>>>>> Signed-off-by: Zoltan Kiss <zoltan.kiss@xxxxxxxxxx>
>>>>>> Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
>>>>>> Cc: Ian Campbell <Ian.Campbell@xxxxxxxxxx>
>>>>>> Cc: Paul Durrant <paul.durrant@xxxxxxxxxx>
>>>>>> Cc: netdev@xxxxxxxxxxxxxxx
>>>>>> Cc: linux-kernel@xxxxxxxxxxxxxxx
>>>>>> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx
>>>>>
>>>>> This does not seem to be marked explicitly as stable. Has someone already 
>>>>> asked
>>>>> David Miller to put it on his stable queue? IMO it qualifies quite well 
>>>>> and the
>>>>> actual change should be simple to pick/backport.
>>>>>
>>>>
>>>> Thank you Stefan, I'm queuing this for the next 3.16 kernel release.
>>>
>>> Don't backport this yes.  It's broken.  It produces malformed requests
>>> and netback will report a fatal error and stop all traffic on the VIF.
>>
>> Thanks David. Did this just come up? I don't remember seeing any report of 
>> the
>> regression. :/
> 
> There's been a couple of reports on xen-devel recently with 3.17
> frontends and I've just repro'd it (by always forcing a skb_linearize()
> in netfront).

Ah ok. Found at least one now (plus your fixup proposal). Too long ago to
remember for sure but I thought the change was tested by reporters. Could be
that I only had tried the approach I was working on back then (which was more
complicated trying to replace individual frags by aligned copies). :( And on our
devel we have not yet switched to past 3.16. Hope I can squeeze in some testing
there.

-Stefan
> 
> David
>

Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel