Re: [Xen-devel] [PATCH RFC] xen-netback: remove guest RX path dependence on MAX_SKB_FRAGS

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 02.07.13 at 17:02, Wei Liu <wei.liu2@xxxxxxxxxx> wrote:
> On Tue, Jul 02, 2013 at 03:19:12PM +0100, Jan Beulich wrote:
>> >>> On 02.07.13 at 15:40, Wei Liu <wei.liu2@xxxxxxxxxx> wrote:
>> >  int xen_netbk_rx_ring_full(struct xenvif *vif)
>> >  {
>> > -  RING_IDX peek   = vif->rx_req_cons_peek;
>> > -  RING_IDX needed = max_required_rx_slots(vif);
>> > +  RING_IDX peek = vif->rx_req_cons_peek;
>> >  
>> > -  return ((vif->rx.sring->req_prod - peek) < needed) ||
>> > -         ((vif->rx.rsp_prod_pvt + XEN_NETIF_RX_RING_SIZE - peek) < 
>> > needed);
>> > +  return ((vif->rx.sring->req_prod < peek) ||
>> > +          (vif->rx.rsp_prod_pvt + XEN_NETIF_RX_RING_SIZE < peek));
>> 
>> This transformation is definitely wrong: You need to retain the
>> code's capability of dealing with the indexes wrapping, i.e.
>> simple comparisons won't do.
> 
> Hmm... so I don't quite understand this wrapping around thing and I have
> a question here, what if req_prod < peek so that (req_prod - peek)
> results in a very large number. In that case the ring is actually full
> (cannot accommodate that packet), however "needed" is only like 20-ish
> something, so the first comparision would fail. Are we relying on the
> second comparion to tell the ring is full?

Yes, and I'm struggling to understand what the first one actually
checked for. Your new first one in any case doesn't check whether
the ring is full, but whether there's enough slots available on the
ring to consume all the way up to "peek" (plus is having a wrap
around issue just like the second half).

> The only thing I need to do here is to tell whether rep_prod is ahead of
> peek in the first comparison and in the second case there is enough
> slots between rsp_prod_pvt and peek, what's the correct line?

Not sure - in any event, as said above, the first part isn't really
matching the name of the function anymore (so would at least
deserve a comment, to save future readers from struggling to
understand the purpose just like I do with the original one now).

Jan


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