From mboxrd@z Thu Jan 1 00:00:00 1970 From: Zoltan Kiss Subject: Re: RING_HAS_UNCONSUMED_REQUESTS oddness Date: Wed, 12 Mar 2014 16:13:47 +0000 Message-ID: <532087BB.6000100@citrix.com> References: <5318987C.3030303@citrix.com> <1394552666.30915.64.camel@kazak.uk.xensource.com> <531F9B17.5060107@citrix.com> <1394620083.21145.21.camel@kazak.uk.xensource.com> <9AAE0902D5BC7E449B7C8E4E778ABCD0282CBF@AMSPEX01CL01.citrite.net> <53207226.7090002@citrix.com> <9AAE0902D5BC7E449B7C8E4E778ABCD02833BD@AMSPEX01CL01.citrite.net> <20140312154249.GP19620@zion.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1WNlmy-0006w1-Fy for xen-devel@lists.xenproject.org; Wed, 12 Mar 2014 16:13:52 +0000 In-Reply-To: <20140312154249.GP19620@zion.uk.xensource.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: Wei Liu , Paul Durrant Cc: "xen-devel@lists.xenproject.org" , "Tim (Xen.org)" , Ian Campbell List-Id: xen-devel@lists.xenproject.org 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