diff mbox series

[v3] vhost: add new `rte_vhost_vring_call_nonblock` API

Message ID 20221017071457.28589-1-changpeng.liu@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers show
Series [v3] vhost: add new `rte_vhost_vring_call_nonblock` API | expand

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/iol-mellanox-Performance success Performance Testing PASS
ci/intel-Testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS

Commit Message

Liu, Changpeng Oct. 17, 2022, 7:14 a.m. UTC
Vhost-user library locks all VQ's access lock when processing
vring based messages, such as SET_VRING_KICK and SET_VRING_CALL,
and the data processing thread may already be started, e.g: SPDK
vhost-blk and vhost-scsi will start the data processing thread
when one vring is ready, then deadlock may happen when SPDK is
posting interrupts to VM.  Here, we add a new API which allows
caller to try again later for this case.

Bugzilla ID: 1015
Fixes: c5736998305d ("vhost: fix missing virtqueue lock protection")

Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
---
 doc/guides/prog_guide/vhost_lib.rst    |  6 ++++++
 doc/guides/rel_notes/release_22_11.rst |  6 ++++++
 lib/vhost/rte_vhost.h                  | 15 +++++++++++++
 lib/vhost/version.map                  |  3 +++
 lib/vhost/vhost.c                      | 30 ++++++++++++++++++++++++++
 5 files changed, 60 insertions(+)

Comments

Xia, Chenbo Oct. 17, 2022, 7:39 a.m. UTC | #1
Hi Changpeng,

> -----Original Message-----
> From: Liu, Changpeng <changpeng.liu@intel.com>
> Sent: Monday, October 17, 2022 3:15 PM
> To: dev@dpdk.org
> Cc: Liu, Changpeng <changpeng.liu@intel.com>; Maxime Coquelin
> <maxime.coquelin@redhat.com>; Xia, Chenbo <chenbo.xia@intel.com>; David
> Marchand <david.marchand@redhat.com>
> Subject: [PATCH v3] vhost: add new `rte_vhost_vring_call_nonblock` API
> 
> Vhost-user library locks all VQ's access lock when processing
> vring based messages, such as SET_VRING_KICK and SET_VRING_CALL,
> and the data processing thread may already be started, e.g: SPDK
> vhost-blk and vhost-scsi will start the data processing thread
> when one vring is ready, then deadlock may happen when SPDK is
> posting interrupts to VM.  Here, we add a new API which allows
> caller to try again later for this case.
> 
> Bugzilla ID: 1015
> Fixes: c5736998305d ("vhost: fix missing virtqueue lock protection")
> 
> Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
> ---
>  doc/guides/prog_guide/vhost_lib.rst    |  6 ++++++
>  doc/guides/rel_notes/release_22_11.rst |  6 ++++++
>  lib/vhost/rte_vhost.h                  | 15 +++++++++++++
>  lib/vhost/version.map                  |  3 +++
>  lib/vhost/vhost.c                      | 30 ++++++++++++++++++++++++++
>  5 files changed, 60 insertions(+)
> 
> diff --git a/doc/guides/prog_guide/vhost_lib.rst
> b/doc/guides/prog_guide/vhost_lib.rst
> index bad4d819e1..52f0589f51 100644
> --- a/doc/guides/prog_guide/vhost_lib.rst
> +++ b/doc/guides/prog_guide/vhost_lib.rst
> @@ -297,6 +297,12 @@ The following is an overview of some key Vhost API
> functions:
>    Clear in-flight packets which are submitted to async channel in vhost
> async data
>    path. Completed packets are returned to applications through ``pkts``.
> 
> +* ``rte_vhost_vring_call_nonblock(int vid, uint16_t vring_idx)``
> +
> +  Notify the guest that used descriptors have been added to the vring.
> This function
> +  will return -EAGAIN when vq's access lock is held by other thread, user
> should try
> +  again later.
> +
>  * ``rte_vhost_vring_stats_get_names(int vid, uint16_t queue_id, struct
> rte_vhost_stat_name *names, unsigned int size)``
> 
>    This function returns the names of the queue statistics. It requires
> diff --git a/doc/guides/rel_notes/release_22_11.rst
> b/doc/guides/rel_notes/release_22_11.rst
> index 2da8bc9661..9b3783ffd8 100644
> --- a/doc/guides/rel_notes/release_22_11.rst
> +++ b/doc/guides/rel_notes/release_22_11.rst
> @@ -236,6 +236,12 @@ New Features
> 
>       strings $dpdk_binary_or_driver | sed -n 's/^PMD_INFO_STRING= //p'
> 
> +* **Added non-block notify API to the vhost library.**
> +
> +  Added API to notify the guest that used descriptors have been added
> +  to the vring in non-block way, user should check the return value of
> +  this API.
> +

