[v6,2/2] eal: add additional info if core mask too long

Message ID 20211103143229.34503-2-david.hunt@intel.com (mailing list archive)
State Accepted, archived
Delegated to: David Marchand
Headers
Series [v6,1/2] eal: add additional info if core list too long |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-x86_64-unit-testing fail Testing issues

Commit Message

Hunt, David Nov. 3, 2021, 2:32 p.m. UTC
  If the user requests to use an lcore above 128 using -c,
the eal will exit with "EAL: invalid coremask syntax" and
very little else useful information.

This patch adds some extra information suggesting to use --lcores
so that physical cores above RTE_MAX_LCORE (default 128) can be
used. This is achieved by using the --lcores option by mapping
the logical cores in the application to physical cores.

For example, if "-c 0x300000000000000000000000000000000" is
used, we see the following additional output on the command line:

EAL: lcore 128 >= RTE_MAX_LCORE (128)
EAL: lcore 129 >= RTE_MAX_LCORE (128)
EAL: to use high physical core ids , please use --lcores to
map them to lcore ids below RTE_MAX_LCORE,
EAL:     e.g. --lcores 0@128,1@129

Signed-off-by: David Hunt <david.hunt@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
changes in v6
   * Fixed typo (gto -> got).
   * Removed dead code for which the intention was to check for 0 at
     the start of the coremask. Also now means that '0xf' is now as
     valid coremask (previously needed to be '0x0f').
changes in v5
   * replaced strdup and frees with a const char *, as we
     just need to keep track of original pointer location.
   * reverted err: usage to return -1, as no free() needed.
   * other minod code cleanups.
changes in v4
   * fixed buffer overrun in populating lcore array.
   * switched from strlcpy to strdup due to a clang error.
changes in v3
   * added this patch to the set. Addresses the changes for
     the -c option.
---
 lib/eal/common/eal_common_options.c | 45 +++++++++++++++++++++++------
 1 file changed, 36 insertions(+), 9 deletions(-)
  

Comments

David Marchand Nov. 5, 2021, 10:50 a.m. UTC | #1
On Wed, Nov 3, 2021 at 3:33 PM David Hunt <david.hunt@intel.com> wrote:
>
> If the user requests to use an lcore above 128 using -c,
> the eal will exit with "EAL: invalid coremask syntax" and
> very little else useful information.
>
> This patch adds some extra information suggesting to use --lcores
> so that physical cores above RTE_MAX_LCORE (default 128) can be
> used. This is achieved by using the --lcores option by mapping
> the logical cores in the application to physical cores.
>
> For example, if "-c 0x300000000000000000000000000000000" is
> used, we see the following additional output on the command line:
>
> EAL: lcore 128 >= RTE_MAX_LCORE (128)
> EAL: lcore 129 >= RTE_MAX_LCORE (128)
> EAL: to use high physical core ids , please use --lcores to
> map them to lcore ids below RTE_MAX_LCORE,
> EAL:     e.g. --lcores 0@128,1@129
>
> Signed-off-by: David Hunt <david.hunt@intel.com>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Series applied, thanks.
  

Patch

diff --git a/lib/eal/common/eal_common_options.c b/lib/eal/common/eal_common_options.c
index c35798b288..0fe3220892 100644
--- a/lib/eal/common/eal_common_options.c
+++ b/lib/eal/common/eal_common_options.c
@@ -750,10 +750,12 @@  check_core_list(int *lcores, unsigned int count)
 static int
 eal_parse_coremask(const char *coremask, int *cores)
 {
-	unsigned count = 0;
+	unsigned int count = 0;
 	int i, j, idx;
 	int val;
 	char c;
+	int lcores[RTE_MAX_LCORE];
+	const char *coremask_orig = coremask;
 
 	for (idx = 0; idx < RTE_MAX_LCORE; idx++)
 		cores[idx] = -1;
@@ -770,29 +772,54 @@  eal_parse_coremask(const char *coremask, int *cores)
 	i = strlen(coremask);
 	while ((i > 0) && isblank(coremask[i - 1]))
 		i--;
-	if (i == 0)
+	if (i == 0) {
+		RTE_LOG(ERR, EAL, "No lcores in coremask: [%s]\n",
+				coremask_orig);
 		return -1;
+	}
 
-	for (i = i - 1; i >= 0 && idx < RTE_MAX_LCORE; i--) {
+	for (i = i - 1; i >= 0; i--) {
 		c = coremask[i];
 		if (isxdigit(c) == 0) {
 			/* invalid characters */
+			RTE_LOG(ERR, EAL, "invalid characters in coremask: [%s]\n",
+					coremask_orig);
 			return -1;
 		}
 		val = xdigit2val(c);
-		for (j = 0; j < BITS_PER_HEX && idx < RTE_MAX_LCORE; j++, idx++)
+		for (j = 0; j < BITS_PER_HEX; j++, idx++)
 		{
 			if ((1 << j) & val) {
-				cores[idx] = count;
+				if (count >= RTE_MAX_LCORE) {
+					RTE_LOG(ERR, EAL, "Too many lcores provided. Cannot exceed %d\n",
+							RTE_MAX_LCORE);
+					return -1;
+				}
+				lcores[count] = idx;
 				count++;
 			}
 		}
 	}
-	for (; i >= 0; i--)
-		if (coremask[i] != '0')
-			return -1;
-	if (count == 0)
+	if (count == 0) {
+		RTE_LOG(ERR, EAL, "No lcores in coremask: [%s]\n",
+				coremask_orig);
+		return -1;
+	}
+
+	if (check_core_list(lcores, count))
 		return -1;
+
+	/*
+	 * Now that we've got a list of cores no longer than
+	 * RTE_MAX_LCORE, and no lcore in that list is greater
+	 * than RTE_MAX_LCORE, populate the cores
+	 * array and return.
+	 */
+	do {
+		count--;
+		cores[lcores[count]] = count;
+	} while (count != 0);
+
 	return 0;
 }