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

[Neil Horman <nhorman@tuxdriver.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
> 
Do you want to burn an option letter on that?  It seems like it might be better
to search the string for 0x and base the selection of bitmap of list parsing
based on its presence or absence.


> Signed-off-by: Didier Pallard <didier.pallard@6wind.com>
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.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
> 
>