diff mbox series

vhost: promote some APIs to stable

Message ID 20210907025800.55680-1-chenbo.xia@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Maxime Coquelin
Headers show
Series vhost: promote some APIs to stable | expand

Checks

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

Commit Message

Xia, Chenbo Sept. 7, 2021, 2:58 a.m. UTC
As reported by symbol bot, APIs listed in this patch have been
experimental for more than two years. This patch promotes these
18 APIs to stable.

Signed-off-by: Chenbo Xia <chenbo.xia@intel.com>
---
 lib/vhost/rte_vhost.h        | 13 -------------
 lib/vhost/rte_vhost_crypto.h |  5 -----
 lib/vhost/version.map        | 36 ++++++++++++++++++------------------
 3 files changed, 18 insertions(+), 36 deletions(-)

Comments

Kinsella, Ray Sept. 8, 2021, 8:56 a.m. UTC | #1
On 07/09/2021 03:58, Chenbo Xia wrote:
> As reported by symbol bot, APIs listed in this patch have been
> experimental for more than two years. This patch promotes these
> 18 APIs to stable.
> 
> Signed-off-by: Chenbo Xia <chenbo.xia@intel.com>
> ---
>  lib/vhost/rte_vhost.h        | 13 -------------
>  lib/vhost/rte_vhost_crypto.h |  5 -----
>  lib/vhost/version.map        | 36 ++++++++++++++++++------------------
>  3 files changed, 18 insertions(+), 36 deletions(-)
> 

Acked-by: Ray Kinsella <mdr@ashroe.eu>
Kevin Traynor Sept. 8, 2021, 12:01 p.m. UTC | #2
On 07/09/2021 03:58, Chenbo Xia wrote:
> As reported by symbol bot, APIs listed in this patch have been
> experimental for more than two years. This patch promotes these
> 18 APIs to stable.
> 

Patch lgtm. One question about a possible follow on below.

Acked-by: Kevin Traynor <ktraynor@redhat.com>

