Re: [Xen-devel] RING_HAS_UNCONSUMED_REQUESTS oddness

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Paul Durrant
> Sent: 12 March 2014 15:57
> To: Wei Liu
> Cc: Zoltan Kiss; Ian Campbell; xen-devel@xxxxxxxxxxxxxxxxxxxx; Wei Liu; Tim
> (Xen.org)
> Subject: RE: [Xen-devel] RING_HAS_UNCONSUMED_REQUESTS oddness
> 
> > -----Original Message-----
> > From: Wei Liu [mailto:wei.liu2@xxxxxxxxxx]
> > Sent: 12 March 2014 15:43
> > To: Paul Durrant
> > Cc: Zoltan Kiss; Ian Campbell; xen-devel@xxxxxxxxxxxxxxxxxxxx; Wei Liu; Tim
> > (Xen.org)
> > Subject: Re: [Xen-devel] RING_HAS_UNCONSUMED_REQUESTS oddness
> >
> > On Wed, Mar 12, 2014 at 03:23:09PM +0000, Paul Durrant wrote:
> > [...]
> > > > > Actually ancient memory tells me that, unfortunately, netback's
> > backend-
> > > > >frontend GSO protocol is broken in this way... it requires one more
> > > > response slot than the number of requests it consumes (for the extra
> > > > metadata), which means that if the frontend keeps the ring full you can
> > get
> > > > overflow. It's a bit of a tangent though, because that code doesn't use
> > this
> > > > macro (or in fact check the ring has space in any way IIRC). The prefix
> > variant
> > > > of the protocol is ok though.
> > > >
> > > > I think it's not: it consumes a request for the metadata, and when the
> > > > packet is grant copied to the guest, it creates a response for that slot
> > > > as well.
> > >
> > > As explained verbally, it doesn't consume a request for the 'extra' info.
> Let
> > me elaborate here for the benefit of the list...
> > >
> > > In xenvif_gop_skb(), in the non-prefix GSO case, a single request is
> > consumed for the header along with a meta slot which is used to hold the
> > GSO data. Later on in xenvif_rx_action() the code calls
> make_rx_response()
> > for the header, but then *before* moving onto the next meta slot it makes
> > an 'extra' response for the GSO metadata. So - one meta slot - one request
> > consumed, but two responses produced.
> > > So this mechanism totally relies on the netfront driver not completely
> filling
> > the shared ring. If it ever does, you'll get overflow.
> > >
> >
> > (... which reminds me of the heisenbug Sander is seeing.)
> >
> > But do we not check for there's enough space in the ring before
> > procceeding?
> >
> 
> Apparently not, but TBH I cannot figure out how on earth this ever worked at
> all. If netback is genuinely issuing 2 reponses for 1 consumed request then it
> must trash an unconsumed request at some point - but from my reading of
> the code that really does seem to be what it's doing.
> 

Aha, no - I've found the sneaky bit of code that restores balance to the 
force... It's at the bottom of xenvif_gop_frag_copy(). In the non-prefix GSO 
case req_cons is bumped if it's processing the head frag. Phew!

  Paul


>   Paul
> 
> > Wei.
> >
> > >   Paul
> > >
> > >

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