diff mbox

[dpdk-dev] vhost: add API for getting last_idx of vrings

Message ID 20180305155959.21212-1-tomaszx.kulasek@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail apply issues

Commit Message

Tomasz Kulasek March 5, 2018, 3:59 p.m. UTC 
  vhost-net devices might keep track of last descriptors indices by
themselves, and assuming they initially start at 0, but that is not the
case for vhost-scsi. Initial last descriptor indices are set via
VHOST_USER_SET_VRING_BASE message, and we cannot possibly predict what
will they be. Setting these to vqueue->used->idx is also not an option,
because there might be some yet unprocessed requests between these and
the actual last_idx. This patch adds API for getting/setting last
descriptor indices of vrings, so that they can be synchronized between
user-device and rte_vhost.

The last_idx flow could be as following:

 * vhost start,
 * received SET_VRING_BASE msg, last_idx is set on rte_vhost side,
 * created user-device, last_idx pulled from rte_vhost,
 * requests are being processed by user-device, last_idx changes,
 * destroyed user-device, last_idx pushed to rte_vhost,
 * at this point, vrings could be recreated and another SET_VRING_BASE
   message could arrive, so last_idx would be set
 * recreated user-device, last_idx pulled from rte_vhost.


Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Signed-off-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>
---
 lib/librte_vhost/rte_vhost.h | 24 ++++++++++++++++++++++++
 lib/librte_vhost/vhost.c     | 27 +++++++++++++++++++++++++++
 2 files changed, 51 insertions(+)

Comments

Maxime Coquelin March 28, 2018, 8:57 a.m. UTC | #1 
Hi Tomasz,

On 03/05/2018 04:59 PM, Tomasz Kulasek wrote:
> vhost-net devices might keep track of last descriptors indices by
> themselves, and assuming they initially start at 0, but that is not the
> case for vhost-scsi. Initial last descriptor indices are set via
> VHOST_USER_SET_VRING_BASE message, and we cannot possibly predict what
> will they be. Setting these to vqueue->used->idx is also not an option,
> because there might be some yet unprocessed requests between these and
> the actual last_idx. This patch adds API for getting/setting last
> descriptor indices of vrings, so that they can be synchronized between
> user-device and rte_vhost.
> 
> The last_idx flow could be as following:
> 
>   * vhost start,
>   * received SET_VRING_BASE msg, last_idx is set on rte_vhost side,
>   * created user-device, last_idx pulled from rte_vhost,
>   * requests are being processed by user-device, last_idx changes,
>   * destroyed user-device, last_idx pushed to rte_vhost,
>   * at this point, vrings could be recreated and another SET_VRING_BASE
>     message could arrive, so last_idx would be set
>   * recreated user-device, last_idx pulled from rte_vhost.
> 
> 
> Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
> Signed-off-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>
> ---
>   lib/librte_vhost/rte_vhost.h | 24 ++++++++++++++++++++++++
>   lib/librte_vhost/vhost.c     | 27 +++++++++++++++++++++++++++
>   2 files changed, 51 insertions(+)
> 

I agree with the patch, but it is missing the declaration of the new API
in rte_vhost_version.map.

Thanks,
Maxime
Tomasz Kulasek March 28, 2018, 9:31 a.m. UTC | #2 
Hi Maxime,

> -----Original Message-----

> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]

> Sent: Wednesday, March 28, 2018 10:57

> To: Kulasek, TomaszX <tomaszx.kulasek@intel.com>; yliu@fridaylinux.org

> Cc: Verkamp, Daniel <daniel.verkamp@intel.com>; Harris, James R

> <james.r.harris@intel.com>; Wodkowski, PawelX <pawelx.wodkowski@intel.com>;

> dev@dpdk.org; Stojaczyk, DariuszX <dariuszx.stojaczyk@intel.com>

> Subject: Re: [dpdk-dev] [PATCH] vhost: add API for getting last_idx of vrings

> 

> Hi Tomasz,

> 

> On 03/05/2018 04:59 PM, Tomasz Kulasek wrote:

> > vhost-net devices might keep track of last descriptors indices by

> > themselves, and assuming they initially start at 0, but that is not the

> > case for vhost-scsi. Initial last descriptor indices are set via

> > VHOST_USER_SET_VRING_BASE message, and we cannot possibly predict what

> > will they be. Setting these to vqueue->used->idx is also not an option,

> > because there might be some yet unprocessed requests between these and

> > the actual last_idx. This patch adds API for getting/setting last

> > descriptor indices of vrings, so that they can be synchronized between

> > user-device and rte_vhost.

> >

> > The last_idx flow could be as following:

> >

