On 02/09/2021 10:50, Ferruh Yigit wrote:
> On 9/1/2021 2:25 PM, Burakov, Anatoly wrote:
>> On 01-Sep-21 12:42 PM, Ferruh Yigit wrote:
>>> On 9/1/2021 12:01 PM, Burakov, Anatoly wrote:
>>>> On 01-Sep-21 10:56 AM, Ferruh Yigit wrote:
>>>>> On 9/1/2021 2:41 AM, Ding, Xuan wrote:
>>>>>> Hi Ferruh,
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Yigit, Ferruh <ferruh.yi...@intel.com>
>>>>>>> Sent: Wednesday, September 1, 2021 12:01 AM
>>>>>>> To: Ding, Xuan <xuan.d...@intel.com>; dev@dpdk.org; Burakov, Anatoly
>>>>>>> <anatoly.bura...@intel.com>
>>>>>>> Cc: maxime.coque...@redhat.com; Xia, Chenbo <chenbo....@intel.com>; Hu,
>>>>>>> Jiayu <jiayu...@intel.com>; Richardson, Bruce
>>>>>>> <bruce.richard...@intel.com>
>>>>>>> Subject: Re: [PATCH] doc: announce change in vfio dma mapping
>>>>>>>
>>>>>>> On 8/31/2021 2:10 PM, Xuan Ding wrote:
>>>>>>>> Currently, the VFIO subsystem will compact adjacent DMA regions for the
>>>>>>>> purposes of saving space in the internal list of mappings. This has a
>>>>>>>> side effect of compacting two separate mappings that just happen to be
>>>>>>>> adjacent in memory. Since VFIO implementation on IA platforms also does
>>>>>>>> not allow partial unmapping of memory mapped for DMA, the current
>>>>>>> DPDK
>>>>>>>> VFIO implementation will prevent unmapping of accidentally adjacent
>>>>>>>> maps even though it could have been unmapped [1].
>>>>>>>>
>>>>>>>> The proper fix for this issue is to change the VFIO DMA mapping API to
>>>>>>>> also include page size, and always map memory page-by-page.
>>>>>>>>
>>>>>>>> [1] https://mails.dpdk.org/archives/dev/2021-July/213493.html
>>>>>>>>
>>>>>>>> Signed-off-by: Xuan Ding <xuan.d...@intel.com>
>>>>>>>> ---
>>>>>>>> doc/guides/rel_notes/deprecation.rst | 3 +++
>>>>>>>> 1 file changed, 3 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/doc/guides/rel_notes/deprecation.rst
>>>>>>> b/doc/guides/rel_notes/deprecation.rst
>>>>>>>> index 76a4abfd6b..1234420caf 100644
>>>>>>>> --- a/doc/guides/rel_notes/deprecation.rst
>>>>>>>> +++ b/doc/guides/rel_notes/deprecation.rst
>>>>>>>> @@ -287,3 +287,6 @@ Deprecation Notices
>>>>>>>> reserved bytes to 2 (from 3), and use 1 byte to indicate warnings
>>>>>>>> and
>>>>>>> other
>>>>>>>> information from the crypto/security operation. This field will be
>>>>>>>> used to
>>>>>>>> communicate events such as soft expiry with IPsec in lookaside
>>>>>>>> mode.
>>>>>>>> +
>>>>>>>> +* vfio: the functions `rte_vfio_container_dma_map` will be amended to
>>>>>>>> + include page size. This change is targeted for DPDK 22.02.
>>>>>>>>
>>>>>>>
>>>>>>> Is this means adding a new parameter to API?
>>>>>>> If so this is an ABI/API break and we can't do this change in the 22.02.
>>>>>>
>>>>>> Our original plan is add a new parameter in order not to use a new
>>>>>> function
>>>>>> name, so you mean, any changes to the API can only be done in the LTS
>>>>>> version?
>>>>>> If so, we can only add a new API and retire the old one in 22.11.
>>>>>>
>>>>>
>>>>> We can add a new API anytime. Adding new parameter to an existing API can
>>>>> be
>>>>> done on the ABI break release.
>>>>
>>>> So, 22.11 then?
>>>>
>>>
>>> Yes.
>>>
>>>>>
>>>>> You can add the new API in this release, and start using it.
>>>>> And mark the old API as deprecated in this release. This lets existing
>>>>> binaries
>>>>> to keep using it, but app needs to switch to new API for compilation.
>>>>> Old API can be removed on 22.11, and you will need a deprecation notice
>>>>> before
>>>>> 22.11 for it.
>>>>>
>>>>> Is above plan works for you?
>>>>>
>>>>
>>>> We have slightly rethought our approach, and the functionality that Xuan
>>>> requires does not rely on this API. They can, for all intents and
>>>> purposes, be
>>>> considered unrelated issues.
>>>>
>>>> I still think it's a good idea to update the API that way, so I would like
>>>> to do
>>>> that, and if we have to wait until 22.11 to fix it, I'm OK with that. Since
>>>> there no longer is any urgency here, it's acceptable to wait for the next
>>>> LTS to
>>>> break it.
>>>>
>>>
>>> Got it.
>>>
>>> As far as I understand, main motivation in techboard decision was to
>>> prevent the
>>> ABI break as much as possible (main reason of decision wasn't deprecation
>>> notice
>>> being late). But if the correct thing to do is to rename the API (and break
>>> the
>>> ABI), I don't see the benefit to wait one more year, it is just delaying the
>>> impact and adding overhead to us.
>>> I am for being pragmatic and doing the change in this release if API rename
>>> is
>>> better option, perhaps we can visit the issue again in techboard.
>>>
>>> Can you please describe why renaming API is better option, comparing to
>>> adding
>>> new API with new parameter?
>>
>> I take it you meant "why renaming API *isn't* a better option".
>>
>> The problem we're solving is that the API in question does not know about
>> page
>> sizes and thus can't map segments page-by-page. I mean I /guess/ we could
>> have
>> two API's (one paged, one not paged), but then we get into all kinds of hairy
>> things about the API leaking the details of underlying platform.
>>
>> Bottom line: i like current API function name. It's concise, it's
>> descriptive.
>> It's only missing a parameter, which i would like to add. A rename has been
>> suggested (deprecate old API, add new API with a different name, and with
>> added
>> parameter), but honestly, I don't see why we have to do that because this is
>> predicated upon the assumption that we *can't* break ABI at all, under any
>> circumstances.
>>
>> Can you please explain to me what is wrong with keeping a versioned symbol?
>> Like, keep the old function around to keep ABI compatibility, but break the
>> API
>> compatibility for those who target 22.02 or later? That's what symbol
>> versioning
>> is *for*, is it not?
>>
>
> Nothing wrong with symbol versioning, indeed that is preferred way if it works
> for you, I didn't get that symbol versioning is planned.
>
> @Ray,
> Since symbol versioning is planned, ABI won't break, but API will change, can
> this change be done in this release without deprecation notice?
Yes - I would think so.
Since we are going to the effort of using symbol versioning nothing is being
depreciated as such (yet).
> Later we can have a deprecation notice to remove old symbol on 22.11.
>
> Thanks,
> ferruh
>
