[Qemu-devel] [PATCH 0/2] virtio-net: Fix TX data discard on backend disconnection

[Qemu-devel] [PATCH 0/2] virtio-net: Fix TX data discard on backend disconnection

[Maxime Coquelin]

This series fixes TX data discard when backend disconnects.

On backend disconnection, QEMU cannot retrieve its internal
avail index, which makes the virtio queue internal state
inconsistent (last_avail_idx will always be 0, whereas used_idx
is correct).

It results in discarding TX data to corrupt the vring state, by
completing descriptors already completed by the backend.

This series fixes this by restoring last_avail_idx to the vring
used idx, as some backends used to do on reconnection.

Maxime Coquelin (2):
  virtio: Add queue interface to restore avail index from vring used
    index
  vhost: restore avail index from vring used index on disconnection

 hw/virtio/vhost.c          |  4 ++++
 hw/virtio/virtio.c         | 10 ++++++++++
 include/hw/virtio/virtio.h |  1 +
 3 files changed, 15 insertions(+)

-- 
2.14.3

[Qemu-devel] [PATCH 1/2] virtio: Add queue interface to restore avail index from vring used index

[Maxime Coquelin]

In case of backend crash, it is not possible to restore internal
avail index from the backend value as vhost_get_vring_base
callback fails.

This patch provides a new interface to restore internal avail index
from the vring used index, as done by some vhost-user backend on
reconnection.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 hw/virtio/virtio.c         | 10 ++++++++++
 include/hw/virtio/virtio.h |  1 +
 2 files changed, 11 insertions(+)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 5884ce3480..583516c428 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2310,6 +2310,16 @@ void virtio_queue_set_last_avail_idx(VirtIODevice *vdev, int n, uint16_t idx)
     vdev->vq[n].shadow_avail_idx = idx;
 }
 
+void virtio_queue_restore_last_avail_idx(VirtIODevice *vdev, int n)
+{
+    rcu_read_lock();
+    if (vdev->vq[n].vring.desc) {
+        vdev->vq[n].last_avail_idx = vring_used_idx(&vdev->vq[n]);
+        vdev->vq[n].shadow_avail_idx = vdev->vq[n].last_avail_idx;
+    }
+    rcu_read_unlock();
+}
+
 void virtio_queue_update_used_idx(VirtIODevice *vdev, int n)
 {
     rcu_read_lock();
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 5abada6966..098bdaaea3 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -272,6 +272,7 @@ hwaddr virtio_queue_get_avail_size(VirtIODevice *vdev, int n);
 hwaddr virtio_queue_get_used_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_restore_last_avail_idx(VirtIODevice *vdev, int n);
 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);
-- 
2.14.3

[Qemu-devel] [PATCH 2/2] vhost: restore avail index from vring used index on disconnection

[Maxime Coquelin]

vhost_virtqueue_stop() gets avail index value from the backend,
except if the backend is not responding.

It happens when the backend crashes, and in this case, internal
state of the virtio queue is inconsistent, making packets
to corrupt the vring state.

With a Linux guest, it results in following error message on
backend reconnection:

[   22.444905] virtio_net virtio0: output.0:id 0 is not a head!
[   22.446746] net enp0s3: Unexpected TXQ (0) queue failure: -5
[   22.476360] net enp0s3: Unexpected TXQ (0) queue failure: -5

Fixes: 283e2c2adcb8 ("net: virtio-net discards TX data after link down")
Cc: qemu-stable@nongnu.org
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 hw/virtio/vhost.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index ddc42f0f93..54041948cf 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1138,6 +1138,10 @@ static void vhost_virtqueue_stop(struct vhost_dev *dev,
     r = dev->vhost_ops->vhost_get_vring_base(dev, &state);
     if (r < 0) {
         VHOST_OPS_DEBUG("vhost VQ %d ring restore failed: %d", idx, r);
+        /* Connection to the backend is broken, so let's sync internal
+         * last avail idx to the device used idx.
+         */
+        virtio_queue_restore_last_avail_idx(vdev, idx);
     } else {
         virtio_queue_set_last_avail_idx(vdev, idx, state.num);
     }
-- 
2.14.3

Re: [Qemu-devel] [PATCH 0/2] virtio-net: Fix TX data discard on backend disconnection

[Jason Wang]

On 2017年11月17日 02:48, Maxime Coquelin wrote:
> This series fixes TX data discard when backend disconnects.
>
> On backend disconnection, QEMU cannot retrieve its internal
> avail index, which makes the virtio queue internal state
> inconsistent (last_avail_idx will always be 0, whereas used_idx
> is correct).
>
> It results in discarding TX data to corrupt the vring state, by
> completing descriptors already completed by the backend.
>
> This series fixes this by restoring last_avail_idx to the vring
> used idx, as some backends used to do on reconnection.
>
> Maxime Coquelin (2):
>    virtio: Add queue interface to restore avail index from vring used
>      index
>    vhost: restore avail index from vring used index on disconnection
>
>   hw/virtio/vhost.c          |  4 ++++
>   hw/virtio/virtio.c         | 10 ++++++++++
>   include/hw/virtio/virtio.h |  1 +
>   3 files changed, 15 insertions(+)
>

Reviewed-by: Jason Wang <jasowang@redhat.com>

Re: [Qemu-devel] [PATCH 2/2] vhost: restore avail index from vring used index on disconnection

[Jens Freimann]

On Thu, Nov 16, 2017 at 06:48:35PM +0000, Maxime Coquelin wrote:
>vhost_virtqueue_stop() gets avail index value from the backend,
>except if the backend is not responding.
>
>It happens when the backend crashes, and in this case, internal
>state of the virtio queue is inconsistent, making packets
>to corrupt the vring state.
>
>With a Linux guest, it results in following error message on
>backend reconnection:
>
>[   22.444905] virtio_net virtio0: output.0:id 0 is not a head!
>[   22.446746] net enp0s3: Unexpected TXQ (0) queue failure: -5
>[   22.476360] net enp0s3: Unexpected TXQ (0) queue failure: -5
>
>Fixes: 283e2c2adcb8 ("net: virtio-net discards TX data after link down")
>Cc: qemu-stable@nongnu.org
>Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>---
> hw/virtio/vhost.c | 4 ++++
> 1 file changed, 4 insertions(+)
>

Reviewed-by: Jens Freimann <jfreimann@redhat.com>

Re: [Qemu-devel] [PATCH 1/2] virtio: Add queue interface to restore avail index from vring used index

[Jens Freimann]

On Thu, Nov 16, 2017 at 06:48:34PM +0000, Maxime Coquelin wrote:
>In case of backend crash, it is not possible to restore internal
>avail index from the backend value as vhost_get_vring_base
>callback fails.
>
>This patch provides a new interface to restore internal avail index
>from the vring used index, as done by some vhost-user backend on
>reconnection.
>
>Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>---
> hw/virtio/virtio.c         | 10 ++++++++++
> include/hw/virtio/virtio.h |  1 +
> 2 files changed, 11 insertions(+)
>

Reviewed-by: Jens Freimann <jfreimann@redhat.com>