> >   * vhost start,

> >   * received SET_VRING_BASE msg, last_idx is set on rte_vhost side,

> >   * created user-device, last_idx pulled from rte_vhost,

> >   * requests are being processed by user-device, last_idx changes,

> >   * destroyed user-device, last_idx pushed to rte_vhost,

> >   * at this point, vrings could be recreated and another SET_VRING_BASE

> >     message could arrive, so last_idx would be set

> >   * recreated user-device, last_idx pulled from rte_vhost.

> >

> >

> > Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>

> > Signed-off-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>

> > ---

> >   lib/librte_vhost/rte_vhost.h | 24 ++++++++++++++++++++++++

> >   lib/librte_vhost/vhost.c     | 27 +++++++++++++++++++++++++++

> >   2 files changed, 51 insertions(+)

> >

> 

> I agree with the patch, but it is missing the declaration of the new API

> in rte_vhost_version.map.

> 

> Thanks,

> Maxime


Yes, I will send v2.

Tomasz
Maxime Coquelin April 19, 2018, 2:57 p.m. UTC | #3 
Hi Tomasz,

On 03/28/2018 11:31 AM, Kulasek, TomaszX wrote:
> Hi Maxime,
> 
>> -----Original Message-----
>> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
>> Sent: Wednesday, March 28, 2018 10:57
>> To: Kulasek, TomaszX <tomaszx.kulasek@intel.com>; yliu@fridaylinux.org
>> Cc: Verkamp, Daniel <daniel.verkamp@intel.com>; Harris, James R
>> <james.r.harris@intel.com>; Wodkowski, PawelX <pawelx.wodkowski@intel.com>;
>> dev@dpdk.org; Stojaczyk, DariuszX <dariuszx.stojaczyk@intel.com>
>> Subject: Re: [dpdk-dev] [PATCH] vhost: add API for getting last_idx of vrings
>>
>> Hi Tomasz,
>>
>> On 03/05/2018 04:59 PM, Tomasz Kulasek wrote:
>>> vhost-net devices might keep track of last descriptors indices by
>>> themselves, and assuming they initially start at 0, but that is not the
>>> case for vhost-scsi. Initial last descriptor indices are set via
>>> VHOST_USER_SET_VRING_BASE message, and we cannot possibly predict what
>>> will they be. Setting these to vqueue->used->idx is also not an option,
>>> because there might be some yet unprocessed requests between these and
>>> the actual last_idx. This patch adds API for getting/setting last
>>> descriptor indices of vrings, so that they can be synchronized between
>>> user-device and rte_vhost.
>>>
>>> The last_idx flow could be as following:
>>>
>>>    * vhost start,
>>>    * received SET_VRING_BASE msg, last_idx is set on rte_vhost side,
>>>    * created user-device, last_idx pulled from rte_vhost,
>>>    * requests are being processed by user-device, last_idx changes,
>>>    * destroyed user-device, last_idx pushed to rte_vhost,
>>>    * at this point, vrings could be recreated and another SET_VRING_BASE
>>>      message could arrive, so last_idx would be set
>>>    * recreated user-device, last_idx pulled from rte_vhost.
>>>
>>>
>>> Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
>>> Signed-off-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>
>>> ---
>>>    lib/librte_vhost/rte_vhost.h | 24 ++++++++++++++++++++++++
>>>    lib/librte_vhost/vhost.c     | 27 +++++++++++++++++++++++++++
>>>    2 files changed, 51 insertions(+)
>>>
>>
>> I agree with the patch, but it is missing the declaration of the new API
>> in rte_vhost_version.map.
>>
>> Thanks,
>> Maxime
> 
> Yes, I will send v2.

Do you plan to send v2 for v18.02?
It can still make it to -rc2 if you post it early next week.

Thanks,
Maxime

> Tomasz
>
Thomas Monjalon July 26, 2018, 3:43 p.m. UTC | #4 
What is the status of this patch?

