DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Burakov, Anatoly" <anatoly.burakov@intel.com>
To: Hariprasad Govindharajan <hariprasad.govindharajan@intel.com>,
	Wenzhuo Lu <wenzhuo.lu@intel.com>,
	Jingjing Wu <jingjing.wu@intel.com>,
	Bernard Iremonger <bernard.iremonger@intel.com>,
	John McNamara <john.mcnamara@intel.com>,
	Marko Kovacevic <marko.kovacevic@intel.com>
Cc: dev@dpdk.org, ferruh.yigit@intel.com, stephen@networkplumber.org,
	david.marchand@redhat.com
Subject: Re: [dpdk-dev] [PATCH v8] app/testpmd: add portlist option
Date: Tue, 11 Feb 2020 16:51:31 +0000
Message-ID: <685ff0d1-750c-3d10-4f27-fb3523f92ba1@intel.com> (raw)
In-Reply-To: <1581436375-16910-1-git-send-email-hariprasad.govindharajan@intel.com>

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).

> + * @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 :)

> +
> +	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.

> +		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.


-- 
Thanks,
Anatoly

  reply	other threads:[~2020-02-11 16:51 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 [this message]
2020-02-12 10:25       ` Govindharajan, Hariprasad
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=685ff0d1-750c-3d10-4f27-fb3523f92ba1@intel.com \
    --to=anatoly.burakov@intel.com \
    --cc=bernard.iremonger@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=hariprasad.govindharajan@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