[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

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:


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.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.