From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36574) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZFH3E-0001BN-2r for qemu-devel@nongnu.org; Wed, 15 Jul 2015 03:24:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZFH3A-0002wc-RE for qemu-devel@nongnu.org; Wed, 15 Jul 2015 03:24:20 -0400 Received: from mx1.redhat.com ([209.132.183.28]:47903) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZFH3A-0002wC-LU for qemu-devel@nongnu.org; Wed, 15 Jul 2015 03:24:16 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (Postfix) with ESMTPS id 472A7C03E6 for ; Wed, 15 Jul 2015 07:24:16 +0000 (UTC) Message-ID: <55A60A9B.70902@redhat.com> Date: Wed, 15 Jul 2015 15:24:11 +0800 From: Jason Wang MIME-Version: 1.0 References: <1436929347-11991-1-git-send-email-famz@redhat.com> In-Reply-To: <1436929347-11991-1-git-send-email-famz@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH for-2.4] virtio-net: Flush incoming queues when DRIVER_OK is being set List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng , qemu-devel@nongnu.org Cc: stefanha@redhat.com, "Michael S. Tsirkin" On 07/15/2015 11:02 AM, 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 > > --- Reviewed-by: Jason Wang btw, there's another condition in can_receive() which is suspicious: ... if (nc->queue_index >= n->curr_queues) { return 0; } ... This requires queue to be flushed when the number of queues was increased. But it looks unnecessary since both guest and vhost does not care about this. So I think we could safely just remove this condition. Maybe a patch on top. Thanks > > v2: Limit to "virtio_net_started(n, queue_status) && > !n->vhost_started".[MST] > --- > hw/net/virtio-net.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > index d728233..24c7be1 100644 > --- a/hw/net/virtio-net.c > +++ b/hw/net/virtio-net.c > @@ -162,6 +162,8 @@ 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); > + bool queue_started; > q = &n->vqs[i]; > > if ((!n->multiqueue && i != 0) || i >= n->curr_queues) { > @@ -169,12 +171,18 @@ static void virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status) > } else { > queue_status = status; > } > + queue_started = > + virtio_net_started(n, queue_status) && !n->vhost_started; > + > + if (queue_started) { > + qemu_flush_queued_packets(ncs); > + } > > if (!q->tx_waiting) { > continue; > } > > - if (virtio_net_started(n, queue_status) && !n->vhost_started) { > + if (queue_started) { > if (q->tx_timer) { > timer_mod(q->tx_timer, > qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + n->tx_timeout);