Re: [PATCH RFC] xen/public/ring.h: simplify RING_HAS_UNCONSUMED_REQUESTS()

[Zoltan Kiss <zoltan.kiss@citrix.com>]

On 24/03/14 07:38, Jan Beulich wrote:
>>>> On 22.03.14 at 18:14, <tim@xen.org> wrote:
>> At 14:18 +0000 on 22 Mar (1395494283), Zoltan Kiss wrote:
>>> I think I might have an explanation why do we need this, see this mailing:
>>>
>>> https://lkml.org/lkml/2014/3/20/710
>>> https://lkml.org/lkml/2014/3/21/111
>>> https://lkml.org/lkml/2014/3/21/390
>>
>> Quoting from the third of these:
>>
>> | But consuming overrunning requests after rsp_prod_pvt is a problem:
>> | - NAPI instance races with dealloc thread over the slots. The first
>> | reads them as requests, the second writes them as responses
>> | - the NAPI instance overwrites used pending slots as well, so skb frag
>> | release go wrong etc.
>>
>> OK, so the backend needs to be careful not to follow the frontend into
>> overrun, not because of the ring itself being corrupted but because it
>> will mess up the backend's internal bookkeeping.
>
> With s/will/may/ I'm not sure that's a reason to withdraw the patch:
> The generic macros in ring.h imo shouldn't dictate any particular
> protection policy beyond protecting the ring itself. I.e. I'd think if
> netback need protection beyond the one provided by ring.h macros,
> it should take care to implement them itself.

It's not "may", it is a "will". In case of Linux netback for sure, but I 
think it's reasonable for any backend to rely on the fact that the ring 
macros protect them from abusive frontends. Protecting a backend to read 
requests from the [rsp_prod_pvt, req_cons] range is a sensible thing to 
do in the ring macros.
Also, RING_FINAL_CHECK_FOR_REQUESTS relies on this macro, removing this 
protection may cause other issues, e.g. netback keeps the NAPI instance 
spinning while it's not consuming any requests.

Zoli