[dpdk-dev] [PATCH RFC] eal: change core mask input format

[dpdk-dev] [PATCH RFC] eal: change core mask input format

[David Marchand]

From: Didier Pallard <didier.pallard@6wind.com>

In current version, coremask can only be specified using a bitmask.
It will now be possible to specify core masks in 2 different ways:
- Using a bitmask (-c 0xnnn): bitmask must be in hex format and start with 0x
- Using a core set description in following format: -c [c1[-c2]][,c3[-c4]]...

-c 0-7,16-23,31 being equivalent to -c 0x80FF00FF

Signed-off-by: Didier Pallard <didier.pallard@6wind.com>
---
 lib/librte_eal/bsdapp/eal/eal.c   |  103 ++++++++++++++++++++++++----------
 lib/librte_eal/linuxapp/eal/eal.c |  111 ++++++++++++++++++++++++++-----------
 2 files changed, 152 insertions(+), 62 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index f8a6fc1..5c181b3 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -302,10 +302,13 @@ rte_config_init(void)
 static void
 eal_usage(const char *prgname)
 {
-	printf("\nUsage: %s -c COREMASK -n NUM [-m NB] [-r NUM] [-b <domain:bus:devid.func>]"
+	printf("\nUsage: %s -c CORESET -n NUM [-m NB] [-r NUM] [-b <domain:bus:devid.func>]"
 	       "[--proc-type primary|secondary|auto] \n\n"
 	       "EAL options:\n"
-	       "  -c COREMASK  : A hexadecimal bitmask of cores to run on\n"
+	       "  -c CORESET   : Set of cores to run on, supports two different formats:\n"
+	       "                 - A hexadecimal bitmask of cores starting with 0x\n"
+	       "                 - A list of cores in following format c1[-c2][,c3[-c4]]...\n"
+	       "                   where c1,c2,c3,... are core indexes between 0 and %d\n"
 	       "  -n NUM       : Number of memory channels\n"
 		   "  -v           : Display version information on startup\n"
 	       "                 (multiple -b options are allowed)\n"
@@ -330,7 +333,7 @@ eal_usage(const char *prgname)
 	       "  --"OPT_NO_PCI"   : disable pci\n"
 	       "  --"OPT_NO_SHCONF": no shared config (mmap'd files)\n"
 	       "\n",
-	       prgname);
+	       prgname, RTE_MAX_LCORE-1);
 	/* Allow the application to print its usage message too if hook is set */
 	if ( rte_application_usage_hook ) {
 		printf("===== Application Usage =====\n\n");
@@ -384,37 +387,79 @@ eal_parse_coremask(const char *coremask)
 	while (isblank(*coremask))
 		coremask++;
 	if (coremask[0] == '0' && ((coremask[1] == 'x')
-		||  (coremask[1] == 'X')) )
+		||  (coremask[1] == 'X')) ) {
 		coremask += 2;
-	i = strnlen(coremask, sysconf(_SC_ARG_MAX));
-	while ((i > 0) && isblank(coremask[i - 1]))
-		i--;
-	if (i == 0)
-		return -1;
+		i = strnlen(coremask, sysconf(_SC_ARG_MAX));
+		while ((i > 0) && isblank(coremask[i - 1]))
+			i--;
+		if (i == 0)
+			return -1;
 
-	for (i = i - 1; i >= 0 && idx < RTE_MAX_LCORE; i--) {
-		c = coremask[i];
-		if (isxdigit(c) == 0) {
-			/* invalid characters */
-			return (-1);
-		}
-		val = xdigit2val(c);
-		for(j = 0; j < BITS_PER_HEX && idx < RTE_MAX_LCORE; j++, idx++) {
-			if((1 << j) & val) {
-				cfg->lcore_role[idx] = ROLE_RTE;
-				if(count == 0)
-					cfg->master_lcore = idx;
-				count++;
-			} else  {
-				cfg->lcore_role[idx] = ROLE_OFF;
+		for (i = i - 1; i >= 0 && idx < RTE_MAX_LCORE; i--) {
+			c = coremask[i];
+			if (isxdigit(c) == 0) {
+				/* invalid characters */
+				return (-1);
+			}
+			val = xdigit2val(c);
+			for(j = 0; j < BITS_PER_HEX && idx < RTE_MAX_LCORE; j++, idx++) {
+				if((1 << j) & val) {
+					cfg->lcore_role[idx] = ROLE_RTE;
+					if(count == 0)
+						cfg->master_lcore = idx;
+					count++;
+				} else  {
+					cfg->lcore_role[idx] = ROLE_OFF;
+				}
 			}
 		}
-	}
-	for(; i >= 0; i--)
-		if(coremask[i] != '0')
+
+		for(; i >= 0; i--)
+			if(coremask[i] != '0')
+				return -1;
+		for(; idx < RTE_MAX_LCORE; idx++)
+			cfg->lcore_role[idx] = ROLE_OFF;
+	} else {
+		char *end = NULL;
+		int min, max;
+
+		/* Reset core roles */
+		for(idx = 0; idx < RTE_MAX_LCORE; idx++)
+			cfg->lcore_role[idx] = ROLE_OFF;
+
+		/* else this is a list of cores */
+		min = RTE_MAX_LCORE;
+		do {
+			while (isblank(*coremask))
+				coremask++;
+			idx = strtoul(coremask, &end, 10);
+			if (end != NULL) {
+				while (isblank(*end))
+					end++;
+				if (*end == '-') {
+					min = idx;
+					coremask = end + 1;
+				} 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;
+					if (*end != '\0')
+						coremask = end + 1;
+				} else {
+					break;
+				}
+			}
+		} while ((*coremask != '\0') && (end != NULL) && (*end != '\0'));
+		if ((*coremask == '\0') || (end == NULL) || (*end != '\0'))
 			return -1;
-	for(; idx < RTE_MAX_LCORE; idx++)
-		cfg->lcore_role[idx] = ROLE_OFF;
+	}
 	if(count == 0)
 		return -1;
 	return 0;
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index d7a59de..de082ab 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -330,10 +330,13 @@ eal_hugedirs_unlock(void)
 static void
 eal_usage(const char *prgname)
 {
-	printf("\nUsage: %s -c COREMASK -n NUM [-m NB] [-r NUM] [-b <domain:bus:devid.func>]"
+	printf("\nUsage: %s -c CORESET -n NUM [-m NB] [-r NUM] [-b <domain:bus:devid.func>]"
 	       "[--proc-type primary|secondary|auto] \n\n"
 	       "EAL options:\n"
-	       "  -c COREMASK  : A hexadecimal bitmask of cores to run on\n"
+	       "  -c CORESET   : Set of cores to run on, supports two different formats:\n"
+	       "                 - A hexadecimal bitmask of cores starting with 0x\n"
+	       "                 - A list of cores in following format c1[-c2][,c3[-c4]]...\n"
+	       "                   where c1,c2,c3,... are core indexes between 0 and %d\n"
 	       "  -n NUM       : Number of memory channels\n"
 		   "  -v           : Display version information on startup\n"
 	       "  -d LIB.so    : add driver (can be used multiple times)\n"
@@ -368,7 +371,7 @@ eal_usage(const char *prgname)
 	       "  --"OPT_NO_HPET"  : disable hpet\n"
 	       "  --"OPT_NO_SHCONF": no shared config (mmap'd files)\n"
 	       "\n",
-	       prgname);
+	       prgname, RTE_MAX_LCORE-1);
 	/* Allow the application to print its usage message too if hook is set */
 	if ( rte_application_usage_hook ) {
 		printf("===== Application Usage =====\n\n");
@@ -422,42 +425,84 @@ eal_parse_coremask(const char *coremask)
 	while (isblank(*coremask))
 		coremask++;
 	if (coremask[0] == '0' && ((coremask[1] == 'x')
-		||  (coremask[1] == 'X')) )
+		||  (coremask[1] == 'X')) ) {
 		coremask += 2;
-	i = strnlen(coremask, PATH_MAX);
-	while ((i > 0) && isblank(coremask[i - 1]))
-		i--;
-	if (i == 0)
-		return -1;
+		i = strnlen(coremask, PATH_MAX);
+		while ((i > 0) && isblank(coremask[i - 1]))
+			i--;
+		if (i == 0)
+			return -1;
 
-	for (i = i - 1; i >= 0 && idx < RTE_MAX_LCORE; i--) {
-		c = coremask[i];
-		if (isxdigit(c) == 0) {
-			/* invalid characters */
-			return (-1);
-		}
-		val = xdigit2val(c);
-		for(j = 0; j < BITS_PER_HEX && idx < RTE_MAX_LCORE; j++, idx++) {
-			if((1 << j) & val) {
-				if (!lcore_config[idx].detected) {
-					RTE_LOG(ERR, EAL, "lcore %u "
-					        "unavailable\n", idx);
-					return -1;
+		for (i = i - 1; i >= 0 && idx < RTE_MAX_LCORE; i--) {
+			c = coremask[i];
+			if (isxdigit(c) == 0) {
+				/* invalid characters */
+				return (-1);
+			}
+			val = xdigit2val(c);
+			for(j = 0; j < BITS_PER_HEX && idx < RTE_MAX_LCORE; j++, idx++) {
+				if((1 << j) & val) {
+					if (!lcore_config[idx].detected) {
+						RTE_LOG(ERR, EAL, "lcore %u "
+								"unavailable\n", idx);
+						return -1;
+					}
+					cfg->lcore_role[idx] = ROLE_RTE;
+					if(count == 0)
+						cfg->master_lcore = idx;
+					count++;
+				} else  {
+					cfg->lcore_role[idx] = ROLE_OFF;
 				}
-				cfg->lcore_role[idx] = ROLE_RTE;
-				if(count == 0)
-					cfg->master_lcore = idx;
-				count++;
-			} else  {
-				cfg->lcore_role[idx] = ROLE_OFF;
 			}
 		}
-	}
-	for(; i >= 0; i--)
-		if(coremask[i] != '0')
+
+		for(; i >= 0; i--)
+			if(coremask[i] != '0')
+				return -1;
+		for(; idx < RTE_MAX_LCORE; idx++)
+			cfg->lcore_role[idx] = ROLE_OFF;
+	} else {
+		char *end = NULL;
+		int min, max;
+
+		/* Reset core roles */
+		for(idx = 0; idx < RTE_MAX_LCORE; idx++)
+			cfg->lcore_role[idx] = ROLE_OFF;
+
+		/* else this is a list of cores */
+		min = RTE_MAX_LCORE;
+		do {
+			while (isblank(*coremask))
+				coremask++;
+			idx = strtoul(coremask, &end, 10);
+			if (end != NULL) {
+				while (isblank(*end))
+					end++;
+				if (*end == '-') {
+					min = idx;
+					coremask = end + 1;
+				} 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;
+					if (*end != '\0')
+						coremask = end + 1;
+				} else {
+					break;
+				}
+			}
+		} while ((*coremask != '\0') && (end != NULL) && (*end != '\0'));
+		if ((*coremask == '\0') || (end == NULL) || (*end != '\0'))
 			return -1;
-	for(; idx < RTE_MAX_LCORE; idx++)
-		cfg->lcore_role[idx] = ROLE_OFF;
+	}
 	if(count == 0)
 		return -1;
 	/* Update the count of enabled logical cores of the EAL configuration */
-- 
1.7.10.4

Re: [dpdk-dev] [PATCH RFC] eal: change core mask input format

[Burakov, Anatoly]

> From: Didier Pallard <didier.pallard@6wind.com>
> 
> In current version, coremask can only be specified using a bitmask.
> It will now be possible to specify core masks in 2 different ways:
> - Using a bitmask (-c 0xnnn): bitmask must be in hex format and start with 0x
> - Using a core set description in following format: -c [c1[-c2]][,c3[-c4]]...
> 
> -c 0-7,16-23,31 being equivalent to -c 0x80FF00FF
> 
> Signed-off-by: Didier Pallard <didier.pallard@6wind.com>

Can we perhaps use a different parameter for that? E.g. --use-cores=[c1[-c2]][,c3[-c4]]... instead of -c?

Also, it would be great if corresponding unit tests were also added.

Best regards,
Anatoly Burakov
DPDK SW Engineer

--------------------------------------------------------------
Intel Shannon Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263
Business address: Dromore House, East Park, Shannon, Co. Clare


 

Re: [dpdk-dev] [PATCH RFC] eal: change core mask input format

[Thomas Monjalon]

Hi Anatoly,

2014-05-01 15:43, Burakov, Anatoly:
> > From: Didier Pallard <didier.pallard@6wind.com>
> > 
> > In current version, coremask can only be specified using a bitmask.
> > It will now be possible to specify core masks in 2 different ways:
> > - Using a bitmask (-c 0xnnn): bitmask must be in hex format and start with
> > 0x - Using a core set description in following format: -c
> > [c1[-c2]][,c3[-c4]]...
> > 
> > -c 0-7,16-23,31 being equivalent to -c 0x80FF00FF
> > 
> > Signed-off-by: Didier Pallard <didier.pallard@6wind.com>
> 
> Can we perhaps use a different parameter for that? E.g.
> --use-cores=[c1[-c2]][,c3[-c4]]... instead of -c?

I don't understand what would be improved by adding a new parameter.
I think being able to handle the 2 syntaxes within the same option is nice.

-- 
Thomas

Re: [dpdk-dev] [PATCH RFC] eal: change core mask input format

[Burakov, Anatoly]

Hi Thomas,

> I don't understand what would be improved by adding a new parameter.
> I think being able to handle the 2 syntaxes within the same option is nice.

Well, -c option has been handling everything as hex since forever. Our own documentation usually has coremasks without the 0x prefix. That means that there's probably a lot of legacy code (including our own internal validation code, and probably a lot of customers' code too) that depend on this behavior and that will fail in an unobvious way (e.g. you specified -c 3, that gave you cores 0,1 previously, now it gives you core 3). So I personally would much prefer leaving coremask as a *coremask* (e.g. a hex mask of cores), and add a new parameter to have a core *list* that would be interchangeable with coremask.

Best regards,
Anatoly Burakov
DPDK SW Engineer