From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id BF609A328D for ; Tue, 22 Oct 2019 14:29:14 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 4DC251BECA; Tue, 22 Oct 2019 14:29:14 +0200 (CEST) Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [148.163.129.52]) by dpdk.org (Postfix) with ESMTP id 544141BEC5 for ; Tue, 22 Oct 2019 14:29:12 +0200 (CEST) X-Virus-Scanned: Proofpoint Essentials engine Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by mx1-us1.ppe-hosted.com (PPE Hosted ESMTP Server) with ESMTPS id E139D10007A; Tue, 22 Oct 2019 12:29:09 +0000 (UTC) Received: from [192.168.38.17] (91.220.146.112) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Tue, 22 Oct 2019 13:29:01 +0100 To: Vamsi Krishna Attunuru , "olivier.matz@6wind.com" CC: "thomas@monjalon.net" , Jerin Jacob Kollanukkaran , Kiran Kumar Kokkilagadda , "anatoly.burakov@intel.com" , "stephen@networkplumber.org" , Ferruh Yigit , "dev@dpdk.org" References: <20190816061252.17214-1-vattunuru@marvell.com> <20191021080324.10659-1-vattunuru@marvell.com> <20191021080324.10659-3-vattunuru@marvell.com> <4bd1acf5-2da2-b2da-2b0c-7ee243d5aeb9@intel.com> <77f8eaf0-52ca-1295-973d-c8085f7b7736@intel.com> From: Andrew Rybchenko Message-ID: <08c426d1-6fc9-1c3f-02d4-8632a8e3c337@solarflare.com> Date: Tue, 22 Oct 2019 15:28:58 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-GB X-Originating-IP: [91.220.146.112] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-12.5.0.1300-8.5.1010-24994.003 X-TM-AS-Result: No-3.583500-8.000000-10 X-TMASE-MatchedRID: hwsFDxVJcFnmLzc6AOD8DfHkpkyUphL9q5U6sO8HwtMm3tlnC/E6VNQ2 78bCBeruAqpHmtGPhFN+lHqSVvyRgJbVgMuuPJP8tGM0cBDUM432X2nyY2WSCXrjo3X9e+DHXWj vA8TpWFgtL39QFW9vMBjiwFiGJGqibFP2eHvWcMA3X0+M8lqGUoeAPCkZqxntB/FMznsE8cOpqX HGFXvCOOdRkWB60/F/HVEh/7RYbw64WGh1vUSL/zCMW7zNwFaInKpQna4coUBTHhSneq8wp9WZD ZFp9sK+IcLBcXc5tzEc+7/rXCZH5CZ7yAgScddfwY28o+cGA5rHSOYUp9HFyANZ45IRIZKg7Anu 4Jv00sJF4L1kDjGn7ZsN1FAz2iw4jrFE/BBOzi8Sq+XFWzaAylG+BHSGRsbgC/U4++8MvOw9o/z fJjIK5C9s/iTjpBqs0p9wEhtW6NzuxoIYPSP14G6HurDH4PpPElLH7tCb90ourUcwuzZNE09P9E Rudm7OsljE9ujVfaIpCTmwclB0j2t5A6iSzaOuhUy0TABax1wEa8g1x8eqF8bJavo8UnfRAlsZc XuE2WiHb83Lc+sQhQCUsM5+RthrYKZZxkxcUv8qkSeDPauzr+DTYjejIZTwlv7ccf/HfoH4LEtl 9KUU0oc+FkrvgAdlkZ52zqoz5uL51WNA1rE6t6mukiZOfPi2a2pCAnJvQFGX5+55DMJT0yzybVq WyY2NoifFN1WHISOAXYJTrU0+pE1+zyfzlN7ygxsfzkNRlfKTzVw79BoLWCXVtBmyOiKujoczmu oPCq3zf5vsv4csT5/jhSiZm5Jo7qiCsCxpYeB+nXBJm9YhY8nWoHwogD8yfnRMvKfUnB4I9tTAB jXkgB4QYOLEHsPGmV6ww2ANp/Ti+fTMx9KaNitss6PUa4/cD6GAt+UbooSj1CO4X0Eqed8x3qzo MNxx X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--3.583500-8.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-24994.003 X-MDID: 1571747351-fYanWNuGmr_d Subject: Re: [dpdk-dev] [EXT] Re: [PATCH v11 2/4] eal: add legacy kni option X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 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" Hi Vasmi, On 10/21/19 5:38 PM, Vamsi Krishna Attunuru wrote: > Hi Olivier, Andrew, > >>>>> +#define OPT_LEGACY_KNI "legacy-kni" >>>>> + OPT_LEGACY_KNI_NUM, >>>>> OPT_LONG_MAX_NUM >>>>> }; >>>> Two concerns, >>>> >>>> 1- "legacy-kni" doesn't have enough context >>>> >>>> 2- I prefer to keep existing behavior default, at least for a while, >>>> something like next LTS etc, meanwhile this patch can be around for a >>>> good time and can be good to switch. >>>> >>>> Based on above to, what do you think to rename the option to >>>> 'kni-iova-va', if not set by default it will be "IOVA=PA", when set it will >> enable "IOVA=VA" >>>> mode? >>> Hi Ferruh, >>> >>> I think the new eal flag(legacy-kni) is quite intuitive. Since release notes will >> be having the required details about it's purpose and how it enables users to >> use existing applications on latest dpdk. >> >> what exactly 'legacy' means, what has been changed, is the old one >> completely replaced/re-written ????, but whoever not following what is >> happening won't underase tand what is old in the KNI, digging through >> release notes and commits will give this information but it will be hard to get >> it from binary and get harder by a few releases passed. >> >>> Current EAL does set iova as va if bus iommu returns DC, meaning iova=va >> is the kind of default mode(in most of use cases) and for running kni, we >> have to explicitly set the flag to run kni in iova=va mode all the time. I think >> having a flag for legacy usage(PA mode) is more appropriate than having kni- >> iova-va kind of flag. >> >> It is about keeping the existing behavior same, right now if the kni module is >> inserted it will force the PA mode. With your update it will be possible to run >> iova=va with kni module inserted when flag is set. I suggest giving some time >> to this new behavior before making it default. >> >>> Otx2 net pmd that runs on Octeontx2 platforms only supports iova=va >> mode, we would like to have KNI running by default without any flags >> passed. >> I see, but other way around will affect all existing KNI users, they will either >> need to add this flag or update their application. >> >> This is new feature, who want to use it adding a specific flag makes more >> sense to me than all old users have to add the flag. > > Ferruh suggested to have a flag for enabling these new feature and also not interested in having newer mempool alloc APIs for KNI(see V10 review comments). Before reworking on the flag changes, I would like check with you whether the same flag can be used in mempool lib for checking and fulfilling the mempool page boundary requirement (mempool patch v11 1/4), by doing so, it can avoid newer exported APIs both in mempool and KNI lib. Anyways, these mempool requirement can be addressed with Olivier's below patches. > > http://patchwork.dpdk.org/project/dpdk/list/?series=5624 > > When those patches are merged, flag check can be removed. It is really hard to follow, sorry. Is it really a problem to have KNI switcher to enable new behaviour and document it that dedicated function should be used to allocate mbufs? Andrew. > Regards > A Vamsi > > > > > > >> -----Original Message----- >> From: Ferruh Yigit >> Sent: Monday, October 21, 2019 7:02 PM >> To: Vamsi Krishna Attunuru ; dev@dpdk.org >> Cc: thomas@monjalon.net; Jerin Jacob Kollanukkaran ; >> Kiran Kumar Kokkilagadda ; >> olivier.matz@6wind.com; anatoly.burakov@intel.com; >> arybchenko@solarflare.com; stephen@networkplumber.org >> Subject: Re: [EXT] Re: [dpdk-dev] [PATCH v11 2/4] eal: add legacy kni option >> >> On 10/21/2019 2:13 PM, Vamsi Krishna Attunuru wrote: >>> >>>> -----Original Message----- >>>> From: Ferruh Yigit >>>> Sent: Monday, October 21, 2019 5:25 PM >>>> To: Vamsi Krishna Attunuru ; dev@dpdk.org >>>> Cc: thomas@monjalon.net; Jerin Jacob Kollanukkaran >>>> ; Kiran Kumar Kokkilagadda >>>> ; olivier.matz@6wind.com; >>>> anatoly.burakov@intel.com; arybchenko@solarflare.com; >>>> stephen@networkplumber.org >>>> Subject: [EXT] Re: [dpdk-dev] [PATCH v11 2/4] eal: add legacy kni >>>> option >>>> >>>> External Email >>>> >>>> --------------------------------------------------------------------- >>>> - On 10/21/2019 9:03 AM, vattunuru@marvell.com wrote: >>>>> From: Vamsi Attunuru >>>>> >>>>> This adds a "--legacy-kni" command-line option. It will be used to >>>>> run existing KNI applications with DPDK 19.11 and later. >>>>> >>>>> Signed-off-by: Vamsi Attunuru >>>>> Suggested-by: Ferruh Yigit >>>> <...> >>>> >>>>> diff --git a/lib/librte_eal/common/eal_options.h >>>>> b/lib/librte_eal/common/eal_options.h >>>>> index 9855429..1010ed3 100644 >>>>> --- a/lib/librte_eal/common/eal_options.h >>>>> +++ b/lib/librte_eal/common/eal_options.h >>>>> @@ -69,6 +69,8 @@ enum { >>>>> OPT_IOVA_MODE_NUM, >>>>> #define OPT_MATCH_ALLOCATIONS "match-allocations" >>>>> OPT_MATCH_ALLOCATIONS_NUM, >>>>> +#define OPT_LEGACY_KNI "legacy-kni" >>>>> + OPT_LEGACY_KNI_NUM, >>>>> OPT_LONG_MAX_NUM >>>>> }; >>>> Two concerns, >>>> >>>> 1- "legacy-kni" doesn't have enough context >>>> >>>> 2- I prefer to keep existing behavior default, at least for a while, >>>> something like next LTS etc, meanwhile this patch can be around for a >>>> good time and can be good to switch. >>>> >>>> Based on above to, what do you think to rename the option to >>>> 'kni-iova-va', if not set by default it will be "IOVA=PA", when set it will >> enable "IOVA=VA" >>>> mode? >>> Hi Ferruh, >>> >>> I think the new eal flag(legacy-kni) is quite intuitive. Since release notes will >> be having the required details about it's purpose and how it enables users to >> use existing applications on latest dpdk. >> >> what exactly 'legacy' means, what has been changed, is the old one >> completely replaced/re-written ????, but whoever not following what is >> happening won't underase tand what is old in the KNI, digging through >> release notes and commits will give this information but it will be hard to get >> it from binary and get harder by a few releases passed. >> >>> Current EAL does set iova as va if bus iommu returns DC, meaning iova=va >> is the kind of default mode(in most of use cases) and for running kni, we >> have to explicitly set the flag to run kni in iova=va mode all the time. I think >> having a flag for legacy usage(PA mode) is more appropriate than having kni- >> iova-va kind of flag. >> >> It is about keeping the existing behavior same, right now if the kni module is >> inserted it will force the PA mode. With your update it will be possible to run >> iova=va with kni module inserted when flag is set. I suggest giving some time >> to this new behavior before making it default. >> >>> Otx2 net pmd that runs on Octeontx2 platforms only supports iova=va >> mode, we would like to have KNI running by default without any flags >> passed. >> I see, but other way around will affect all existing KNI users, they will either >> need to add this flag or update their application. >> >> This is new feature, who want to use it adding a specific flag makes more >> sense to me than all old users have to add the flag.