19/04/2018 16:57, Maxime Coquelin:
> Hi Tomasz,
> 
> On 03/28/2018 11:31 AM, Kulasek, TomaszX wrote:
> > Hi Maxime,
> > 
> >> -----Original Message-----
> >> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
> >> Sent: Wednesday, March 28, 2018 10:57
> >> To: Kulasek, TomaszX <tomaszx.kulasek@intel.com>; yliu@fridaylinux.org
> >> Cc: Verkamp, Daniel <daniel.verkamp@intel.com>; Harris, James R
> >> <james.r.harris@intel.com>; Wodkowski, PawelX <pawelx.wodkowski@intel.com>;
> >> dev@dpdk.org; Stojaczyk, DariuszX <dariuszx.stojaczyk@intel.com>
> >> Subject: Re: [dpdk-dev] [PATCH] vhost: add API for getting last_idx of vrings
> >>
> >> Hi Tomasz,
> >>
> >> On 03/05/2018 04:59 PM, Tomasz Kulasek wrote:
> >>> vhost-net devices might keep track of last descriptors indices by
> >>> themselves, and assuming they initially start at 0, but that is not the
> >>> case for vhost-scsi. Initial last descriptor indices are set via
> >>> VHOST_USER_SET_VRING_BASE message, and we cannot possibly predict what
> >>> will they be. Setting these to vqueue->used->idx is also not an option,
> >>> because there might be some yet unprocessed requests between these and
> >>> the actual last_idx. This patch adds API for getting/setting last
> >>> descriptor indices of vrings, so that they can be synchronized between
> >>> user-device and rte_vhost.
> >>>
> >>> The last_idx flow could be as following:
> >>>
> >>>    * vhost start,
> >>>    * received SET_VRING_BASE msg, last_idx is set on rte_vhost side,
> >>>    * created user-device, last_idx pulled from rte_vhost,
> >>>    * requests are being processed by user-device, last_idx changes,
> >>>    * destroyed user-device, last_idx pushed to rte_vhost,
> >>>    * at this point, vrings could be recreated and another SET_VRING_BASE
> >>>      message could arrive, so last_idx would be set
> >>>    * recreated user-device, last_idx pulled from rte_vhost.
> >>>
> >>>
> >>> Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
> >>> Signed-off-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>
> >>> ---
> >>>    lib/librte_vhost/rte_vhost.h | 24 ++++++++++++++++++++++++
> >>>    lib/librte_vhost/vhost.c     | 27 +++++++++++++++++++++++++++
> >>>    2 files changed, 51 insertions(+)
> >>>
> >>
> >> I agree with the patch, but it is missing the declaration of the new API
> >> in rte_vhost_version.map.
> >>
> >> Thanks,
> >> Maxime
> > 
> > Yes, I will send v2.
> 
> Do you plan to send v2 for v18.02?
> It can still make it to -rc2 if you post it early next week.
> 
> Thanks,
> Maxime
> 
> > Tomasz
> > 
>
Stojaczyk, DariuszX July 26, 2018, 7:11 p.m. UTC | #5 
It can be abandoned. I can see Zhihong added equivalent APIs called rte_vhost_get_vring_base/set_vring_base back in April.

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Thursday, July 26, 2018 5:44 PM
> To: Maxime Coquelin <maxime.coquelin@redhat.com>; Kulasek, TomaszX
> <tomaszx.kulasek@intel.com>
> Cc: dev@dpdk.org; yliu@fridaylinux.org; Verkamp, Daniel
> <daniel.verkamp@intel.com>; Harris, James R <james.r.harris@intel.com>;
> Wodkowski, PawelX <pawelx.wodkowski@intel.com>; Stojaczyk, DariuszX
> <dariuszx.stojaczyk@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] vhost: add API for getting last_idx of vrings
> 
> What is the status of this patch?
> 
> 19/04/2018 16:57, Maxime Coquelin:
> > Hi Tomasz,
> >
> > On 03/28/2018 11:31 AM, Kulasek, TomaszX wrote:
> > > Hi Maxime,
> > >
> > >> -----Original Message-----
> > >> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
> > >> Sent: Wednesday, March 28, 2018 10:57
> > >> To: Kulasek, TomaszX <tomaszx.kulasek@intel.com>; yliu@fridaylinux.org
> > >> Cc: Verkamp, Daniel <daniel.verkamp@intel.com>; Harris, James R
> > >> <james.r.harris@intel.com>; Wodkowski, PawelX
> <pawelx.wodkowski@intel.com>;
> > >> dev@dpdk.org; Stojaczyk, DariuszX <dariuszx.stojaczyk@intel.com>
> > >> Subject: Re: [dpdk-dev] [PATCH] vhost: add API for getting last_idx of vrings
> > >>
> > >> Hi Tomasz,
> > >>
> > >> On 03/05/2018 04:59 PM, Tomasz Kulasek wrote:
> > >>> vhost-net devices might keep track of last descriptors indices by
> > >>> themselves, and assuming they initially start at 0, but that is not the
> > >>> case for vhost-scsi. Initial last descriptor indices are set via
> > >>> VHOST_USER_SET_VRING_BASE message, and we cannot possibly predict
> what
> > >>> will they be. Setting these to vqueue->used->idx is also not an option,
> > >>> because there might be some yet unprocessed requests between these and
> > >>> the actual last_idx. This patch adds API for getting/setting last
> > >>> descriptor indices of vrings, so that they can be synchronized between
> > >>> user-device and rte_vhost.
> > >>>
> > >>> The last_idx flow could be as following:
> > >>>
> > >>>    * vhost start,
> > >>>    * received SET_VRING_BASE msg, last_idx is set on rte_vhost side,
> > >>>    * created user-device, last_idx pulled from rte_vhost,
> > >>>    * requests are being processed by user-device, last_idx changes,
> > >>>    * destroyed user-device, last_idx pushed to rte_vhost,
> > >>>    * at this point, vrings could be recreated and another SET_VRING_BASE
> > >>>      message could arrive, so last_idx would be set
> > >>>    * recreated user-device, last_idx pulled from rte_vhost.
> > >>>
> > >>>
> > >>> Signed-off-by: Dariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
> > >>> Signed-off-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>
> > >>> ---
> > >>>    lib/librte_vhost/rte_vhost.h | 24 ++++++++++++++++++++++++
> > >>>    lib/librte_vhost/vhost.c     | 27 +++++++++++++++++++++++++++
> > >>>    2 files changed, 51 insertions(+)
> > >>>
> > >>
> > >> I agree with the patch, but it is missing the declaration of the new API
> > >> in rte_vhost_version.map.
> > >>
> > >> Thanks,
> > >> Maxime
> > >
> > > Yes, I will send v2.
> >
> > Do you plan to send v2 for v18.02?
> > It can still make it to -rc2 if you post it early next week.
> >
> > Thanks,
> > Maxime
> >
> > > Tomasz
> > >
> >
> 
> 
> 
>
Thomas Monjalon July 26, 2018, 7:15 p.m. UTC | #6 
26/07/2018 21:11, Stojaczyk, DariuszX:
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > 
> > What is the status of this patch?
> 
> It can be abandoned. I can see Zhihong added equivalent APIs called rte_vhost_get_vring_base/set_vring_base back in April.

