[dpdk-dev,v1,02/15] eal: new eal option '--lcores' for cpu assignment

  It supports one new eal long option '--lcores' for EAL thread cpuset assignment.

The format pattern:
	--lcores='lcores[@cpus]<,lcores[@cpus]>'
lcores, cpus could be a single digit or a group.
'(' and ')' are necessary if it's a group.
If not supply '@cpus', the value of cpus uses the same as lcores.

e.g. '1,2@(5-7),(3-5)@(0,2),(0,6)' means starting 7 EAL thread as below
  lcore 0 runs on cpuset 0x41 (cpu 0,6)
  lcore 1 runs on cpuset 0x2 (cpu 1)
  lcore 2 runs on cpuset 0xe0 (cpu 5,6,7)
  lcore 3,4,5 runs on cpuset 0x5 (cpu 0,2)
  lcore 6 runs on cpuset 0x41 (cpu 0,6)

Signed-off-by: Cunming Liang <cunming.liang@intel.com>
---
 lib/librte_eal/common/eal_common_launch.c  |   1 -
 lib/librte_eal/common/eal_common_options.c | 262 ++++++++++++++++++++++++++++-
 lib/librte_eal/common/eal_options.h        |   2 +
 lib/librte_eal/linuxapp/eal/Makefile       |   1 +
 4 files changed, 261 insertions(+), 5 deletions(-)
  

On Thu, Jan 22, 2015 at 04:16:25PM +0800, Cunming Liang wrote:
> It supports one new eal long option '--lcores' for EAL thread cpuset assignment.
> 
> The format pattern:
> 	--lcores='lcores[@cpus]<,lcores[@cpus]>'
> lcores, cpus could be a single digit or a group.
> '(' and ')' are necessary if it's a group.
> If not supply '@cpus', the value of cpus uses the same as lcores.
> 
> e.g. '1,2@(5-7),(3-5)@(0,2),(0,6)' means starting 7 EAL thread as below
>   lcore 0 runs on cpuset 0x41 (cpu 0,6)
>   lcore 1 runs on cpuset 0x2 (cpu 1)
>   lcore 2 runs on cpuset 0xe0 (cpu 5,6,7)
>   lcore 3,4,5 runs on cpuset 0x5 (cpu 0,2)
>   lcore 6 runs on cpuset 0x41 (cpu 0,6)
> 

This strikes me as very confusing, though a couple of tweaks might help with
readability. The lcore 0 at the end is especially confusing. Perhaps we can 
limit the allowed formats here,
* require the lcore_id to be specified - the lack of an lcore id for the last part
makes having it as lcore 0 surprising.
* only allow one lcore id to be given for each set of cores. 

I think it may still be readable if we allow the core set to be omitted if its
to be the same as the lcore_id.

It's probably still not going to be very tidy, but I think we can improve things.

/Bruce

> Signed-off-by: Cunming Liang <cunming.liang@intel.com>
> ---
>  lib/librte_eal/common/eal_common_launch.c  |   1 -
>  lib/librte_eal/common/eal_common_options.c | 262 ++++++++++++++++++++++++++++-
>  lib/librte_eal/common/eal_options.h        |   2 +
>  lib/librte_eal/linuxapp/eal/Makefile       |   1 +
>  4 files changed, 261 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/librte_eal/common/eal_common_launch.c b/lib/librte_eal/common/eal_common_launch.c
> index 599f83b..2d732b1 100644
> --- a/lib/librte_eal/common/eal_common_launch.c
> +++ b/lib/librte_eal/common/eal_common_launch.c
> @@ -117,4 +117,3 @@ rte_eal_mp_wait_lcore(void)
>  		rte_eal_wait_lcore(lcore_id);
>  	}
>  }
> -
> diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
> index e2810ab..fc47588 100644
> --- a/lib/librte_eal/common/eal_common_options.c
> +++ b/lib/librte_eal/common/eal_common_options.c
> @@ -45,6 +45,7 @@
>  #include <rte_lcore.h>
>  #include <rte_version.h>
>  #include <rte_devargs.h>
> +#include <rte_memcpy.h>
>  
>  #include "eal_internal_cfg.h"
>  #include "eal_options.h"
> @@ -85,6 +86,7 @@ eal_long_options[] = {
>  	{OPT_XEN_DOM0, 0, 0, OPT_XEN_DOM0_NUM},
>  	{OPT_CREATE_UIO_DEV, 1, NULL, OPT_CREATE_UIO_DEV_NUM},
>  	{OPT_VFIO_INTR, 1, NULL, OPT_VFIO_INTR_NUM},
> +	{OPT_LCORES, 1, 0, OPT_LCORES_NUM},
>  	{0, 0, 0, 0}
>  };
>  
> @@ -255,9 +257,11 @@ eal_parse_corelist(const char *corelist)
>  			if (min == RTE_MAX_LCORE)
>  				min = idx;
>  			for (idx = min; idx <= max; idx++) {
> -				cfg->lcore_role[idx] = ROLE_RTE;
> -				lcore_config[idx].core_index = count;
> -				count++;
> +				if (cfg->lcore_role[idx] != ROLE_RTE) {
> +					cfg->lcore_role[idx] = ROLE_RTE;
> +					lcore_config[idx].core_index = count;
> +					count++;
> +				}
>  			}
>  			min = RTE_MAX_LCORE;
>  		} else
> @@ -289,6 +293,241 @@ eal_parse_master_lcore(const char *arg)
>  	return 0;
>  }
>  
> +/*
> + * Parse elem, the elem could be single number or '(' ')' group
> + * Within group elem, '-' used for a range seperator;
> + *                    ',' used for a single number.
> + */
> +static int
> +eal_parse_set(const char *input, uint16_t set[], unsigned num)
> +{
> +	unsigned idx;
> +	const char *str = input;
> +	char *end = NULL;
> +	unsigned min, max;
> +
> +	memset(set, 0, num * sizeof(uint16_t));
> +
> +	while (isblank(*str))
> +		str++;
> +
> +	/* only digit or left bracket is qulify for start point */
> +	if ((!isdigit(*str) && *str != '(') || *str == '\0')
> +		return -1;
> +
> +	/* process single number */
> +	if (*str != '(') {
> +		errno = 0;
> +		idx = strtoul(str, &end, 10);
> +		if (errno || end == NULL || idx >= num)
> +			return -1;
> +		else {
> +			while (isblank(*end))
> +				end++;
> +
> +			if (*end != ',' && *end != '\0' &&
> +			    *end != '@')
> +				return -1;
> +
> +			set[idx] = 1;
> +			return end - input;
> +		}
> +	}
> +
> +	/* process set within bracket */
> +	str++;
> +	while (isblank(*str))
> +		str++;
> +	if (*str == '\0')
> +		return -1;
> +
> +	min = RTE_MAX_LCORE;
> +	do {
> +
> +		/* go ahead to the first digit */
> +		while (isblank(*str))
> +			str++;
> +		if (!isdigit(*str))
> +			return -1;
> +
> +		/* get the digit value */
> +		errno = 0;
> +		idx = strtoul(str, &end, 10);
> +		if (errno || end == NULL || idx >= num)
> +			return -1;
> +
> +		/* go ahead to separator '-',',' and ')' */
> +		while (isblank(*end))
> +			end++;
> +		if (*end == '-') {
> +			if (min == RTE_MAX_LCORE)
> +				min = idx;
> +			else /* avoid continuous '-' */
> +				return -1;
> +		} else if ((*end == ',') || (*end == ')')) {
> +			max = idx;
> +			if (min == RTE_MAX_LCORE)
> +				min = idx;
> +			for (idx = RTE_MIN(min, max);
> +			     idx <= RTE_MAX(min, max); idx++)
> +				set[idx] = 1;
> +
> +			min = RTE_MAX_LCORE;
> +		} else
> +			return -1;
> +
> +		str = end + 1;
> +	} while (*end != '\0' && *end != ')');
> +
> +	return str - input;
> +}
> +
> +/* convert from set array to cpuset bitmap */
> +static inline int
> +convert_to_cpuset(rte_cpuset_t *cpusetp,
> +	      uint16_t *set, unsigned num)
> +{
> +	unsigned idx;
> +
> +	CPU_ZERO(cpusetp);
> +
> +	for (idx = 0; idx < num; idx++) {
> +		if (!set[idx])
> +			continue;
> +
> +		if (!lcore_config[idx].detected) {
> +			RTE_LOG(ERR, EAL, "core %u "
> +				"unavailable\n", idx);
> +			return -1;
> +		}
> +
> +		CPU_SET(idx, cpusetp);
> +	}
> +
> +	return 0;
> +}
> +
> +/*
> + * The format pattern: --lcores='lcores[@cpus]<,lcores[@cpus]>'
> + * lcores, cpus could be a single digit or a group.
> + * '(' and ')' are necessary if it's a group.
> + * If not supply '@cpus', the value of cpus uses the same as lcores.
> + * e.g. '1,2@(5-7),(3-5)@(0,2),(0,6)' means start 7 EAL thread as below
> + *   lcore 0 runs on cpuset 0x41 (cpu 0,6)
> + *   lcore 1 runs on cpuset 0x2 (cpu 1)
> + *   lcore 2 runs on cpuset 0xe0 (cpu 5,6,7)
> + *   lcore 3,4,5 runs on cpuset 0x5 (cpu 0,2)
> + *   lcore 6 runs on cpuset 0x41 (cpu 0,6)
> + */
> +static int
> +eal_parse_lcores(const char *lcores)
> +{
> +	struct rte_config *cfg = rte_eal_get_configuration();
> +	static uint16_t set[RTE_MAX_LCORE];
> +	unsigned idx = 0;
> +	int i;
> +	unsigned count = 0;
> +	const char *lcore_start = NULL;
> +	const char *end = NULL;
> +	int offset;
> +	rte_cpuset_t cpuset;
> +	int ret = -1;
> +
> +	if (lcores == NULL)
> +		return -1;
> +
> +	/* Remove all blank characters ahead and after */
> +	while (isblank(*lcores))
> +		lcores++;
> +	i = strnlen(lcores, sysconf(_SC_ARG_MAX));
> +	while ((i > 0) && isblank(lcores[i - 1]))
> +		i--;
> +
> +	CPU_ZERO(&cpuset);
> +
> +	/* Reset lcore config */
> +	for (idx = 0; idx < RTE_MAX_LCORE; idx++) {
> +		cfg->lcore_role[idx] = ROLE_OFF;
> +		lcore_config[idx].core_index = -1;
> +		CPU_ZERO(&lcore_config[idx].cpuset);
> +	}
> +
> +	/* Get list of cores */
> +	do {
> +		while (isblank(*lcores))
> +			lcores++;
> +		if (*lcores == '\0')
> +			goto err;
> +
> +		/* record lcore_set start point */
> +		lcore_start = lcores;
> +
> +		/* go across a complete bracket */
> +		if (*lcore_start == '(') {
> +			lcores += strcspn(lcores, ")");
> +			if (*lcores++ == '\0')
> +				goto err;
> +		}
> +
> +		/* scan the separator '@', ','(next) or '\0'(finish) */
> +		lcores += strcspn(lcores, "@,");
> +
> +		if (*lcores == '@') {
> +			/* explict assign cpu_set */
> +			offset = eal_parse_set(lcores + 1, set, RTE_DIM(set));
> +			if (offset < 0)
> +				goto err;
> +
> +			/* prepare cpu_set and update the end cursor */
> +			if (0 > convert_to_cpuset(&cpuset,
> +						  set, RTE_DIM(set)))
> +				goto err;
> +			end = lcores + 1 + offset;
> +		} else  /* ',' or '\0' */
> +			/* haven't given cpu_set, current loop done */
> +			end = lcores;
> +
> +		if (*end != ',' && *end != '\0')
> +			goto err;
> +
> +		/* parse lcore_set from start point */
> +		if (0 > eal_parse_set(lcore_start, set, RTE_DIM(set)))
> +			goto err;
> +
> +		/* without '@', by default using lcore_set as cpu_set */
> +		if (*lcores != '@' &&
> +		    0 > convert_to_cpuset(&cpuset, set, RTE_DIM(set)))
> +			goto err;
> +
> +		/* start to update lcore_set */
> +		for (idx = 0; idx < RTE_MAX_LCORE; idx++) {
> +			if (!set[idx])
> +				continue;
> +
> +			if (cfg->lcore_role[idx] != ROLE_RTE) {
> +				lcore_config[idx].core_index = count;
> +				cfg->lcore_role[idx] = ROLE_RTE;
> +				count++;
> +			}
> +			rte_memcpy(&lcore_config[idx].cpuset, &cpuset,
> +				   sizeof(rte_cpuset_t));
> +		}
> +
> +		lcores = end + 1;
> +	} while (*end != '\0');
> +
> +	if (count == 0)
> +		goto err;
> +
> +	cfg->lcore_count = count;
> +	lcores_parsed = 1;
> +	ret = 0;
> +
> +err:
> +
> +	return ret;
> +}
> +
>  static int
>  eal_parse_syslog(const char *facility, struct internal_config *conf)
>  {
> @@ -489,6 +728,13 @@ eal_parse_common_option(int opt, const char *optarg,
>  		conf->log_level = log;
>  		break;
>  	}
> +	case OPT_LCORES_NUM:
> +		if (eal_parse_lcores(optarg) < 0) {
> +			RTE_LOG(ERR, EAL, "invalid parameter for --"
> +				OPT_LCORES "\n");
> +			return -1;
> +		}
> +		break;
>  
>  	/* don't know what to do, leave this to caller */
>  	default:
> @@ -527,7 +773,7 @@ eal_check_common_options(struct internal_config *internal_cfg)
>  
>  	if (!lcores_parsed) {
>  		RTE_LOG(ERR, EAL, "CPU cores must be enabled with options "
> -			"-c or -l\n");
> +			"-c, -l or --lcores\n");
>  		return -1;
>  	}
>  	if (cfg->lcore_role[cfg->master_lcore] != ROLE_RTE) {
> @@ -583,6 +829,14 @@ eal_common_usage(void)
>  	       "                 The argument format is <c1>[-c2][,c3[-c4],...]\n"
>  	       "                 where c1, c2, etc are core indexes between 0 and %d\n"
>  	       "  --"OPT_MASTER_LCORE" ID: Core ID that is used as master\n"
> +	       "  --"OPT_LCORES" MAP: maps between lcore_set to phys_cpu_set\n"
> +	       "                 The argument format is\n"
> +	       "                       'lcores[@cpus]<,lcores[@cpus],...>'\n"
> +	       "                 lcores and cpus list are grouped by '(' and ')'\n"
> +	       "                 Within the group, '-' is used for range separator,\n"
> +	       "                 ',' is used for single number separator.\n"
> +	       "                 '( )' can be omitted for single element group, '@' \n"
> +	       "                 can be omitted if cpus and lcores has the same value\n"
>  	       "  -n NUM       : Number of memory channels\n"
>  	       "  -v           : Display version information on startup\n"
>  	       "  -m MB        : memory to allocate (see also --"OPT_SOCKET_MEM")\n"
> diff --git a/lib/librte_eal/common/eal_options.h b/lib/librte_eal/common/eal_options.h
> index e476f8d..a1cc59f 100644
> --- a/lib/librte_eal/common/eal_options.h
> +++ b/lib/librte_eal/common/eal_options.h
> @@ -77,6 +77,8 @@ enum {
>  	OPT_CREATE_UIO_DEV_NUM,
>  #define OPT_VFIO_INTR    "vfio-intr"
>  	OPT_VFIO_INTR_NUM,
> +#define OPT_LCORES "lcores"
> +	OPT_LCORES_NUM,
>  	OPT_LONG_MAX_NUM
>  };
>  
> diff --git a/lib/librte_eal/linuxapp/eal/Makefile b/lib/librte_eal/linuxapp/eal/Makefile
> index 0e9c447..025d836 100644
> --- a/lib/librte_eal/linuxapp/eal/Makefile
> +++ b/lib/librte_eal/linuxapp/eal/Makefile
> @@ -95,6 +95,7 @@ CFLAGS_eal_hugepage_info.o := -D_GNU_SOURCE
>  CFLAGS_eal_pci.o := -D_GNU_SOURCE
>  CFLAGS_eal_pci_vfio.o := -D_GNU_SOURCE
>  CFLAGS_eal_common_whitelist.o := -D_GNU_SOURCE
> +CFLAGS_eal_common_options.o := -D_GNU_SOURCE
>  
>  # workaround for a gcc bug with noreturn attribute
>  # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12603
> -- 
> 1.8.1.4
>
  

Hi Bruce,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
> Sent: Thursday, January 22, 2015 12:19 PM
> To: Liang, Cunming
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v1 02/15] eal: new eal option '--lcores' for cpu assignment
> 
> On Thu, Jan 22, 2015 at 04:16:25PM +0800, Cunming Liang wrote:
> > It supports one new eal long option '--lcores' for EAL thread cpuset assignment.
> >
> > The format pattern:
> > 	--lcores='lcores[@cpus]<,lcores[@cpus]>'
> > lcores, cpus could be a single digit or a group.
> > '(' and ')' are necessary if it's a group.
> > If not supply '@cpus', the value of cpus uses the same as lcores.
> >
> > e.g. '1,2@(5-7),(3-5)@(0,2),(0,6)' means starting 7 EAL thread as below
> >   lcore 0 runs on cpuset 0x41 (cpu 0,6)
> >   lcore 1 runs on cpuset 0x2 (cpu 1)
> >   lcore 2 runs on cpuset 0xe0 (cpu 5,6,7)
> >   lcore 3,4,5 runs on cpuset 0x5 (cpu 0,2)
> >   lcore 6 runs on cpuset 0x41 (cpu 0,6)
> >
> 
> This strikes me as very confusing, though a couple of tweaks might help with
> readability. The lcore 0 at the end is especially confusing.

