[dpdk-dev] [PATCH v2 0/2] vhost: Add API to get negotiated protocol features

[dpdk-dev] [PATCH v2 0/2] vhost: Add API to get negotiated protocol features

[Keiichi Watanabe]

This patchset adds an API to get negotiated vhost protocol features in
librte_vhost so we can use it to check if an optional feature is enabled.

v1: https://mails.dpdk.org/archives/dev/2021-February/199044.html

Keiichi Watanabe (2):
  vhost: Add API to get negotiated protocol features
  examples/vhost_blk: Check protocol features before getting inflight
    info

 examples/vhost_blk/vhost_blk.c | 23 ++++++++++++++++++-----
 lib/librte_vhost/rte_vhost.h   | 15 +++++++++++++++
 lib/librte_vhost/version.map   |  1 +
 lib/librte_vhost/vhost.c       | 14 ++++++++++++++
 4 files changed, 48 insertions(+), 5 deletions(-)

--
2.31.0.rc2.261.g7f71774620-goog

[dpdk-dev] [PATCH v2 1/2] vhost: Add API to get negotiated protocol features

[Keiichi Watanabe]

Add rte_vhost_get_negotiated_protocol_features, which returns a set of
enabled protocol features.

Signed-off-by: Keiichi Watanabe <keiichiw@chromium.org>
---
 lib/librte_vhost/rte_vhost.h | 15 +++++++++++++++
 lib/librte_vhost/version.map |  1 +
 lib/librte_vhost/vhost.c     | 14 ++++++++++++++
 3 files changed, 30 insertions(+)

diff --git a/lib/librte_vhost/rte_vhost.h b/lib/librte_vhost/rte_vhost.h
index 010f16086..d0a8ae31f 100644
--- a/lib/librte_vhost/rte_vhost.h
+++ b/lib/librte_vhost/rte_vhost.h
@@ -567,6 +567,21 @@ rte_vhost_driver_get_queue_num(const char *path, uint32_t *queue_num);
  */
 int rte_vhost_get_negotiated_features(int vid, uint64_t *features);
 
+/**
+ * Get the protocol feature bits after negotiation
+ *
+ * @param vid
+ *  Vhost device ID
+ * @param protocol_features
+ *  A pointer to store the queried protocol feature bits
+ * @return
+ *  0 on success, -1 on failure
+ */
+__rte_experimental
+int
+rte_vhost_get_negotiated_protocol_features(int vid,
+					   uint64_t *protocol_features);
+
 /* Register callbacks. */
 int rte_vhost_driver_callback_register(const char *path,
 	struct vhost_device_ops const * const ops);
diff --git a/lib/librte_vhost/version.map b/lib/librte_vhost/version.map
index 9183d6f2f..95c4c0990 100644
--- a/lib/librte_vhost/version.map
+++ b/lib/librte_vhost/version.map
@@ -63,6 +63,7 @@ EXPERIMENTAL {
 	rte_vhost_va_from_guest_pa;
 	rte_vhost_extern_callback_register;
 	rte_vhost_driver_set_protocol_features;
+	rte_vhost_get_negotiated_protocol_features;
 	rte_vhost_set_inflight_desc_split;
 	rte_vhost_set_inflight_desc_packed;
 	rte_vhost_set_last_inflight_io_split;
diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c
index b83cf639e..c43248e4b 100644
--- a/lib/librte_vhost/vhost.c
+++ b/lib/librte_vhost/vhost.c
@@ -868,6 +868,20 @@ rte_vhost_get_negotiated_features(int vid, uint64_t *features)
 	return 0;
 }
 
