From: "Govindharajan, Hariprasad" <hariprasad.govindharajan@intel.com> To: "Burakov, Anatoly" <anatoly.burakov@intel.com>, "Lu, Wenzhuo" <wenzhuo.lu@intel.com>, "Wu, Jingjing" <jingjing.wu@intel.com>, "Iremonger, Bernard" <bernard.iremonger@intel.com>, "Mcnamara, John" <john.mcnamara@intel.com>, "Kovacevic, Marko" <marko.kovacevic@intel.com> Cc: "dev@dpdk.org" <dev@dpdk.org>, "Yigit, Ferruh" <ferruh.yigit@intel.com>, "stephen@networkplumber.org" <stephen@networkplumber.org>, "david.marchand@redhat.com" <david.marchand@redhat.com> Subject: Re: [dpdk-dev] [PATCH v8] app/testpmd: add portlist option Date: Wed, 12 Feb 2020 10:25:40 +0000 Message-ID: <BYAPR11MB264800987B30F2F0C4D84D32841B0@BYAPR11MB2648.namprd11.prod.outlook.com> (raw) In-Reply-To: <685ff0d1-750c-3d10-4f27-fb3523f92ba1@intel.com> > -----Original Message----- > From: Burakov, Anatoly <anatoly.burakov@intel.com> > Sent: Tuesday, February 11, 2020 4:52 PM > To: Govindharajan, Hariprasad <hariprasad.govindharajan@intel.com>; Lu, > Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; > Iremonger, Bernard <bernard.iremonger@intel.com>; Mcnamara, John > <john.mcnamara@intel.com>; Kovacevic, Marko > <marko.kovacevic@intel.com> > Cc: dev@dpdk.org; Yigit, Ferruh <ferruh.yigit@intel.com>; > stephen@networkplumber.org; david.marchand@redhat.com > Subject: Re: [dpdk-dev] [PATCH v8] app/testpmd: add portlist option > > On 11-Feb-20 3:52 PM, Hariprasad Govindharajan wrote: > > In current version, we are setting the ports using portmask. With > > portmask, we can use only upto 64 ports. This portlist option enables > > the user to use more than 64 ports. > > Now we can specify the ports in 2 different ways > > - Using portmask (-p [0x]nnn): mask must be in hex format > > - Using portlist in the following format > > --portlist <p1>[-p2][,p3[-p4],...] > > > > --portmask 0x2 is same as --portlist 1 > > --portmask 0x3 is same as --portlist 0-1 > > > > Signed-off-by: Hariprasad Govindharajan > > <hariprasad.govindharajan@intel.com> > > --- > > v8: > > changed the data types of the variables. > > optimised the code by checking for blank spaces only once. > > > > v7: > > moved the port validation outside the parser function. > > added meaningful comments describing the new functionality. > > renamed the variables with meaningful names > > > > v6: > > optimized the code to check for duplicates > > > > v5: > > added a check to validate the ports available before setting them. > > also added comments in the testpmd file for the new function > > > > v4: > > the parser is modified so that we don't ues 2 arrays to convert the > > listed port values > > > > v3: > > squashed the 2 patches and made it 1 patch with changes only in > > testpmd. Also working on optmizing the parser > > > > v2: > > moved the parser function to testpmd > > --- > > app/test-pmd/config.c | 108 > ++++++++++++++++++++++++++++++++++ > > app/test-pmd/parameters.c | 5 ++ > > app/test-pmd/testpmd.h | 3 + > > doc/guides/testpmd_app_ug/run_app.rst | 7 +++ > > 4 files changed, 123 insertions(+) > > > > diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index > > 9669cbd..86566d9 100644 > > --- a/app/test-pmd/config.c > > +++ b/app/test-pmd/config.c > > @@ -2587,6 +2587,114 @@ set_fwd_ports_list(unsigned int *portlist, > unsigned int nb_pt) > > } > > } > > > > +/** > > + * Parse the user input and obtain the list of forwarding ports > > + * > > + * @param[in] list > > + * String containing the user input. User can specify > > + * in these formats 1,3,5 or 1-3 or 1-2,5 or 3,5-6. > > + * For example, if the user wants to use all the available > > + * 4 ports in his system, then the input can be 0-3 or 0,1,2,3. > > + * If the user wants to use only the ports 1,2 then the input > > + * is 1,2. > > + * valid characters are '-' and ',' > > + * invalid chars like '.' or '#' will result in > > + * EAL: Error - exiting with code: 1 > > + * Cause: Invalid fwd port list > > + * @param[out] values > > + * This array will be filled with a list of port IDs > > + * based on the user input > > + * Note that duplicate entries are discarded and only the first > > + * count entries in this array are port IDs and all the rest > > + * will contain default values > > + * @param[in] maxsize > > + * This parameter denotes 2 things > > + * 1) Maximum size of the values array > > + * 2) Maximum value of each element in the values array > > I still suspect the first item should say "number", not size. The 2) takes care of > how big each individual value is, and 1) presumably takes care of how many > of these values there can be. Therefore i think it should be "number" (as in > how many), not "size" (as in how big). > [Govindharajan, Hariprasad] Changed > > + * @return > > + * -returns total count of parsed port IDs > > + */ > > +static unsigned int > > +parse_port_list(const char *list, unsigned int *values, unsigned int > > +maxsize) { > > + unsigned int count = 0; > > + char *end = NULL; > > + int min, max; > > + int value, i; > > + unsigned int marked[maxsize]; > > + > > + if (list == NULL || values == NULL) > > + return -1; > > + > > + for (i = 0; i < (int)maxsize; i++) > > + marked[i] = 0; > > Then memset(), but that's just nitpicking, so feel free to disregard :) [Govindharajan, Hariprasad] It is disregarded.. 😊 > > > + > > + min = INT_MAX; > > + > > + do { > > + /*Remove the blank spaces if any*/ > > + while (*list != '\0' && isblank(*list)) > > + list++; > > My apologies. I've just checked if isblank() returns 0 on '\0', and it does. So, > the `*list != '\0'` check is not necessary here after all. [Govindharajan, Hariprasad] Removed the isblank check > > > + if (*list == '\0') > > + break; > > + errno = 0; > > + value = strtol(list, &end, 10); > > + if (errno || end == NULL) > > + return 0; > > + if (value < 0 || value >= (int)maxsize) > > + return 0; > > + while (isblank(*end)) > > + end++; > > + if (*end == '-') { > > + min = value; > > This would accept input such as "1-2-3" and parse it as "2-3". Maybe > > if (*end == '-' && min == INT_MAX) > > ? This would then fall through to the failure path if end was '-' and min was > already set. > [Govindharajan, Hariprasad] corrected. > > -- > Thanks, > Anatoly
next prev parent reply other threads:[~2020-02-12 10:25 UTC|newest] Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-01-27 10:30 [dpdk-dev] [PATCH 0/2] app/testpmd: add portlist option to the testpmd Hariprasad Govindharajan 2020-01-27 10:30 ` [dpdk-dev] [PATCH 1/2] " Hariprasad Govindharajan 2020-01-31 16:22 ` [dpdk-dev] [PATCH v2] " Hariprasad Govindharajan 2020-01-31 17:41 ` [dpdk-dev] [PATCH v3] " Hariprasad Govindharajan 2020-02-03 16:53 ` [dpdk-dev] [PATCH v4] " Hariprasad Govindharajan 2020-02-04 16:48 ` [dpdk-dev] [PATCH v5] " Hariprasad Govindharajan 2020-02-05 10:42 ` Ferruh Yigit 2020-02-06 15:03 ` [dpdk-dev] [PATCH v6] " Hariprasad Govindharajan 2020-02-10 17:19 ` [dpdk-dev] [PATCH v7] app/testpmd: add portlist option Hariprasad Govindharajan 2020-02-11 12:00 ` Burakov, Anatoly 2020-02-11 15:37 ` Govindharajan, Hariprasad 2020-02-11 15:52 ` [dpdk-dev] [PATCH v8] " Hariprasad Govindharajan 2020-02-11 16:51 ` Burakov, Anatoly 2020-02-12 10:25 ` Govindharajan, Hariprasad [this message] 2020-02-12 10:29 ` [dpdk-dev] [PATCH v9] " Hariprasad Govindharajan 2020-02-12 13:03 ` Burakov, Anatoly 2020-02-12 13:59 ` Ferruh Yigit 2020-02-12 17:27 ` Lipiec, Herakliusz 2020-02-13 11:31 ` Ferruh Yigit 2020-01-27 10:30 ` [dpdk-dev] [PATCH 2/2] eal: add eal_parse_optionlist to parse user input Hariprasad Govindharajan 2020-01-28 17:35 ` Ferruh Yigit 2020-01-29 17:44 ` David Marchand 2020-01-29 18:07 ` Ferruh Yigit 2020-01-29 19:19 ` Stephen Hemminger 2020-01-31 11:25 ` Govindharajan, Hariprasad 2020-01-31 14:42 ` Govindharajan, Hariprasad
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=BYAPR11MB264800987B30F2F0C4D84D32841B0@BYAPR11MB2648.namprd11.prod.outlook.com \ --to=hariprasad.govindharajan@intel.com \ --cc=anatoly.burakov@intel.com \ --cc=bernard.iremonger@intel.com \ --cc=david.marchand@redhat.com \ --cc=dev@dpdk.org \ --cc=ferruh.yigit@intel.com \ --cc=jingjing.wu@intel.com \ --cc=john.mcnamara@intel.com \ --cc=marko.kovacevic@intel.com \ --cc=stephen@networkplumber.org \ --cc=wenzhuo.lu@intel.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
DPDK patches and discussions This inbox may be cloned and mirrored by anyone: git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \ dev@dpdk.org public-inbox-index dev Example config snippet for mirrors. Newsgroup available over NNTP: nntp://inbox.dpdk.org/inbox.dpdk.dev AGPL code for this site: git clone https://public-inbox.org/public-inbox.git