Hi,

-----Original Message-----
From: Burakov, Anatoly <redacted>
Sent: Thursday, August 26, 2021 6:15 PM
To: Richardson, Bruce <redacted>
Cc: Yigit, Ferruh <redacted>; Ding, Xuan
[off-list ref]; dev@dpdk.org; maxime.coquelin@redhat.com; Xia,
Chenbo [off-list ref]; Hu, Jiayu [off-list ref];
techboard@dpdk.org; David Marchand [off-list ref]
Subject: Re: [PATCH] doc: announce change in dma mapping/unmapping

On 26-Aug-21 11:09 AM, Bruce Richardson wrote:

quoted

On Thu, Aug 26, 2021 at 10:46:07AM +0100, Burakov, Anatoly wrote:

quoted

On 26-Aug-21 10:29 AM, Ferruh Yigit wrote:

quoted

On 8/25/2021 12:47 PM, Burakov, Anatoly wrote:

quoted

On 25-Aug-21 12:27 PM, Xuan Ding wrote:

quoted

Currently, the VFIO subsystem will compact adjacent DMA regions for

the

quoted

quoted

quoted

quoted

quoted

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

quoted

quoted

quoted

quoted

quoted

adjacent in memory. Since VFIO implementation on IA platforms also

does

quoted

quoted

quoted

quoted

quoted

not allow partial unmapping of memory mapped for DMA, the current

DPDK

quoted

quoted

quoted

quoted

quoted

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

quoted

quoted

quoted

quoted

quoted

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 <redacted>
---
    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..272ffa993e 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

quoted

quoted

quoted

quoted

quoted

      information from the crypto/security operation. This field will be

used to

quoted

quoted

quoted

quoted

quoted

      communicate events such as soft expiry with IPsec in lookaside

mode.

quoted

quoted

quoted

quoted

quoted

+
+  * vfio: the functions `rte_vfio_container_dma_map` and
`rte_vfio_container_dma_unmap`
+  will be amended to include page size. This change is targeted for

DPDK 21.11.

quoted

quoted

quoted

quoted

quoted

Acked-by: Anatoly Burakov <redacted>

Techboard decision was to add a new API, instead of updating existing

ones, to

quoted

quoted

quoted

not break the apps using this API.

@Xuan, @Anatoly, can you please confirm if this will solve your problem?

I don't think adding a new API is a particularly good solution. The "new"
API will be almost exactly as the old one, but adding one parameter. I

don't

quoted

quoted

expect code duplication to be an issue, but having two API's that do the
same thing seems like it's rife for potential confusion.

Well, if one API is marked as deprecated, then there will be no confusion
for users, since using the wrong one will give a warning pointing to the
right one.

quoted

If we add a new API, we can then either remove the old API entirely in
22.11 (effectively renaming it), or we remove the new API in 22.11 and
rename it back to the old function name. I don't think neither of these
is a good solution, as we risk introducing more users for the API that
will later change.

The new API will not be renamed to the old one, since that would break

apps

quoted

using it without proper deprecation process. Removing the old one alone
would be the approach to be used, but it would be correctly following the
deprecation process and giving users at least 1 year, if no 2, of notice
about the change.

quoted

I think the pain of updating current software for 21.11 (while keeping
compatibility with 20.11 ABI!) is going to happen regardless, and whether

we

quoted

quoted

decide to add a "temporary" new API or permanently rename the old one.

It's

quoted

quoted

(in my opinion) easier to just bite the bullet and update the function in
21.11.

I fail to see the issue with adding a new function. Whether we add a new
function or add a parameter to the existing one, code will have to change
either way. The advantage of the former scheme, adding the new function,

is

quoted

that it shows that we are serious about our ABI/API compatibility process,
and are not lax about passing exceptions when other options are available.

quoted

However, if the tech board feels like adding a new API is a good solution,
then okay, but we need to flesh out roadmap a bit better. Do we rename

the

quoted

quoted

old API, or do we add a temporary new API?

New API added, old API deprecated. In future old API goes away leaving

new

quoted

API as the only option.

/Bruce

Okay, so it's settled then. I revoke my ack for this patch, and we need
a new deprecation notice.

A new depreciation notice was sent [1], targeting for API change in DPDK-22.02.
For the unmapping issue mentioned before, we developed a compromised solution
to optimize the partial unmap logic in DPDK-21.11, and it is compatible with current
API.

[1] https://mails.dpdk.org/archives/dev/2021-August/217802.html

Thanks for your suggestion and support!

Regards,
Xuan

--
Thanks,
Anatoly