This may be better:

* **Added non-blocking notify API to the vhost library.**

  Added ``rte_vhost_vring_call_nonblock`` API to notify the guest that used
  descriptors have been added to the vring in non-blocking way. User should
  check the return value of this API and try again if needed.

And for new features, library should come before scripts, so you need to move
this before ' Rewritten pmdinfo script. ' item.

Thanks,
Chenbo

> 
>  Removed Items
>  -------------
> diff --git a/lib/vhost/rte_vhost.h b/lib/vhost/rte_vhost.h
> index bb7d86a432..d22b25cd4e 100644
> --- a/lib/vhost/rte_vhost.h
> +++ b/lib/vhost/rte_vhost.h
> @@ -909,6 +909,21 @@ rte_vhost_clr_inflight_desc_packed(int vid, uint16_t
> vring_idx,
>   */
>  int rte_vhost_vring_call(int vid, uint16_t vring_idx);
> 
> +/**
> + * Notify the guest that used descriptors have been added to the vring.
> This
> + * function acts as a memory barrier.  This function will return -EAGAIN
> when
> + * vq's access lock is held by other thread, user should try again later.
> + *
> + * @param vid
> + *  vhost device ID
> + * @param vring_idx
> + *  vring index
> + * @return
> + *  0 on success, -1 on failure, -EAGAIN for another retry
> + */
> +__rte_experimental
> +int rte_vhost_vring_call_nonblock(int vid, uint16_t vring_idx);
> +
>  /**
>   * Get vhost RX queue avail count.
>   *
> diff --git a/lib/vhost/version.map b/lib/vhost/version.map
> index 7a00b65740..c8c44b8326 100644
> --- a/lib/vhost/version.map
> +++ b/lib/vhost/version.map
> @@ -94,6 +94,9 @@ EXPERIMENTAL {
>  	rte_vhost_async_try_dequeue_burst;
>  	rte_vhost_driver_get_vdpa_dev_type;
>  	rte_vhost_clear_queue;
> +
> +	# added in 22.11
> +	rte_vhost_vring_call_nonblock;
>  };
> 
>  INTERNAL {
> diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c
> index 8740aa2788..ed6efb003f 100644
> --- a/lib/vhost/vhost.c
> +++ b/lib/vhost/vhost.c
> @@ -1317,6 +1317,36 @@ rte_vhost_vring_call(int vid, uint16_t vring_idx)
>  	return 0;
>  }
> 
> +int
> +rte_vhost_vring_call_nonblock(int vid, uint16_t vring_idx)
> +{
> +	struct virtio_net *dev;
> +	struct vhost_virtqueue *vq;
> +
> +	dev = get_device(vid);
> +	if (!dev)
> +		return -1;
> +
> +	if (vring_idx >= VHOST_MAX_VRING)
> +		return -1;
> +
> +	vq = dev->virtqueue[vring_idx];
> +	if (!vq)
> +		return -1;
> +
> +	if (!rte_spinlock_trylock(&vq->access_lock))
> +		return -EAGAIN;
> +
> +	if (vq_is_packed(dev))
> +		vhost_vring_call_packed(dev, vq);
> +	else
> +		vhost_vring_call_split(dev, vq);
> +
> +	rte_spinlock_unlock(&vq->access_lock);
> +
> +	return 0;
> +}
> +
>  uint16_t
>  rte_vhost_avail_entries(int vid, uint16_t queue_id)
>  {
> --
> 2.21.3
Liu, Changpeng Oct. 17, 2022, 7:50 a.m. UTC | #2
> -----Original Message-----
> From: Xia, Chenbo <chenbo.xia@intel.com>
> Sent: Monday, October 17, 2022 3:40 PM
> To: Liu, Changpeng <changpeng.liu@intel.com>; dev@dpdk.org
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>; David Marchand
> <david.marchand@redhat.com>
> Subject: RE: [PATCH v3] vhost: add new `rte_vhost_vring_call_nonblock` API
> 
> Hi Changpeng,
> 
> > -----Original Message-----
> > From: Liu, Changpeng <changpeng.liu@intel.com>
> > Sent: Monday, October 17, 2022 3:15 PM
> > To: dev@dpdk.org
> > Cc: Liu, Changpeng <changpeng.liu@intel.com>; Maxime Coquelin
> > <maxime.coquelin@redhat.com>; Xia, Chenbo <chenbo.xia@intel.com>; David
> > Marchand <david.marchand@redhat.com>
> > Subject: [PATCH v3] vhost: add new `rte_vhost_vring_call_nonblock` API
> >
> > Vhost-user library locks all VQ's access lock when processing
> > vring based messages, such as SET_VRING_KICK and SET_VRING_CALL,
> > and the data processing thread may already be started, e.g: SPDK
> > vhost-blk and vhost-scsi will start the data processing thread
> > when one vring is ready, then deadlock may happen when SPDK is
> > posting interrupts to VM.  Here, we add a new API which allows
> > caller to try again later for this case.
> >
> > Bugzilla ID: 1015
> > Fixes: c5736998305d ("vhost: fix missing virtqueue lock protection")
> >
> > Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
> > ---
> >  doc/guides/prog_guide/vhost_lib.rst    |  6 ++++++
> >  doc/guides/rel_notes/release_22_11.rst |  6 ++++++
> >  lib/vhost/rte_vhost.h                  | 15 +++++++++++++
> >  lib/vhost/version.map                  |  3 +++
> >  lib/vhost/vhost.c                      | 30 ++++++++++++++++++++++++++
> >  5 files changed, 60 insertions(+)
> >
> > diff --git a/doc/guides/prog_guide/vhost_lib.rst
> > b/doc/guides/prog_guide/vhost_lib.rst
> > index bad4d819e1..52f0589f51 100644
> > --- a/doc/guides/prog_guide/vhost_lib.rst
> > +++ b/doc/guides/prog_guide/vhost_lib.rst
> > @@ -297,6 +297,12 @@ The following is an overview of some key Vhost API
> > functions:
> >    Clear in-flight packets which are submitted to async channel in vhost
> > async data
> >    path. Completed packets are returned to applications through ``pkts``.
> >
> > +* ``rte_vhost_vring_call_nonblock(int vid, uint16_t vring_idx)``
> > +
> > +  Notify the guest that used descriptors have been added to the vring.
> > This function
> > +  will return -EAGAIN when vq's access lock is held by other thread, user
> > should try
> > +  again later.
> > +
> >  * ``rte_vhost_vring_stats_get_names(int vid, uint16_t queue_id, struct
> > rte_vhost_stat_name *names, unsigned int size)``
> >
> >    This function returns the names of the queue statistics. It requires
> > diff --git a/doc/guides/rel_notes/release_22_11.rst
> > b/doc/guides/rel_notes/release_22_11.rst
> > index 2da8bc9661..9b3783ffd8 100644
> > --- a/doc/guides/rel_notes/release_22_11.rst
> > +++ b/doc/guides/rel_notes/release_22_11.rst
> > @@ -236,6 +236,12 @@ New Features
> >
> >       strings $dpdk_binary_or_driver | sed -n 's/^PMD_INFO_STRING= //p'
> >
> > +* **Added non-block notify API to the vhost library.**
> > +
> > +  Added API to notify the guest that used descriptors have been added
> > +  to the vring in non-block way, user should check the return value of
> > +  this API.
> > +
> 
> This may be better:
> 
> * **Added non-blocking notify API to the vhost library.**
> 
>   Added ``rte_vhost_vring_call_nonblock`` API to notify the guest that used
>   descriptors have been added to the vring in non-blocking way. User should
>   check the return value of this API and try again if needed.
> 
> And for new features, library should come before scripts, so you need to move
> this before ' Rewritten pmdinfo script. ' item.
Thanks Chenbo, applied them with v4.
> 
> Thanks,
> Chenbo
> 
> >
> >  Removed Items
> >  -------------
> > diff --git a/lib/vhost/rte_vhost.h b/lib/vhost/rte_vhost.h
> > index bb7d86a432..d22b25cd4e 100644
> > --- a/lib/vhost/rte_vhost.h
> > +++ b/lib/vhost/rte_vhost.h
> > @@ -909,6 +909,21 @@ rte_vhost_clr_inflight_desc_packed(int vid, uint16_t
> > vring_idx,
> >   */
> >  int rte_vhost_vring_call(int vid, uint16_t vring_idx);
> >
> > +/**
> > + * Notify the guest that used descriptors have been added to the vring.
> > This
> > + * function acts as a memory barrier.  This function will return -EAGAIN
> > when
> > + * vq's access lock is held by other thread, user should try again later.
> > + *
> > + * @param vid
> > + *  vhost device ID
> > + * @param vring_idx
> > + *  vring index
> > + * @return
> > + *  0 on success, -1 on failure, -EAGAIN for another retry
> > + */
> > +__rte_experimental
> > +int rte_vhost_vring_call_nonblock(int vid, uint16_t vring_idx);
> > +
> >  /**
> >   * Get vhost RX queue avail count.
> >   *
> > diff --git a/lib/vhost/version.map b/lib/vhost/version.map
> > index 7a00b65740..c8c44b8326 100644
> > --- a/lib/vhost/version.map
> > +++ b/lib/vhost/version.map
> > @@ -94,6 +94,9 @@ EXPERIMENTAL {
> >  	rte_vhost_async_try_dequeue_burst;
> >  	rte_vhost_driver_get_vdpa_dev_type;
> >  	rte_vhost_clear_queue;
> > +
> > +	# added in 22.11
> > +	rte_vhost_vring_call_nonblock;
> >  };
> >
> >  INTERNAL {
> > diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c
> > index 8740aa2788..ed6efb003f 100644
> > --- a/lib/vhost/vhost.c
> > +++ b/lib/vhost/vhost.c
> > @@ -1317,6 +1317,36 @@ rte_vhost_vring_call(int vid, uint16_t vring_idx)
> >  	return 0;
> >  }
> >
> > +int
> > +rte_vhost_vring_call_nonblock(int vid, uint16_t vring_idx)
> > +{
> > +	struct virtio_net *dev;
> > +	struct vhost_virtqueue *vq;
> > +
> > +	dev = get_device(vid);
> > +	if (!dev)
> > +		return -1;
> > +
> > +	if (vring_idx >= VHOST_MAX_VRING)
> > +		return -1;
> > +
> > +	vq = dev->virtqueue[vring_idx];
> > +	if (!vq)
> > +		return -1;
> > +
> > +	if (!rte_spinlock_trylock(&vq->access_lock))
> > +		return -EAGAIN;
> > +
> > +	if (vq_is_packed(dev))
> > +		vhost_vring_call_packed(dev, vq);
> > +	else
> > +		vhost_vring_call_split(dev, vq);
> > +
> > +	rte_spinlock_unlock(&vq->access_lock);
> > +
> > +	return 0;
> > +}
> > +
> >  uint16_t
> >  rte_vhost_avail_entries(int vid, uint16_t queue_id)
> >  {
> > --
> > 2.21.3
diff mbox series

Patch

diff --git a/doc/guides/prog_guide/vhost_lib.rst b/doc/guides/prog_guide/vhost_lib.rst
index bad4d819e1..52f0589f51 100644
--- a/doc/guides/prog_guide/vhost_lib.rst
+++ b/doc/guides/prog_guide/vhost_lib.rst
@@ -297,6 +297,12 @@  The following is an overview of some key Vhost API functions:
   Clear in-flight packets which are submitted to async channel in vhost async data
   path. Completed packets are returned to applications through ``pkts``.
 
+* ``rte_vhost_vring_call_nonblock(int vid, uint16_t vring_idx)``
+
+  Notify the guest that used descriptors have been added to the vring. This function
+  will return -EAGAIN when vq's access lock is held by other thread, user should try
+  again later.
+
 * ``rte_vhost_vring_stats_get_names(int vid, uint16_t queue_id, struct rte_vhost_stat_name *names, unsigned int size)``
 
   This function returns the names of the queue statistics. It requires
diff --git a/doc/guides/rel_notes/release_22_11.rst b/doc/guides/rel_notes/release_22_11.rst
index 2da8bc9661..9b3783ffd8 100644
--- a/doc/guides/rel_notes/release_22_11.rst
+++ b/doc/guides/rel_notes/release_22_11.rst
@@ -236,6 +236,12 @@  New Features
 
      strings $dpdk_binary_or_driver | sed -n 's/^PMD_INFO_STRING= //p'
 
+* **Added non-block notify API to the vhost library.**
+
+  Added API to notify the guest that used descriptors have been added
+  to the vring in non-block way, user should check the return value of
+  this API.
+
 
 Removed Items
 -------------
diff --git a/lib/vhost/rte_vhost.h b/lib/vhost/rte_vhost.h
index bb7d86a432..d22b25cd4e 100644
--- a/lib/vhost/rte_vhost.h
+++ b/lib/vhost/rte_vhost.h
@@ -909,6 +909,21 @@  rte_vhost_clr_inflight_desc_packed(int vid, uint16_t vring_idx,
  */
 int rte_vhost_vring_call(int vid, uint16_t vring_idx);
 
+/**
+ * Notify the guest that used descriptors have been added to the vring.  This
+ * function acts as a memory barrier.  This function will return -EAGAIN when
+ * vq's access lock is held by other thread, user should try again later.
+ *
+ * @param vid
+ *  vhost device ID
+ * @param vring_idx
+ *  vring index
+ * @return
+ *  0 on success, -1 on failure, -EAGAIN for another retry
+ */
+__rte_experimental
+int rte_vhost_vring_call_nonblock(int vid, uint16_t vring_idx);
+
 /**
  * Get vhost RX queue avail count.
  *
diff --git a/lib/vhost/version.map b/lib/vhost/version.map
index 7a00b65740..c8c44b8326 100644
--- a/lib/vhost/version.map
+++ b/lib/vhost/version.map
@@ -94,6 +94,9 @@  EXPERIMENTAL {
 	rte_vhost_async_try_dequeue_burst;
 	rte_vhost_driver_get_vdpa_dev_type;
 	rte_vhost_clear_queue;
+
+	# added in 22.11
+	rte_vhost_vring_call_nonblock;
 };
 
 INTERNAL {
diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c
index 8740aa2788..ed6efb003f 100644
--- a/lib/vhost/vhost.c
+++ b/lib/vhost/vhost.c
@@ -1317,6 +1317,36 @@  rte_vhost_vring_call(int vid, uint16_t vring_idx)
 	return 0;
 }
 
+int
+rte_vhost_vring_call_nonblock(int vid, uint16_t vring_idx)
+{
+	struct virtio_net *dev;
+	struct vhost_virtqueue *vq;
+
+	dev = get_device(vid);
+	if (!dev)
+		return -1;
+
+	if (vring_idx >= VHOST_MAX_VRING)
+		return -1;
+
+	vq = dev->virtqueue[vring_idx];
+	if (!vq)
+		return -1;
+
+	if (!rte_spinlock_trylock(&vq->access_lock))
+		return -EAGAIN;
+
+	if (vq_is_packed(dev))
+		vhost_vring_call_packed(dev, vq);
+	else
+		vhost_vring_call_split(dev, vq);
+
+	rte_spinlock_unlock(&vq->access_lock);
+
+	return 0;
+}
+
 uint16_t
 rte_vhost_avail_entries(int vid, uint16_t queue_id)
 {