[for-2.4] virtio-net: Flush incoming queues when DRIVER_OK is being set

This patch fixes network hang after "stop" then "cont", while network
packets keep arriving.

Tested both manually (tap, host pinging guest) and with Jason's qtest
series (plus his "[PATCH 2.4] socket: pass correct size in
net_socket_send()" fix).

As virtio_net_set_status is called when guest driver is setting status
byte and when vm state is changing, it is a good opportunity to flush
queued packets.

This is necessary because during vm stop the backend (e.g. tap) would
stop rx processing after .can_receive returns false, until the queue is
explicitly flushed or purged.

The other interesting condition in .can_receive, virtio_queue_ready(),
is handled by virtio_net_handle_rx() when guest kicks; the 3rd condition
is invalid queue index which doesn't need flushing.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 hw/net/virtio-net.c | 5 +++++
 1 file changed, 5 insertions(+)

On Tue, Jul 14, 2015 at 05:41:04PM +0800, Fam Zheng wrote:
> This patch fixes network hang after "stop" then "cont", while network
> packets keep arriving.
> 
> Tested both manually (tap, host pinging guest) and with Jason's qtest
> series (plus his "[PATCH 2.4] socket: pass correct size in
> net_socket_send()" fix).
> 
> As virtio_net_set_status is called when guest driver is setting status
> byte and when vm state is changing, it is a good opportunity to flush
> queued packets.
> 
> This is necessary because during vm stop the backend (e.g. tap) would
> stop rx processing after .can_receive returns false, until the queue is
> explicitly flushed or purged.
> 
> The other interesting condition in .can_receive, virtio_queue_ready(),
> is handled by virtio_net_handle_rx() when guest kicks; the 3rd condition
> is invalid queue index which doesn't need flushing.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  hw/net/virtio-net.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index d728233..7c178c6 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -162,8 +162,13 @@ static void virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status)
>      virtio_net_vhost_status(n, status);
>  
>      for (i = 0; i < n->max_queues; i++) {
> +        NetClientState *ncs = qemu_get_subqueue(n->nic, i);
>          q = &n->vqs[i];
>  
> +        if (status & VIRTIO_CONFIG_S_DRIVER_OK) {
> +            qemu_flush_queued_packets(ncs);
> +        }
> +
>          if ((!n->multiqueue && i != 0) || i >= n->curr_queues) {
>              queue_status = 0;
>          } else {

I think this should be limited to
	virtio_net_started(n, queue_status) && !n->vhost_started

> -- 
> 2.4.3

On Tue, 07/14 13:09, Michael S. Tsirkin wrote:
> On Tue, Jul 14, 2015 at 05:41:04PM +0800, Fam Zheng wrote:
> > This patch fixes network hang after "stop" then "cont", while network
> > packets keep arriving.
> > 
> > Tested both manually (tap, host pinging guest) and with Jason's qtest
> > series (plus his "[PATCH 2.4] socket: pass correct size in
> > net_socket_send()" fix).
> > 
> > As virtio_net_set_status is called when guest driver is setting status
> > byte and when vm state is changing, it is a good opportunity to flush
> > queued packets.
> > 
> > This is necessary because during vm stop the backend (e.g. tap) would
> > stop rx processing after .can_receive returns false, until the queue is
> > explicitly flushed or purged.
> > 
> > The other interesting condition in .can_receive, virtio_queue_ready(),
> > is handled by virtio_net_handle_rx() when guest kicks; the 3rd condition
> > is invalid queue index which doesn't need flushing.
> > 
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> > ---
> >  hw/net/virtio-net.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > index d728233..7c178c6 100644
> > --- a/hw/net/virtio-net.c
> > +++ b/hw/net/virtio-net.c
> > @@ -162,8 +162,13 @@ static void virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status)
> >      virtio_net_vhost_status(n, status);
> >  
> >      for (i = 0; i < n->max_queues; i++) {
> > +        NetClientState *ncs = qemu_get_subqueue(n->nic, i);
> >          q = &n->vqs[i];
> >  
> > +        if (status & VIRTIO_CONFIG_S_DRIVER_OK) {
> > +            qemu_flush_queued_packets(ncs);
> > +        }
> > +
> >          if ((!n->multiqueue && i != 0) || i >= n->curr_queues) {
> >              queue_status = 0;
> >          } else {
> 
> I think this should be limited to
> 	virtio_net_started(n, queue_status) && !n->vhost_started

Yes, that looks better.

Fam