Re: [dpdk-dev] [PATCH] vhost: promote some APIs to stable

["Kinsella, Ray" <mdr@ashroe.eu>]

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;
>>>
>