DPDK patches and discussions
 help / color / mirror / Atom feed
From: David Marchand <david.marchand@redhat.com>
To: dev@dpdk.org
Cc: didier.pallard@6wind.com, thomas@monjalon.net,
	hari.kumarx.vemula@intel.com, stable@dpdk.org
Subject: [dpdk-dev] [PATCH 2/2] eal: fix corelist validation with disabled cores
Date: Wed, 13 Feb 2019 21:06:59 +0100	[thread overview]
Message-ID: <1550088419-10812-2-git-send-email-david.marchand@redhat.com> (raw)
In-Reply-To: <1550088419-10812-1-git-send-email-david.marchand@redhat.com>

-l and -c options are two ways to select the cores used by DPDK.
Their format differs, but the checks on the selected cores are the same.
Use an intermediate array to separate the specific parsing checks from
the common consistency checks.
The parsing functions now concentrate on validating the passed string
and do nothing more.

We can report all invalid core indexes rather than only the first error.
In the error log message, reporting [0, cfg->lcore_count - 1] as a valid
range is then wrong when the core list is not continuous.

Example on my 8 cpus laptop with core 2 and 6 disabled.
echo 0 > /sys/devices/system/cpu/cpu2/online
echo 0 > /sys/devices/system/cpu/cpu6/online

Before:
./master/app/testpmd -l 0-7 --no-huge -m 512 -- --total-num-mbufs 2048
EAL: Detected 6 lcore(s)
EAL: Detected 1 NUMA nodes
EAL: invalid core list, please check core numbers are in [0, 5] range
...

After:
./master/app/testpmd -l 0-7 --no-huge -m 512 -- --total-num-mbufs 2048
EAL: Detected 6 lcore(s)
EAL: Detected 1 NUMA nodes
EAL: lcore 2 unavailable
EAL: lcore 6 unavailable
EAL: invalid core list, please check specified cores are part of 0-1,3-5,7
...

Fixes: d888cb8b9613 ("eal: add core list input format")
Fixes: b38693b612b4 ("eal: fix core number validation")
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 lib/librte_eal/common/eal_common_options.c | 203 ++++++++++++++++++++---------
 1 file changed, 145 insertions(+), 58 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index c89f7ab..0883a86 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -419,21 +419,44 @@ static int xdigit2val(unsigned char c)
 }
 
 static int
-eal_parse_coremask(const char *coremask)
+update_lcore_config(int *cores)
 {
 	struct rte_config *cfg = rte_eal_get_configuration();
-	int i, j, idx = 0;
+	unsigned int count = 0;
+	unsigned int i;
+	int ret = 0;
+
+	for (i = 0; i < RTE_MAX_LCORE; i++) {
+		if (cores[i] != -1) {
+			if (!lcore_config[i].detected) {
+				RTE_LOG(ERR, EAL, "lcore %u unavailable\n", i);
+				ret = -1;
+				continue;
+			}
+			cfg->lcore_role[i] = ROLE_RTE;
+			count++;
+		} else {
+			cfg->lcore_role[i] = ROLE_OFF;
+		}
+		lcore_config[i].core_index = cores[i];
+	}
+	if (!ret)
+		cfg->lcore_count = count;
+	return ret;
+}
+
+static int
+eal_parse_coremask(const char *coremask, int *cores)
+{
 	unsigned count = 0;
-	char c;
+	int i, j, idx;
 	int val;
+	char c;
 
-	if (eal_service_cores_parsed())
-		RTE_LOG(WARNING, EAL,
-			"Service cores parsed before dataplane cores. "
-			"Please ensure -c is before -s or -S\n");
+	for (idx = 0; idx < RTE_MAX_LCORE; idx++)
+		cores[idx] = -1;
+	idx = 0;
 
-	if (coremask == NULL)
-		return -1;
 	/* Remove all blank characters ahead and after .
 	 * Remove 0x/0X if exists.
 	 */
@@ -458,32 +481,16 @@ static int xdigit2val(unsigned char 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;
-				lcore_config[idx].core_index = count;
+				cores[idx] = count;
 				count++;
-			} else {
-				cfg->lcore_role[idx] = ROLE_OFF;
-				lcore_config[idx].core_index = -1;
 			}
 		}
 	}
 	for (; i >= 0; i--)
 		if (coremask[i] != '0')
 			return -1;