Didn't get you here: do you find (0,6) confusing, right?
Because braces implicitly specifies affinity for group of en-braced lcores? 

> Perhaps we can
> limit the allowed formats here,
> * require the lcore_id to be specified - the lack of an lcore id for the last part
> makes having it as lcore 0 surprising.

Again, not sure I understand you properly:  lcore_id(s) are always specified explicitly. 
Physical cpus part might be omitted.

> * only allow one lcore id to be given for each set of cores.

So you mean for '(3-5)@(0,2)' user would have to: '3@(0,2),4@(0,2),5@(0,2)'?
I don't see big difference here, but imagine you'd like to create a pool of 32 EAL-threads running on same cpu set.
With current syntax it is just something like: '(32-63)@(0-7)'.
With what you proposing it will be a very long list.  

> 
> I think it may still be readable if we allow the core set to be omitted if its
> to be the same as the lcore_id.

I think that is supported.
See lcore_id=1 in Steve's example above.
As I understand: --lcores='0,2,3-5' is equal to '-l 0,2,3-5' and to '-c 0x3d'.

Konstantin

> 
> It's probably still not going to be very tidy, but I think we can improve things.
> 
> /Bruce
> 
> > Signed-off-by: Cunming Liang <cunming.liang@intel.com>
> > ---
> >  lib/librte_eal/common/eal_common_launch.c  |   1 -
> >  lib/librte_eal/common/eal_common_options.c | 262 ++++++++++++++++++++++++++++-
> >  lib/librte_eal/common/eal_options.h        |   2 +
> >  lib/librte_eal/linuxapp/eal/Makefile       |   1 +
> >  4 files changed, 261 insertions(+), 5 deletions(-)
> >
> > diff --git a/lib/librte_eal/common/eal_common_launch.c b/lib/librte_eal/common/eal_common_launch.c
> > index 599f83b..2d732b1 100644
> > --- a/lib/librte_eal/common/eal_common_launch.c
> > +++ b/lib/librte_eal/common/eal_common_launch.c
> > @@ -117,4 +117,3 @@ rte_eal_mp_wait_lcore(void)
> >  		rte_eal_wait_lcore(lcore_id);
> >  	}
> >  }
> > -
> > diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
> > index e2810ab..fc47588 100644
> > --- a/lib/librte_eal/common/eal_common_options.c
> > +++ b/lib/librte_eal/common/eal_common_options.c
> > @@ -45,6 +45,7 @@
> >  #include <rte_lcore.h>
> >  #include <rte_version.h>
> >  #include <rte_devargs.h>
> > +#include <rte_memcpy.h>
> >
> >  #include "eal_internal_cfg.h"
> >  #include "eal_options.h"
> > @@ -85,6 +86,7 @@ eal_long_options[] = {
> >  	{OPT_XEN_DOM0, 0, 0, OPT_XEN_DOM0_NUM},
> >  	{OPT_CREATE_UIO_DEV, 1, NULL, OPT_CREATE_UIO_DEV_NUM},
> >  	{OPT_VFIO_INTR, 1, NULL, OPT_VFIO_INTR_NUM},
> > +	{OPT_LCORES, 1, 0, OPT_LCORES_NUM},
> >  	{0, 0, 0, 0}
> >  };
> >
> > @@ -255,9 +257,11 @@ eal_parse_corelist(const char *corelist)
> >  			if (min == RTE_MAX_LCORE)
> >  				min = idx;
> >  			for (idx = min; idx <= max; idx++) {
> > -				cfg->lcore_role[idx] = ROLE_RTE;
> > -				lcore_config[idx].core_index = count;
> > -				count++;
> > +				if (cfg->lcore_role[idx] != ROLE_RTE) {
> > +					cfg->lcore_role[idx] = ROLE_RTE;
> > +					lcore_config[idx].core_index = count;
> > +					count++;
> > +				}
> >  			}
> >  			min = RTE_MAX_LCORE;
> >  		} else
> > @@ -289,6 +293,241 @@ eal_parse_master_lcore(const char *arg)
> >  	return 0;
> >  }
> >
> > +/*
> > + * Parse elem, the elem could be single number or '(' ')' group
> > + * Within group elem, '-' used for a range seperator;
> > + *                    ',' used for a single number.
> > + */
> > +static int
> > +eal_parse_set(const char *input, uint16_t set[], unsigned num)
> > +{
> > +	unsigned idx;
> > +	const char *str = input;
> > +	char *end = NULL;
> > +	unsigned min, max;
> > +
> > +	memset(set, 0, num * sizeof(uint16_t));
> > +
> > +	while (isblank(*str))
> > +		str++;
> > +
> > +	/* only digit or left bracket is qulify for start point */
> > +	if ((!isdigit(*str) && *str != '(') || *str == '\0')
> > +		return -1;
> > +
> > +	/* process single number */
> > +	if (*str != '(') {
> > +		errno = 0;
> > +		idx = strtoul(str, &end, 10);
> > +		if (errno || end == NULL || idx >= num)
> > +			return -1;
> > +		else {
> > +			while (isblank(*end))
> > +				end++;
> > +
> > +			if (*end != ',' && *end != '\0' &&
> > +			    *end != '@')
> > +				return -1;
> > +
> > +			set[idx] = 1;
> > +			return end - input;
> > +		}
> > +	}
> > +
> > +	/* process set within bracket */
> > +	str++;
> > +	while (isblank(*str))
> > +		str++;
> > +	if (*str == '\0')
> > +		return -1;
> > +
> > +	min = RTE_MAX_LCORE;
> > +	do {
> > +
> > +		/* go ahead to the first digit */
> > +		while (isblank(*str))
> > +			str++;
> > +		if (!isdigit(*str))
> > +			return -1;
> > +
> > +		/* get the digit value */
> > +		errno = 0;
> > +		idx = strtoul(str, &end, 10);
> > +		if (errno || end == NULL || idx >= num)
> > +			return -1;
> > +
> > +		/* go ahead to separator '-',',' and ')' */
> > +		while (isblank(*end))
> > +			end++;
> > +		if (*end == '-') {
> > +			if (min == RTE_MAX_LCORE)
> > +				min = idx;
> > +			else /* avoid continuous '-' */
> > +				return -1;
> > +		} else if ((*end == ',') || (*end == ')')) {
> > +			max = idx;
> > +			if (min == RTE_MAX_LCORE)
> > +				min = idx;
> > +			for (idx = RTE_MIN(min, max);
> > +			     idx <= RTE_MAX(min, max); idx++)
> > +				set[idx] = 1;
> > +
> > +			min = RTE_MAX_LCORE;
> > +		} else
> > +			return -1;
> > +
> > +		str = end + 1;
> > +	} while (*end != '\0' && *end != ')');
> > +
> > +	return str - input;
> > +}
> > +
> > +/* convert from set array to cpuset bitmap */
> > +static inline int
> > +convert_to_cpuset(rte_cpuset_t *cpusetp,
> > +	      uint16_t *set, unsigned num)
> > +{
> > +	unsigned idx;
> > +
> > +	CPU_ZERO(cpusetp);
> > +
> > +	for (idx = 0; idx < num; idx++) {
> > +		if (!set[idx])
> > +			continue;
> > +
> > +		if (!lcore_config[idx].detected) {
> > +			RTE_LOG(ERR, EAL, "core %u "
> > +				"unavailable\n", idx);
> > +			return -1;
> > +		}
> > +
> > +		CPU_SET(idx, cpusetp);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +/*
> > + * The format pattern: --lcores='lcores[@cpus]<,lcores[@cpus]>'
> > + * lcores, cpus could be a single digit or a group.
> > + * '(' and ')' are necessary if it's a group.
> > + * If not supply '@cpus', the value of cpus uses the same as lcores.
> > + * e.g. '1,2@(5-7),(3-5)@(0,2),(0,6)' means start 7 EAL thread as below
> > + *   lcore 0 runs on cpuset 0x41 (cpu 0,6)
> > + *   lcore 1 runs on cpuset 0x2 (cpu 1)
> > + *   lcore 2 runs on cpuset 0xe0 (cpu 5,6,7)
> > + *   lcore 3,4,5 runs on cpuset 0x5 (cpu 0,2)
> > + *   lcore 6 runs on cpuset 0x41 (cpu 0,6)
> > + */
> > +static int
> > +eal_parse_lcores(const char *lcores)
> > +{
> > +	struct rte_config *cfg = rte_eal_get_configuration();
> > +	static uint16_t set[RTE_MAX_LCORE];
> > +	unsigned idx = 0;
> > +	int i;
> > +	unsigned count = 0;
> > +	const char *lcore_start = NULL;
> > +	const char *end = NULL;
> > +	int offset;
> > +	rte_cpuset_t cpuset;
> > +	int ret = -1;
> > +
> > +	if (lcores == NULL)
> > +		return -1;
> > +
> > +	/* Remove all blank characters ahead and after */
> > +	while (isblank(*lcores))
> > +		lcores++;
> > +	i = strnlen(lcores, sysconf(_SC_ARG_MAX));
> > +	while ((i > 0) && isblank(lcores[i - 1]))
> > +		i--;
> > +
> > +	CPU_ZERO(&cpuset);
> > +
> > +	/* Reset lcore config */
> > +	for (idx = 0; idx < RTE_MAX_LCORE; idx++) {
> > +		cfg->lcore_role[idx] = ROLE_OFF;
> > +		lcore_config[idx].core_index = -1;
> > +		CPU_ZERO(&lcore_config[idx].cpuset);
> > +	}
> > +
> > +	/* Get list of cores */
> > +	do {
> > +		while (isblank(*lcores))
> > +			lcores++;
> > +		if (*lcores == '\0')
> > +			goto err;
> > +
> > +		/* record lcore_set start point */
> > +		lcore_start = lcores;
> > +
> > +		/* go across a complete bracket */
> > +		if (*lcore_start == '(') {
> > +			lcores += strcspn(lcores, ")");
> > +			if (*lcores++ == '\0')
> > +				goto err;
> > +		}
> > +
> > +		/* scan the separator '@', ','(next) or '\0'(finish) */
> > +		lcores += strcspn(lcores, "@,");
> > +
> > +		if (*lcores == '@') {
> > +			/* explict assign cpu_set */
> > +			offset = eal_parse_set(lcores + 1, set, RTE_DIM(set));
> > +			if (offset < 0)
> > +				goto err;
> > +
> > +			/* prepare cpu_set and update the end cursor */
> > +			if (0 > convert_to_cpuset(&cpuset,
> > +						  set, RTE_DIM(set)))
> > +				goto err;
> > +			end = lcores + 1 + offset;
> > +		} else  /* ',' or '\0' */
> > +			/* haven't given cpu_set, current loop done */
> > +			end = lcores;
> > +
> > +		if (*end != ',' && *end != '\0')
> > +			goto err;
> > +
> > +		/* parse lcore_set from start point */
> > +		if (0 > eal_parse_set(lcore_start, set, RTE_DIM(set)))
> > +			goto err;
> > +
> > +		/* without '@', by default using lcore_set as cpu_set */
> > +		if (*lcores != '@' &&
> > +		    0 > convert_to_cpuset(&cpuset, set, RTE_DIM(set)))
> > +			goto err;
> > +
> > +		/* start to update lcore_set */
> > +		for (idx = 0; idx < RTE_MAX_LCORE; idx++) {
> > +			if (!set[idx])
> > +				continue;
> > +
> > +			if (cfg->lcore_role[idx] != ROLE_RTE) {
> > +				lcore_config[idx].core_index = count;
> > +				cfg->lcore_role[idx] = ROLE_RTE;
> > +				count++;
> > +			}
> > +			rte_memcpy(&lcore_config[idx].cpuset, &cpuset,
> > +				   sizeof(rte_cpuset_t));
> > +		}
> > +
> > +		lcores = end + 1;
> > +	} while (*end != '\0');
> > +
> > +	if (count == 0)
> > +		goto err;
> > +
> > +	cfg->lcore_count = count;
> > +	lcores_parsed = 1;
> > +	ret = 0;
> > +
> > +err:
> > +
> > +	return ret;
> > +}
> > +
> >  static int
> >  eal_parse_syslog(const char *facility, struct internal_config *conf)
> >  {
> > @@ -489,6 +728,13 @@ eal_parse_common_option(int opt, const char *optarg,
> >  		conf->log_level = log;
> >  		break;
> >  	}
> > +	case OPT_LCORES_NUM:
> > +		if (eal_parse_lcores(optarg) < 0) {
> > +			RTE_LOG(ERR, EAL, "invalid parameter for --"
> > +				OPT_LCORES "\n");
> > +			return -1;
> > +		}
> > +		break;
> >
> >  	/* don't know what to do, leave this to caller */
> >  	default:
> > @@ -527,7 +773,7 @@ eal_check_common_options(struct internal_config *internal_cfg)
> >
> >  	if (!lcores_parsed) {
> >  		RTE_LOG(ERR, EAL, "CPU cores must be enabled with options "
> > -			"-c or -l\n");
> > +			"-c, -l or --lcores\n");
> >  		return -1;
> >  	}
> >  	if (cfg->lcore_role[cfg->master_lcore] != ROLE_RTE) {
> > @@ -583,6 +829,14 @@ eal_common_usage(void)
> >  	       "                 The argument format is <c1>[-c2][,c3[-c4],...]\n"
> >  	       "                 where c1, c2, etc are core indexes between 0 and %d\n"
> >  	       "  --"OPT_MASTER_LCORE" ID: Core ID that is used as master\n"
> > +	       "  --"OPT_LCORES" MAP: maps between lcore_set to phys_cpu_set\n"
> > +	       "                 The argument format is\n"
> > +	       "                       'lcores[@cpus]<,lcores[@cpus],...>'\n"
> > +	       "                 lcores and cpus list are grouped by '(' and ')'\n"
> > +	       "                 Within the group, '-' is used for range separator,\n"
> > +	       "                 ',' is used for single number separator.\n"
> > +	       "                 '( )' can be omitted for single element group, '@' \n"
> > +	       "                 can be omitted if cpus and lcores has the same value\n"
> >  	       "  -n NUM       : Number of memory channels\n"
> >  	       "  -v           : Display version information on startup\n"
> >  	       "  -m MB        : memory to allocate (see also --"OPT_SOCKET_MEM")\n"
> > diff --git a/lib/librte_eal/common/eal_options.h b/lib/librte_eal/common/eal_options.h
> > index e476f8d..a1cc59f 100644
> > --- a/lib/librte_eal/common/eal_options.h
> > +++ b/lib/librte_eal/common/eal_options.h
> > @@ -77,6 +77,8 @@ enum {
> >  	OPT_CREATE_UIO_DEV_NUM,
> >  #define OPT_VFIO_INTR    "vfio-intr"
> >  	OPT_VFIO_INTR_NUM,
> > +#define OPT_LCORES "lcores"
> > +	OPT_LCORES_NUM,
> >  	OPT_LONG_MAX_NUM
> >  };
> >
> > diff --git a/lib/librte_eal/linuxapp/eal/Makefile b/lib/librte_eal/linuxapp/eal/Makefile
> > index 0e9c447..025d836 100644
> > --- a/lib/librte_eal/linuxapp/eal/Makefile
> > +++ b/lib/librte_eal/linuxapp/eal/Makefile
> > @@ -95,6 +95,7 @@ CFLAGS_eal_hugepage_info.o := -D_GNU_SOURCE
> >  CFLAGS_eal_pci.o := -D_GNU_SOURCE
> >  CFLAGS_eal_pci_vfio.o := -D_GNU_SOURCE
> >  CFLAGS_eal_common_whitelist.o := -D_GNU_SOURCE
> > +CFLAGS_eal_common_options.o := -D_GNU_SOURCE
> >
> >  # workaround for a gcc bug with noreturn attribute
> >  # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12603
> > --
> > 1.8.1.4
> >
  

Hi,
I want to mention that similar but for me much more readable syntax have 
Pktgen-DPDK for defining core - port mapping. Maybe we can adopt this syntax
for new '--lcores' parameter.

See '-m' parameter syntax on Pktgen readme.
https://github.com/pktgen/Pktgen-DPDK/blob/master/dpdk/examples/pktgen/README.md

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ananyev, Konstantin
> Sent: Thursday, January 22, 2015 3:34 PM
> To: Richardson, Bruce; Liang, Cunming
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v1 02/15] eal: new eal option '--lcores' for cpu
> assignment
> 
> Hi Bruce,
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
> > Sent: Thursday, January 22, 2015 12:19 PM
> > To: Liang, Cunming
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v1 02/15] eal: new eal option '--lcores' for cpu
> assignment
> >
> > On Thu, Jan 22, 2015 at 04:16:25PM +0800, Cunming Liang wrote:
> > > It supports one new eal long option '--lcores' for EAL thread cpuset
> assignment.
> > >
> > > The format pattern:
> > > 	--lcores='lcores[@cpus]<,lcores[@cpus]>'
> > > lcores, cpus could be a single digit or a group.
> > > '(' and ')' are necessary if it's a group.
> > > If not supply '@cpus', the value of cpus uses the same as lcores.
> > >
> > > e.g. '1,2@(5-7),(3-5)@(0,2),(0,6)' means starting 7 EAL thread as below
> > >   lcore 0 runs on cpuset 0x41 (cpu 0,6)
> > >   lcore 1 runs on cpuset 0x2 (cpu 1)
> > >   lcore 2 runs on cpuset 0xe0 (cpu 5,6,7)
> > >   lcore 3,4,5 runs on cpuset 0x5 (cpu 0,2)
> > >   lcore 6 runs on cpuset 0x41 (cpu 0,6)
> > >
> >
> > This strikes me as very confusing, though a couple of tweaks might help with
> > readability. The lcore 0 at the end is especially confusing.
> 
> Didn't get you here: do you find (0,6) confusing, right?
> Because braces implicitly specifies affinity for group of en-braced lcores?
> 
> > Perhaps we can
> > limit the allowed formats here,
> > * require the lcore_id to be specified - the lack of an lcore id for the last part
> > makes having it as lcore 0 surprising.
> 
> Again, not sure I understand you properly:  lcore_id(s) are always specified
> explicitly.
> Physical cpus part might be omitted.
> 
> > * only allow one lcore id to be given for each set of cores.
> 
> So you mean for '(3-5)@(0,2)' user would have to: '3@(0,2),4@(0,2),5@(0,2)'?
> I don't see big difference here, but imagine you'd like to create a pool of 32 EAL-
> threads running on same cpu set.
> With current syntax it is just something like: '(32-63)@(0-7)'.
> With what you proposing it will be a very long list.
> 
> >
> > I think it may still be readable if we allow the core set to be omitted if its
> > to be the same as the lcore_id.
> 
> I think that is supported.
> See lcore_id=1 in Steve's example above.
> As I understand: --lcores='0,2,3-5' is equal to '-l 0,2,3-5' and to '-c 0x3d'.
> 
> Konstantin
> 
> >
> > It's probably still not going to be very tidy, but I think we can improve things.
> >
> > /Bruce
> >
> > > Signed-off-by: Cunming Liang <cunming.liang@intel.com>
> > > ---
> > >  lib/librte_eal/common/eal_common_launch.c  |   1 -
> > >  lib/librte_eal/common/eal_common_options.c | 262
> ++++++++++++++++++++++++++++-
> > >  lib/librte_eal/common/eal_options.h        |   2 +
> > >  lib/librte_eal/linuxapp/eal/Makefile       |   1 +
> > >  4 files changed, 261 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/lib/librte_eal/common/eal_common_launch.c
> b/lib/librte_eal/common/eal_common_launch.c
> > > index 599f83b..2d732b1 100644
> > > --- a/lib/librte_eal/common/eal_common_launch.c
> > > +++ b/lib/librte_eal/common/eal_common_launch.c
> > > @@ -117,4 +117,3 @@ rte_eal_mp_wait_lcore(void)
> > >  		rte_eal_wait_lcore(lcore_id);
> > >  	}
> > >  }
> > > -
> > > diff --git a/lib/librte_eal/common/eal_common_options.c
> b/lib/librte_eal/common/eal_common_options.c
> > > index e2810ab..fc47588 100644
> > > --- a/lib/librte_eal/common/eal_common_options.c
> > > +++ b/lib/librte_eal/common/eal_common_options.c
> > > @@ -45,6 +45,7 @@
> > >  #include <rte_lcore.h>
> > >  #include <rte_version.h>
> > >  #include <rte_devargs.h>
> > > +#include <rte_memcpy.h>
> > >
> > >  #include "eal_internal_cfg.h"
> > >  #include "eal_options.h"
> > > @@ -85,6 +86,7 @@ eal_long_options[] = {
> > >  	{OPT_XEN_DOM0, 0, 0, OPT_XEN_DOM0_NUM},
> > >  	{OPT_CREATE_UIO_DEV, 1, NULL, OPT_CREATE_UIO_DEV_NUM},
> > >  	{OPT_VFIO_INTR, 1, NULL, OPT_VFIO_INTR_NUM},
> > > +	{OPT_LCORES, 1, 0, OPT_LCORES_NUM},
> > >  	{0, 0, 0, 0}
> > >  };
> > >
> > > @@ -255,9 +257,11 @@ eal_parse_corelist(const char *corelist)
> > >  			if (min == RTE_MAX_LCORE)
> > >  				min = idx;
> > >  			for (idx = min; idx <= max; idx++) {
> > > -				cfg->lcore_role[idx] = ROLE_RTE;
> > > -				lcore_config[idx].core_index = count;
> > > -				count++;
> > > +				if (cfg->lcore_role[idx] != ROLE_RTE) {
> > > +					cfg->lcore_role[idx] = ROLE_RTE;
> > > +					lcore_config[idx].core_index = count;
> > > +					count++;
> > > +				}
> > >  			}
> > >  			min = RTE_MAX_LCORE;
> > >  		} else
> > > @@ -289,6 +293,241 @@ eal_parse_master_lcore(const char *arg)
> > >  	return 0;
> > >  }
> > >
> > > +/*
> > > + * Parse elem, the elem could be single number or '(' ')' group
> > > + * Within group elem, '-' used for a range seperator;
> > > + *                    ',' used for a single number.
> > > + */
> > > +static int
> > > +eal_parse_set(const char *input, uint16_t set[], unsigned num)
> > > +{
> > > +	unsigned idx;
> > > +	const char *str = input;
> > > +	char *end = NULL;
> > > +	unsigned min, max;
> > > +
> > > +	memset(set, 0, num * sizeof(uint16_t));
> > > +
> > > +	while (isblank(*str))
> > > +		str++;
> > > +
> > > +	/* only digit or left bracket is qulify for start point */
> > > +	if ((!isdigit(*str) && *str != '(') || *str == '\0')
> > > +		return -1;
> > > +
> > > +	/* process single number */
> > > +	if (*str != '(') {
> > > +		errno = 0;
> > > +		idx = strtoul(str, &end, 10);
> > > +		if (errno || end == NULL || idx >= num)
> > > +			return -1;
> > > +		else {
> > > +			while (isblank(*end))
> > > +				end++;
> > > +
> > > +			if (*end != ',' && *end != '\0' &&
> > > +			    *end != '@')
> > > +				return -1;
> > > +
> > > +			set[idx] = 1;
> > > +			return end - input;
> > > +		}
> > > +	}
> > > +
> > > +	/* process set within bracket */
> > > +	str++;
> > > +	while (isblank(*str))
> > > +		str++;
> > > +	if (*str == '\0')
> > > +		return -1;
> > > +
> > > +	min = RTE_MAX_LCORE;
> > > +	do {
> > > +
> > > +		/* go ahead to the first digit */
> > > +		while (isblank(*str))
> > > +			str++;
> > > +		if (!isdigit(*str))
> > > +			return -1;
> > > +
> > > +		/* get the digit value */
> > > +		errno = 0;
> > > +		idx = strtoul(str, &end, 10);
> > > +		if (errno || end == NULL || idx >= num)
> > > +			return -1;
> > > +
> > > +		/* go ahead to separator '-',',' and ')' */
> > > +		while (isblank(*end))
> > > +			end++;
> > > +		if (*end == '-') {
> > > +			if (min == RTE_MAX_LCORE)
> > > +				min = idx;
> > > +			else /* avoid continuous '-' */
> > > +				return -1;
> > > +		} else if ((*end == ',') || (*end == ')')) {
> > > +			max = idx;
> > > +			if (min == RTE_MAX_LCORE)
> > > +				min = idx;
> > > +			for (idx = RTE_MIN(min, max);
> > > +			     idx <= RTE_MAX(min, max); idx++)
> > > +				set[idx] = 1;
> > > +
> > > +			min = RTE_MAX_LCORE;
> > > +		} else
> > > +			return -1;
> > > +
> > > +		str = end + 1;
> > > +	} while (*end != '\0' && *end != ')');
> > > +
> > > +	return str - input;
> > > +}
> > > +
> > > +/* convert from set array to cpuset bitmap */
> > > +static inline int
> > > +convert_to_cpuset(rte_cpuset_t *cpusetp,
> > > +	      uint16_t *set, unsigned num)
> > > +{
> > > +	unsigned idx;
> > > +
> > > +	CPU_ZERO(cpusetp);
> > > +
> > > +	for (idx = 0; idx < num; idx++) {
> > > +		if (!set[idx])
> > > +			continue;
> > > +
> > > +		if (!lcore_config[idx].detected) {
> > > +			RTE_LOG(ERR, EAL, "core %u "
> > > +				"unavailable\n", idx);
> > > +			return -1;
> > > +		}
> > > +
> > > +		CPU_SET(idx, cpusetp);
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +/*
> > > + * The format pattern: --lcores='lcores[@cpus]<,lcores[@cpus]>'
> > > + * lcores, cpus could be a single digit or a group.
> > > + * '(' and ')' are necessary if it's a group.
> > > + * If not supply '@cpus', the value of cpus uses the same as lcores.
> > > + * e.g. '1,2@(5-7),(3-5)@(0,2),(0,6)' means start 7 EAL thread as below
> > > + *   lcore 0 runs on cpuset 0x41 (cpu 0,6)
> > > + *   lcore 1 runs on cpuset 0x2 (cpu 1)
> > > + *   lcore 2 runs on cpuset 0xe0 (cpu 5,6,7)
> > > + *   lcore 3,4,5 runs on cpuset 0x5 (cpu 0,2)
> > > + *   lcore 6 runs on cpuset 0x41 (cpu 0,6)
> > > + */
> > > +static int
> > > +eal_parse_lcores(const char *lcores)
> > > +{
> > > +	struct rte_config *cfg = rte_eal_get_configuration();
> > > +	static uint16_t set[RTE_MAX_LCORE];
> > > +	unsigned idx = 0;
> > > +	int i;
> > > +	unsigned count = 0;
> > > +	const char *lcore_start = NULL;
> > > +	const char *end = NULL;
> > > +	int offset;
> > > +	rte_cpuset_t cpuset;
> > > +	int ret = -1;
> > > +
> > > +	if (lcores == NULL)
> > > +		return -1;
> > > +
> > > +	/* Remove all blank characters ahead and after */
> > > +	while (isblank(*lcores))
> > > +		lcores++;
> > > +	i = strnlen(lcores, sysconf(_SC_ARG_MAX));
> > > +	while ((i > 0) && isblank(lcores[i - 1]))
> > > +		i--;
> > > +
> > > +	CPU_ZERO(&cpuset);
> > > +
> > > +	/* Reset lcore config */
> > > +	for (idx = 0; idx < RTE_MAX_LCORE; idx++) {
> > > +		cfg->lcore_role[idx] = ROLE_OFF;
> > > +		lcore_config[idx].core_index = -1;
> > > +		CPU_ZERO(&lcore_config[idx].cpuset);
> > > +	}
> > > +
> > > +	/* Get list of cores */
> > > +	do {
> > > +		while (isblank(*lcores))
> > > +			lcores++;
> > > +		if (*lcores == '\0')
> > > +			goto err;
> > > +
> > > +		/* record lcore_set start point */
> > > +		lcore_start = lcores;
> > > +
> > > +		/* go across a complete bracket */
> > > +		if (*lcore_start == '(') {
> > > +			lcores += strcspn(lcores, ")");
> > > +			if (*lcores++ == '\0')
> > > +				goto err;
> > > +		}
> > > +
> > > +		/* scan the separator '@', ','(next) or '\0'(finish) */
> > > +		lcores += strcspn(lcores, "@,");
> > > +
> > > +		if (*lcores == '@') {
> > > +			/* explict assign cpu_set */
> > > +			offset = eal_parse_set(lcores + 1, set, RTE_DIM(set));
> > > +			if (offset < 0)
> > > +				goto err;
> > > +
> > > +			/* prepare cpu_set and update the end cursor */
> > > +			if (0 > convert_to_cpuset(&cpuset,
> > > +						  set, RTE_DIM(set)))
> > > +				goto err;
> > > +			end = lcores + 1 + offset;
> > > +		} else  /* ',' or '\0' */
> > > +			/* haven't given cpu_set, current loop done */
> > > +			end = lcores;
> > > +
> > > +		if (*end != ',' && *end != '\0')
> > > +			goto err;
> > > +
> > > +		/* parse lcore_set from start point */
> > > +		if (0 > eal_parse_set(lcore_start, set, RTE_DIM(set)))
> > > +			goto err;
> > > +
> > > +		/* without '@', by default using lcore_set as cpu_set */
> > > +		if (*lcores != '@' &&
> > > +		    0 > convert_to_cpuset(&cpuset, set, RTE_DIM(set)))
> > > +			goto err;
> > > +
> > > +		/* start to update lcore_set */
> > > +		for (idx = 0; idx < RTE_MAX_LCORE; idx++) {
> > > +			if (!set[idx])
> > > +				continue;
> > > +
> > > +			if (cfg->lcore_role[idx] != ROLE_RTE) {
> > > +				lcore_config[idx].core_index = count;
> > > +				cfg->lcore_role[idx] = ROLE_RTE;
> > > +				count++;
> > > +			}
> > > +			rte_memcpy(&lcore_config[idx].cpuset, &cpuset,
> > > +				   sizeof(rte_cpuset_t));
> > > +		}
> > > +
> > > +		lcores = end + 1;
> > > +	} while (*end != '\0');
> > > +
> > > +	if (count == 0)
> > > +		goto err;
> > > +
> > > +	cfg->lcore_count = count;
> > > +	lcores_parsed = 1;
> > > +	ret = 0;
> > > +
> > > +err:
> > > +
> > > +	return ret;
> > > +}
> > > +
> > >  static int
> > >  eal_parse_syslog(const char *facility, struct internal_config *conf)
> > >  {
> > > @@ -489,6 +728,13 @@ eal_parse_common_option(int opt, const char
> *optarg,
> > >  		conf->log_level = log;
> > >  		break;
> > >  	}
> > > +	case OPT_LCORES_NUM:
> > > +		if (eal_parse_lcores(optarg) < 0) {
> > > +			RTE_LOG(ERR, EAL, "invalid parameter for --"
> > > +				OPT_LCORES "\n");
> > > +			return -1;
> > > +		}
> > > +		break;
> > >
> > >  	/* don't know what to do, leave this to caller */
> > >  	default:
> > > @@ -527,7 +773,7 @@ eal_check_common_options(struct internal_config
> *internal_cfg)
> > >
> > >  	if (!lcores_parsed) {
> > >  		RTE_LOG(ERR, EAL, "CPU cores must be enabled with options "
> > > -			"-c or -l\n");
> > > +			"-c, -l or --lcores\n");
> > >  		return -1;
> > >  	}
> > >  	if (cfg->lcore_role[cfg->master_lcore] != ROLE_RTE) {
> > > @@ -583,6 +829,14 @@ eal_common_usage(void)
> > >  	       "                 The argument format is <c1>[-c2][,c3[-c4],...]\n"
> > >  	       "                 where c1, c2, etc are core indexes between 0 and %d\n"
> > >  	       "  --"OPT_MASTER_LCORE" ID: Core ID that is used as master\n"
> > > +	       "  --"OPT_LCORES" MAP: maps between lcore_set to
> phys_cpu_set\n"
> > > +	       "                 The argument format is\n"
> > > +	       "                       'lcores[@cpus]<,lcores[@cpus],...>'\n"
> > > +	       "                 lcores and cpus list are grouped by '(' and ')'\n"
> > > +	       "                 Within the group, '-' is used for range separator,\n"
> > > +	       "                 ',' is used for single number separator.\n"
> > > +	       "                 '( )' can be omitted for single element group, '@' \n"
> > > +	       "                 can be omitted if cpus and lcores has the same value\n"
> > >  	       "  -n NUM       : Number of memory channels\n"
> > >  	       "  -v           : Display version information on startup\n"
> > >  	       "  -m MB        : memory to allocate (see also --
> "OPT_SOCKET_MEM")\n"
> > > diff --git a/lib/librte_eal/common/eal_options.h
> b/lib/librte_eal/common/eal_options.h
> > > index e476f8d..a1cc59f 100644
> > > --- a/lib/librte_eal/common/eal_options.h
> > > +++ b/lib/librte_eal/common/eal_options.h
> > > @@ -77,6 +77,8 @@ enum {
> > >  	OPT_CREATE_UIO_DEV_NUM,
> > >  #define OPT_VFIO_INTR    "vfio-intr"
> > >  	OPT_VFIO_INTR_NUM,
> > > +#define OPT_LCORES "lcores"
> > > +	OPT_LCORES_NUM,
> > >  	OPT_LONG_MAX_NUM
> > >  };
> > >
> > > diff --git a/lib/librte_eal/linuxapp/eal/Makefile
> b/lib/librte_eal/linuxapp/eal/Makefile
> > > index 0e9c447..025d836 100644
> > > --- a/lib/librte_eal/linuxapp/eal/Makefile
> > > +++ b/lib/librte_eal/linuxapp/eal/Makefile
> > > @@ -95,6 +95,7 @@ CFLAGS_eal_hugepage_info.o := -D_GNU_SOURCE
> > >  CFLAGS_eal_pci.o := -D_GNU_SOURCE
> > >  CFLAGS_eal_pci_vfio.o := -D_GNU_SOURCE
> > >  CFLAGS_eal_common_whitelist.o := -D_GNU_SOURCE
> > > +CFLAGS_eal_common_options.o := -D_GNU_SOURCE
> > >
> > >  # workaround for a gcc bug with noreturn attribute
> > >  # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12603
> > > --
> > > 1.8.1.4
> > >
  

On Thu, Jan 22, 2015 at 02:34:07PM +0000, Ananyev, Konstantin wrote:
> Hi Bruce,
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
> > Sent: Thursday, January 22, 2015 12:19 PM
> > To: Liang, Cunming
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v1 02/15] eal: new eal option '--lcores' for cpu assignment
> > 
> > On Thu, Jan 22, 2015 at 04:16:25PM +0800, Cunming Liang wrote:
> > > It supports one new eal long option '--lcores' for EAL thread cpuset assignment.
> > >
> > > The format pattern:
> > > 	--lcores='lcores[@cpus]<,lcores[@cpus]>'
> > > lcores, cpus could be a single digit or a group.
> > > '(' and ')' are necessary if it's a group.
> > > If not supply '@cpus', the value of cpus uses the same as lcores.
> > >
> > > e.g. '1,2@(5-7),(3-5)@(0,2),(0,6)' means starting 7 EAL thread as below
> > >   lcore 0 runs on cpuset 0x41 (cpu 0,6)
> > >   lcore 1 runs on cpuset 0x2 (cpu 1)
> > >   lcore 2 runs on cpuset 0xe0 (cpu 5,6,7)
> > >   lcore 3,4,5 runs on cpuset 0x5 (cpu 0,2)
> > >   lcore 6 runs on cpuset 0x41 (cpu 0,6)
> > >
> > 
> > This strikes me as very confusing, though a couple of tweaks might help with
> > readability. The lcore 0 at the end is especially confusing.
> 
> Didn't get you here: do you find (0,6) confusing, right?
> Because braces implicitly specifies affinity for group of en-braced lcores? 
> 
> > Perhaps we can
> > limit the allowed formats here,
> > * require the lcore_id to be specified - the lack of an lcore id for the last part
> > makes having it as lcore 0 surprising.
> 
> Again, not sure I understand you properly:  lcore_id(s) are always specified explicitly. 
> Physical cpus part might be omitted.
> 
> > * only allow one lcore id to be given for each set of cores.
> 
> So you mean for '(3-5)@(0,2)' user would have to: '3@(0,2),4@(0,2),5@(0,2)'?
> I don't see big difference here, but imagine you'd like to create a pool of 32 EAL-threads running on same cpu set.
> With current syntax it is just something like: '(32-63)@(0-7)'.
> With what you proposing it will be a very long list.  
> 
> > 
> > I think it may still be readable if we allow the core set to be omitted if its
> > to be the same as the lcore_id.
> 
> I think that is supported.
> See lcore_id=1 in Steve's example above.
> As I understand: --lcores='0,2,3-5' is equal to '-l 0,2,3-5' and to '-c 0x3d'.
> 
> Konstantin

Ok, thanks for the clarification.

/Bruce
  

> -----Original Message-----
> From: Richardson, Bruce
> Sent: Thursday, January 22, 2015 8:19 PM
> To: Liang, Cunming
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v1 02/15] eal: new eal option '--lcores' for cpu
> assignment
> 
> On Thu, Jan 22, 2015 at 04:16:25PM +0800, Cunming Liang wrote:
> > It supports one new eal long option '--lcores' for EAL thread cpuset assignment.
> >
> > The format pattern:
> > 	--lcores='lcores[@cpus]<,lcores[@cpus]>'
> > lcores, cpus could be a single digit or a group.
> > '(' and ')' are necessary if it's a group.
> > If not supply '@cpus', the value of cpus uses the same as lcores.
> >
> > e.g. '1,2@(5-7),(3-5)@(0,2),(0,6)' means starting 7 EAL thread as below
> >   lcore 0 runs on cpuset 0x41 (cpu 0,6)
> >   lcore 1 runs on cpuset 0x2 (cpu 1)
> >   lcore 2 runs on cpuset 0xe0 (cpu 5,6,7)
> >   lcore 3,4,5 runs on cpuset 0x5 (cpu 0,2)
> >   lcore 6 runs on cpuset 0x41 (cpu 0,6)
> >
> 
> This strikes me as very confusing, though a couple of tweaks might help with
> readability. The lcore 0 at the end is especially confusing. Perhaps we can
> limit the allowed formats here,
> * require the lcore_id to be specified - the lack of an lcore id for the last part
> makes having it as lcore 0 surprising.
> * only allow one lcore id to be given for each set of cores.
[Liang, Cunming] The last one lcore_set (0,6) without cpuset assigned is equal to '(0,6)@(0,6)' or '0@(0,6), 6@(0,6)'.
It's not a typical use case but gives an aggressive sample, it shows the simple way to explain the map.
> 
> I think it may still be readable if we allow the core set to be omitted if its
> to be the same as the lcore_id.
> 
> It's probably still not going to be very tidy, but I think we can improve things.
> 
> /Bruce
> 
> > Signed-off-by: Cunming Liang <cunming.liang@intel.com>
> > ---
> >  lib/librte_eal/common/eal_common_launch.c  |   1 -
> >  lib/librte_eal/common/eal_common_options.c | 262
> ++++++++++++++++++++++++++++-
> >  lib/librte_eal/common/eal_options.h        |   2 +
> >  lib/librte_eal/linuxapp/eal/Makefile       |   1 +
> >  4 files changed, 261 insertions(+), 5 deletions(-)
> >
> > diff --git a/lib/librte_eal/common/eal_common_launch.c
> b/lib/librte_eal/common/eal_common_launch.c
> > index 599f83b..2d732b1 100644
> > --- a/lib/librte_eal/common/eal_common_launch.c
> > +++ b/lib/librte_eal/common/eal_common_launch.c
> > @@ -117,4 +117,3 @@ rte_eal_mp_wait_lcore(void)
> >  		rte_eal_wait_lcore(lcore_id);
> >  	}
> >  }
> > -
> > diff --git a/lib/librte_eal/common/eal_common_options.c
> b/lib/librte_eal/common/eal_common_options.c
> > index e2810ab..fc47588 100644
> > --- a/lib/librte_eal/common/eal_common_options.c
> > +++ b/lib/librte_eal/common/eal_common_options.c
> > @@ -45,6 +45,7 @@
> >  #include <rte_lcore.h>
> >  #include <rte_version.h>
> >  #include <rte_devargs.h>
> > +#include <rte_memcpy.h>
> >
> >  #include "eal_internal_cfg.h"
> >  #include "eal_options.h"
> > @@ -85,6 +86,7 @@ eal_long_options[] = {
> >  	{OPT_XEN_DOM0, 0, 0, OPT_XEN_DOM0_NUM},
> >  	{OPT_CREATE_UIO_DEV, 1, NULL, OPT_CREATE_UIO_DEV_NUM},
> >  	{OPT_VFIO_INTR, 1, NULL, OPT_VFIO_INTR_NUM},
> > +	{OPT_LCORES, 1, 0, OPT_LCORES_NUM},
> >  	{0, 0, 0, 0}
> >  };
> >
> > @@ -255,9 +257,11 @@ eal_parse_corelist(const char *corelist)
> >  			if (min == RTE_MAX_LCORE)
> >  				min = idx;
> >  			for (idx = min; idx <= max; idx++) {
> > -				cfg->lcore_role[idx] = ROLE_RTE;
> > -				lcore_config[idx].core_index = count;
> > -				count++;
> > +				if (cfg->lcore_role[idx] != ROLE_RTE) {
> > +					cfg->lcore_role[idx] = ROLE_RTE;
> > +					lcore_config[idx].core_index = count;
> > +					count++;
> > +				}
> >  			}
> >  			min = RTE_MAX_LCORE;
> >  		} else
> > @@ -289,6 +293,241 @@ eal_parse_master_lcore(const char *arg)
> >  	return 0;
> >  }
> >
> > +/*
> > + * Parse elem, the elem could be single number or '(' ')' group
> > + * Within group elem, '-' used for a range seperator;
> > + *                    ',' used for a single number.
> > + */
> > +static int
> > +eal_parse_set(const char *input, uint16_t set[], unsigned num)
> > +{
> > +	unsigned idx;
> > +	const char *str = input;
> > +	char *end = NULL;
> > +	unsigned min, max;
> > +
> > +	memset(set, 0, num * sizeof(uint16_t));
> > +
> > +	while (isblank(*str))
> > +		str++;
> > +
> > +	/* only digit or left bracket is qulify for start point */
> > +	if ((!isdigit(*str) && *str != '(') || *str == '\0')
> > +		return -1;
> > +
> > +	/* process single number */
> > +	if (*str != '(') {
> > +		errno = 0;
> > +		idx = strtoul(str, &end, 10);
> > +		if (errno || end == NULL || idx >= num)
> > +			return -1;
> > +		else {
> > +			while (isblank(*end))
> > +				end++;
> > +
> > +			if (*end != ',' && *end != '\0' &&
> > +			    *end != '@')
> > +				return -1;
> > +
> > +			set[idx] = 1;
> > +			return end - input;
> > +		}
> > +	}
> > +
> > +	/* process set within bracket */
> > +	str++;
> > +	while (isblank(*str))
> > +		str++;
> > +	if (*str == '\0')
> > +		return -1;
> > +
> > +	min = RTE_MAX_LCORE;
> > +	do {
> > +
> > +		/* go ahead to the first digit */
> > +		while (isblank(*str))
> > +			str++;
> > +		if (!isdigit(*str))
> > +			return -1;
> > +
> > +		/* get the digit value */
> > +		errno = 0;
> > +		idx = strtoul(str, &end, 10);
> > +		if (errno || end == NULL || idx >= num)
> > +			return -1;
> > +
> > +		/* go ahead to separator '-',',' and ')' */
> > +		while (isblank(*end))
> > +			end++;
> > +		if (*end == '-') {
> > +			if (min == RTE_MAX_LCORE)
> > +				min = idx;
> > +			else /* avoid continuous '-' */
> > +				return -1;
> > +		} else if ((*end == ',') || (*end == ')')) {
> > +			max = idx;
> > +			if (min == RTE_MAX_LCORE)
> > +				min = idx;
> > +			for (idx = RTE_MIN(min, max);
> > +			     idx <= RTE_MAX(min, max); idx++)
> > +				set[idx] = 1;
> > +
> > +			min = RTE_MAX_LCORE;
> > +		} else
> > +			return -1;
> > +
> > +		str = end + 1;
> > +	} while (*end != '\0' && *end != ')');
> > +
> > +	return str - input;
> > +}
> > +
> > +/* convert from set array to cpuset bitmap */
> > +static inline int
> > +convert_to_cpuset(rte_cpuset_t *cpusetp,
> > +	      uint16_t *set, unsigned num)
> > +{
> > +	unsigned idx;
> > +
> > +	CPU_ZERO(cpusetp);
> > +
> > +	for (idx = 0; idx < num; idx++) {
> > +		if (!set[idx])
> > +			continue;
> > +
> > +		if (!lcore_config[idx].detected) {
> > +			RTE_LOG(ERR, EAL, "core %u "
> > +				"unavailable\n", idx);
> > +			return -1;
> > +		}
> > +
> > +		CPU_SET(idx, cpusetp);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +/*
> > + * The format pattern: --lcores='lcores[@cpus]<,lcores[@cpus]>'
> > + * lcores, cpus could be a single digit or a group.
> > + * '(' and ')' are necessary if it's a group.
> > + * If not supply '@cpus', the value of cpus uses the same as lcores.
> > + * e.g. '1,2@(5-7),(3-5)@(0,2),(0,6)' means start 7 EAL thread as below
> > + *   lcore 0 runs on cpuset 0x41 (cpu 0,6)
> > + *   lcore 1 runs on cpuset 0x2 (cpu 1)
> > + *   lcore 2 runs on cpuset 0xe0 (cpu 5,6,7)
> > + *   lcore 3,4,5 runs on cpuset 0x5 (cpu 0,2)
> > + *   lcore 6 runs on cpuset 0x41 (cpu 0,6)
> > + */
> > +static int
> > +eal_parse_lcores(const char *lcores)
> > +{
> > +	struct rte_config *cfg = rte_eal_get_configuration();
> > +	static uint16_t set[RTE_MAX_LCORE];
> > +	unsigned idx = 0;
> > +	int i;
> > +	unsigned count = 0;
> > +	const char *lcore_start = NULL;
> > +	const char *end = NULL;
> > +	int offset;
> > +	rte_cpuset_t cpuset;
> > +	int ret = -1;
> > +
> > +	if (lcores == NULL)
> > +		return -1;
> > +
> > +	/* Remove all blank characters ahead and after */
> > +	while (isblank(*lcores))
> > +		lcores++;
> > +	i = strnlen(lcores, sysconf(_SC_ARG_MAX));
> > +	while ((i > 0) && isblank(lcores[i - 1]))
> > +		i--;
> > +
> > +	CPU_ZERO(&cpuset);
> > +
> > +	/* Reset lcore config */
> > +	for (idx = 0; idx < RTE_MAX_LCORE; idx++) {
> > +		cfg->lcore_role[idx] = ROLE_OFF;
> > +		lcore_config[idx].core_index = -1;
> > +		CPU_ZERO(&lcore_config[idx].cpuset);
> > +	}
> > +
> > +	/* Get list of cores */
> > +	do {
> > +		while (isblank(*lcores))
> > +			lcores++;
> > +		if (*lcores == '\0')
> > +			goto err;
> > +
> > +		/* record lcore_set start point */
> > +		lcore_start = lcores;
> > +
> > +		/* go across a complete bracket */
> > +		if (*lcore_start == '(') {
> > +			lcores += strcspn(lcores, ")");
> > +			if (*lcores++ == '\0')
> > +				goto err;
> > +		}
> > +
> > +		/* scan the separator '@', ','(next) or '\0'(finish) */
> > +		lcores += strcspn(lcores, "@,");
> > +
> > +		if (*lcores == '@') {
> > +			/* explict assign cpu_set */
> > +			offset = eal_parse_set(lcores + 1, set, RTE_DIM(set));
> > +			if (offset < 0)
> > +				goto err;
> > +
> > +			/* prepare cpu_set and update the end cursor */
> > +			if (0 > convert_to_cpuset(&cpuset,
> > +						  set, RTE_DIM(set)))
> > +				goto err;
> > +			end = lcores + 1 + offset;
> > +		} else  /* ',' or '\0' */
> > +			/* haven't given cpu_set, current loop done */
> > +			end = lcores;
> > +
> > +		if (*end != ',' && *end != '\0')
> > +			goto err;
> > +
> > +		/* parse lcore_set from start point */
> > +		if (0 > eal_parse_set(lcore_start, set, RTE_DIM(set)))
> > +			goto err;
> > +
> > +		/* without '@', by default using lcore_set as cpu_set */
> > +		if (*lcores != '@' &&
> > +		    0 > convert_to_cpuset(&cpuset, set, RTE_DIM(set)))
> > +			goto err;
> > +
> > +		/* start to update lcore_set */
> > +		for (idx = 0; idx < RTE_MAX_LCORE; idx++) {
> > +			if (!set[idx])
> > +				continue;
> > +
> > +			if (cfg->lcore_role[idx] != ROLE_RTE) {
> > +				lcore_config[idx].core_index = count;
> > +				cfg->lcore_role[idx] = ROLE_RTE;
> > +				count++;
> > +			}
> > +			rte_memcpy(&lcore_config[idx].cpuset, &cpuset,
> > +				   sizeof(rte_cpuset_t));
> > +		}
> > +
> > +		lcores = end + 1;
> > +	} while (*end != '\0');
> > +
> > +	if (count == 0)
> > +		goto err;
> > +
> > +	cfg->lcore_count = count;
> > +	lcores_parsed = 1;
> > +	ret = 0;
> > +
> > +err:
> > +
> > +	return ret;
> > +}
> > +
> >  static int
> >  eal_parse_syslog(const char *facility, struct internal_config *conf)
> >  {
> > @@ -489,6 +728,13 @@ eal_parse_common_option(int opt, const char
> *optarg,
> >  		conf->log_level = log;
> >  		break;
> >  	}
> > +	case OPT_LCORES_NUM:
> > +		if (eal_parse_lcores(optarg) < 0) {
> > +			RTE_LOG(ERR, EAL, "invalid parameter for --"
> > +				OPT_LCORES "\n");
> > +			return -1;
> > +		}
> > +		break;
> >
> >  	/* don't know what to do, leave this to caller */
> >  	default:
> > @@ -527,7 +773,7 @@ eal_check_common_options(struct internal_config
> *internal_cfg)
> >
> >  	if (!lcores_parsed) {
> >  		RTE_LOG(ERR, EAL, "CPU cores must be enabled with options "
> > -			"-c or -l\n");
> > +			"-c, -l or --lcores\n");
> >  		return -1;
> >  	}
> >  	if (cfg->lcore_role[cfg->master_lcore] != ROLE_RTE) {
> > @@ -583,6 +829,14 @@ eal_common_usage(void)
> >  	       "                 The argument format is <c1>[-c2][,c3[-c4],...]\n"
> >  	       "                 where c1, c2, etc are core indexes between 0
> and %d\n"
> >  	       "  --"OPT_MASTER_LCORE" ID: Core ID that is used as master\n"
> > +	       "  --"OPT_LCORES" MAP: maps between lcore_set to
> phys_cpu_set\n"
> > +	       "                 The argument format is\n"
> > +	       "                       'lcores[@cpus]<,lcores[@cpus],...>'\n"
> > +	       "                 lcores and cpus list are grouped by '(' and ')'\n"
> > +	       "                 Within the group, '-' is used for range
> separator,\n"
> > +	       "                 ',' is used for single number separator.\n"
> > +	       "                 '( )' can be omitted for single element group,
> '@' \n"
> > +	       "                 can be omitted if cpus and lcores has the same
> value\n"
> >  	       "  -n NUM       : Number of memory channels\n"
> >  	       "  -v           : Display version information on startup\n"
> >  	       "  -m MB        : memory to allocate (see also
> --"OPT_SOCKET_MEM")\n"
> > diff --git a/lib/librte_eal/common/eal_options.h
> b/lib/librte_eal/common/eal_options.h
> > index e476f8d..a1cc59f 100644
> > --- a/lib/librte_eal/common/eal_options.h
> > +++ b/lib/librte_eal/common/eal_options.h
> > @@ -77,6 +77,8 @@ enum {
> >  	OPT_CREATE_UIO_DEV_NUM,
> >  #define OPT_VFIO_INTR    "vfio-intr"
> >  	OPT_VFIO_INTR_NUM,
> > +#define OPT_LCORES "lcores"
> > +	OPT_LCORES_NUM,
> >  	OPT_LONG_MAX_NUM
> >  };
> >
> > diff --git a/lib/librte_eal/linuxapp/eal/Makefile
> b/lib/librte_eal/linuxapp/eal/Makefile
> > index 0e9c447..025d836 100644
> > --- a/lib/librte_eal/linuxapp/eal/Makefile
> > +++ b/lib/librte_eal/linuxapp/eal/Makefile
> > @@ -95,6 +95,7 @@ CFLAGS_eal_hugepage_info.o := -D_GNU_SOURCE
> >  CFLAGS_eal_pci.o := -D_GNU_SOURCE
> >  CFLAGS_eal_pci_vfio.o := -D_GNU_SOURCE
> >  CFLAGS_eal_common_whitelist.o := -D_GNU_SOURCE
> > +CFLAGS_eal_common_options.o := -D_GNU_SOURCE
> >
> >  # workaround for a gcc bug with noreturn attribute
> >  # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12603
> > --
> > 1.8.1.4
> >
  

Hi Pawel,

I don't see much different there.
If replacing '@' to '.'; '()' to '[]'; and ',' to '/'; they're almost the same.
Without having rx/tx case, so ':' is useless in our case.
Considering the semantic, '@'(at) is more readable than '.' for core assignment.

-Liang Cunming

> -----Original Message-----
> From: Wodkowski, PawelX
> Sent: Thursday, January 22, 2015 11:17 PM
> To: Ananyev, Konstantin; Richardson, Bruce; Liang, Cunming
> Cc: dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v1 02/15] eal: new eal option '--lcores' for cpu
> assignment
> 
> Hi,
> I want to mention that similar but for me much more readable syntax have
> Pktgen-DPDK for defining core - port mapping. Maybe we can adopt this syntax
> for new '--lcores' parameter.
> 
> See '-m' parameter syntax on Pktgen readme.
> https://github.com/pktgen/Pktgen-DPDK/blob/master/dpdk/examples/pktgen/R
> EADME.md
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ananyev, Konstantin
> > Sent: Thursday, January 22, 2015 3:34 PM
> > To: Richardson, Bruce; Liang, Cunming
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v1 02/15] eal: new eal option '--lcores' for cpu
> > assignment
> >
> > Hi Bruce,
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
> > > Sent: Thursday, January 22, 2015 12:19 PM
> > > To: Liang, Cunming
> > > Cc: dev@dpdk.org
> > > Subject: Re: [dpdk-dev] [PATCH v1 02/15] eal: new eal option '--lcores' for cpu
> > assignment
> > >
> > > On Thu, Jan 22, 2015 at 04:16:25PM +0800, Cunming Liang wrote:
> > > > It supports one new eal long option '--lcores' for EAL thread cpuset
> > assignment.
> > > >
> > > > The format pattern:
> > > > 	--lcores='lcores[@cpus]<,lcores[@cpus]>'
> > > > lcores, cpus could be a single digit or a group.
> > > > '(' and ')' are necessary if it's a group.
> > > > If not supply '@cpus', the value of cpus uses the same as lcores.
> > > >
> > > > e.g. '1,2@(5-7),(3-5)@(0,2),(0,6)' means starting 7 EAL thread as below
> > > >   lcore 0 runs on cpuset 0x41 (cpu 0,6)
> > > >   lcore 1 runs on cpuset 0x2 (cpu 1)
> > > >   lcore 2 runs on cpuset 0xe0 (cpu 5,6,7)
> > > >   lcore 3,4,5 runs on cpuset 0x5 (cpu 0,2)
> > > >   lcore 6 runs on cpuset 0x41 (cpu 0,6)
> > > >
> > >
> > > This strikes me as very confusing, though a couple of tweaks might help with
> > > readability. The lcore 0 at the end is especially confusing.
> >
> > Didn't get you here: do you find (0,6) confusing, right?
> > Because braces implicitly specifies affinity for group of en-braced lcores?
> >
> > > Perhaps we can
> > > limit the allowed formats here,
> > > * require the lcore_id to be specified - the lack of an lcore id for the last part
> > > makes having it as lcore 0 surprising.
> >
> > Again, not sure I understand you properly:  lcore_id(s) are always specified
> > explicitly.
> > Physical cpus part might be omitted.
> >
> > > * only allow one lcore id to be given for each set of cores.
> >
> > So you mean for '(3-5)@(0,2)' user would have to: '3@(0,2),4@(0,2),5@(0,2)'?
> > I don't see big difference here, but imagine you'd like to create a pool of 32 EAL-
> > threads running on same cpu set.
> > With current syntax it is just something like: '(32-63)@(0-7)'.
> > With what you proposing it will be a very long list.
> >
> > >
> > > I think it may still be readable if we allow the core set to be omitted if its
> > > to be the same as the lcore_id.
> >
> > I think that is supported.
> > See lcore_id=1 in Steve's example above.
> > As I understand: --lcores='0,2,3-5' is equal to '-l 0,2,3-5' and to '-c 0x3d'.
> >
> > Konstantin
> >
> > >
> > > It's probably still not going to be very tidy, but I think we can improve things.
> > >
> > > /Bruce
> > >
> > > > Signed-off-by: Cunming Liang <cunming.liang@intel.com>
> > > > ---
> > > >  lib/librte_eal/common/eal_common_launch.c  |   1 -
> > > >  lib/librte_eal/common/eal_common_options.c | 262
> > ++++++++++++++++++++++++++++-
> > > >  lib/librte_eal/common/eal_options.h        |   2 +
> > > >  lib/librte_eal/linuxapp/eal/Makefile       |   1 +
> > > >  4 files changed, 261 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/lib/librte_eal/common/eal_common_launch.c
> > b/lib/librte_eal/common/eal_common_launch.c
> > > > index 599f83b..2d732b1 100644
> > > > --- a/lib/librte_eal/common/eal_common_launch.c
> > > > +++ b/lib/librte_eal/common/eal_common_launch.c
> > > > @@ -117,4 +117,3 @@ rte_eal_mp_wait_lcore(void)
> > > >  		rte_eal_wait_lcore(lcore_id);
> > > >  	}
> > > >  }
> > > > -
> > > > diff --git a/lib/librte_eal/common/eal_common_options.c
> > b/lib/librte_eal/common/eal_common_options.c
> > > > index e2810ab..fc47588 100644
> > > > --- a/lib/librte_eal/common/eal_common_options.c
> > > > +++ b/lib/librte_eal/common/eal_common_options.c
> > > > @@ -45,6 +45,7 @@
> > > >  #include <rte_lcore.h>
> > > >  #include <rte_version.h>
> > > >  #include <rte_devargs.h>
> > > > +#include <rte_memcpy.h>
> > > >
> > > >  #include "eal_internal_cfg.h"
> > > >  #include "eal_options.h"
> > > > @@ -85,6 +86,7 @@ eal_long_options[] = {
> > > >  	{OPT_XEN_DOM0, 0, 0, OPT_XEN_DOM0_NUM},
> > > >  	{OPT_CREATE_UIO_DEV, 1, NULL, OPT_CREATE_UIO_DEV_NUM},
> > > >  	{OPT_VFIO_INTR, 1, NULL, OPT_VFIO_INTR_NUM},
> > > > +	{OPT_LCORES, 1, 0, OPT_LCORES_NUM},
> > > >  	{0, 0, 0, 0}
> > > >  };
> > > >
> > > > @@ -255,9 +257,11 @@ eal_parse_corelist(const char *corelist)
> > > >  			if (min == RTE_MAX_LCORE)
> > > >  				min = idx;
> > > >  			for (idx = min; idx <= max; idx++) {
> > > > -				cfg->lcore_role[idx] = ROLE_RTE;
> > > > -				lcore_config[idx].core_index = count;
> > > > -				count++;
> > > > +				if (cfg->lcore_role[idx] != ROLE_RTE) {
> > > > +					cfg->lcore_role[idx] = ROLE_RTE;
> > > > +					lcore_config[idx].core_index = count;
> > > > +					count++;
> > > > +				}
> > > >  			}
> > > >  			min = RTE_MAX_LCORE;
> > > >  		} else
> > > > @@ -289,6 +293,241 @@ eal_parse_master_lcore(const char *arg)
> > > >  	return 0;
> > > >  }
> > > >
> > > > +/*
> > > > + * Parse elem, the elem could be single number or '(' ')' group
> > > > + * Within group elem, '-' used for a range seperator;
> > > > + *                    ',' used for a single number.
> > > > + */
> > > > +static int
> > > > +eal_parse_set(const char *input, uint16_t set[], unsigned num)
> > > > +{
> > > > +	unsigned idx;
> > > > +	const char *str = input;
> > > > +	char *end = NULL;
> > > > +	unsigned min, max;
> > > > +
> > > > +	memset(set, 0, num * sizeof(uint16_t));
> > > > +
> > > > +	while (isblank(*str))
> > > > +		str++;
> > > > +
> > > > +	/* only digit or left bracket is qulify for start point */
> > > > +	if ((!isdigit(*str) && *str != '(') || *str == '\0')
> > > > +		return -1;
> > > > +
> > > > +	/* process single number */
> > > > +	if (*str != '(') {
> > > > +		errno = 0;
> > > > +		idx = strtoul(str, &end, 10);
> > > > +		if (errno || end == NULL || idx >= num)
> > > > +			return -1;
> > > > +		else {
> > > > +			while (isblank(*end))
> > > > +				end++;
> > > > +
> > > > +			if (*end != ',' && *end != '\0' &&
> > > > +			    *end != '@')
> > > > +				return -1;
> > > > +
> > > > +			set[idx] = 1;
> > > > +			return end - input;
> > > > +		}
> > > > +	}
> > > > +
> > > > +	/* process set within bracket */
> > > > +	str++;
> > > > +	while (isblank(*str))
> > > > +		str++;
> > > > +	if (*str == '\0')
> > > > +		return -1;
> > > > +
> > > > +	min = RTE_MAX_LCORE;
> > > > +	do {
> > > > +
> > > > +		/* go ahead to the first digit */
> > > > +		while (isblank(*str))
> > > > +			str++;
> > > > +		if (!isdigit(*str))
> > > > +			return -1;
> > > > +
> > > > +		/* get the digit value */
> > > > +		errno = 0;
> > > > +		idx = strtoul(str, &end, 10);
> > > > +		if (errno || end == NULL || idx >= num)
> > > > +			return -1;
> > > > +
> > > > +		/* go ahead to separator '-',',' and ')' */
> > > > +		while (isblank(*end))
> > > > +			end++;
> > > > +		if (*end == '-') {
> > > > +			if (min == RTE_MAX_LCORE)
> > > > +				min = idx;
> > > > +			else /* avoid continuous '-' */
> > > > +				return -1;
> > > > +		} else if ((*end == ',') || (*end == ')')) {
> > > > +			max = idx;
> > > > +			if (min == RTE_MAX_LCORE)
> > > > +				min = idx;
> > > > +			for (idx = RTE_MIN(min, max);
> > > > +			     idx <= RTE_MAX(min, max); idx++)
> > > > +				set[idx] = 1;
> > > > +
> > > > +			min = RTE_MAX_LCORE;
> > > > +		} else
> > > > +			return -1;
> > > > +
> > > > +		str = end + 1;
> > > > +	} while (*end != '\0' && *end != ')');
> > > > +
> > > > +	return str - input;
> > > > +}
> > > > +
> > > > +/* convert from set array to cpuset bitmap */
> > > > +static inline int
> > > > +convert_to_cpuset(rte_cpuset_t *cpusetp,
> > > > +	      uint16_t *set, unsigned num)
> > > > +{
> > > > +	unsigned idx;
> > > > +
> > > > +	CPU_ZERO(cpusetp);
> > > > +
> > > > +	for (idx = 0; idx < num; idx++) {
> > > > +		if (!set[idx])
> > > > +			continue;
> > > > +
> > > > +		if (!lcore_config[idx].detected) {
> > > > +			RTE_LOG(ERR, EAL, "core %u "
> > > > +				"unavailable\n", idx);
> > > > +			return -1;
> > > > +		}
> > > > +
> > > > +		CPU_SET(idx, cpusetp);
> > > > +	}
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +/*
> > > > + * The format pattern: --lcores='lcores[@cpus]<,lcores[@cpus]>'
> > > > + * lcores, cpus could be a single digit or a group.
> > > > + * '(' and ')' are necessary if it's a group.
> > > > + * If not supply '@cpus', the value of cpus uses the same as lcores.
> > > > + * e.g. '1,2@(5-7),(3-5)@(0,2),(0,6)' means start 7 EAL thread as below
> > > > + *   lcore 0 runs on cpuset 0x41 (cpu 0,6)
> > > > + *   lcore 1 runs on cpuset 0x2 (cpu 1)
> > > > + *   lcore 2 runs on cpuset 0xe0 (cpu 5,6,7)
> > > > + *   lcore 3,4,5 runs on cpuset 0x5 (cpu 0,2)
> > > > + *   lcore 6 runs on cpuset 0x41 (cpu 0,6)
> > > > + */
> > > > +static int
> > > > +eal_parse_lcores(const char *lcores)
> > > > +{
> > > > +	struct rte_config *cfg = rte_eal_get_configuration();
> > > > +	static uint16_t set[RTE_MAX_LCORE];
> > > > +	unsigned idx = 0;
> > > > +	int i;
> > > > +	unsigned count = 0;
> > > > +	const char *lcore_start = NULL;
> > > > +	const char *end = NULL;
> > > > +	int offset;
> > > > +	rte_cpuset_t cpuset;
> > > > +	int ret = -1;
> > > > +
> > > > +	if (lcores == NULL)
> > > > +		return -1;
> > > > +
> > > > +	/* Remove all blank characters ahead and after */
> > > > +	while (isblank(*lcores))
> > > > +		lcores++;
> > > > +	i = strnlen(lcores, sysconf(_SC_ARG_MAX));
> > > > +	while ((i > 0) && isblank(lcores[i - 1]))
> > > > +		i--;
> > > > +
> > > > +	CPU_ZERO(&cpuset);
> > > > +
> > > > +	/* Reset lcore config */
> > > > +	for (idx = 0; idx < RTE_MAX_LCORE; idx++) {
> > > > +		cfg->lcore_role[idx] = ROLE_OFF;
> > > > +		lcore_config[idx].core_index = -1;
> > > > +		CPU_ZERO(&lcore_config[idx].cpuset);
> > > > +	}
> > > > +
> > > > +	/* Get list of cores */
> > > > +	do {
> > > > +		while (isblank(*lcores))
> > > > +			lcores++;
> > > > +		if (*lcores == '\0')
> > > > +			goto err;
> > > > +
> > > > +		/* record lcore_set start point */
> > > > +		lcore_start = lcores;
> > > > +
> > > > +		/* go across a complete bracket */
> > > > +		if (*lcore_start == '(') {
> > > > +			lcores += strcspn(lcores, ")");
> > > > +			if (*lcores++ == '\0')
> > > > +				goto err;
> > > > +		}
> > > > +
> > > > +		/* scan the separator '@', ','(next) or '\0'(finish) */
> > > > +		lcores += strcspn(lcores, "@,");
> > > > +
> > > > +		if (*lcores == '@') {
> > > > +			/* explict assign cpu_set */
> > > > +			offset = eal_parse_set(lcores + 1, set, RTE_DIM(set));
> > > > +			if (offset < 0)
> > > > +				goto err;
> > > > +
> > > > +			/* prepare cpu_set and update the end cursor */
> > > > +			if (0 > convert_to_cpuset(&cpuset,
> > > > +						  set, RTE_DIM(set)))
> > > > +				goto err;
> > > > +			end = lcores + 1 + offset;
> > > > +		} else  /* ',' or '\0' */
> > > > +			/* haven't given cpu_set, current loop done */
> > > > +			end = lcores;
> > > > +
> > > > +		if (*end != ',' && *end != '\0')
> > > > +			goto err;
> > > > +
> > > > +		/* parse lcore_set from start point */
> > > > +		if (0 > eal_parse_set(lcore_start, set, RTE_DIM(set)))
> > > > +			goto err;
> > > > +
> > > > +		/* without '@', by default using lcore_set as cpu_set */
> > > > +		if (*lcores != '@' &&
> > > > +		    0 > convert_to_cpuset(&cpuset, set, RTE_DIM(set)))
> > > > +			goto err;
> > > > +
> > > > +		/* start to update lcore_set */
> > > > +		for (idx = 0; idx < RTE_MAX_LCORE; idx++) {
> > > > +			if (!set[idx])
> > > > +				continue;
> > > > +
> > > > +			if (cfg->lcore_role[idx] != ROLE_RTE) {
> > > > +				lcore_config[idx].core_index = count;
> > > > +				cfg->lcore_role[idx] = ROLE_RTE;
> > > > +				count++;
> > > > +			}
> > > > +			rte_memcpy(&lcore_config[idx].cpuset, &cpuset,
> > > > +				   sizeof(rte_cpuset_t));
> > > > +		}
> > > > +
> > > > +		lcores = end + 1;
> > > > +	} while (*end != '\0');
> > > > +
> > > > +	if (count == 0)
> > > > +		goto err;
> > > > +
> > > > +	cfg->lcore_count = count;
> > > > +	lcores_parsed = 1;
> > > > +	ret = 0;
> > > > +
> > > > +err:
> > > > +
> > > > +	return ret;
> > > > +}
> > > > +
> > > >  static int
> > > >  eal_parse_syslog(const char *facility, struct internal_config *conf)
> > > >  {
> > > > @@ -489,6 +728,13 @@ eal_parse_common_option(int opt, const char
> > *optarg,
> > > >  		conf->log_level = log;
> > > >  		break;
> > > >  	}
> > > > +	case OPT_LCORES_NUM:
> > > > +		if (eal_parse_lcores(optarg) < 0) {
> > > > +			RTE_LOG(ERR, EAL, "invalid parameter for --"
> > > > +				OPT_LCORES "\n");
> > > > +			return -1;
> > > > +		}
> > > > +		break;
> > > >
> > > >  	/* don't know what to do, leave this to caller */
> > > >  	default:
> > > > @@ -527,7 +773,7 @@ eal_check_common_options(struct internal_config
> > *internal_cfg)
> > > >
> > > >  	if (!lcores_parsed) {
> > > >  		RTE_LOG(ERR, EAL, "CPU cores must be enabled with options "
> > > > -			"-c or -l\n");
> > > > +			"-c, -l or --lcores\n");
> > > >  		return -1;
> > > >  	}
> > > >  	if (cfg->lcore_role[cfg->master_lcore] != ROLE_RTE) {
> > > > @@ -583,6 +829,14 @@ eal_common_usage(void)
> > > >  	       "                 The argument format is
> <c1>[-c2][,c3[-c4],...]\n"
> > > >  	       "                 where c1, c2, etc are core indexes between
> 0 and %d\n"
> > > >  	       "  --"OPT_MASTER_LCORE" ID: Core ID that is used as
> master\n"
> > > > +	       "  --"OPT_LCORES" MAP: maps between lcore_set to
> > phys_cpu_set\n"
> > > > +	       "                 The argument format is\n"
> > > > +	       "
> 'lcores[@cpus]<,lcores[@cpus],...>'\n"
> > > > +	       "                 lcores and cpus list are grouped by '(' and
> ')'\n"
> > > > +	       "                 Within the group, '-' is used for range
> separator,\n"
> > > > +	       "                 ',' is used for single number separator.\n"
> > > > +	       "                 '( )' can be omitted for single element
> group, '@' \n"
> > > > +	       "                 can be omitted if cpus and lcores has the
> same value\n"
> > > >  	       "  -n NUM       : Number of memory channels\n"
> > > >  	       "  -v           : Display version information on startup\n"
> > > >  	       "  -m MB        : memory to allocate (see also --
> > "OPT_SOCKET_MEM")\n"
> > > > diff --git a/lib/librte_eal/common/eal_options.h
> > b/lib/librte_eal/common/eal_options.h
> > > > index e476f8d..a1cc59f 100644
> > > > --- a/lib/librte_eal/common/eal_options.h
> > > > +++ b/lib/librte_eal/common/eal_options.h
> > > > @@ -77,6 +77,8 @@ enum {
> > > >  	OPT_CREATE_UIO_DEV_NUM,
> > > >  #define OPT_VFIO_INTR    "vfio-intr"
> > > >  	OPT_VFIO_INTR_NUM,
> > > > +#define OPT_LCORES "lcores"
> > > > +	OPT_LCORES_NUM,
> > > >  	OPT_LONG_MAX_NUM
> > > >  };
> > > >
> > > > diff --git a/lib/librte_eal/linuxapp/eal/Makefile
> > b/lib/librte_eal/linuxapp/eal/Makefile
> > > > index 0e9c447..025d836 100644
> > > > --- a/lib/librte_eal/linuxapp/eal/Makefile
> > > > +++ b/lib/librte_eal/linuxapp/eal/Makefile
> > > > @@ -95,6 +95,7 @@ CFLAGS_eal_hugepage_info.o := -D_GNU_SOURCE
> > > >  CFLAGS_eal_pci.o := -D_GNU_SOURCE
> > > >  CFLAGS_eal_pci_vfio.o := -D_GNU_SOURCE
> > > >  CFLAGS_eal_common_whitelist.o := -D_GNU_SOURCE
> > > > +CFLAGS_eal_common_options.o := -D_GNU_SOURCE
> > > >
> > > >  # workaround for a gcc bug with noreturn attribute
> > > >  # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12603
> > > > --
> > > > 1.8.1.4
> > > >