[dpdk-dev] [EXT] Re: [PATCH v11 2/4] eal: add legacy kni option

[Vamsi Krishna Attunuru]

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.yigit at intel.com>
> Sent: Monday, October 21, 2019 7:02 PM
> To: Vamsi Krishna Attunuru <vattunuru at marvell.com>; dev at dpdk.org
> Cc: thomas at monjalon.net; Jerin Jacob Kollanukkaran <jerinj at marvell.com>;
> Kiran Kumar Kokkilagadda <kirankumark at marvell.com>;
> olivier.matz at 6wind.com; anatoly.burakov at intel.com;
> arybchenko at solarflare.com; stephen at 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.yigit at intel.com>
> >> Sent: Monday, October 21, 2019 5:25 PM
> >> To: Vamsi Krishna Attunuru <vattunuru at marvell.com>; dev at dpdk.org
> >> Cc: thomas at monjalon.net; Jerin Jacob Kollanukkaran
> >> <jerinj at marvell.com>; Kiran Kumar Kokkilagadda
> >> <kirankumark at marvell.com>; olivier.matz at 6wind.com;
> >> anatoly.burakov at intel.com; arybchenko at solarflare.com;
> >> stephen at 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 at marvell.com wrote:
> >>> From: Vamsi Attunuru <vattunuru at 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 <vattunuru at marvell.com>
> >>> Suggested-by: Ferruh Yigit <ferruh.yigit at 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.