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

[Zoltan Kiss <zoltan.kiss@xxxxxxxxxx>]

On 24/03/14 07:38, Jan Beulich wrote:
> > > > On 22.03.14 at 18:14, <tim@xxxxxxx> 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

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