-	for (; idx < RTE_MAX_LCORE; idx++) {
-		cfg->lcore_role[idx] = ROLE_OFF;
-		lcore_config[idx].core_index = -1;
-	}
 	if (count == 0)
 		return -1;
-	/* Update the count of enabled logical cores of the EAL configuration */
-	cfg->lcore_count = count;
 	return 0;
 }
 
@@ -564,32 +571,20 @@ static int xdigit2val(unsigned char c)
 }
 
 static int
-eal_parse_corelist(const char *corelist)
+eal_parse_corelist(const char *corelist, int *cores)
 {
-	struct rte_config *cfg = rte_eal_get_configuration();
 	unsigned count = 0;
 	char *end = NULL;
 	int min, max;
 	int idx;
 
-	if (eal_service_cores_parsed())
-		RTE_LOG(WARNING, EAL,
-			"Service cores parsed before dataplane cores. "
-			"Please ensure -l is before -s or -S\n");
-
-	if (corelist == NULL)
-		return -1;
+	for (idx = 0; idx < RTE_MAX_LCORE; idx++)
+		cores[idx] = -1;
 
 	/* Remove all blank characters ahead */
 	while (isblank(*corelist))
 		corelist++;
 
-	/* Reset config */
-	for (idx = 0; idx < RTE_MAX_LCORE; idx++) {
-		cfg->lcore_role[idx] = ROLE_OFF;
-		lcore_config[idx].core_index = -1;
-	}
-
 	/* Get list of cores */
 	min = RTE_MAX_LCORE;
 	do {
@@ -599,10 +594,10 @@ static int xdigit2val(unsigned char c)
 			return -1;
 		errno = 0;
 		idx = strtol(corelist, &end, 10);
-		if (idx < 0 || idx >= (int)cfg->lcore_count)
-			return -1;
 		if (errno || end == NULL)
 			return -1;
+		if (idx < 0 || idx >= RTE_MAX_LCORE)
+			return -1;
 		while (isblank(*end))
 			end++;
 		if (*end == '-') {
@@ -612,9 +607,8 @@ static int xdigit2val(unsigned char c)
 			if (min == RTE_MAX_LCORE)
 				min = idx;
 			for (idx = min; idx <= max; idx++) {
-				if (cfg->lcore_role[idx] != ROLE_RTE) {
-					cfg->lcore_role[idx] = ROLE_RTE;
-					lcore_config[idx].core_index = count;
+				if (cores[idx] == -1) {
+					cores[idx] = count;
 					count++;
 				}
 			}
@@ -626,10 +620,6 @@ static int xdigit2val(unsigned char c)
 
 	if (count == 0)
 		return -1;
-
-	/* Update the count of enabled logical cores of the EAL configuration */
-	cfg->lcore_count = count;
-
 	return 0;
 }
 
@@ -1105,13 +1095,81 @@ static int xdigit2val(unsigned char c)
 	return 0;
 }
 
+/* caller is responsible for freeing the returned string */
+static char *
+available_cores(void)
+{
+	char *str = NULL;
+	int previous;
+	int sequence;
+	char *tmp;
+	int idx;
+
+	/* find the first available cpu */
+	for (idx = 0; idx < RTE_MAX_LCORE; idx++) {
+		if (!lcore_config[idx].detected)
+			continue;
+		break;
+	}
+	if (idx >= RTE_MAX_LCORE)
+		return NULL;
+
+	/* first sequence */
+	if (asprintf(&str, "%d", idx) < 0)
+		return NULL;
+	previous = idx;
+	sequence = 0;
+
+	for (idx++ ; idx < RTE_MAX_LCORE; idx++) {
+		if (!lcore_config[idx].detected)
+			continue;
+
+		if (idx == previous + 1) {
+			previous = idx;
+			sequence = 1;
+			continue;
+		}
+
+		/* finish current sequence */
+		if (sequence) {
+			if (asprintf(&tmp, "%s-%d", str, previous) < 0) {
+				free(str);
+				return NULL;
+			}
+			free(str);
+			str = tmp;
+		}
+
+		/* new sequence */
+		if (asprintf(&tmp, "%s,%d", str, idx) < 0) {
+			free(str);
+			return NULL;
+		}
+		free(str);
+		str = tmp;
+		previous = idx;
+		sequence = 0;
+	}
+
+	/* finish last sequence */
+	if (sequence) {
+		if (asprintf(&tmp, "%s-%d", str, previous) < 0) {
+			free(str);
+			return NULL;
+		}
+		free(str);
+		str = tmp;
+	}
+
+	return str;
+}
+
 int
 eal_parse_common_option(int opt, const char *optarg,
 			struct internal_config *conf)
 {
 	static int b_used;
 	static int w_used;
-	struct rte_config *cfg = rte_eal_get_configuration();
 
 	switch (opt) {
 	/* blacklist */
@@ -1135,9 +1193,23 @@ static int xdigit2val(unsigned char c)
 		w_used = 1;
 		break;
 	/* coremask */
-	case 'c':
-		if (eal_parse_coremask(optarg) < 0) {
-			RTE_LOG(ERR, EAL, "invalid coremask\n");
+	case 'c': {
+		int lcore_indexes[RTE_MAX_LCORE];
+
+		if (eal_service_cores_parsed())
+			RTE_LOG(WARNING, EAL,
+				"Service cores parsed before dataplane cores. Please ensure -c is before -s or -S\n");
+		if (eal_parse_coremask(optarg, lcore_indexes) < 0) {
+			RTE_LOG(ERR, EAL, "invalid coremask syntax\n");
+			return -1;
+		}
+		if (update_lcore_config(lcore_indexes) < 0) {
+			char *available = available_cores();
+
+			RTE_LOG(ERR, EAL,
+				"invalid coremask, please check specified cores are part of %s\n",
+				available);
+			free(available);
 			return -1;
 		}
 
@@ -1151,12 +1223,26 @@ static int xdigit2val(unsigned char c)
 
 		core_parsed = LCORE_OPT_MSK;
 		break;
+	}
 	/* corelist */
-	case 'l':
-		if (eal_parse_corelist(optarg) < 0) {
+	case 'l': {
+		int lcore_indexes[RTE_MAX_LCORE];
+
+		if (eal_service_cores_parsed())
+			RTE_LOG(WARNING, EAL,
+				"Service cores parsed before dataplane cores. Please ensure -l is before -s or -S\n");
+
+		if (eal_parse_corelist(optarg, lcore_indexes) < 0) {
+			RTE_LOG(ERR, EAL, "invalid core list syntax\n");
+			return -1;
+		}
+		if (update_lcore_config(lcore_indexes) < 0) {
+			char *available = available_cores();
+
 			RTE_LOG(ERR, EAL,
-				"invalid core list, please check core numbers are in [0, %u] range\n",
-					cfg->lcore_count-1);
+				"invalid core list, please check specified cores are part of %s\n",
+				available);
+			free(available);
 			return -1;
 		}
 
@@ -1170,6 +1256,7 @@ static int xdigit2val(unsigned char c)
 
 		core_parsed = LCORE_OPT_LST;
 		break;
+	}
 	/* service coremask */
 	case 's':
 		if (eal_parse_service_coremask(optarg) < 0) {
-- 
1.8.3.1

  reply	other threads:[~2019-02-13 20:07 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-13 20:06 [dpdk-dev] [PATCH 1/2] eal: remove dead code in corelist parsing David Marchand
2019-02-13 20:06 ` David Marchand [this message]
2019-03-07 20:23   ` [dpdk-dev] [dpdk-stable] [PATCH 2/2] eal: fix corelist validation with disabled cores Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1550088419-10812-2-git-send-email-david.marchand@redhat.com \
    --to=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=didier.pallard@6wind.com \
    --cc=hari.kumarx.vemula@intel.com \
    --cc=stable@dpdk.org \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).