From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefan Bader Subject: Re: [PATCH] xen-netfront: Fix handling packets on compound pages with skb_linearize Date: Mon, 08 Dec 2014 13:30:38 +0100 Message-ID: <548599EE.4000204@canonical.com> References: <1407778343-13622-1-git-send-email-zoltan.kiss@citrix.com> <547C2CFC.7060908@canonical.com> <20141208101936.GA7491@hercules> <54858753.1070801@citrix.com> <548589B0.8050608@canonical.com> <54858BFC.2020906@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============8681200982878047249==" Return-path: In-Reply-To: <54858BFC.2020906@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: David Vrabel , xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --===============8681200982878047249== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="f7iA47JQTeHghJ9oOxmoFewQk37w8ALJa" This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --f7iA47JQTeHghJ9oOxmoFewQk37w8ALJa Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable 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 oc= cur since >>>>>> compound pages were introduced, as the ring protocol slice them up= into >>>>>> individual (non-compound) page aligned slots. The theoretical wors= t 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 =3D 45 = slots >>>>>> last 2 frags: 1 + 1 bytes, overlapping page boundary, 2 * 2 =3D 4 = slots >>>>>> Although I don't think this 51 slots skb can really happen, we nee= d a solution >>>>>> which can deal with every scenario. In real life there is only a f= ew slots >>>>>> overdue, but usually it causes the TCP stream to be blocked, as th= e 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 magn= itude than >>>>>> anything else. Probably this code path is not touched very frequen= tly anyway. >>>>>> >>>>>> Signed-off-by: Zoltan Kiss >>>>>> Cc: Wei Liu >>>>>> Cc: Ian Campbell >>>>>> Cc: Paul Durrant >>>>>> Cc: netdev@vger.kernel.org >>>>>> Cc: linux-kernel@vger.kernel.org >>>>>> Cc: xen-devel@lists.xenproject.org >>>>> >>>>> This does not seem to be marked explicitly as stable. Has someone a= lready 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 request= s >>> and netback will report a fatal error and stop all traffic on the VIF= =2E >> >> Thanks David. Did this just come up? I don't remember seeing any repor= t of the >> regression. :/ >=20 > 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 t= esting there. -Stefan >=20 > David >=20 --f7iA47JQTeHghJ9oOxmoFewQk37w8ALJa Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBCgAGBQJUhZnuAAoJEOhnXe7L7s6jJScQAIBUR+QoWXOaZFdhS3HarkGU SD5UlLLRLHJLQmbpVxfN7pggd8VFW5bLA0f5r8kffSOmuMf7Zm86Y4KGaO5k+qG7 cPvYkv8rrJUY1kRGsVv/9y3IsA3ppR/mKT9E/5ow+o2AGdmBONKTjMuSJwXjId6A wfq5M0RNSRlAnWrAbdYbBQZo50gMW+UgW7d5JfvigRfNMVGT6BJrD6MteWW9p4+P NlMpangg1O1wUbo1UZwItszdxovBWz2qkm0qsUgNI9bvBMIlGd4voO1da3mraeN8 LWnpAQVyU8wTVgG6TNX3S+cnq4Bd8J3MR9TIyqMrqBPMw+YqI4NgXGJSa6NwyOc4 mpEZoFC/uaE8Nf28gj+FqtDejWt5ZUmnQLBQdUN32e+U80VZBwJ7DlzRbh0+WO16 BlI2lqAorCxpz9Z1eKSQwx7w2Bm7Kylxnrg0hCA/smIOBZdZolOaH7op6qDyTsKE IqbjUxeR4a0KtNxz0L4W6+2TpHZEwlN3uiFffF6ORjBevB6mkRufLMF3slJ3z7ju QHWMFHNWxXOse8IFgBqFG/fNkC0664GACejuIU3bm4nsFls7Wv9QvpzSWzTvKbtO tgjm/gsmzXVOOrMQjKwbIVYreBvTrQjnPpndtFo+YF28YsguMdUGuJdtN+WBPDoX hyPtAR/GBSecbCEZFfQl =e9Bj -----END PGP SIGNATURE----- --f7iA47JQTeHghJ9oOxmoFewQk37w8ALJa-- --===============8681200982878047249== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============8681200982878047249==--