From patchwork Tue Nov 16 15:17:56 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Hu, Jiayu" X-Patchwork-Id: 104374 X-Patchwork-Delegate: maxime.coquelin@redhat.com Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id D61E6A0C43; Tue, 16 Nov 2021 08:15:55 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 65E9F407FF; Tue, 16 Nov 2021 08:15:55 +0100 (CET) Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by mails.dpdk.org (Postfix) with ESMTP id 460604068C for ; Tue, 16 Nov 2021 08:15:54 +0100 (CET) X-IronPort-AV: E=McAfee;i="6200,9189,10169"; a="233473860" X-IronPort-AV: E=Sophos;i="5.87,238,1631602800"; d="scan'208";a="233473860" Received: from orsmga007.jf.intel.com ([10.7.209.58]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 15 Nov 2021 23:15:53 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.87,238,1631602800"; d="scan'208";a="494353290" Received: from npgdpdkvirtiojiayuhu117.sh.intel.com ([10.67.119.202]) by orsmga007.jf.intel.com with ESMTP; 15 Nov 2021 23:15:51 -0800 From: Jiayu Hu To: dev@dpdk.org Cc: maxime.coquelin@redhat.com, chenbo.xia@intel.com, xingguang.he@intel.com, Jiayu Hu Subject: [PATCH v3] vhost: fix packed ring descriptor update in async enqueue Date: Tue, 16 Nov 2021 10:17:56 -0500 Message-Id: <20211116151756.2420165-1-jiayu.hu@intel.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20211110124009.2250592-1-jiayu.hu@intel.com> References: <20211110124009.2250592-1-jiayu.hu@intel.com> MIME-Version: 1.0 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org If the packet uses multiple descriptors 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 are 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 Reviewed-by: Chenbo Xia Reviewed-by: Maxime Coquelin --- v3: * fix typo 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