Re: [Xen-devel] RING_HAS_UNCONSUMED_REQUESTS oddness

[Zoltan Kiss <zoltan.kiss@xxxxxxxxxx>]

On 12/03/14 15:42, Wei Liu wrote:
> 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?
Thinking further what Paul said, we might be actually OK. At the end of 
xenvif_gop_frag_copy there is this:
if (*head && ((1 << gso_type) & vif->gso_mask))
        vif->rx.req_cons++;

So although netback doesn't call RING_GET_REQUEST to consume the 
request, it does so by increasing req_cons. And netfront automagically 
detect that in xennet_get_extras, and calls xennet_move_rx_slot to move 
that buffer to req_prod_pvt. Same happens when the backend send a 
response that it couldn't use the slot and it doesn't contain any data.
Zoli

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