[v2,01/16] vhost: add single packet enqueue function

Message ID 20190919163643.24130-2-yong.liu@intel.com (mailing list archive)
State Superseded, archived
Headers
Series vhost packed ring performance optimization |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-dpdk_compile_spdk success Compile Testing PASS
ci/iol-dpdk_compile success Compile Testing PASS
ci/iol-dpdk_compile_ovs success Compile Testing PASS
ci/intel-Performance success Performance Testing PASS
ci/mellanox-Performance success Performance Testing PASS
ci/Intel-compilation fail Compilation issues

Commit Message

Marvin Liu Sept. 19, 2019, 4:36 p.m. UTC
  Add vhost enqueue function for single packet and meanwhile left space
for flush used ring function.

Signed-off-by: Marvin Liu <yong.liu@intel.com>
  

Comments

Gavin Hu Sept. 23, 2019, 9:09 a.m. UTC | #1
Hi Marvin,

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Marvin Liu
> Sent: Friday, September 20, 2019 12:36 AM
> To: maxime.coquelin@redhat.com; tiwei.bie@intel.com;
> zhihong.wang@intel.com
> Cc: dev@dpdk.org; Marvin Liu <yong.liu@intel.com>
> Subject: [dpdk-dev] [PATCH v2 01/16] vhost: add single packet enqueue
> function
>
> Add vhost enqueue function for single packet and meanwhile left space
> for flush used ring function.
>
> Signed-off-by: Marvin Liu <yong.liu@intel.com>
>
> diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c
> index 5b85b832d..2b5c47145 100644
> --- a/lib/librte_vhost/virtio_net.c
> +++ b/lib/librte_vhost/virtio_net.c
> @@ -774,6 +774,70 @@ copy_mbuf_to_desc(struct virtio_net *dev, struct
> vhost_virtqueue *vq,
>       return error;
>  }
>
> +/*
> + * Returns -1 on fail, 0 on success
> + */
> +static __rte_always_inline int
> +vhost_enqueue_single_packed(struct virtio_net *dev, struct
> vhost_virtqueue *vq,
> +     struct rte_mbuf *pkt, struct buf_vector *buf_vec, uint16_t
> *nr_descs)
> +{
> +     uint16_t nr_vec = 0;
> +
> +     uint16_t avail_idx;
> +     uint16_t max_tries, tries = 0;
> +
> +     uint16_t buf_id = 0;
> +     uint32_t len = 0;
> +     uint16_t desc_count;
> +
> +     uint32_t size = pkt->pkt_len + dev->vhost_hlen;
> +     avail_idx = vq->last_avail_idx;
> +
> +     if (rxvq_is_mergeable(dev))
> +             max_tries = vq->size - 1;
> +     else
> +             max_tries = 1;
> +
> +     uint16_t num_buffers = 0;
> +
> +     while (size > 0) {
> +             /*
> +              * if we tried all available ring items, and still
> +              * can't get enough buf, it means something abnormal
> +              * happened.
> +              */
> +             if (unlikely(++tries > max_tries))
> +                     return -1;
> +
> +             if (unlikely(fill_vec_buf_packed(dev, vq,
> +                                             avail_idx, &desc_count,
> +                                             buf_vec, &nr_vec,
> +                                             &buf_id, &len,
> +                                             VHOST_ACCESS_RW) < 0)) {
> +                     return -1;
> +             }
> +
> +             len = RTE_MIN(len, size);
> +
> +             size -= len;
> +
> +             avail_idx += desc_count;
> +             if (avail_idx >= vq->size)
> +                     avail_idx -= vq->size;
> +
> +             *nr_descs += desc_count;
> +             num_buffers += 1;
> +     }
> +
> +     if (copy_mbuf_to_desc(dev, vq, pkt,
> +                                     buf_vec, nr_vec,
> +                                     num_buffers) < 0) {
> +             return 0;
Why return 0, which means success, while "copy_mbuf_to_desc" encounters some problems and failed?
/Gavin
> +     }
> +
> +     return 0;
> +}
> +
>  static __rte_noinline uint32_t
>  virtio_dev_rx_split(struct virtio_net *dev, struct vhost_virtqueue *vq,
>       struct rte_mbuf **pkts, uint32_t count)
> @@ -831,6 +895,35 @@ virtio_dev_rx_split(struct virtio_net *dev, struct
> vhost_virtqueue *vq,
>       return pkt_idx;
>  }
>
> +static __rte_unused int16_t
> +virtio_dev_rx_single_packed(struct virtio_net *dev, struct vhost_virtqueue
> *vq,
> +     struct rte_mbuf *pkt)
> +{
> +     struct buf_vector buf_vec[BUF_VECTOR_MAX];
> +     uint16_t nr_descs = 0;
> +
> +     rte_smp_rmb();
> +     if (unlikely(vhost_enqueue_single_packed(dev, vq, pkt, buf_vec,
> +                                              &nr_descs) < 0)) {
> +             VHOST_LOG_DEBUG(VHOST_DATA,
> +                             "(%d) failed to get enough desc from
> vring\n",
> +                             dev->vid);
> +             return -1;
> +     }
> +
> +     VHOST_LOG_DEBUG(VHOST_DATA, "(%d) current index %d | end
> index %d\n",
> +                     dev->vid, vq->last_avail_idx,
> +                     vq->last_avail_idx + nr_descs);
> +
> +     vq->last_avail_idx += nr_descs;
> +     if (vq->last_avail_idx >= vq->size) {
> +             vq->last_avail_idx -= vq->size;
> +             vq->avail_wrap_counter ^= 1;
> +     }
> +
> +     return 0;
> +}
> +
>  static __rte_noinline uint32_t
>  virtio_dev_rx_packed(struct virtio_net *dev, struct vhost_virtqueue *vq,
>       struct rte_mbuf **pkts, uint32_t count)
> --
> 2.17.1

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
  
