[dpdk-stable] [PATCH 17.11] net/virtio: add barrier before reading the flags

[dpdk-stable] [PATCH 17.11] net/virtio: add barrier before reading the flags

[Ilya Maximets]

[ backported from upstream commit d21d05c7a9c1c49ec927f961c4ab797a598af5ca ]

Reading the used->flags could be reordered with avail->idx update.
vhost in kernel disables notifications for the time of packets
receiving, like this:

    1. disable notify
    2. process packets
    3. enable notify
    4. has more packets ? goto 1

In case of reordering, virtio driver could read the flags on
step 2 while notifications disabled and update avail->idx after
the step 4, i.e. vhost will exit the loop on step 4 with
notifications enabled, but virtio will not notify.

Fixes: c1f86306a026 ("virtio: add new driver")
Cc: stable@dpdk.org

Reported-by: Shahaf Shuler <shahafs@mellanox.com>
Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/net/virtio/virtqueue.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/virtio/virtqueue.h b/drivers/net/virtio/virtqueue.h
index 788d04dfa..6efd4f01c 100644
--- a/drivers/net/virtio/virtqueue.h
+++ b/drivers/net/virtio/virtqueue.h
@@ -354,6 +354,11 @@ vq_update_avail_ring(struct virtqueue *vq, uint16_t desc_idx)
 static inline int
 virtqueue_kick_prepare(struct virtqueue *vq)
 {
+	/*
+	 * Ensure updated avail->idx is visible to vhost before reading
+	 * the used->flags.
+	 */
+	virtio_mb();
 	return !(vq->vq_ring.used->flags & VRING_USED_F_NO_NOTIFY);
 }
 
-- 
2.17.1

Re: [dpdk-stable] [PATCH 17.11] net/virtio: add barrier before reading the flags

[Yongseok Koh]

> On Mar 13, 2019, at 8:27 AM, Ilya Maximets <i.maximets@samsung.com> wrote:
> 
> [ backported from upstream commit d21d05c7a9c1c49ec927f961c4ab797a598af5ca ]
> 
> Reading the used->flags could be reordered with avail->idx update.
> vhost in kernel disables notifications for the time of packets
> receiving, like this:
> 
>    1. disable notify
>    2. process packets
>    3. enable notify
>    4. has more packets ? goto 1
> 
> In case of reordering, virtio driver could read the flags on
> step 2 while notifications disabled and update avail->idx after
> the step 4, i.e. vhost will exit the loop on step 4 with
> notifications enabled, but virtio will not notify.
> 
> Fixes: c1f86306a026 ("virtio: add new driver")
> Cc: stable@dpdk.org
> 
> Reported-by: Shahaf Shuler <shahafs@mellanox.com>
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
applied to stable/17.11

Thanks,
Yongseok


> drivers/net/virtio/virtqueue.h | 5 +++++
> 1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/net/virtio/virtqueue.h b/drivers/net/virtio/virtqueue.h
> index 788d04dfa..6efd4f01c 100644
> --- a/drivers/net/virtio/virtqueue.h
> +++ b/drivers/net/virtio/virtqueue.h
> @@ -354,6 +354,11 @@ vq_update_avail_ring(struct virtqueue *vq, uint16_t desc_idx)
> static inline int
> virtqueue_kick_prepare(struct virtqueue *vq)
> {
> +	/*
> +	 * Ensure updated avail->idx is visible to vhost before reading
> +	 * the used->flags.
> +	 */
> +	virtio_mb();
> 	return !(vq->vq_ring.used->flags & VRING_USED_F_NO_NOTIFY);
> }
> 
> -- 
> 2.17.1
>