[v3] vhost: add new `rte_vhost_vring_call_nonblock` API
Checks
Commit Message
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
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
> -----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
@@ -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
@@ -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
-------------
@@ -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.
*
@@ -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 {
@@ -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)
{