From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jens Freimann Subject: Re: [PATCH v3 15/21] vhost: packed queue enqueue path Date: Fri, 6 Apr 2018 14:17:50 +0200 Message-ID: <20180406121750.e3joojiyb2utzz6e@dhcp-192-241.str.redhat.com> References: <20180405101031.26468-1-jfreimann@redhat.com> <20180405101031.26468-16-jfreimann@redhat.com> <9cbcb9c1-ee1f-1118-ed39-ad9b0d4be0b2@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Cc: dev@dpdk.org, tiwei.bie@intel.com, yliu@fridaylinux.org, mst@redhat.com To: Maxime Coquelin Return-path: Received: from mx1.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by dpdk.org (Postfix) with ESMTP id C85B71CF80 for ; Fri, 6 Apr 2018 14:17:52 +0200 (CEST) Content-Disposition: inline In-Reply-To: <9cbcb9c1-ee1f-1118-ed39-ad9b0d4be0b2@redhat.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Fri, Apr 06, 2018 at 11:36:03AM +0200, Maxime Coquelin wrote: > > >On 04/05/2018 12:10 PM, Jens Freimann wrote: >>Implement enqueue of packets to the receive virtqueue. >> >>Set descriptor flag VIRTQ_DESC_F_USED and toggle used wrap counter if >>last descriptor in ring is used. Perform a write memory barrier before >>flags are written to descriptor. >> >>Chained descriptors are not supported with this patch. >> >>Signed-off-by: Jens Freimann >>--- >> lib/librte_vhost/virtio_net.c | 129 ++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 129 insertions(+) >> >>diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c >>index 7eea1da04..578e5612e 100644 >>--- a/lib/librte_vhost/virtio_net.c >>+++ b/lib/librte_vhost/virtio_net.c >>@@ -695,6 +695,135 @@ virtio_dev_merge_rx(struct virtio_net *dev, uint16_t queue_id, >> return pkt_idx; >> } >>+static inline uint32_t __attribute__((always_inline)) >>+vhost_enqueue_burst_packed(struct virtio_net *dev, uint16_t queue_id, >>+ struct rte_mbuf **pkts, uint32_t count) >>+{ >>+ struct vhost_virtqueue *vq; >>+ struct vring_desc_packed *descs; >>+ uint16_t idx; >>+ uint16_t mask; >>+ uint16_t i; >>+ >>+ vq = dev->virtqueue[queue_id]; >>+ >>+ rte_spinlock_lock(&vq->access_lock); >>+ >>+ if (unlikely(vq->enabled == 0)) { >>+ i = 0; >>+ goto out_access_unlock; >>+ } >>+ >>+ if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)) >>+ vhost_user_iotlb_rd_lock(vq); >>+ >>+ descs = vq->desc_packed; >>+ mask = vq->size - 1; >>+ >>+ for (i = 0; i < count; i++) { >>+ uint32_t desc_avail, desc_offset; >>+ uint32_t mbuf_avail, mbuf_offset; >>+ uint32_t cpy_len; >>+ struct vring_desc_packed *desc; >>+ uint64_t desc_addr; >>+ struct virtio_net_hdr_mrg_rxbuf *hdr; >>+ struct rte_mbuf *m = pkts[i]; >>+ >>+ /* XXX: there is an assumption that no desc will be chained */ >Is this assumption still true? >If not what are the plan to fix this? This is a leftover from the prototype code. I checked the code and don't see what it could still relate to except if it is supposed to mean indirect instead of chained. I think the comment can be removed. > >>+ idx = vq->last_used_idx & mask; >>+ desc = &descs[idx]; >>+ >>+ if (!desc_is_avail(vq, desc)) >IIUC, it means the ring is full. >I think this is an unlikely case, so maybe better to use the unlikely >macro here. yes, we can use unlikely here, will fix. thanks! regards, Jens