Re: [Xen-devel] RING_HAS_UNCONSUMED_REQUESTS oddness

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Zoltan Kiss
> Sent: 12 March 2014 14:42
> To: Paul Durrant; Ian Campbell
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; Wei Liu; Tim (Xen.org)
> Subject: Re: [Xen-devel] RING_HAS_UNCONSUMED_REQUESTS oddness
> 
> On 12/03/14 11:38, Paul Durrant wrote:
> >> -----Original Message-----
> >> From: Paul Durrant
> >> Sent: 12 March 2014 11:26
> >> To: Ian Campbell; Zoltan Kiss
> >> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; Wei Liu; Tim (Xen.org)
> >> Subject: RE: [Xen-devel] RING_HAS_UNCONSUMED_REQUESTS oddness
> >>
> >>> -----Original Message-----
> >>> From: xen-devel-bounces@xxxxxxxxxxxxx [mailto:xen-devel-
> >>> bounces@xxxxxxxxxxxxx] On Behalf Of Ian Campbell
> >>> Sent: 12 March 2014 10:28
> >>> To: Zoltan Kiss
> >>> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; Wei Liu; Tim (Xen.org)
> >>> Subject: Re: [Xen-devel] RING_HAS_UNCONSUMED_REQUESTS
> oddness
> >>>
> >>> On Tue, 2014-03-11 at 23:24 +0000, Zoltan Kiss wrote:
> >>>> On 11/03/14 15:44, Ian Campbell wrote:
> >>>
> >>>>> Is it the case that this macro considers a request to be unconsumed if
> >>>>> the *response* to a request is outstanding as well as if the request
> >>>>> itself is still on the ring?
> >>>> I don't think that would make sense. I think everywhere where this
> >> macro
> >>>> is called the caller is not interested in pending request (pending means
> >>>> consumed but not responded)
> >>>
> >>> It might be interested in such pending requests in some of the
> >>> pathological cases I allude to in the next paragraph though?
> >>>
> >>> For example if the ring has unconsumed requests but there are no slots
> >>> free for a response, it would be better to treat it as no unconsumed
> >>> requests until space opens up for a response, otherwise something else
> >>> just has to abort the processing of the request when it notices the lack
> >>> of space.
> >>>
> >>
> >> Huh? The act of consuming the request will create the space for the
> >> response. If responses and requests are not balanced then I'd argue that
> the
> >> protocol is broken.
> >>
> >
> > 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.

  Paul




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