From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43212) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c4ZTr-00089M-Lk for qemu-devel@nongnu.org; Wed, 09 Nov 2016 15:28:24 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c4ZTm-0002OG-G8 for qemu-devel@nongnu.org; Wed, 09 Nov 2016 15:28:23 -0500 Received: from mx1.redhat.com ([209.132.183.28]:54960) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c4ZTm-0002Ny-8C for qemu-devel@nongnu.org; Wed, 09 Nov 2016 15:28:18 -0500 Date: Wed, 9 Nov 2016 22:28:12 +0200 From: "Michael S. Tsirkin" Message-ID: <20161109221300-mutt-send-email-mst@kernel.org> References: <1478704922-3400-1-git-send-email-yuri.benditovich@daynix.com> <1478704922-3400-4-git-send-email-yuri.benditovich@daynix.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1478704922-3400-4-git-send-email-yuri.benditovich@daynix.com> Subject: Re: [Qemu-devel] [PATCH v2 3/3] net: virtio-net discards TX data after link down List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: yuri.benditovich@daynix.com Cc: Jason Wang , qemu-devel@nongnu.org, dmitry@daynix.com, yan@daynix.com On Wed, Nov 09, 2016 at 05:22:02PM +0200, yuri.benditovich@daynix.com wrote: > From: Yuri Benditovich > > https://bugzilla.redhat.com/show_bug.cgi?id=1295637 > Upon set_link monitor command or upon netdev deletion > virtio-net sends link down indication to the guest > and stops vhost if one is used. > Guest driver can still submit data for TX until it > recognizes link loss. If these packets not returned by > the host, the Windows guest will never be able to finish > disable/removal/shutdown. > Now each packet sent by guest after NIC indicated link > down will be completed immediately. > > Signed-off-by: Yuri Benditovich > --- > hw/net/virtio-net.c | 28 ++++++++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > index 06bfe4b..ab4e18a 100644 > --- a/hw/net/virtio-net.c > +++ b/hw/net/virtio-net.c > @@ -218,6 +218,16 @@ static void virtio_net_vnet_endian_status(VirtIONet *n, uint8_t status) > } > } > > +static void virtio_net_drop_tx_queue_data(VirtIODevice *vdev, VirtQueue *vq) > +{ > + VirtQueueElement *elem; > + while ((elem = virtqueue_pop(vq, sizeof(VirtQueueElement)))) { > + virtqueue_push(vq, elem, 0); > + virtio_notify(vdev, vq); > + g_free(elem); > + } > +} > + > static void virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status) > { > VirtIONet *n = VIRTIO_NET(vdev); I don't like this part. This does too much queue parsing, I would like to just copy head from avail to used ring. For example, people want to support rings >1K in size. Let's add bool virtqueue_drop(vq) and be done with it. > @@ -262,6 +272,14 @@ static void virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status) > } else { > qemu_bh_cancel(q->tx_bh); > } > + if ((n->status & VIRTIO_NET_S_LINK_UP) == 0 && > + (queue_status & VIRTIO_CONFIG_S_DRIVER_OK)) { > + /* if tx is waiting we are likely have some packets in tx queue > + * and disabled notification */ > + q->tx_waiting = 0; > + virtio_queue_set_notification(q->tx_vq, 1); > + virtio_net_drop_tx_queue_data(vdev, q->tx_vq); > + } > } > } > } OK but what if guest keeps sending packets? What will drop them? > @@ -1319,6 +1337,11 @@ static void virtio_net_handle_tx_timer(VirtIODevice *vdev, VirtQueue *vq) > VirtIONet *n = VIRTIO_NET(vdev); > VirtIONetQueue *q = &n->vqs[vq2q(virtio_get_queue_index(vq))]; > > + if (unlikely((n->status & VIRTIO_NET_S_LINK_UP) == 0)) { > + virtio_net_drop_tx_queue_data(vdev, vq); > + return; > + } > + > /* This happens when device was stopped but VCPU wasn't. */ > if (!vdev->vm_running) { > q->tx_waiting = 1; > @@ -1345,6 +1368,11 @@ static void virtio_net_handle_tx_bh(VirtIODevice *vdev, VirtQueue *vq) > VirtIONet *n = VIRTIO_NET(vdev); > VirtIONetQueue *q = &n->vqs[vq2q(virtio_get_queue_index(vq))]; > > + if (unlikely((n->status & VIRTIO_NET_S_LINK_UP) == 0)) { > + virtio_net_drop_tx_queue_data(vdev, vq); > + return; > + } > + > if (unlikely(q->tx_waiting)) { > return; > } > -- > 1.9.1