Re: [Qemu-devel] [PATCH] virtio-net: Drop net_virtio_info.can_receive

[Stefan Hajnoczi]

On Tue, Jun 30, 2015 at 04:35:24PM +0800, Jason Wang wrote:
> On 06/30/2015 11:06 AM, Fam Zheng wrote:
> > virtio_net_receive still does the check by calling
> > virtio_net_can_receive, if the device or driver is not ready, the packet
> > is dropped.
> >
> > This is necessary because returning false from can_receive complicates
> > things: the peer would disable sending until we explicitly flush the
> > queue.
> >
> > Signed-off-by: Fam Zheng <address@hidden>
> > ---
> >  hw/net/virtio-net.c | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > index d728233..dbef0d0 100644
> > --- a/hw/net/virtio-net.c
> > +++ b/hw/net/virtio-net.c
> > @@ -1503,7 +1503,6 @@ static int virtio_net_load_device(VirtIODevice *vdev, 
> > QEMUFile *f,
> >  static NetClientInfo net_virtio_info = {
> >      .type = NET_CLIENT_OPTIONS_KIND_NIC,
> >      .size = sizeof(NICState),
> > -    .can_receive = virtio_net_can_receive,
> >      .receive = virtio_net_receive,
> >      .link_status_changed = virtio_net_set_link_status,
> >      .query_rx_filter = virtio_net_query_rxfilter,
> 
> A side effect of this patch is it will read and then drop packet is
> guest driver is no ok.

I think that the semantics of .can_receive() and .receive() return
values are currently incorrect in many NICs.  They have .can_receive()
functions that return false for conditions where .receive() would
discard the packet.  So what happens is that packets get queued when
they should actually be discarded.

The purpose of the flow control (queuing) mechanism is to tell the
sender to hold off until the receiver has more rx buffers available.
It's a short-term thing that doesn't included link down, rx disable, or
NIC reset states.

Therefore, I think this patch will not introduce a regression.  It is
adjusting the code to stop queuing packets when they should actually be
dropped.

Thoughts?

Reviewed-by: Stefan Hajnoczi <address@hidden>

pgpwoRye4WTQ7.pgp
Description: PGP signature