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. Regards A Vamsi > -----Original Message----- > From: Ferruh Yigit <ferruh.yi...@intel.com> > Sent: Monday, October 21, 2019 7:02 PM > To: Vamsi Krishna Attunuru <vattun...@marvell.com>; dev@dpdk.org > Cc: tho...@monjalon.net; Jerin Jacob Kollanukkaran <jer...@marvell.com>; > Kiran Kumar Kokkilagadda <kirankum...@marvell.com>; > olivier.m...@6wind.com; anatoly.bura...@intel.com; > arybche...@solarflare.com; step...@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 <ferruh.yi...@intel.com> > >> Sent: Monday, October 21, 2019 5:25 PM > >> To: Vamsi Krishna Attunuru <vattun...@marvell.com>; dev@dpdk.org > >> Cc: tho...@monjalon.net; Jerin Jacob Kollanukkaran > >> <jer...@marvell.com>; Kiran Kumar Kokkilagadda > >> <kirankum...@marvell.com>; olivier.m...@6wind.com; > >> anatoly.bura...@intel.com; arybche...@solarflare.com; > >> step...@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, vattun...@marvell.com wrote: > >>> From: Vamsi Attunuru <vattun...@marvell.com> > >>> > >>> 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 <vattun...@marvell.com> > >>> Suggested-by: Ferruh Yigit <ferruh.yi...@intel.com> > >> > >> <...> > >> > >>> 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.