+int
+rte_vhost_get_negotiated_protocol_features(int vid,
+					   uint64_t *protocol_features)
+{
+	struct virtio_net *dev;
+
+	dev = get_device(vid);
+	if (dev == NULL || protocol_features == NULL)
+		return -1;
+
+	*protocol_features = dev->protocol_features;
+	return 0;
+}
+
 int
 rte_vhost_get_mem_table(int vid, struct rte_vhost_memory **mem)
 {
-- 
2.31.0.rc2.261.g7f71774620-goog

[dpdk-dev] [PATCH v2 2/2] examples/vhost_blk: Check protocol features before getting inflight info

[Keiichi Watanabe]

Avoid calling rte_vhost_get_vhost_ring_inflight() and
rte_vhost_get_vring_base_from_inflight() when
VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD is not set.

Signed-off-by: Keiichi Watanabe <keiichiw@chromium.org>
---
 examples/vhost_blk/vhost_blk.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/examples/vhost_blk/vhost_blk.c b/examples/vhost_blk/vhost_blk.c
index bb293d492..dd05881b4 100644
--- a/examples/vhost_blk/vhost_blk.c
+++ b/examples/vhost_blk/vhost_blk.c
@@ -603,10 +603,10 @@ new_device(int vid)
 	struct vhost_blk_ctrlr *ctrlr;
 	struct vhost_blk_queue *vq;
 	char path[PATH_MAX];
-	uint64_t features;
+	uint64_t features, protocol_features;
 	pthread_t tid;
 	int i, ret;
-	bool packed_ring;
+	bool packed_ring, inflight_shmfd;
 
 	ret = rte_vhost_get_ifname(vid, path, PATH_MAX);
 	if (ret) {
@@ -631,6 +631,16 @@ new_device(int vid)
 	}
 	packed_ring = !!(features & (1ULL << VIRTIO_F_RING_PACKED));
 
+	ret = rte_vhost_get_negotiated_protocol_features(
+		vid, &protocol_features);
+	if (ret) {
+		fprintf(stderr,
+			"Failed to get the negotiated protocol features\n");
+		return -1;
+	}
+	inflight_shmfd = !!(features &
+			    (1ULL << VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD));
+
 	/* Disable Notifications and init last idx */
 	for (i = 0; i < NUM_OF_BLK_QUEUES; i++) {
 		vq = &ctrlr->queues[i];
@@ -641,10 +651,13 @@ new_device(int vid)
 		assert(rte_vhost_get_vring_base(ctrlr->vid, i,
 					       &vq->last_avail_idx,
 					       &vq->last_used_idx) == 0);
-		assert(rte_vhost_get_vhost_ring_inflight(ctrlr->vid, i,
-						&vq->inflight_ring) == 0);
 
-		if (packed_ring) {
+		if (inflight_shmfd)
+			assert(rte_vhost_get_vhost_ring_inflight(
+				       ctrlr->vid, i,
+				       &vq->inflight_ring) == 0);
+
+		if (packed_ring && inflight_shmfd) {
 			/* for the reconnection */
 			assert(rte_vhost_get_vring_base_from_inflight(
 				ctrlr->vid, i,
-- 
2.31.0.rc2.261.g7f71774620-goog

Re: [dpdk-dev] [PATCH v2 1/2] vhost: Add API to get negotiated protocol features

[Maxime Coquelin]

On 3/22/21 8:22 AM, Keiichi Watanabe wrote:
> Add rte_vhost_get_negotiated_protocol_features, which returns a set of
> enabled protocol features.
> 
> Signed-off-by: Keiichi Watanabe <keiichiw@chromium.org>
> ---
>  lib/librte_vhost/rte_vhost.h | 15 +++++++++++++++
>  lib/librte_vhost/version.map |  1 +
>  lib/librte_vhost/vhost.c     | 14 ++++++++++++++
>  3 files changed, 30 insertions(+)
> 


Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime

Re: [dpdk-dev] [PATCH v2 2/2] examples/vhost_blk: Check protocol features before getting inflight info

[Maxime Coquelin]

On 3/22/21 8:22 AM, Keiichi Watanabe wrote:
> Avoid calling rte_vhost_get_vhost_ring_inflight() and
> rte_vhost_get_vring_base_from_inflight() when
> VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD is not set.
> 
> Signed-off-by: Keiichi Watanabe <keiichiw@chromium.org>
> ---
>  examples/vhost_blk/vhost_blk.c | 23 ++++++++++++++++++-----
>  1 file changed, 18 insertions(+), 5 deletions(-)
> 

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime

Re: [dpdk-dev] [PATCH v2 0/2] vhost: Add API to get negotiated protocol features

[Xia, Chenbo]

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Keiichi Watanabe
> Sent: Monday, March 22, 2021 3:23 PM
> To: dev@dpdk.org
> Cc: dgreid@chromium.org; chirantan@chromium.org; Keiichi Watanabe
> <keiichiw@chromium.org>
> Subject: [dpdk-dev] [PATCH v2 0/2] vhost: Add API to get negotiated protocol
> features
> 
> This patchset adds an API to get negotiated vhost protocol features in
> librte_vhost so we can use it to check if an optional feature is enabled.
> 
> v1: https://mails.dpdk.org/archives/dev/2021-February/199044.html
> 
> Keiichi Watanabe (2):
>   vhost: Add API to get negotiated protocol features
>   examples/vhost_blk: Check protocol features before getting inflight
>     info
> 
>  examples/vhost_blk/vhost_blk.c | 23 ++++++++++++++++++-----
>  lib/librte_vhost/rte_vhost.h   | 15 +++++++++++++++
>  lib/librte_vhost/version.map   |  1 +
>  lib/librte_vhost/vhost.c       | 14 ++++++++++++++
>  4 files changed, 48 insertions(+), 5 deletions(-)
> 
> --
> 2.31.0.rc2.261.g7f71774620-goog

Fix the headlines due to check-git-log errors:

vhost: add API to get negotiated protocol features

examples/vhost_blk: check features before inflight API

With above fixed, series applied to next-virtio/main.

Thanks!

Re: [dpdk-dev] [PATCH v2 1/2] vhost: Add API to get negotiated protocol features

[Ferruh Yigit]

On 3/22/2021 7:22 AM, Keiichi Watanabe wrote:
> Add rte_vhost_get_negotiated_protocol_features, which returns a set of
> enabled protocol features.
> 
> Signed-off-by: Keiichi Watanabe <keiichiw@chromium.org>

<...>

> diff --git a/lib/librte_vhost/version.map b/lib/librte_vhost/version.map
> index 9183d6f2f..95c4c0990 100644
> --- a/lib/librte_vhost/version.map
> +++ b/lib/librte_vhost/version.map
> @@ -63,6 +63,7 @@ EXPERIMENTAL {
>   	rte_vhost_va_from_guest_pa;
>   	rte_vhost_extern_callback_register;
>   	rte_vhost_driver_set_protocol_features;
> +	rte_vhost_get_negotiated_protocol_features;
>   	rte_vhost_set_inflight_desc_split;
>   	rte_vhost_set_inflight_desc_packed;
>   	rte_vhost_set_last_inflight_io_split;

Added the release version that experimental API is added as comment in next-net, 
like following:

  +++ b/lib/librte_vhost/version.map
  @@ -76,4 +76,7 @@ EXPERIMENTAL {
          rte_vhost_async_channel_unregister;
          rte_vhost_submit_enqueue_burst;
          rte_vhost_poll_enqueue_completed;
  +
  +       # added in 21.05
  +       rte_vhost_get_negotiated_protocol_features;
   };


We are doing this already in many libraries [1], this helps us easily observe 
how long an API is staying as experimental in a library.

[1]: https://git.dpdk.org/dpdk/tree/lib/librte_eal/version.map?h=v21.02#n407

Re: [dpdk-dev] [PATCH v2 1/2] vhost: Add API to get negotiated protocol features

[Kinsella, Ray]

On 31/03/2021 17:45, Ferruh Yigit wrote:
> On 3/22/2021 7:22 AM, Keiichi Watanabe wrote:
>> Add rte_vhost_get_negotiated_protocol_features, which returns a set of
>> enabled protocol features.
>>
>> Signed-off-by: Keiichi Watanabe <keiichiw@chromium.org>
> 
> <...>
> 
>> diff --git a/lib/librte_vhost/version.map b/lib/librte_vhost/version.map
>> index 9183d6f2f..95c4c0990 100644
>> --- a/lib/librte_vhost/version.map
>> +++ b/lib/librte_vhost/version.map
>> @@ -63,6 +63,7 @@ EXPERIMENTAL {
>>       rte_vhost_va_from_guest_pa;
>>       rte_vhost_extern_callback_register;
>>       rte_vhost_driver_set_protocol_features;
>> +    rte_vhost_get_negotiated_protocol_features;
>>       rte_vhost_set_inflight_desc_split;
>>       rte_vhost_set_inflight_desc_packed;
>>       rte_vhost_set_last_inflight_io_split;
> 
> Added the release version that experimental API is added as comment in next-net, like following:
> 
> +++ b/lib/librte_vhost/version.map
> @@ -76,4 +76,7 @@ EXPERIMENTAL {
>         rte_vhost_async_channel_unregister;
>         rte_vhost_submit_enqueue_burst;
>         rte_vhost_poll_enqueue_completed;
> +
> +       # added in 21.05
> +       rte_vhost_get_negotiated_protocol_features;
>  };
> 
> 
> We are doing this already in many libraries [1], this helps us easily observe how long an API is staying as experimental in a library.

Good idea.
 
> [1]: https://git.dpdk.org/dpdk/tree/lib/librte_eal/version.map?h=v21.02#n407

Re: [dpdk-dev] [PATCH v2 1/2] vhost: Add API to get negotiated protocol features

[Keiichi Watanabe]

Thanks for the review!
Should I send the updated version of the patch series? Or, can I ask you to
make the follow up changes as a maintainer?

Keiichi

On Thu, Apr 1, 2021 at 5:42 PM Kinsella, Ray <mdr@ashroe.eu> wrote:

>
>
> On 31/03/2021 17:45, Ferruh Yigit wrote:
> > On 3/22/2021 7:22 AM, Keiichi Watanabe wrote:
> >> Add rte_vhost_get_negotiated_protocol_features, which returns a set of
> >> enabled protocol features.
> >>
> >> Signed-off-by: Keiichi Watanabe <keiichiw@chromium.org>
> >
> > <...>
> >
> >> diff --git a/lib/librte_vhost/version.map b/lib/librte_vhost/version.map
> >> index 9183d6f2f..95c4c0990 100644
> >> --- a/lib/librte_vhost/version.map
> >> +++ b/lib/librte_vhost/version.map
> >> @@ -63,6 +63,7 @@ EXPERIMENTAL {
> >>       rte_vhost_va_from_guest_pa;
> >>       rte_vhost_extern_callback_register;
> >>       rte_vhost_driver_set_protocol_features;
> >> +    rte_vhost_get_negotiated_protocol_features;
> >>       rte_vhost_set_inflight_desc_split;
> >>       rte_vhost_set_inflight_desc_packed;
> >>       rte_vhost_set_last_inflight_io_split;
> >
> > Added the release version that experimental API is added as comment in
> next-net, like following:
> >
> > +++ b/lib/librte_vhost/version.map
> > @@ -76,4 +76,7 @@ EXPERIMENTAL {
> >         rte_vhost_async_channel_unregister;
> >         rte_vhost_submit_enqueue_burst;
> >         rte_vhost_poll_enqueue_completed;
> > +
> > +       # added in 21.05
> > +       rte_vhost_get_negotiated_protocol_features;
> >  };
> >
> >
> > We are doing this already in many libraries [1], this helps us easily
> observe how long an API is staying as experimental in a library.
>
> Good idea.
>
> > [1]:
> https://git.dpdk.org/dpdk/tree/lib/librte_eal/version.map?h=v21.02#n407
>

Re: [dpdk-dev] [PATCH v2 1/2] vhost: Add API to get negotiated protocol features

[Ferruh Yigit]

On 4/1/2021 5:05 PM, Keiichi Watanabe wrote:
> Thanks for the review!
> Should I send the updated version of the patch series? Or, can I ask you to make 
> the follow up changes as a maintainer?
> 

No new version is required, already updated in the next-net [1] repo.

[1] https://git.dpdk.org/next/dpdk-next-net/

> Keiichi
> 
> On Thu, Apr 1, 2021 at 5:42 PM Kinsella, Ray <mdr@ashroe.eu 
> <mailto:mdr@ashroe.eu>> wrote:
> 
> 
> 
>     On 31/03/2021 17:45, Ferruh Yigit wrote:
>      > On 3/22/2021 7:22 AM, Keiichi Watanabe wrote:
>      >> Add rte_vhost_get_negotiated_protocol_features, which returns a set of
>      >> enabled protocol features.
>      >>
>      >> Signed-off-by: Keiichi Watanabe <keiichiw@chromium.org
>     <mailto:keiichiw@chromium.org>>
>      >
>      > <...>
>      >
>      >> diff --git a/lib/librte_vhost/version.map b/lib/librte_vhost/version.map
>      >> index 9183d6f2f..95c4c0990 100644
>      >> --- a/lib/librte_vhost/version.map
>      >> +++ b/lib/librte_vhost/version.map
>      >> @@ -63,6 +63,7 @@ EXPERIMENTAL {
>      >>       rte_vhost_va_from_guest_pa;
>      >>       rte_vhost_extern_callback_register;
>      >>       rte_vhost_driver_set_protocol_features;
>      >> +    rte_vhost_get_negotiated_protocol_features;
>      >>       rte_vhost_set_inflight_desc_split;
>      >>       rte_vhost_set_inflight_desc_packed;
>      >>       rte_vhost_set_last_inflight_io_split;
>      >
>      > Added the release version that experimental API is added as comment in
>     next-net, like following:
>      >
>      > +++ b/lib/librte_vhost/version.map
>      > @@ -76,4 +76,7 @@ EXPERIMENTAL {
>      >         rte_vhost_async_channel_unregister;
>      >         rte_vhost_submit_enqueue_burst;
>      >         rte_vhost_poll_enqueue_completed;
>      > +
>      > +       # added in 21.05
>      > +       rte_vhost_get_negotiated_protocol_features;
>      >  };
>      >
>      >
>      > We are doing this already in many libraries [1], this helps us easily
>     observe how long an API is staying as experimental in a library.
> 
>     Good idea.
> 
>      > [1]:
>     https://git.dpdk.org/dpdk/tree/lib/librte_eal/version.map?h=v21.02#n407
>     <https://git.dpdk.org/dpdk/tree/lib/librte_eal/version.map?h=v21.02#n407>
> 

Re: [dpdk-dev] [PATCH v2 1/2] vhost: Add API to get negotiated protocol features

[Keiichi Watanabe]

On Fri, Apr 2, 2021 at 1:28 AM Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> On 4/1/2021 5:05 PM, Keiichi Watanabe wrote:
> > Thanks for the review!
> > Should I send the updated version of the patch series? Or, can I ask you
> to make
> > the follow up changes as a maintainer?
> >
>
> No new version is required, already updated in the next-net [1] repo.
>

Oh, you already added the version number there. Cool!
Thank you!

Keiichi



>
> [1] https://git.dpdk.org/next/dpdk-next-net/
>
> > Keiichi
> >
> > On Thu, Apr 1, 2021 at 5:42 PM Kinsella, Ray <mdr@ashroe.eu
> > <mailto:mdr@ashroe.eu>> wrote:
> >
> >
> >
> >     On 31/03/2021 17:45, Ferruh Yigit wrote:
> >      > On 3/22/2021 7:22 AM, Keiichi Watanabe wrote:
> >      >> Add rte_vhost_get_negotiated_protocol_features, which returns a
> set of
> >      >> enabled protocol features.
> >      >>
> >      >> Signed-off-by: Keiichi Watanabe <keiichiw@chromium.org
> >     <mailto:keiichiw@chromium.org>>
> >      >
> >      > <...>
> >      >
> >      >> diff --git a/lib/librte_vhost/version.map
> b/lib/librte_vhost/version.map
> >      >> index 9183d6f2f..95c4c0990 100644
> >      >> --- a/lib/librte_vhost/version.map
> >      >> +++ b/lib/librte_vhost/version.map
> >      >> @@ -63,6 +63,7 @@ EXPERIMENTAL {
> >      >>       rte_vhost_va_from_guest_pa;
> >      >>       rte_vhost_extern_callback_register;
> >      >>       rte_vhost_driver_set_protocol_features;
> >      >> +    rte_vhost_get_negotiated_protocol_features;
> >      >>       rte_vhost_set_inflight_desc_split;
> >      >>       rte_vhost_set_inflight_desc_packed;
> >      >>       rte_vhost_set_last_inflight_io_split;
> >      >
> >      > Added the release version that experimental API is added as
> comment in
> >     next-net, like following:
> >      >
> >      > +++ b/lib/librte_vhost/version.map
> >      > @@ -76,4 +76,7 @@ EXPERIMENTAL {
> >      >         rte_vhost_async_channel_unregister;
> >      >         rte_vhost_submit_enqueue_burst;
> >      >         rte_vhost_poll_enqueue_completed;
> >      > +
> >      > +       # added in 21.05
> >      > +       rte_vhost_get_negotiated_protocol_features;
> >      >  };
> >      >
> >      >
> >      > We are doing this already in many libraries [1], this helps us
> easily
> >     observe how long an API is staying as experimental in a library.
> >
> >     Good idea.
> >
> >      > [1]:
> >
> https://git.dpdk.org/dpdk/tree/lib/librte_eal/version.map?h=v21.02#n407
> >     <
> https://git.dpdk.org/dpdk/tree/lib/librte_eal/version.map?h=v21.02#n407>
> >
>
>