From mboxrd@z Thu Jan 1 00:00:00 1970 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) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 374AF818B10B; Fri, 6 Apr 2018 12:17:52 +0000 (UTC) Received: from localhost (dhcp-192-241.str.redhat.com [10.33.192.241]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 923FF2024CA5; Fri, 6 Apr 2018 12:17:51 +0000 (UTC) Date: Fri, 6 Apr 2018 14:17:50 +0200 From: Jens Freimann To: Maxime Coquelin Cc: dev@dpdk.org, tiwei.bie@intel.com, yliu@fridaylinux.org, mst@redhat.com 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 Content-Disposition: inline In-Reply-To: <9cbcb9c1-ee1f-1118-ed39-ad9b0d4be0b2@redhat.com> User-Agent: NeoMutt/20180223 X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Fri, 06 Apr 2018 12:17:52 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.8]); Fri, 06 Apr 2018 12:17:52 +0000 (UTC) for IP:'10.11.54.4' DOMAIN:'int-mx04.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'jfreimann@redhat.com' RCPT:'' Subject: Re: [dpdk-dev] [PATCH v3 15/21] vhost: packed queue enqueue path X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 06 Apr 2018 12:17:53 -0000 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