If the packet uses multiple descrptors and its descriptor indices are wrapped, the first descriptor flag is not updated last, which may cause virtio read the incomplete packet. For example, given a packet uses 64 descriptors, and virtio ring size is 256, and its descriptor indices is 224~255 and 0~31, current implementation will update 224~255 descriptor flags earlier than 0~31 descriptor flags.
This patch fixes this issue by updating descriptor flags in one loop, so that the first descriptor flag is always updated last. Fixes: 873e8dad6f49 ("vhost: support packed ring in async datapath") Signed-off-by: Jiayu Hu <jiayu...@intel.com> --- v2: * update commit log --- lib/vhost/virtio_net.c | 122 ++++++++++++++++++----------------------- 1 file changed, 54 insertions(+), 68 deletions(-) diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c index cef4bcf15c..b3d954aab4 100644 --- a/lib/vhost/virtio_net.c +++ b/lib/vhost/virtio_net.c @@ -1549,60 +1549,6 @@ virtio_dev_rx_async_submit_split(struct virtio_net *dev, return pkt_idx; } -static __rte_always_inline void -vhost_update_used_packed(struct vhost_virtqueue *vq, - struct vring_used_elem_packed *shadow_ring, - uint16_t count) -{ - int i; - uint16_t used_idx = vq->last_used_idx; - uint16_t head_idx = vq->last_used_idx; - uint16_t head_flags = 0; - - if (count == 0) - return; - - /* Split loop in two to save memory barriers */ - for (i = 0; i < count; i++) { - vq->desc_packed[used_idx].id = shadow_ring[i].id; - vq->desc_packed[used_idx].len = shadow_ring[i].len; - - used_idx += shadow_ring[i].count; - if (used_idx >= vq->size) - used_idx -= vq->size; - } - - /* The ordering for storing desc flags needs to be enforced. */ - rte_atomic_thread_fence(__ATOMIC_RELEASE); - - for (i = 0; i < count; i++) { - uint16_t flags; - - if (vq->shadow_used_packed[i].len) - flags = VRING_DESC_F_WRITE; - else - flags = 0; - - if (vq->used_wrap_counter) { - flags |= VRING_DESC_F_USED; - flags |= VRING_DESC_F_AVAIL; - } else { - flags &= ~VRING_DESC_F_USED; - flags &= ~VRING_DESC_F_AVAIL; - } - - if (i > 0) { - vq->desc_packed[vq->last_used_idx].flags = flags; - } else { - head_idx = vq->last_used_idx; - head_flags = flags; - } - - vq_inc_last_used_packed(vq, shadow_ring[i].count); - } - - vq->desc_packed[head_idx].flags = head_flags; -} static __rte_always_inline int vhost_enqueue_async_packed(struct virtio_net *dev, @@ -1819,23 +1765,63 @@ write_back_completed_descs_packed(struct vhost_virtqueue *vq, uint16_t n_buffers) { struct vhost_async *async = vq->async; - uint16_t nr_left = n_buffers; - uint16_t from, to; + uint16_t from = async->last_buffer_idx_packed; + uint16_t used_idx = vq->last_used_idx; + uint16_t head_idx = vq->last_used_idx; + uint16_t head_flags = 0; + uint16_t i; - do { - from = async->last_buffer_idx_packed; - to = (from + nr_left) % vq->size; - if (to > from) { - vhost_update_used_packed(vq, async->buffers_packed + from, to - from); - async->last_buffer_idx_packed += nr_left; - nr_left = 0; + /* Split loop in two to save memory barriers */ + for (i = 0; i < n_buffers; i++) { + vq->desc_packed[used_idx].id = async->buffers_packed[from].id; + vq->desc_packed[used_idx].len = async->buffers_packed[from].len; + + used_idx += async->buffers_packed[from].count; + if (used_idx >= vq->size) + used_idx -= vq->size; + + from++; + if (from >= vq->size) + from = 0; + } + + /* The ordering for storing desc flags needs to be enforced. */ + rte_atomic_thread_fence(__ATOMIC_RELEASE); + + from = async->last_buffer_idx_packed; + + for (i = 0; i < n_buffers; i++) { + uint16_t flags; + + if (async->buffers_packed[from].len) + flags = VRING_DESC_F_WRITE; + else + flags = 0; + + if (vq->used_wrap_counter) { + flags |= VRING_DESC_F_USED; + flags |= VRING_DESC_F_AVAIL; } else { - vhost_update_used_packed(vq, async->buffers_packed + from, - vq->size - from); - async->last_buffer_idx_packed = 0; - nr_left -= vq->size - from; + flags &= ~VRING_DESC_F_USED; + flags &= ~VRING_DESC_F_AVAIL; } - } while (nr_left > 0); + + if (i > 0) { + vq->desc_packed[vq->last_used_idx].flags = flags; + } else { + head_idx = vq->last_used_idx; + head_flags = flags; + } + + vq_inc_last_used_packed(vq, async->buffers_packed[from].count); + + from++; + if (from == vq->size) + from = 0; + } + + vq->desc_packed[head_idx].flags = head_flags; + async->last_buffer_idx_packed = from; } static __rte_always_inline uint16_t -- 2.25.1