> -----Original Message----- > From: Stephen Hemminger <step...@networkplumber.org> > Sent: Wednesday, January 29, 2020 7:20 PM > To: Yigit, Ferruh <ferruh.yi...@intel.com> > Cc: David Marchand <david.march...@redhat.com>; Thomas Monjalon > <tho...@monjalon.net>; Govindharajan, Hariprasad > <hariprasad.govindhara...@intel.com>; dev <dev@dpdk.org> > Subject: Re: [dpdk-dev] [PATCH 2/2] eal: add eal_parse_optionlist to parse > user input > > On Wed, 29 Jan 2020 18:07:05 +0000 > Ferruh Yigit <ferruh.yi...@intel.com> wrote: > > > On 1/29/2020 5:44 PM, David Marchand wrote: > > > On Tue, Jan 28, 2020 at 6:35 PM Ferruh Yigit <ferruh.yi...@intel.com> > wrote: > > >> On 1/27/2020 10:30 AM, Hariprasad Govindharajan wrote: > > >>> In current version, there is a function which parses the corelist > > >>> based on user value. A new generic function eal_parse_optionlist > > >>> is added which will parse corelist as well as similar user input > > >>> so that we can use it as a public API too. > > >>> > > >>> Signed-off-by: Hariprasad Govindharajan > > >>> <hariprasad.govindhara...@intel.com> > > >> > > >> Hi David, > > >> > > >> Overall this patchset is to add '--portlist' command to testpmd and > > >> remove existing 64 port limitation. > > >> > > >> And in this patch re-uses the exiting parser function in eal and > > >> converts it to API, question is if eal is good place to have this API, > > >> what > do you think about it? > > > > > > Exporting string parsers from the EAL has little value. > > > Ok we avoid code duplication (and I can see other places in the tree > > > where it might be used), but in the end we will have to maintain > > > this API in the ABI when it enters the stable ABI. > > > > > > I am for avoiding this. > > > > > > > > > > The same function can be used in some sample applications too (which > > are using port mask), so instead of duplicating it multiple times, it > > would be nice to have this function somewhere that applications can use. > > > > Does it makes sense to have a rte_util.c (under in eal or as a > > separate library) to have this kind of application helper functions? > > It makes sense to have a rte library that handles arbitrary size bitvector and > has string handling functions. Kind of like what kernel has for the cpuset > parsing. This could be used for cpus in EAL and port-masks or other arrays in > applications. > > But just doing copy/paste of existing code without thinking about how API > should work is a bad idea.
[Govindharajan, Hariprasad] Hi, PLEASE IGNORE MY PREVIOUS EMAIL. I am planning to move the existing parser function to the testpmd file instead of keeping it in the eal and will revert the eal back. Also, I am planning to create an util file in eal with this parser and do a RFC. @Stephen Hemminger, We already investigated the existing function and then decided to re-use it as seen in the patch. For creating an API, is there any other specific requirements that should be addressed? Please clarify us. Thanks G Hariprasad