> Signed-off-by: Chenbo Xia <chenbo.xia@intel.com>
> ---
>  lib/vhost/rte_vhost.h        | 13 -------------
>  lib/vhost/rte_vhost_crypto.h |  5 -----
>  lib/vhost/version.map        | 36 ++++++++++++++++++------------------
>  3 files changed, 18 insertions(+), 36 deletions(-)
> 
> diff --git a/lib/vhost/rte_vhost.h b/lib/vhost/rte_vhost.h
> index 8d875e9322..fd372d5259 100644
> --- a/lib/vhost/rte_vhost.h
> +++ b/lib/vhost/rte_vhost.h
> @@ -342,7 +342,6 @@ rte_vhost_gpa_to_vva(struct rte_vhost_memory *mem, uint64_t gpa)
>   * @return
>   *  the host virtual address on success, 0 on failure
>   */
> -__rte_experimental
>  static __rte_always_inline uint64_t
>  rte_vhost_va_from_guest_pa(struct rte_vhost_memory *mem,
>  						   uint64_t gpa, uint64_t *len)
> @@ -522,7 +521,6 @@ int rte_vhost_driver_get_features(const char *path, uint64_t *features);
>   * @return
>   *  0 on success, -1 on failure
>   */
> -__rte_experimental
>  int
>  rte_vhost_driver_set_protocol_features(const char *path,
>  		uint64_t protocol_features);
> @@ -537,7 +535,6 @@ rte_vhost_driver_set_protocol_features(const char *path,
>   * @return
>   *  0 on success, -1 on failure
>   */
> -__rte_experimental
>  int
>  rte_vhost_driver_get_protocol_features(const char *path,
>  		uint64_t *protocol_features);
> @@ -552,7 +549,6 @@ rte_vhost_driver_get_protocol_features(const char *path,
>   * @return
>   *  0 on success, -1 on failure
>   */
> -__rte_experimental
>  int
>  rte_vhost_driver_get_queue_num(const char *path, uint32_t *queue_num);
>  
> @@ -768,7 +764,6 @@ int rte_vhost_get_vhost_vring(int vid, uint16_t vring_idx,
>   * @return
>   *  0 on success, -1 on failure
>   */
> -__rte_experimental
>  int
>  rte_vhost_get_vhost_ring_inflight(int vid, uint16_t vring_idx,
>  	struct rte_vhost_ring_inflight *vring);
> @@ -788,7 +783,6 @@ rte_vhost_get_vhost_ring_inflight(int vid, uint16_t vring_idx,
>   * @return
>   *  0 on success, -1 on failure
>   */
> -__rte_experimental
>  int
>  rte_vhost_set_inflight_desc_split(int vid, uint16_t vring_idx,
>  	uint16_t idx);
> @@ -811,7 +805,6 @@ rte_vhost_set_inflight_desc_split(int vid, uint16_t vring_idx,
>   * @return
>   *  0 on success, -1 on failure
>   */
> -__rte_experimental
>  int
>  rte_vhost_set_inflight_desc_packed(int vid, uint16_t vring_idx,
>  	uint16_t head, uint16_t last, uint16_t *inflight_entry);
> @@ -828,7 +821,6 @@ rte_vhost_set_inflight_desc_packed(int vid, uint16_t vring_idx,
>   * @return
>   *  0 on success, -1 on failure
>   */
> -__rte_experimental
>  int
>  rte_vhost_set_last_inflight_io_split(int vid,
>  	uint16_t vring_idx, uint16_t idx);
> @@ -848,7 +840,6 @@ rte_vhost_set_last_inflight_io_split(int vid,
>   * @return
>   *  0 on success, -1 on failure
>   */
> -__rte_experimental
>  int
>  rte_vhost_set_last_inflight_io_packed(int vid,
>  	uint16_t vring_idx, uint16_t head);
> @@ -867,7 +858,6 @@ rte_vhost_set_last_inflight_io_packed(int vid,
>   * @return
>   *  0 on success, -1 on failure
>   */
> -__rte_experimental
>  int
>  rte_vhost_clr_inflight_desc_split(int vid, uint16_t vring_idx,
>  	uint16_t last_used_idx, uint16_t idx);
> @@ -884,7 +874,6 @@ rte_vhost_clr_inflight_desc_split(int vid, uint16_t vring_idx,
>   * @return
>   *  0 on success, -1 on failure
>   */
> -__rte_experimental
>  int
>  rte_vhost_clr_inflight_desc_packed(int vid, uint16_t vring_idx,
>  	uint16_t head);
> @@ -965,7 +954,6 @@ rte_vhost_get_vring_base(int vid, uint16_t queue_id,
>   * @return
>   *  0 on success, -1 on failure
>   */
> -__rte_experimental
>  int
>  rte_vhost_get_vring_base_from_inflight(int vid,
>  	uint16_t queue_id, uint16_t *last_avail_idx, uint16_t *last_used_idx);
> @@ -1000,7 +988,6 @@ rte_vhost_set_vring_base(int vid, uint16_t queue_id,
>   * @return
>   *  0 on success, -1 on failure
>   */
> -__rte_experimental
>  int
>  rte_vhost_extern_callback_register(int vid,
>  		struct rte_vhost_user_extern_ops const * const ops, void *ctx);
> diff --git a/lib/vhost/rte_vhost_crypto.h b/lib/vhost/rte_vhost_crypto.h
> index 8531757285..f54d731139 100644
> --- a/lib/vhost/rte_vhost_crypto.h
> +++ b/lib/vhost/rte_vhost_crypto.h
> @@ -58,7 +58,6 @@ rte_vhost_crypto_driver_start(const char *path);
>   *  0 if the Vhost Crypto Instance is created successfully.
>   *  Negative integer if otherwise
>   */
> -__rte_experimental
>  int
>  rte_vhost_crypto_create(int vid, uint8_t cryptodev_id,
>  		struct rte_mempool *sess_pool,
> @@ -74,7 +73,6 @@ rte_vhost_crypto_create(int vid, uint8_t cryptodev_id,
>   *  0 if the Vhost Crypto Instance is created successfully.
>   *  Negative integer if otherwise.
>   */
> -__rte_experimental
>  int
>  rte_vhost_crypto_free(int vid);
>  
> @@ -89,7 +87,6 @@ rte_vhost_crypto_free(int vid);
>   *  0 if completed successfully.
>   *  Negative integer if otherwise.
>   */
> -__rte_experimental
>  int
>  rte_vhost_crypto_set_zero_copy(int vid, enum rte_vhost_crypto_zero_copy option);
>  
> @@ -110,7 +107,6 @@ rte_vhost_crypto_set_zero_copy(int vid, enum rte_vhost_crypto_zero_copy option);
>   * @return
>   *  The number of fetched and processed vhost crypto request operations.
>   */
> -__rte_experimental
>  uint16_t
>  rte_vhost_crypto_fetch_requests(int vid, uint32_t qid,
>  		struct rte_crypto_op **ops, uint16_t nb_ops);
> @@ -132,7 +128,6 @@ rte_vhost_crypto_fetch_requests(int vid, uint32_t qid,
>   * @return
>   *  The number of ops processed.
>   */
> -__rte_experimental
>  uint16_t
>  rte_vhost_crypto_finalize_requests(struct rte_crypto_op **ops,
>  		uint16_t nb_ops, int *callfds, uint16_t *nb_callfds);
> diff --git a/lib/vhost/version.map b/lib/vhost/version.map
> index c92a9d4962..8ebde3f694 100644
> --- a/lib/vhost/version.map
> +++ b/lib/vhost/version.map
> @@ -13,6 +13,13 @@ DPDK_22 {
>  	rte_vdpa_reset_stats;
>  	rte_vdpa_unregister_device;
>  	rte_vhost_avail_entries;
> +	rte_vhost_clr_inflight_desc_packed;
> +	rte_vhost_clr_inflight_desc_split;
> +	rte_vhost_crypto_create;
> +	rte_vhost_crypto_fetch_requests;
> +	rte_vhost_crypto_finalize_requests;
> +	rte_vhost_crypto_free;
> +	rte_vhost_crypto_set_zero_copy;
>  	rte_vhost_dequeue_burst;
>  	rte_vhost_driver_attach_vdpa_device;
>  	rte_vhost_driver_callback_register;
> @@ -20,13 +27,17 @@ DPDK_22 {
>  	rte_vhost_driver_disable_features;
>  	rte_vhost_driver_enable_features;
>  	rte_vhost_driver_get_features;
> +	rte_vhost_driver_get_protocol_features;
> +	rte_vhost_driver_get_queue_num;
>  	rte_vhost_driver_get_vdpa_device;
>  	rte_vhost_driver_register;
>  	rte_vhost_driver_set_features;
> +	rte_vhost_driver_set_protocol_features;
>  	rte_vhost_driver_start;
>  	rte_vhost_driver_unregister;
>  	rte_vhost_enable_guest_notification;
>  	rte_vhost_enqueue_burst;
> +	rte_vhost_extern_callback_register;
>  	rte_vhost_get_ifname;
>  	rte_vhost_get_log_base;
>  	rte_vhost_get_mem_table;
> @@ -35,15 +46,22 @@ DPDK_22 {
>  	rte_vhost_get_numa_node;
>  	rte_vhost_get_queue_num;
>  	rte_vhost_get_vdpa_device;
> +	rte_vhost_get_vhost_ring_inflight;
>  	rte_vhost_get_vhost_vring;
>  	rte_vhost_get_vring_base;
> +	rte_vhost_get_vring_base_from_inflight;
>  	rte_vhost_get_vring_num;

>  	rte_vhost_gpa_to_vva;

Can this ^^^ be also removed now that rte_vhost_va_from_guest_pa() is
promoted to non-experimental? It is marked as deprecated in API (see
below) but i don't see anything in the deprecation documentation.

commit 9553e6e408883b3677e208dc66049bcd7f758529
Author: Maxime Coquelin <maxime.coquelin@redhat.com>
Date:   Wed Mar 14 17:31:25 2018 +0100

    vhost: deprecate unsafe GPA translation API

    This patch marks rte_vhost_gpa_to_vva() as deprecated because
    it is unsafe. Application relying on this API should move
    to the new rte_vhost_va_from_guest_pa() API, and check
    returned length to avoid out-of-bound accesses.

    This issue has been assigned CVE-2018-1059.

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


>  	rte_vhost_host_notifier_ctrl;
>  	rte_vhost_log_used_vring;
>  	rte_vhost_log_write;
>  	rte_vhost_rx_queue_count;
> +	rte_vhost_set_inflight_desc_packed;
> +	rte_vhost_set_inflight_desc_split;
> +	rte_vhost_set_last_inflight_io_packed;
> +	rte_vhost_set_last_inflight_io_split;
>  	rte_vhost_set_vring_base;
> +	rte_vhost_va_from_guest_pa;
>  	rte_vhost_vring_call;
>  
>  	local: *;
> @@ -52,25 +70,7 @@ DPDK_22 {
>  EXPERIMENTAL {
>  	global:
>  
> -	rte_vhost_driver_get_protocol_features;
> -	rte_vhost_driver_get_queue_num;
> -	rte_vhost_crypto_create;
>  	rte_vhost_crypto_driver_start;
> -	rte_vhost_crypto_free;
> -	rte_vhost_crypto_fetch_requests;
> -	rte_vhost_crypto_finalize_requests;
> -	rte_vhost_crypto_set_zero_copy;
> -	rte_vhost_va_from_guest_pa;
> -	rte_vhost_extern_callback_register;
> -	rte_vhost_driver_set_protocol_features;
> -	rte_vhost_set_inflight_desc_split;
> -	rte_vhost_set_inflight_desc_packed;
> -	rte_vhost_set_last_inflight_io_split;
> -	rte_vhost_set_last_inflight_io_packed;
> -	rte_vhost_clr_inflight_desc_split;
> -	rte_vhost_clr_inflight_desc_packed;
> -	rte_vhost_get_vhost_ring_inflight;
> -	rte_vhost_get_vring_base_from_inflight;
>  	rte_vhost_slave_config_change;
>  	rte_vhost_async_channel_register;
>  	rte_vhost_async_channel_unregister;
>
Xia, Chenbo Sept. 9, 2021, 2:13 a.m. UTC | #3
Hi Kevin,

> -----Original Message-----
> From: Kevin Traynor <ktraynor@redhat.com>
> Sent: Wednesday, September 8, 2021 8:01 PM
> To: Xia, Chenbo <chenbo.xia@intel.com>; dev@dpdk.org;
> maxime.coquelin@redhat.com
> Cc: Liu, Changpeng <changpeng.liu@intel.com>; mdr@ashroe.eu
> Subject: Re: [dpdk-dev] [PATCH] vhost: promote some APIs to stable
> 
> On 07/09/2021 03:58, Chenbo Xia wrote:
> > As reported by symbol bot, APIs listed in this patch have been
> > experimental for more than two years. This patch promotes these
> > 18 APIs to stable.
> >
> 
> Patch lgtm. One question about a possible follow on below.
> 
> Acked-by: Kevin Traynor <ktraynor@redhat.com>
> 
> > Signed-off-by: Chenbo Xia <chenbo.xia@intel.com>
> > ---
> >  lib/vhost/rte_vhost.h        | 13 -------------
> >  lib/vhost/rte_vhost_crypto.h |  5 -----
> >  lib/vhost/version.map        | 36 ++++++++++++++++++------------------
> >  3 files changed, 18 insertions(+), 36 deletions(-)
> >

[...]

> > diff --git a/lib/vhost/version.map b/lib/vhost/version.map
> > index c92a9d4962..8ebde3f694 100644
> > --- a/lib/vhost/version.map
> > +++ b/lib/vhost/version.map
> > @@ -13,6 +13,13 @@ DPDK_22 {
> >  	rte_vdpa_reset_stats;
> >  	rte_vdpa_unregister_device;
> >  	rte_vhost_avail_entries;
> > +	rte_vhost_clr_inflight_desc_packed;
> > +	rte_vhost_clr_inflight_desc_split;
> > +	rte_vhost_crypto_create;
> > +	rte_vhost_crypto_fetch_requests;
> > +	rte_vhost_crypto_finalize_requests;
> > +	rte_vhost_crypto_free;
> > +	rte_vhost_crypto_set_zero_copy;
> >  	rte_vhost_dequeue_burst;
> >  	rte_vhost_driver_attach_vdpa_device;
> >  	rte_vhost_driver_callback_register;
> > @@ -20,13 +27,17 @@ DPDK_22 {
> >  	rte_vhost_driver_disable_features;
> >  	rte_vhost_driver_enable_features;
> >  	rte_vhost_driver_get_features;
> > +	rte_vhost_driver_get_protocol_features;
> > +	rte_vhost_driver_get_queue_num;
> >  	rte_vhost_driver_get_vdpa_device;
> >  	rte_vhost_driver_register;
> >  	rte_vhost_driver_set_features;
> > +	rte_vhost_driver_set_protocol_features;
> >  	rte_vhost_driver_start;
> >  	rte_vhost_driver_unregister;
> >  	rte_vhost_enable_guest_notification;
> >  	rte_vhost_enqueue_burst;
> > +	rte_vhost_extern_callback_register;
> >  	rte_vhost_get_ifname;
> >  	rte_vhost_get_log_base;
> >  	rte_vhost_get_mem_table;
> > @@ -35,15 +46,22 @@ DPDK_22 {
> >  	rte_vhost_get_numa_node;
> >  	rte_vhost_get_queue_num;
> >  	rte_vhost_get_vdpa_device;
> > +	rte_vhost_get_vhost_ring_inflight;
> >  	rte_vhost_get_vhost_vring;
> >  	rte_vhost_get_vring_base;
> > +	rte_vhost_get_vring_base_from_inflight;
> >  	rte_vhost_get_vring_num;
> 
> >  	rte_vhost_gpa_to_vva;
> 
> Can this ^^^ be also removed now that rte_vhost_va_from_guest_pa() is
> promoted to non-experimental? It is marked as deprecated in API (see
> below) but i don't see anything in the deprecation documentation.

Good point. I think it can be removed now. But we didn't send the deprecation
notice last release. I am not sure if it's ok to remove it this release.

@Ray & Maxime,

What do you think? I think since this API is unsafe and the safe version is
promoted, it makes sense to remove this.

Thanks,
Chenbo

> 
> commit 9553e6e408883b3677e208dc66049bcd7f758529
> Author: Maxime Coquelin <maxime.coquelin@redhat.com>
> Date:   Wed Mar 14 17:31:25 2018 +0100
> 
>     vhost: deprecate unsafe GPA translation API
> 
>     This patch marks rte_vhost_gpa_to_vva() as deprecated because
>     it is unsafe. Application relying on this API should move
>     to the new rte_vhost_va_from_guest_pa() API, and check
>     returned length to avoid out-of-bound accesses.
> 
>     This issue has been assigned CVE-2018-1059.
> 
>     Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> 
> 
> >  	rte_vhost_host_notifier_ctrl;
> >  	rte_vhost_log_used_vring;
> >  	rte_vhost_log_write;
> >  	rte_vhost_rx_queue_count;
> > +	rte_vhost_set_inflight_desc_packed;
> > +	rte_vhost_set_inflight_desc_split;
> > +	rte_vhost_set_last_inflight_io_packed;
> > +	rte_vhost_set_last_inflight_io_split;
> >  	rte_vhost_set_vring_base;
> > +	rte_vhost_va_from_guest_pa;
> >  	rte_vhost_vring_call;
> >
> >  	local: *;
> > @@ -52,25 +70,7 @@ DPDK_22 {
> >  EXPERIMENTAL {
> >  	global:
> >
> > -	rte_vhost_driver_get_protocol_features;
> > -	rte_vhost_driver_get_queue_num;
> > -	rte_vhost_crypto_create;
> >  	rte_vhost_crypto_driver_start;
> > -	rte_vhost_crypto_free;
> > -	rte_vhost_crypto_fetch_requests;
> > -	rte_vhost_crypto_finalize_requests;
> > -	rte_vhost_crypto_set_zero_copy;
> > -	rte_vhost_va_from_guest_pa;
> > -	rte_vhost_extern_callback_register;
> > -	rte_vhost_driver_set_protocol_features;
> > -	rte_vhost_set_inflight_desc_split;
> > -	rte_vhost_set_inflight_desc_packed;
> > -	rte_vhost_set_last_inflight_io_split;
> > -	rte_vhost_set_last_inflight_io_packed;
> > -	rte_vhost_clr_inflight_desc_split;
> > -	rte_vhost_clr_inflight_desc_packed;
> > -	rte_vhost_get_vhost_ring_inflight;
> > -	rte_vhost_get_vring_base_from_inflight;
> >  	rte_vhost_slave_config_change;
> >  	rte_vhost_async_channel_register;
> >  	rte_vhost_async_channel_unregister;
> >
Kinsella, Ray Sept. 9, 2021, 11:19 a.m. UTC | #4
On 09/09/2021 03:13, Xia, Chenbo wrote:
> Hi Kevin,
> 
>> -----Original Message-----
>> From: Kevin Traynor <ktraynor@redhat.com>
>> Sent: Wednesday, September 8, 2021 8:01 PM
>> To: Xia, Chenbo <chenbo.xia@intel.com>; dev@dpdk.org;
>> maxime.coquelin@redhat.com
>> Cc: Liu, Changpeng <changpeng.liu@intel.com>; mdr@ashroe.eu
>> Subject: Re: [dpdk-dev] [PATCH] vhost: promote some APIs to stable
>>
>> On 07/09/2021 03:58, Chenbo Xia wrote:
>>> As reported by symbol bot, APIs listed in this patch have been
>>> experimental for more than two years. This patch promotes these
>>> 18 APIs to stable.
>>>
>>
>> Patch lgtm. One question about a possible follow on below.
>>
>> Acked-by: Kevin Traynor <ktraynor@redhat.com>
>>
>>> Signed-off-by: Chenbo Xia <chenbo.xia@intel.com>
>>> ---
>>>  lib/vhost/rte_vhost.h        | 13 -------------
>>>  lib/vhost/rte_vhost_crypto.h |  5 -----
>>>  lib/vhost/version.map        | 36 ++++++++++++++++++------------------
>>>  3 files changed, 18 insertions(+), 36 deletions(-)
>>>
> 
> [...]
> 
>>> diff --git a/lib/vhost/version.map b/lib/vhost/version.map
>>> index c92a9d4962..8ebde3f694 100644
>>> --- a/lib/vhost/version.map
>>> +++ b/lib/vhost/version.map
>>> @@ -13,6 +13,13 @@ DPDK_22 {
>>>  	rte_vdpa_reset_stats;
>>>  	rte_vdpa_unregister_device;
>>>  	rte_vhost_avail_entries;
>>> +	rte_vhost_clr_inflight_desc_packed;
>>> +	rte_vhost_clr_inflight_desc_split;
>>> +	rte_vhost_crypto_create;
>>> +	rte_vhost_crypto_fetch_requests;
>>> +	rte_vhost_crypto_finalize_requests;
>>> +	rte_vhost_crypto_free;
>>> +	rte_vhost_crypto_set_zero_copy;
>>>  	rte_vhost_dequeue_burst;
>>>  	rte_vhost_driver_attach_vdpa_device;
>>>  	rte_vhost_driver_callback_register;
>>> @@ -20,13 +27,17 @@ DPDK_22 {
>>>  	rte_vhost_driver_disable_features;
>>>  	rte_vhost_driver_enable_features;
>>>  	rte_vhost_driver_get_features;
>>> +	rte_vhost_driver_get_protocol_features;
>>> +	rte_vhost_driver_get_queue_num;
>>>  	rte_vhost_driver_get_vdpa_device;
>>>  	rte_vhost_driver_register;
>>>  	rte_vhost_driver_set_features;
>>> +	rte_vhost_driver_set_protocol_features;
>>>  	rte_vhost_driver_start;
>>>  	rte_vhost_driver_unregister;
>>>  	rte_vhost_enable_guest_notification;
>>>  	rte_vhost_enqueue_burst;
>>> +	rte_vhost_extern_callback_register;
>>>  	rte_vhost_get_ifname;
>>>  	rte_vhost_get_log_base;
>>>  	rte_vhost_get_mem_table;
>>> @@ -35,15 +46,22 @@ DPDK_22 {
>>>  	rte_vhost_get_numa_node;
>>>  	rte_vhost_get_queue_num;
>>>  	rte_vhost_get_vdpa_device;
>>> +	rte_vhost_get_vhost_ring_inflight;
>>>  	rte_vhost_get_vhost_vring;
>>>  	rte_vhost_get_vring_base;
>>> +	rte_vhost_get_vring_base_from_inflight;
>>>  	rte_vhost_get_vring_num;
>>
>>>  	rte_vhost_gpa_to_vva;
>>
>> Can this ^^^ be also removed now that rte_vhost_va_from_guest_pa() is
>> promoted to non-experimental? It is marked as deprecated in API (see
>> below) but i don't see anything in the deprecation documentation.
> 
> Good point. I think it can be removed now. But we didn't send the deprecation
> notice last release. I am not sure if it's ok to remove it this release.
> 
> @Ray & Maxime,
> 
> What do you think? I think since this API is unsafe and the safe version is
> promoted, it makes sense to remove this.

Strictly speaking there should have been depreciation notice. 
However if the API has been marked depreciated since 2018 and _is_ unsafe.
You'd have to imagine that is sufficient to warrant removal at this stage. 

Thomas, David and Ferruh - any inputs/comments or objections?

> 
> Thanks,
> Chenbo
> 
>>
>> commit 9553e6e408883b3677e208dc66049bcd7f758529
>> Author: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Date:   Wed Mar 14 17:31:25 2018 +0100
>>
>>     vhost: deprecate unsafe GPA translation API
>>
>>     This patch marks rte_vhost_gpa_to_vva() as deprecated because
>>     it is unsafe. Application relying on this API should move
>>     to the new rte_vhost_va_from_guest_pa() API, and check
>>     returned length to avoid out-of-bound accesses.
>>
>>     This issue has been assigned CVE-2018-1059.
>>
>>     Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>>
>>
>>>  	rte_vhost_host_notifier_ctrl;
>>>  	rte_vhost_log_used_vring;
>>>  	rte_vhost_log_write;
>>>  	rte_vhost_rx_queue_count;
>>> +	rte_vhost_set_inflight_desc_packed;
>>> +	rte_vhost_set_inflight_desc_split;
>>> +	rte_vhost_set_last_inflight_io_packed;
>>> +	rte_vhost_set_last_inflight_io_split;
>>>  	rte_vhost_set_vring_base;
>>> +	rte_vhost_va_from_guest_pa;
>>>  	rte_vhost_vring_call;
>>>
>>>  	local: *;
>>> @@ -52,25 +70,7 @@ DPDK_22 {
>>>  EXPERIMENTAL {
>>>  	global:
>>>
>>> -	rte_vhost_driver_get_protocol_features;
>>> -	rte_vhost_driver_get_queue_num;
>>> -	rte_vhost_crypto_create;
>>>  	rte_vhost_crypto_driver_start;
>>> -	rte_vhost_crypto_free;
>>> -	rte_vhost_crypto_fetch_requests;
>>> -	rte_vhost_crypto_finalize_requests;
>>> -	rte_vhost_crypto_set_zero_copy;
>>> -	rte_vhost_va_from_guest_pa;
>>> -	rte_vhost_extern_callback_register;
>>> -	rte_vhost_driver_set_protocol_features;
>>> -	rte_vhost_set_inflight_desc_split;
>>> -	rte_vhost_set_inflight_desc_packed;
>>> -	rte_vhost_set_last_inflight_io_split;
>>> -	rte_vhost_set_last_inflight_io_packed;
>>> -	rte_vhost_clr_inflight_desc_split;
>>> -	rte_vhost_clr_inflight_desc_packed;
>>> -	rte_vhost_get_vhost_ring_inflight;
>>> -	rte_vhost_get_vring_base_from_inflight;
>>>  	rte_vhost_slave_config_change;
>>>  	rte_vhost_async_channel_register;
>>>  	rte_vhost_async_channel_unregister;
>>>
>
Maxime Coquelin Sept. 13, 2021, 4:02 p.m. UTC | #5
On 9/9/21 1:19 PM, Kinsella, Ray wrote:
> 
> 
> On 09/09/2021 03:13, Xia, Chenbo wrote:
>> Hi Kevin,
>>
>>> -----Original Message-----
>>> From: Kevin Traynor <ktraynor@redhat.com>
>>> Sent: Wednesday, September 8, 2021 8:01 PM
>>> To: Xia, Chenbo <chenbo.xia@intel.com>; dev@dpdk.org;
>>> maxime.coquelin@redhat.com
>>> Cc: Liu, Changpeng <changpeng.liu@intel.com>; mdr@ashroe.eu
>>> Subject: Re: [dpdk-dev] [PATCH] vhost: promote some APIs to stable
>>>
>>> On 07/09/2021 03:58, Chenbo Xia wrote:
>>>> As reported by symbol bot, APIs listed in this patch have been
>>>> experimental for more than two years. This patch promotes these
>>>> 18 APIs to stable.
>>>>
>>>
>>> Patch lgtm. One question about a possible follow on below.
>>>
>>> Acked-by: Kevin Traynor <ktraynor@redhat.com>
>>>
>>>> Signed-off-by: Chenbo Xia <chenbo.xia@intel.com>
>>>> ---
>>>>  lib/vhost/rte_vhost.h        | 13 -------------
>>>>  lib/vhost/rte_vhost_crypto.h |  5 -----
>>>>  lib/vhost/version.map        | 36 ++++++++++++++++++------------------
>>>>  3 files changed, 18 insertions(+), 36 deletions(-)
>>>>
>>
>> [...]
>>
>>>> diff --git a/lib/vhost/version.map b/lib/vhost/version.map
>>>> index c92a9d4962..8ebde3f694 100644
>>>> --- a/lib/vhost/version.map
>>>> +++ b/lib/vhost/version.map
>>>> @@ -13,6 +13,13 @@ DPDK_22 {
>>>>  	rte_vdpa_reset_stats;
>>>>  	rte_vdpa_unregister_device;
>>>>  	rte_vhost_avail_entries;
>>>> +	rte_vhost_clr_inflight_desc_packed;
>>>> +	rte_vhost_clr_inflight_desc_split;
>>>> +	rte_vhost_crypto_create;
>>>> +	rte_vhost_crypto_fetch_requests;
>>>> +	rte_vhost_crypto_finalize_requests;
>>>> +	rte_vhost_crypto_free;
>>>> +	rte_vhost_crypto_set_zero_copy;
>>>>  	rte_vhost_dequeue_burst;
>>>>  	rte_vhost_driver_attach_vdpa_device;
>>>>  	rte_vhost_driver_callback_register;
>>>> @@ -20,13 +27,17 @@ DPDK_22 {
>>>>  	rte_vhost_driver_disable_features;
>>>>  	rte_vhost_driver_enable_features;
>>>>  	rte_vhost_driver_get_features;
>>>> +	rte_vhost_driver_get_protocol_features;
>>>> +	rte_vhost_driver_get_queue_num;
>>>>  	rte_vhost_driver_get_vdpa_device;
>>>>  	rte_vhost_driver_register;
>>>>  	rte_vhost_driver_set_features;
>>>> +	rte_vhost_driver_set_protocol_features;
>>>>  	rte_vhost_driver_start;
>>>>  	rte_vhost_driver_unregister;
>>>>  	rte_vhost_enable_guest_notification;
>>>>  	rte_vhost_enqueue_burst;
>>>> +	rte_vhost_extern_callback_register;
>>>>  	rte_vhost_get_ifname;
>>>>  	rte_vhost_get_log_base;
>>>>  	rte_vhost_get_mem_table;
>>>> @@ -35,15 +46,22 @@ DPDK_22 {
>>>>  	rte_vhost_get_numa_node;
>>>>  	rte_vhost_get_queue_num;
>>>>  	rte_vhost_get_vdpa_device;
>>>> +	rte_vhost_get_vhost_ring_inflight;
>>>>  	rte_vhost_get_vhost_vring;
>>>>  	rte_vhost_get_vring_base;
>>>> +	rte_vhost_get_vring_base_from_inflight;
>>>>  	rte_vhost_get_vring_num;
>>>
>>>>  	rte_vhost_gpa_to_vva;
>>>
>>> Can this ^^^ be also removed now that rte_vhost_va_from_guest_pa() is
>>> promoted to non-experimental? It is marked as deprecated in API (see
>>> below) but i don't see anything in the deprecation documentation.
>>
>> Good point. I think it can be removed now. But we didn't send the deprecation
>> notice last release. I am not sure if it's ok to remove it this release.
>>
>> @Ray & Maxime,
>>
>> What do you think? I think since this API is unsafe and the safe version is
>> promoted, it makes sense to remove this.
> 
> Strictly speaking there should have been depreciation notice. 
> However if the API has been marked depreciated since 2018 and _is_ unsafe.
> You'd have to imagine that is sufficient to warrant removal at this stage. 
> 
> Thomas, David and Ferruh - any inputs/comments or objections?

I aagree it can be removed in this release. SPDK project was the only
user I'm aware of, and they migrated to the safe variant long time ago.

I propose to apply this patch first, then I will post a patch removing
this deprecated symbol if nobody disagree.

Thanks,
Maxime

>>
>> Thanks,
>> Chenbo
>>
>>>
>>> commit 9553e6e408883b3677e208dc66049bcd7f758529
>>> Author: Maxime Coquelin <maxime.coquelin@redhat.com>
>>> Date:   Wed Mar 14 17:31:25 2018 +0100
>>>
>>>     vhost: deprecate unsafe GPA translation API
>>>
>>>     This patch marks rte_vhost_gpa_to_vva() as deprecated because
>>>     it is unsafe. Application relying on this API should move
>>>     to the new rte_vhost_va_from_guest_pa() API, and check
>>>     returned length to avoid out-of-bound accesses.
>>>
>>>     This issue has been assigned CVE-2018-1059.
>>>
>>>     Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>>>
>>>
>>>>  	rte_vhost_host_notifier_ctrl;
>>>>  	rte_vhost_log_used_vring;
>>>>  	rte_vhost_log_write;
>>>>  	rte_vhost_rx_queue_count;
>>>> +	rte_vhost_set_inflight_desc_packed;
>>>> +	rte_vhost_set_inflight_desc_split;
>>>> +	rte_vhost_set_last_inflight_io_packed;
>>>> +	rte_vhost_set_last_inflight_io_split;
>>>>  	rte_vhost_set_vring_base;
>>>> +	rte_vhost_va_from_guest_pa;
>>>>  	rte_vhost_vring_call;
>>>>
>>>>  	local: *;
>>>> @@ -52,25 +70,7 @@ DPDK_22 {
>>>>  EXPERIMENTAL {
>>>>  	global:
>>>>
>>>> -	rte_vhost_driver_get_protocol_features;
>>>> -	rte_vhost_driver_get_queue_num;
>>>> -	rte_vhost_crypto_create;
>>>>  	rte_vhost_crypto_driver_start;
>>>> -	rte_vhost_crypto_free;
>>>> -	rte_vhost_crypto_fetch_requests;
>>>> -	rte_vhost_crypto_finalize_requests;
>>>> -	rte_vhost_crypto_set_zero_copy;
>>>> -	rte_vhost_va_from_guest_pa;
>>>> -	rte_vhost_extern_callback_register;
>>>> -	rte_vhost_driver_set_protocol_features;
>>>> -	rte_vhost_set_inflight_desc_split;
>>>> -	rte_vhost_set_inflight_desc_packed;
>>>> -	rte_vhost_set_last_inflight_io_split;
>>>> -	rte_vhost_set_last_inflight_io_packed;
>>>> -	rte_vhost_clr_inflight_desc_split;
>>>> -	rte_vhost_clr_inflight_desc_packed;
>>>> -	rte_vhost_get_vhost_ring_inflight;
>>>> -	rte_vhost_get_vring_base_from_inflight;
>>>>  	rte_vhost_slave_config_change;
>>>>  	rte_vhost_async_channel_register;
>>>>  	rte_vhost_async_channel_unregister;
>>>>
>>
>
Maxime Coquelin Sept. 13, 2021, 7:18 p.m. UTC | #6
On 9/7/21 4:58 AM, Chenbo Xia wrote:
> As reported by symbol bot, APIs listed in this patch have been
> experimental for more than two years. This patch promotes these
> 18 APIs to stable.
> 
> Signed-off-by: Chenbo Xia <chenbo.xia@intel.com>
> ---
>  lib/vhost/rte_vhost.h        | 13 -------------
>  lib/vhost/rte_vhost_crypto.h |  5 -----
>  lib/vhost/version.map        | 36 ++++++++++++++++++------------------
>  3 files changed, 18 insertions(+), 36 deletions(-)
> 

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

Thanks,
Maxime
Maxime Coquelin Sept. 14, 2021, 11:30 a.m. UTC | #7
On 9/7/21 4:58 AM, Chenbo Xia wrote:
> As reported by symbol bot, APIs listed in this patch have been
> experimental for more than two years. This patch promotes these
> 18 APIs to stable.
> 
> Signed-off-by: Chenbo Xia <chenbo.xia@intel.com>
> ---
>  lib/vhost/rte_vhost.h        | 13 -------------
>  lib/vhost/rte_vhost_crypto.h |  5 -----
>  lib/vhost/version.map        | 36 ++++++++++++++++++------------------
>  3 files changed, 18 insertions(+), 36 deletions(-)
> 

Applied to dpdk-next-virtio/main.

Thanks,
Maxime
diff mbox series

Patch

diff --git a/lib/vhost/rte_vhost.h b/lib/vhost/rte_vhost.h
index 8d875e9322..fd372d5259 100644
--- a/lib/vhost/rte_vhost.h
+++ b/lib/vhost/rte_vhost.h
@@ -342,7 +342,6 @@  rte_vhost_gpa_to_vva(struct rte_vhost_memory *mem, uint64_t gpa)
  * @return
  *  the host virtual address on success, 0 on failure
  */
-__rte_experimental
 static __rte_always_inline uint64_t
 rte_vhost_va_from_guest_pa(struct rte_vhost_memory *mem,
 						   uint64_t gpa, uint64_t *len)
@@ -522,7 +521,6 @@  int rte_vhost_driver_get_features(const char *path, uint64_t *features);
  * @return
  *  0 on success, -1 on failure
  */
-__rte_experimental
 int
 rte_vhost_driver_set_protocol_features(const char *path,
 		uint64_t protocol_features);
@@ -537,7 +535,6 @@  rte_vhost_driver_set_protocol_features(const char *path,
  * @return
  *  0 on success, -1 on failure
  */
-__rte_experimental
 int
 rte_vhost_driver_get_protocol_features(const char *path,
 		uint64_t *protocol_features);
@@ -552,7 +549,6 @@  rte_vhost_driver_get_protocol_features(const char *path,
  * @return
  *  0 on success, -1 on failure
  */
-__rte_experimental
 int
 rte_vhost_driver_get_queue_num(const char *path, uint32_t *queue_num);
 
@@ -768,7 +764,6 @@  int rte_vhost_get_vhost_vring(int vid, uint16_t vring_idx,
  * @return
  *  0 on success, -1 on failure
  */
-__rte_experimental
 int
 rte_vhost_get_vhost_ring_inflight(int vid, uint16_t vring_idx,
 	struct rte_vhost_ring_inflight *vring);
@@ -788,7 +783,6 @@  rte_vhost_get_vhost_ring_inflight(int vid, uint16_t vring_idx,
  * @return
  *  0 on success, -1 on failure
  */
-__rte_experimental
 int
 rte_vhost_set_inflight_desc_split(int vid, uint16_t vring_idx,
 	uint16_t idx);
@@ -811,7 +805,6 @@  rte_vhost_set_inflight_desc_split(int vid, uint16_t vring_idx,
  * @return
  *  0 on success, -1 on failure
  */
-__rte_experimental
 int
 rte_vhost_set_inflight_desc_packed(int vid, uint16_t vring_idx,
 	uint16_t head, uint16_t last, uint16_t *inflight_entry);
@@ -828,7 +821,6 @@  rte_vhost_set_inflight_desc_packed(int vid, uint16_t vring_idx,
  * @return
  *  0 on success, -1 on failure
  */
-__rte_experimental
 int
 rte_vhost_set_last_inflight_io_split(int vid,
 	uint16_t vring_idx, uint16_t idx);
@@ -848,7 +840,6 @@  rte_vhost_set_last_inflight_io_split(int vid,
  * @return
  *  0 on success, -1 on failure
  */
-__rte_experimental
 int
 rte_vhost_set_last_inflight_io_packed(int vid,
 	uint16_t vring_idx, uint16_t head);
@@ -867,7 +858,6 @@  rte_vhost_set_last_inflight_io_packed(int vid,
  * @return
  *  0 on success, -1 on failure
  */
-__rte_experimental
 int
 rte_vhost_clr_inflight_desc_split(int vid, uint16_t vring_idx,
 	uint16_t last_used_idx, uint16_t idx);
@@ -884,7 +874,6 @@  rte_vhost_clr_inflight_desc_split(int vid, uint16_t vring_idx,
  * @return
  *  0 on success, -1 on failure
  */
-__rte_experimental
 int
 rte_vhost_clr_inflight_desc_packed(int vid, uint16_t vring_idx,
 	uint16_t head);
@@ -965,7 +954,6 @@  rte_vhost_get_vring_base(int vid, uint16_t queue_id,
  * @return
  *  0 on success, -1 on failure
  */
-__rte_experimental
 int
 rte_vhost_get_vring_base_from_inflight(int vid,
 	uint16_t queue_id, uint16_t *last_avail_idx, uint16_t *last_used_idx);
@@ -1000,7 +988,6 @@  rte_vhost_set_vring_base(int vid, uint16_t queue_id,
  * @return
  *  0 on success, -1 on failure
  */
-__rte_experimental
 int
 rte_vhost_extern_callback_register(int vid,
 		struct rte_vhost_user_extern_ops const * const ops, void *ctx);
diff --git a/lib/vhost/rte_vhost_crypto.h b/lib/vhost/rte_vhost_crypto.h
index 8531757285..f54d731139 100644
--- a/lib/vhost/rte_vhost_crypto.h
+++ b/lib/vhost/rte_vhost_crypto.h
@@ -58,7 +58,6 @@  rte_vhost_crypto_driver_start(const char *path);
  *  0 if the Vhost Crypto Instance is created successfully.
  *  Negative integer if otherwise
  */
-__rte_experimental
 int
 rte_vhost_crypto_create(int vid, uint8_t cryptodev_id,
 		struct rte_mempool *sess_pool,
@@ -74,7 +73,6 @@  rte_vhost_crypto_create(int vid, uint8_t cryptodev_id,
  *  0 if the Vhost Crypto Instance is created successfully.
  *  Negative integer if otherwise.
  */
-__rte_experimental
 int
 rte_vhost_crypto_free(int vid);
 
@@ -89,7 +87,6 @@  rte_vhost_crypto_free(int vid);
  *  0 if completed successfully.
  *  Negative integer if otherwise.
  */
-__rte_experimental
 int
 rte_vhost_crypto_set_zero_copy(int vid, enum rte_vhost_crypto_zero_copy option);
 
@@ -110,7 +107,6 @@  rte_vhost_crypto_set_zero_copy(int vid, enum rte_vhost_crypto_zero_copy option);
  * @return
  *  The number of fetched and processed vhost crypto request operations.
  */
-__rte_experimental
 uint16_t
 rte_vhost_crypto_fetch_requests(int vid, uint32_t qid,
 		struct rte_crypto_op **ops, uint16_t nb_ops);
@@ -132,7 +128,6 @@  rte_vhost_crypto_fetch_requests(int vid, uint32_t qid,
  * @return
  *  The number of ops processed.
  */
-__rte_experimental
 uint16_t
 rte_vhost_crypto_finalize_requests(struct rte_crypto_op **ops,
 		uint16_t nb_ops, int *callfds, uint16_t *nb_callfds);
diff --git a/lib/vhost/version.map b/lib/vhost/version.map
index c92a9d4962..8ebde3f694 100644
--- a/lib/vhost/version.map
+++ b/lib/vhost/version.map
@@ -13,6 +13,13 @@  DPDK_22 {
 	rte_vdpa_reset_stats;
 	rte_vdpa_unregister_device;
 	rte_vhost_avail_entries;
+	rte_vhost_clr_inflight_desc_packed;
+	rte_vhost_clr_inflight_desc_split;
+	rte_vhost_crypto_create;
+	rte_vhost_crypto_fetch_requests;
+	rte_vhost_crypto_finalize_requests;
+	rte_vhost_crypto_free;
+	rte_vhost_crypto_set_zero_copy;
 	rte_vhost_dequeue_burst;
 	rte_vhost_driver_attach_vdpa_device;
 	rte_vhost_driver_callback_register;
@@ -20,13 +27,17 @@  DPDK_22 {
 	rte_vhost_driver_disable_features;
 	rte_vhost_driver_enable_features;
 	rte_vhost_driver_get_features;
+	rte_vhost_driver_get_protocol_features;
+	rte_vhost_driver_get_queue_num;
 	rte_vhost_driver_get_vdpa_device;
 	rte_vhost_driver_register;
 	rte_vhost_driver_set_features;
+	rte_vhost_driver_set_protocol_features;
 	rte_vhost_driver_start;
 	rte_vhost_driver_unregister;
 	rte_vhost_enable_guest_notification;
 	rte_vhost_enqueue_burst;
+	rte_vhost_extern_callback_register;
 	rte_vhost_get_ifname;
 	rte_vhost_get_log_base;
 	rte_vhost_get_mem_table;
@@ -35,15 +46,22 @@  DPDK_22 {
 	rte_vhost_get_numa_node;
 	rte_vhost_get_queue_num;
 	rte_vhost_get_vdpa_device;
+	rte_vhost_get_vhost_ring_inflight;
 	rte_vhost_get_vhost_vring;
 	rte_vhost_get_vring_base;
+	rte_vhost_get_vring_base_from_inflight;
 	rte_vhost_get_vring_num;
 	rte_vhost_gpa_to_vva;
 	rte_vhost_host_notifier_ctrl;
 	rte_vhost_log_used_vring;
 	rte_vhost_log_write;
 	rte_vhost_rx_queue_count;
+	rte_vhost_set_inflight_desc_packed;
+	rte_vhost_set_inflight_desc_split;
+	rte_vhost_set_last_inflight_io_packed;
+	rte_vhost_set_last_inflight_io_split;
 	rte_vhost_set_vring_base;
+	rte_vhost_va_from_guest_pa;
 	rte_vhost_vring_call;
 
 	local: *;
@@ -52,25 +70,7 @@  DPDK_22 {
 EXPERIMENTAL {
 	global:
 
-	rte_vhost_driver_get_protocol_features;
-	rte_vhost_driver_get_queue_num;
-	rte_vhost_crypto_create;
 	rte_vhost_crypto_driver_start;
-	rte_vhost_crypto_free;
-	rte_vhost_crypto_fetch_requests;
-	rte_vhost_crypto_finalize_requests;
-	rte_vhost_crypto_set_zero_copy;
-	rte_vhost_va_from_guest_pa;
-	rte_vhost_extern_callback_register;
-	rte_vhost_driver_set_protocol_features;
-	rte_vhost_set_inflight_desc_split;
-	rte_vhost_set_inflight_desc_packed;
-	rte_vhost_set_last_inflight_io_split;
-	rte_vhost_set_last_inflight_io_packed;
-	rte_vhost_clr_inflight_desc_split;
-	rte_vhost_clr_inflight_desc_packed;
-	rte_vhost_get_vhost_ring_inflight;
-	rte_vhost_get_vring_base_from_inflight;
 	rte_vhost_slave_config_change;
 	rte_vhost_async_channel_register;
 	rte_vhost_async_channel_unregister;