Re: [Xen-devel] [PATCH] Mini-OS: netfront: Fix rx ring starvation in network_rx

[Samuel Thibault <samuel.thibault@xxxxxxxxxxxx>]

Hello,

Martin Lucina, le Thu 04 Dec 2014 14:55:50 +0100, a écrit :
> following is a patch against vanilla Mini-OS in upstream xen.git for a
> problem we have found in the netfront driver. When subjected to load
> network receive would freeze due to the rx ring running out of free
> request slots.

Oh, indeed nr_consumed seems to have been wrongly initialized.  Could
you submit a patch that only moves the nr_consumed = 0 initialization
and the bogus BUG_ON?

> -        if (rx->flags & NETRXF_extra_info)
> -        {
> -            printk("+++++++++++++++++++++ we have extras!\n");
> -            continue;
> -        }
> -
> -
> -        if (rx->status == NETIF_RSP_NULL) continue;

I believe we want to keep them.  Or turn them into
if (rx->status != NETIF_RSP_OKAY)
  continue;

the HAVE_LIBC codepath will be fine with the change.

> Further, I would like to clean this function up; the code is an impossible
> to follow mess. What I'd like to arrive at is at [2],

I agree with stuffing the "cons" initialization into the for loop, but:

> however this also needs(?) to address the -DHAVE_LIBC codepath.

Yes, and I introduced the "some" variable especially for this. The
idea was to implement a basic support for opening the device with a
tap behavior.  But we didn't want to introduce dynamic buffers to
store incoming packets.  In that case we don't run network_rx from the
interrupt, but netfront_select_handler instead, which will wake the
select() call, the application then calls netfront_receive(), which eats
exactly one packet only from the ring.  Thus the "some" variable in
order to break the loop when exactly one packet was eaten (and no, we
can't use break or set cons to rp, since that'd would eat 0 or more than
1 packets).

Samuel

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