netdev - RE: [Xen-devel] [PATCH net v2 1/3] xen-netback: remove pointless clause from if statement

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9AAE0902D5BC7E449B7C8E4E778ABCD029E0ED@AMSPEX01CL01.citrite.net>
Date:	Fri, 28 Mar 2014 10:12:26 +0000
From:	Paul Durrant <Paul.Durrant@...rix.com>
To:	Sander Eikelenboom <linux@...elenboom.it>
CC:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	Wei Liu <wei.liu2@...rix.com>,
	Ian Campbell <Ian.Campbell@...rix.com>,
	"xen-devel@...ts.xen.org" <xen-devel@...ts.xen.org>
Subject: RE: [Xen-devel] [PATCH net v2 1/3] xen-netback: remove pointless
 clause from if statement

> -----Original Message-----
> From: netdev-owner@...r.kernel.org [mailto:netdev-
> owner@...r.kernel.org] On Behalf Of Sander Eikelenboom
> Sent: 28 March 2014 10:00
> To: Paul Durrant
> Cc: netdev@...r.kernel.org; Wei Liu; Ian Campbell; xen-devel@...ts.xen.org
> Subject: Re: [Xen-devel] [PATCH net v2 1/3] xen-netback: remove pointless
> clause from if statement
> 
> 
> Friday, March 28, 2014, 10:47:20 AM, you wrote:
> 
> >> -----Original Message-----
> >> From: Sander Eikelenboom [mailto:linux@...elenboom.it]
> >> Sent: 28 March 2014 09:39
> >> To: Paul Durrant
> >> Cc: netdev@...r.kernel.org; Wei Liu; Ian Campbell; xen-
> devel@...ts.xen.org
> >> Subject: Re: [Xen-devel] [PATCH net v2 1/3] xen-netback: remove
> pointless
> >> clause from if statement
> >>
> >>
> >> Friday, March 28, 2014, 10:30:27 AM, you wrote:
> >>
> >> >> -----Original Message-----
> >> >> From: Sander Eikelenboom [mailto:linux@...elenboom.it]
> >> >> Sent: 27 March 2014 19:23
> >> >> To: Paul Durrant
> >> >> Cc: netdev@...r.kernel.org; Wei Liu; Ian Campbell; xen-
> >> devel@...ts.xen.org
> >> >> Subject: Re: [Xen-devel] [PATCH net v2 1/3] xen-netback: remove
> >> pointless
> >> >> clause from if statement
> >> >>
> >> >>
> >> >> Thursday, March 27, 2014, 7:34:54 PM, you wrote:
> >> >>
> >> >> > <big snip>
> >> >>
> >> >> >>> >>
> >> >> >>> >> > So, it may be that the worse-case estimate is now too bad. In
> >> the
> >> >> case
> >> >> >>> >> where it's failing for you it would be nice to know what the
> >> estimate
> >> >> was
> >> >> >>>
> >> >> >>>
> >> >> >>> > Ok, so we cannot be too pessimistic. In that case I don't see
> there's
> >> a
> > > The behaviour of the Windows frontend is different to netfront; it tries to
> > keep the shared ring as full as possible so the estimate could be as
> > pessimistic as you like (as long as it doesn't exceed ring size ;-)) and you'd
> > never see the lock-up. For some reason (best known to the originator of
> the
> > code I suspect) the Linux netfront driver limits the number of requests it
> > posts into the shared ring leading to the possibility of lock-up in the case
> > where the backend needs more slots than the fontend 'thinks' it should.
> > But from what i read the ring size is determined by the frontend .. so that
> PV
> > driver should be able to guarantee that itself ..
> >
> 
> > The ring size is 256 - that's baked in. The number of pending requests
> > available to backend *is* determined by the frontend.
> 
> Ah ok, does it also reverse that space ?
> (if so .. why not use it to allow multiple complete packets to be shoveled in)
> 
> > Which begs for the question .. was that change of max_slots_needed
> > calculation *needed* to prevent the problem you saw on "Windows Server
> > 2008R2",
> > or was that just changed for correctness ?
> >
> 
> > It was changed for correctness. As I understand it, use of MAX_SKB_FRAGS
> is
> > incorrect if compound pages are in use as the page size is no longer the slot
> > size. It's also wasteful to always wait for space for a maximal packet if the
> > packet you have is smaller so the intention of the max estimate was that it
> > should be at least the number of slots required but not excessive. I think
> > you've proved that making such an estimate is just too hard and since we
> don't
> > want to fall back to the old dry-run style of slot counting (which meant you
> > had two codepaths that *must* arrive at the same number - and they
> didn't,
> > which is why I was getting the lock-up with Windows guests) I think we
> should
> > just go with full-packing so that we don't need to estimate.
> 
> Ok i asked this question since the about to be released 3.14 does now
> underestimate and
> it causes a regression.
> So if that part of your patches is not involved in fixing the stated problem /
> regression i think
> just that calculation change should be reverted to the MAX_SKB_FRAGS
> variant again.
> It's more wasteful (as it always has been) but that is better than incorrect and
> inducing buffer overrun IMHO.

But I'm not sure even that is correct. Are you?

> 
> That would give time to think, revise and test this for 3.15.
> 
> BTW: if a slot is always 4k, should it check with PAGE_SIZE then on a lot of
> occasions or just with the
> hardcoded 4k slot size ? (at the moment you only have x86 dom0 so probably
> the page_size==4k is guaranteed that way,
> but nevertheless.)
> 

Well, it's 4k because that's the smallest x86 page size and that's what Xen uses in its ABI so I guess the slot size should really be acquired from Xen to be architecture agnostic.

  Paul

> > Paul
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html