OK, thank you
diff mbox

Patch

diff --git a/lib/librte_vhost/rte_vhost.h b/lib/librte_vhost/rte_vhost.h
index d33206997..b9ba058d1 100644
--- a/lib/librte_vhost/rte_vhost.h
+++ b/lib/librte_vhost/rte_vhost.h
@@ -62,6 +62,9 @@  struct rte_vhost_vring {
 
 	int			kickfd;
 	uint16_t		size;
+
+	uint16_t		last_avail_idx;
+	uint16_t		last_used_idx;
 };
 
 /**
@@ -434,6 +437,27 @@  int rte_vhost_vring_call(int vid, uint16_t vring_idx);
  */
 uint32_t rte_vhost_rx_queue_count(int vid, uint16_t qid);
 
+/**
+ * Set id of the last descriptors in avail and used guest vrings.
+ *
+ * In case user application operates directly on buffers, it should use this
+ * function on device destruction to retrieve the same values later on in device
+ * creation via rte_vhost_get_vhost_vring(int, uint16_t, struct rte_vhost_vring *)
+ *
+ * @param vid
+ *  vhost device ID
+ * @param vring_idx
+ *  vring index
+ * @param last_avail_idx
+ *  id of the last descriptor in avail ring to be set
+ * @param last_used_idx
+ *  id of the last descriptor in used ring to be set
+ * @return
+ *  0 on success, -1 on failure
+ */
+int rte_vhost_set_vhost_vring_last_idx(int vid, uint16_t vring_idx,
+			      uint16_t last_avail_idx, uint16_t last_used_idx);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c
index a407067e2..a82dc5a62 100644
--- a/lib/librte_vhost/vhost.c
+++ b/lib/librte_vhost/vhost.c
@@ -512,6 +512,9 @@  rte_vhost_get_vhost_vring(int vid, uint16_t vring_idx,
 	vring->kickfd  = vq->kickfd;
 	vring->size    = vq->size;
 
+	vring->last_avail_idx = vq->last_avail_idx;
+	vring->last_used_idx = vq->last_used_idx;
+
 	return 0;
 }
 
@@ -627,3 +630,27 @@  rte_vhost_rx_queue_count(int vid, uint16_t qid)
 
 	return *((volatile uint16_t *)&vq->avail->idx) - vq->last_avail_idx;
 }
+
+int
+rte_vhost_set_vhost_vring_last_idx(int vid, uint16_t vring_idx,
+			      uint16_t last_avail_idx, uint16_t last_used_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;
+
+	vq->last_avail_idx = last_avail_idx;
+	vq->last_used_idx = last_used_idx;
+
+	return 0;
+}