Marvin Liu Sept. 23, 2019, 9:15 a.m. UTC | #2
> -----Original Message-----
> From: Gavin Hu (Arm Technology China) [mailto:Gavin.Hu@arm.com]
> Sent: Monday, September 23, 2019 5:09 PM
> To: Liu, Yong <yong.liu@intel.com>; maxime.coquelin@redhat.com; Bie, Tiwei
> <tiwei.bie@intel.com>; Wang, Zhihong <zhihong.wang@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v2 01/16] vhost: add single packet enqueue
> function
> 
> Hi Marvin,
> 
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Marvin Liu
> > Sent: Friday, September 20, 2019 12:36 AM
> > To: maxime.coquelin@redhat.com; tiwei.bie@intel.com;
> > zhihong.wang@intel.com
> > Cc: dev@dpdk.org; Marvin Liu <yong.liu@intel.com>
> > Subject: [dpdk-dev] [PATCH v2 01/16] vhost: add single packet enqueue
> > function
> >
> > Add vhost enqueue function for single packet and meanwhile left space
> > for flush used ring function.
> >
> > Signed-off-by: Marvin Liu <yong.liu@intel.com>
> >
> > diff --git a/lib/librte_vhost/virtio_net.c
> b/lib/librte_vhost/virtio_net.c
> > index 5b85b832d..2b5c47145 100644
> > --- a/lib/librte_vhost/virtio_net.c
> > +++ b/lib/librte_vhost/virtio_net.c
> > @@ -774,6 +774,70 @@ copy_mbuf_to_desc(struct virtio_net *dev, struct
> > vhost_virtqueue *vq,
> >       return error;
> >  }
> >
> > +/*
> > + * Returns -1 on fail, 0 on success
> > + */
> > +static __rte_always_inline int
> > +vhost_enqueue_single_packed(struct virtio_net *dev, struct
> > vhost_virtqueue *vq,
> > +     struct rte_mbuf *pkt, struct buf_vector *buf_vec, uint16_t
> > *nr_descs)
> > +{
> > +     uint16_t nr_vec = 0;
> > +
> > +     uint16_t avail_idx;
> > +     uint16_t max_tries, tries = 0;
> > +
> > +     uint16_t buf_id = 0;
> > +     uint32_t len = 0;
> > +     uint16_t desc_count;
> > +
> > +     uint32_t size = pkt->pkt_len + dev->vhost_hlen;
> > +     avail_idx = vq->last_avail_idx;
> > +
> > +     if (rxvq_is_mergeable(dev))
> > +             max_tries = vq->size - 1;
> > +     else
> > +             max_tries = 1;
> > +
> > +     uint16_t num_buffers = 0;
> > +
> > +     while (size > 0) {
> > +             /*
> > +              * if we tried all available ring items, and still
> > +              * can't get enough buf, it means something abnormal
> > +              * happened.
> > +              */
> > +             if (unlikely(++tries > max_tries))
> > +                     return -1;
> > +
> > +             if (unlikely(fill_vec_buf_packed(dev, vq,
> > +                                             avail_idx, &desc_count,
> > +                                             buf_vec, &nr_vec,
> > +                                             &buf_id, &len,
> > +                                             VHOST_ACCESS_RW) < 0)) {
> > +                     return -1;
> > +             }
> > +
> > +             len = RTE_MIN(len, size);
> > +
> > +             size -= len;
> > +
> > +             avail_idx += desc_count;
> > +             if (avail_idx >= vq->size)
> > +                     avail_idx -= vq->size;
> > +
> > +             *nr_descs += desc_count;
> > +             num_buffers += 1;
> > +     }
> > +
> > +     if (copy_mbuf_to_desc(dev, vq, pkt,
> > +                                     buf_vec, nr_vec,
> > +                                     num_buffers) < 0) {
> > +             return 0;
> Why return 0, which means success, while "copy_mbuf_to_desc" encounters
> some problems and failed?
> /Gavin

Gavin,
Thanks for notice this typo issue. Here should return negative value -1.

Regards,
Marvin

> > +     }
> > +
> > +     return 0;
> > +}
> > +
> >  static __rte_noinline uint32_t
> >  virtio_dev_rx_split(struct virtio_net *dev, struct vhost_virtqueue *vq,
> >       struct rte_mbuf **pkts, uint32_t count)
> > @@ -831,6 +895,35 @@ virtio_dev_rx_split(struct virtio_net *dev, struct
> > vhost_virtqueue *vq,
> >       return pkt_idx;
> >  }
> >
> > +static __rte_unused int16_t
> > +virtio_dev_rx_single_packed(struct virtio_net *dev, struct
> vhost_virtqueue
> > *vq,
> > +     struct rte_mbuf *pkt)
> > +{
> > +     struct buf_vector buf_vec[BUF_VECTOR_MAX];
> > +     uint16_t nr_descs = 0;
> > +
> > +     rte_smp_rmb();
> > +     if (unlikely(vhost_enqueue_single_packed(dev, vq, pkt, buf_vec,
> > +                                              &nr_descs) < 0)) {
> > +             VHOST_LOG_DEBUG(VHOST_DATA,
> > +                             "(%d) failed to get enough desc from
> > vring\n",
> > +                             dev->vid);
> > +             return -1;
> > +     }
> > +
> > +     VHOST_LOG_DEBUG(VHOST_DATA, "(%d) current index %d | end
> > index %d\n",
> > +                     dev->vid, vq->last_avail_idx,
> > +                     vq->last_avail_idx + nr_descs);
> > +
> > +     vq->last_avail_idx += nr_descs;
> > +     if (vq->last_avail_idx >= vq->size) {
> > +             vq->last_avail_idx -= vq->size;
> > +             vq->avail_wrap_counter ^= 1;
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> >  static __rte_noinline uint32_t
> >  virtio_dev_rx_packed(struct virtio_net *dev, struct vhost_virtqueue *vq,
> >       struct rte_mbuf **pkts, uint32_t count)
> > --
> > 2.17.1
> 
> IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended
> recipient, please notify the sender immediately and do not disclose the
> contents to any other person, use it for any purpose, or store or copy the
> information in any medium. Thank you.
  

Patch

diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c
index 5b85b832d..2b5c47145 100644
--- a/lib/librte_vhost/virtio_net.c
+++ b/lib/librte_vhost/virtio_net.c
@@ -774,6 +774,70 @@  copy_mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
 	return error;
 }
 
+/*
+ * Returns -1 on fail, 0 on success
+ */
+static __rte_always_inline int
+vhost_enqueue_single_packed(struct virtio_net *dev, struct vhost_virtqueue *vq,
+	struct rte_mbuf *pkt, struct buf_vector *buf_vec, uint16_t *nr_descs)
+{
+	uint16_t nr_vec = 0;
+
+	uint16_t avail_idx;
+	uint16_t max_tries, tries = 0;
+
+	uint16_t buf_id = 0;
+	uint32_t len = 0;
+	uint16_t desc_count;
+
+	uint32_t size = pkt->pkt_len + dev->vhost_hlen;
+	avail_idx = vq->last_avail_idx;
+
+	if (rxvq_is_mergeable(dev))
+		max_tries = vq->size - 1;
+	else
+		max_tries = 1;
+
+	uint16_t num_buffers = 0;
+
+	while (size > 0) {
+		/*
+		 * if we tried all available ring items, and still
+		 * can't get enough buf, it means something abnormal
+		 * happened.
+		 */
+		if (unlikely(++tries > max_tries))
+			return -1;
+
+		if (unlikely(fill_vec_buf_packed(dev, vq,
+						avail_idx, &desc_count,
+						buf_vec, &nr_vec,
+						&buf_id, &len,
+						VHOST_ACCESS_RW) < 0)) {
+			return -1;
+		}
+
+		len = RTE_MIN(len, size);
+
+		size -= len;
+
+		avail_idx += desc_count;
+		if (avail_idx >= vq->size)
+			avail_idx -= vq->size;
+
+		*nr_descs += desc_count;
+		num_buffers += 1;
+	}
+
+	if (copy_mbuf_to_desc(dev, vq, pkt,
+					buf_vec, nr_vec,
+					num_buffers) < 0) {
+		return 0;
+	}
+
+	return 0;
+}
+
 static __rte_noinline uint32_t
 virtio_dev_rx_split(struct virtio_net *dev, struct vhost_virtqueue *vq,
 	struct rte_mbuf **pkts, uint32_t count)
@@ -831,6 +895,35 @@  virtio_dev_rx_split(struct virtio_net *dev, struct vhost_virtqueue *vq,
 	return pkt_idx;
 }
 
+static __rte_unused int16_t
+virtio_dev_rx_single_packed(struct virtio_net *dev, struct vhost_virtqueue *vq,
+	struct rte_mbuf *pkt)
+{
+	struct buf_vector buf_vec[BUF_VECTOR_MAX];
+	uint16_t nr_descs = 0;
+
+	rte_smp_rmb();
+	if (unlikely(vhost_enqueue_single_packed(dev, vq, pkt, buf_vec,
+						 &nr_descs) < 0)) {
+		VHOST_LOG_DEBUG(VHOST_DATA,
+				"(%d) failed to get enough desc from vring\n",
+				dev->vid);
+		return -1;
+	}
+
+	VHOST_LOG_DEBUG(VHOST_DATA, "(%d) current index %d | end index %d\n",
+			dev->vid, vq->last_avail_idx,
+			vq->last_avail_idx + nr_descs);
+
+	vq->last_avail_idx += nr_descs;
+	if (vq->last_avail_idx >= vq->size) {
+		vq->last_avail_idx -= vq->size;
+		vq->avail_wrap_counter ^= 1;
+	}
+
+	return 0;
+}
+
 static __rte_noinline uint32_t
 virtio_dev_rx_packed(struct virtio_net *dev, struct vhost_virtqueue *vq,
 	struct rte_mbuf **pkts, uint32_t count)