Re: [dpdk-dev] [PATCH 07/10] eal: add core list input format

[Bruce Richardson <bruce.richardson@intel.com>]

On Sat, Nov 22, 2014 at 10:43:39PM +0100, Thomas Monjalon wrote:
> From: Didier Pallard <didier.pallard@6wind.com>
> 
> In current version, used cores can only be specified using a bitmask.
> It will now be possible to specify cores in 2 different ways:
> - Using a bitmask (-c [0x]nnn): bitmask must be in hex format
> - Using a list in following format: -l <c1>[-c2][,c3[-c4],...]
> 
> The letter -l can stand for lcore or list.
> 
> -l 0-7,16-23,31 being equivalent to -c 0x80FF00FF
> 
> Signed-off-by: Didier Pallard <didier.pallard@6wind.com>
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>

Majority of people seem happy enough using the -l flag, so

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

> ---
>  app/test/test_eal_flags.c                  | 28 ++++++++++-
>  lib/librte_eal/common/eal_common_options.c | 81 ++++++++++++++++++++++++++++--
>  2 files changed, 103 insertions(+), 6 deletions(-)
> 
> diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c
> index 1f95d7f..5ad89c5 100644
> --- a/app/test/test_eal_flags.c
> +++ b/app/test/test_eal_flags.c
> @@ -484,7 +484,7 @@ test_invalid_r_flag(void)
>  }
>  
>  /*
> - * Test that the app doesn't run without the coremask flag. In all cases
> + * Test that the app doesn't run without the coremask/corelist flags. In all cases
>   * should give an error and fail to run
>   */
>  static int
> @@ -504,12 +504,22 @@ test_missing_c_flag(void)
>  
>  	/* -c flag but no coremask value */
>  	const char *argv1[] = { prgname, prefix, mp_flag, "-n", "3", "-c"};
> -	/* No -c flag at all */
> +	/* No -c or -l flag at all */
>  	const char *argv2[] = { prgname, prefix, mp_flag, "-n", "3"};
>  	/* bad coremask value */
>  	const char *argv3[] = { prgname, prefix, mp_flag, "-n", "3", "-c", "error" };
>  	/* sanity check of tests - valid coremask value */
>  	const char *argv4[] = { prgname, prefix, mp_flag, "-n", "3", "-c", "1" };
> +	/* -l flag but no corelist value */
> +	const char *argv5[] = { prgname, prefix, mp_flag, "-n", "3", "-l"};
> +	const char *argv6[] = { prgname, prefix, mp_flag, "-n", "3", "-l", " " };
> +	/* bad corelist values */
> +	const char *argv7[] = { prgname, prefix, mp_flag, "-n", "3", "-l", "error" };
> +	const char *argv8[] = { prgname, prefix, mp_flag, "-n", "3", "-l", "1-" };
> +	const char *argv9[] = { prgname, prefix, mp_flag, "-n", "3", "-l", "1," };
> +	const char *argv10[] = { prgname, prefix, mp_flag, "-n", "3", "-l", "1#2" };
> +	/* sanity check test - valid corelist value */
> +	const char *argv11[] = { prgname, prefix, mp_flag, "-n", "3", "-l", "1-2,3" };
>  
>  	if (launch_proc(argv1) == 0
>  			|| launch_proc(argv2) == 0
> @@ -521,6 +531,20 @@ test_missing_c_flag(void)
>  		printf("Error - process did not run ok with valid coremask value\n");
>  		return -1;
>  	}
> +
> +	if (launch_proc(argv5) == 0
> +			|| launch_proc(argv6) == 0
> +			|| launch_proc(argv7) == 0
> +			|| launch_proc(argv8) == 0
> +			|| launch_proc(argv9) == 0
> +			|| launch_proc(argv10) == 0) {
> +		printf("Error - process ran without error with invalid -l flag\n");
> +		return -1;
> +	}
> +	if (launch_proc(argv11) != 0) {
> +		printf("Error - process did not run ok with valid corelist value\n");
> +		return -1;
> +	}
>  	return 0;
>  }
>  
> diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
> index 63710b0..18d03e3 100644
> --- a/lib/librte_eal/common/eal_common_options.c
> +++ b/lib/librte_eal/common/eal_common_options.c
> @@ -32,6 +32,7 @@
>   */
>  
>  #include <stdlib.h>
> +#include <unistd.h>
>  #include <string.h>
>  #include <syslog.h>
>  #include <ctype.h>
> @@ -55,8 +56,9 @@ const char
>  eal_short_options[] =
>  	"b:" /* pci-blacklist */
>  	"w:" /* pci-whitelist */
> -	"c:"
> +	"c:" /* coremask */
>  	"d:"
> +	"l:" /* corelist */
>  	"m:"
>  	"n:"
>  	"r:"
> @@ -205,6 +207,67 @@ eal_parse_coremask(const char *coremask)
>  }
>  
>  static int
> +eal_parse_corelist(const char *corelist)
> +{
> +	struct rte_config *cfg = rte_eal_get_configuration();
> +	int i, idx = 0;
> +	unsigned count = 0;
> +	char *end = NULL;
> +	int min, max;
> +
> +	if (corelist == NULL)
> +		return -1;
> +
> +	/* Remove all blank characters ahead and after */
> +	while (isblank(*corelist))
> +		corelist++;
> +	i = strnlen(corelist, sysconf(_SC_ARG_MAX));
> +	while ((i > 0) && isblank(corelist[i - 1]))
> +		i--;
> +
> +	/* Reset core roles */
> +	for (idx = 0; idx < RTE_MAX_LCORE; idx++)
> +		cfg->lcore_role[idx] = ROLE_OFF;
> +
> +	/* Get list of cores */
> +	min = RTE_MAX_LCORE;
> +	do {
> +		while (isblank(*corelist))
> +			corelist++;
> +		if (*corelist == '\0')
> +			return -1;
> +		errno = 0;
> +		idx = strtoul(corelist, &end, 10);
> +		if (errno || end == NULL)
> +			return -1;
> +		while (isblank(*end))
> +			end++;
> +		if (*end == '-') {
> +			min = idx;
> +		} else if ((*end == ',') || (*end == '\0')) {
> +			max = idx;
> +			if (min == RTE_MAX_LCORE)
> +				min = idx;
> +			for (idx = min; idx <= max; idx++) {
> +				cfg->lcore_role[idx] = ROLE_RTE;
> +				if (count == 0)
> +					cfg->master_lcore = idx;
> +				count++;
> +			}
> +			min = RTE_MAX_LCORE;
> +		} else
> +			return -1;
> +		corelist = end + 1;
> +	} while (*end != '\0');
> +
> +	if (count == 0)
> +		return -1;
> +
> +	lcores_parsed = 1;
> +	return 0;
> +}
> +
> +static int
>  eal_parse_syslog(const char *facility, struct internal_config *conf)
>  {
>  	int i;
> @@ -304,6 +367,13 @@ eal_parse_common_option(int opt, const char *optarg,
>  			return -1;
>  		}
>  		break;
> +	/* corelist */
> +	case 'l':
> +		if (eal_parse_corelist(optarg) < 0) {
> +			RTE_LOG(ERR, EAL, "invalid core list\n");
> +			return -1;
> +		}
> +		break;
>  	/* size of memory */
>  	case 'm':
>  		conf->memory = atoi(optarg);
> @@ -421,8 +491,8 @@ eal_check_common_options(struct internal_config *internal_cfg)
>  	struct rte_config *cfg = rte_eal_get_configuration();
>  
>  	if (!lcores_parsed) {
> -		RTE_LOG(ERR, EAL, "CPU cores must be enabled with option "
> -			"-c\n");
> +		RTE_LOG(ERR, EAL, "CPU cores must be enabled with options "
> +			"-c or -l\n");
>  		return -1;
>  	}
>  
> @@ -470,6 +540,9 @@ eal_common_usage(void)
>  	       "[--proc-type primary|secondary|auto]\n\n"
>  	       "EAL common options:\n"
>  	       "  -c COREMASK  : A hexadecimal bitmask of cores to run on\n"
> +	       "  -l CORELIST  : List of cores to run on\n"
> +	       "                 The argument format is <c1>[-c2][,c3[-c4],...]\n"
> +	       "                 where c1, c2, etc are core indexes between 0 and %d\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"
> @@ -494,5 +567,5 @@ eal_common_usage(void)
>  	       "  --"OPT_NO_PCI"   : disable pci\n"
>  	       "  --"OPT_NO_HPET"  : disable hpet\n"
>  	       "  --"OPT_NO_SHCONF": no shared config (mmap'd files)\n"
> -	       "\n");
> +	       "\n", RTE_MAX_LCORE);
>  }
> -- 
> 2.1.3
>