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

[Maxime Coquelin <maxime.coquelin@redhat.com>]

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