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?
--
Thanks,
Anatoly
