From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 79305A0C4C; Thu, 2 Sep 2021 18:13:29 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 01D8440041; Thu, 2 Sep 2021 18:13:29 +0200 (CEST) Received: from mail-108-mta11.mxroute.com (mail-108-mta11.mxroute.com [136.175.108.11]) by mails.dpdk.org (Postfix) with ESMTP id 5BC724003E for ; Thu, 2 Sep 2021 18:13:27 +0200 (CEST) Received: from filter004.mxroute.com ([149.28.56.236] filter004.mxroute.com) (Authenticated sender: mN4UYu2MZsgR) by mail-108-mta11.mxroute.com (ZoneMTA) with ESMTPSA id 17ba749493400074ba.001 for (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES128-GCM-SHA256); Thu, 02 Sep 2021 16:13:23 +0000 X-Zone-Loop: 651784a97b231515559010616c6b40f82fa16295f953 X-Originating-IP: [149.28.56.236] DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=ashroe.eu; s=x; h=Content-Transfer-Encoding:Content-Type:In-Reply-To:MIME-Version:Date: Message-ID:From:References:Cc:To:Subject:Sender:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=jvMcMCMNIBMxhsBa8DRyMN8ftF2/uEGIlieREMFh3+0=; b=qie+1XfrB9H2yZIqaUJ9F3cKaG I+FC7COSQAwcZ6bfTBef2a4Ato8honMUFcVyNgEAPB86r35f+IS9gbDw2gwEInV2BUcPlyQqWH808 jOZqiOydZzMohOaJylRwDbSmHSXBmG1QlbiyqLwS4EskgEJFxupbPcz5R8URG4H9ia839zo6HLVQh jRMa4C2HQEontXPlJKom7Hy02+iU3DxhdbXIZvGSI+w9Ih5DsUCCxM7V6wR6nXB/LbNQMvc1xX3/n 38lcJWHJOI0RkRD5DxjJEdpUb+qakgLgn49U2enyOkKWrki+qtuM4rbJaasTtzhSDgelgxP7ZZqzG r6i+Fjcw==; To: Ferruh Yigit , "Burakov, Anatoly" , "Ding, Xuan" , "dev@dpdk.org" Cc: "maxime.coquelin@redhat.com" , "Xia, Chenbo" , "Hu, Jiayu" , "Richardson, Bruce" , Thomas Monjalon References: <20210831131039.29964-1-xuan.ding@intel.com> <2c9fc397-df61-2ef6-0cce-db5dbbb1c952@intel.com> <926d7065-481f-b84e-1d05-bdeebd31d6ea@intel.com> <032e666a-9ada-f4c0-36bc-ad9b5fac4359@intel.com> <8779d165-6e98-a294-9f4b-a6ce76d9c77a@intel.com> <0998ae41-2c0d-c5f6-806c-50ccbcaf2139@intel.com> From: "Kinsella, Ray" Message-ID: <695fb104-533f-a9cd-42b0-953769ebf7c9@ashroe.eu> Date: Thu, 2 Sep 2021 17:13:18 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: <0998ae41-2c0d-c5f6-806c-50ccbcaf2139@intel.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-AuthUser: mdr@ashroe.eu Subject: Re: [dpdk-dev] [PATCH] doc: announce change in vfio dma mapping X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 >>>>>>> Sent: Wednesday, September 1, 2021 12:01 AM >>>>>>> To: Ding, Xuan ; dev@dpdk.org; Burakov, Anatoly >>>>>>> >>>>>>> Cc: maxime.coquelin@redhat.com; Xia, Chenbo ; Hu, >>>>>>> Jiayu ; Richardson, Bruce >>>>>>> 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 >>>>>>>> --- >>>>>>>>    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 >