[Qemu-devel] [PATCH 0/3] virtio-net discards TX data after link down

[Qemu-devel] [PATCH 0/3] virtio-net discards TX data after link down

[yuri.benditovich]

From: Yuri Benditovich <yuri.benditovich@daynix.com>

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. In order to allow qemu to
discard these packets, virtio queue shall update
its internal structure upon vhost stop.

Yuri Benditovich (3):
  net: Add virtio queue interface to update used index from vring state
  net: vhost stop updates virtio queue state
  net: virtio-net discards TX data after link down

 hw/net/virtio-net.c        | 15 +++++++++++++++
 hw/virtio/vhost.c          |  1 +
 hw/virtio/virtio.c         |  5 +++++
 include/hw/virtio/virtio.h |  1 +
 4 files changed, 22 insertions(+)

-- 
1.9.1

[Qemu-devel] [PATCH 1/3] net: Add virtio queue interface to update used index from vring state

[yuri.benditovich]

From: Yuri Benditovich <yuri.benditovich@daynix.com>

Bring virtio queue to correct internal  state for host-to-guest
operations when vhost is temporary stopped.

Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
---
 hw/virtio/virtio.c         | 5 +++++
 include/hw/virtio/virtio.h | 1 +
 2 files changed, 6 insertions(+)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index d48d1a9..7e1274a 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1983,6 +1983,11 @@ void virtio_queue_set_last_avail_idx(VirtIODevice *vdev, int n, uint16_t idx)
     vdev->vq[n].shadow_avail_idx = idx;
 }
 
+void virtio_queue_update_used_idx(VirtIODevice *vdev, int n)
+{
+    vdev->vq[n].used_idx = vring_used_idx(&vdev->vq[n]);
+}
+
 void virtio_queue_invalidate_signalled_used(VirtIODevice *vdev, int n)
 {
     vdev->vq[n].signalled_used_valid = false;
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index b913aac..b9a9d6e 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -260,6 +260,7 @@ hwaddr virtio_queue_get_ring_size(VirtIODevice *vdev, int n);
 uint16_t virtio_queue_get_last_avail_idx(VirtIODevice *vdev, int n);
 void virtio_queue_set_last_avail_idx(VirtIODevice *vdev, int n, uint16_t idx);
 void virtio_queue_invalidate_signalled_used(VirtIODevice *vdev, int n);
+void virtio_queue_update_used_idx(VirtIODevice *vdev, int n);
 VirtQueue *virtio_get_queue(VirtIODevice *vdev, int n);
 uint16_t virtio_get_queue_index(VirtQueue *vq);
 EventNotifier *virtio_queue_get_guest_notifier(VirtQueue *vq);
-- 
1.9.1

[Qemu-devel] [PATCH 2/3] net: vhost stop updates virtio queue state

[yuri.benditovich]

From: Yuri Benditovich <yuri.benditovich@daynix.com>

Prepare virtio queue for push operation from qemu
after vhost was stopped.

Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
---
 hw/virtio/vhost.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index bd051ab..2e990d0 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -963,6 +963,7 @@ static void vhost_virtqueue_stop(struct vhost_dev *dev,
         virtio_queue_set_last_avail_idx(vdev, idx, state.num);
     }
     virtio_queue_invalidate_signalled_used(vdev, idx);
+    virtio_queue_update_used_idx(vdev, idx);
 
     /* In the cross-endian case, we need to reset the vring endianness to
      * native as legacy devices expect so by default.
-- 
1.9.1

[Qemu-devel] [PATCH 3/3] net: virtio-net discards TX data after link down

[yuri.benditovich]

From: Yuri Benditovich <yuri.benditovich@daynix.com>

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 <yuri.benditovich@daynix.com>
---
 hw/net/virtio-net.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 06bfe4b..6158de0 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1200,6 +1200,16 @@ static ssize_t virtio_net_receive(NetClientState *nc, const uint8_t *buf, size_t
     return size;
 }
 
+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 int32_t virtio_net_flush_tx(VirtIONetQueue *q);
 
 static void virtio_net_tx_complete(NetClientState *nc, ssize_t len)
@@ -1345,6 +1355,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

Re: [Qemu-devel] [PATCH 3/3] net: virtio-net discards TX data after link down

[Jason Wang]

On 2016年11月07日 16:20, yuri.benditovich@daynix.com wrote:
> From: Yuri Benditovich <yuri.benditovich@daynix.com>
>
> 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 <yuri.benditovich@daynix.com>
> ---
>   hw/net/virtio-net.c | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 06bfe4b..6158de0 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -1200,6 +1200,16 @@ static ssize_t virtio_net_receive(NetClientState *nc, const uint8_t *buf, size_t
>       return size;
>   }
>   
> +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 int32_t virtio_net_flush_tx(VirtIONetQueue *q);
>   
>   static void virtio_net_tx_complete(NetClientState *nc, ssize_t len)
> @@ -1345,6 +1355,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;
>       }

This doesn't work for tx timer, you may want to do the modification on 
virtio_net_flush_tx().

What's more, when link_down is true, qemu_send_packet_async() will 
return size of iov, can we do some check there?

Thanks

Re: [Qemu-devel] [PATCH 3/3] net: virtio-net discards TX data after link down

[Yuri Benditovich]

On Tue, Nov 8, 2016 at 8:48 AM, Jason Wang <jasowang@redhat.com> wrote:

>
>
> On 2016年11月07日 16:20, yuri.benditovich@daynix.com wrote:
>
>> From: Yuri Benditovich <yuri.benditovich@daynix.com>
>>
>> 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 <yuri.benditovich@daynix.com>
>> ---
>>   hw/net/virtio-net.c | 15 +++++++++++++++
>>   1 file changed, 15 insertions(+)
>>
>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>> index 06bfe4b..6158de0 100644
>> --- a/hw/net/virtio-net.c
>> +++ b/hw/net/virtio-net.c
>> @@ -1200,6 +1200,16 @@ static ssize_t virtio_net_receive(NetClientState
>> *nc, const uint8_t *buf, size_t
>>       return size;
>>   }
>>   +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 int32_t virtio_net_flush_tx(VirtIONetQueue *q);
>>     static void virtio_net_tx_complete(NetClientState *nc, ssize_t len)
>> @@ -1345,6 +1355,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;
>>       }
>>
>
> This doesn't work for tx timer, you may want to do the modification on
> virtio_net_flush_tx().
>
> What's more, when link_down is true, qemu_send_packet_async() will return
> size of iov, can we do some check there?
>
> Thanks
>

I will prepare v2 with support for timer configuration. But in case of
vhost virtio_net_flush_tx() never called and without vhost I did not
succeed to reproduce the problem.