Hi Olivier, Andrew, Please share your thoughts/comments on below email.
> -----Original Message----- > From: Vamsi Krishna Attunuru > Sent: Monday, October 21, 2019 8:08 PM > To: olivier.m...@6wind.com; Andrew Rybchenko > <arybche...@solarflare.com> > 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; Ferruh Yigit > <ferruh.yi...@intel.com>; dev@dpdk.org > Subject: RE: [EXT] Re: [dpdk-dev] [PATCH v11 2/4] eal: add legacy kni option > > 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.
