Hi David, On 16/9/2021 1:34 PM, David Marchand wrote:
On Wed, Sep 15, 2021 at 2:11 PM David Hunt <david.h...@intel.com> wrote:If the user requests to use an lcore above 128 using -l or -c, the eal will exit with "EAL: invalid core list syntax" and very little other 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 onto to physical cores. There is no change in functionalty, just additional messages suggesting how the --lcores option might be used for the supplied list of lcores. For example, if "-l 12-14,130,132" is used, we see the following additional output on the command line: EAL: Error = One of the 5 cores provided exceeds RTE_MAX_LCORE (128) EAL: Please use --lcores instead, e.g. --lcores 0@12,1@13,2@14,3@130,4@132 Signed-off-by: David Hunt <david.h...@intel.com> --- changes in v2 * Rather than increasing the default max lcores (as in v1), it was agreed to do this instead (switch to --lcores). * As the other patches in the v1 of the set are no longer related to this change, I'll submit as a separate patch set.The -c option can use the same kind of warning.
Agreed, I'll include in the next version.
--- lib/eal/common/eal_common_options.c | 31 +++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/lib/eal/common/eal_common_options.c b/lib/eal/common/eal_common_options.c index ff5861b5f3..5c7a5a45a5 100644 --- a/lib/eal/common/eal_common_options.c +++ b/lib/eal/common/eal_common_options.c @@ -836,6 +836,8 @@ eal_parse_service_corelist(const char *corelist) return 0; } +#define MAX_LCORES_STRING 512 + static int eal_parse_corelist(const char *corelist, int *cores) { @@ -843,6 +845,9 @@ eal_parse_corelist(const char *corelist, int *cores) char *end = NULL; int min, max; int idx; + bool overflow = false; + char lcores[MAX_LCORES_STRING] = "";This code is not performance sensitive. In the worst case, like for RTE_MAX_LCORES lcores, it gives this: 0@0,1@1,2@2,3@3,4@4,5@5,6@6,7@7,8@8,9@9,10@10,11@11,12@12,13@13,14@14,15@15,16@16,17@17,18@18,19@19,20@20,21@21,22@22,23@23,24@24,25@25,26@26,27@27,28@28,29@29,30@30,31@31,32@32,33@33,34@34,35@35,36@36,37@37,38@38,39@39,40@40,41@41,42@42,43@43,44@44,45@45,46@46,47@47,48@48,49@49,50@50,51@51,52@52,53@53,54@54,55@55,56@56,57@57,58@58,59@59,60@60,61@61,62@62,63@63,64@64,65@65,66@66,67@67,68@68,69@69,70@70,71@71,72@72,73@73,74@74,75@75,76@76,77@77,78@78,79@79,80@80,81@81,82@82,83@83,84@84,85@85,86@86,87@87,88@88,89@89,90@90,91@91,92@92,93@93,94@94,95@95,96@96,97@97,98@98,99@99,100@100,101@101,102@102,103@103,104@104,105@105,106@106,107@107,108@108,109@109,110@110,111@111,112@112,113@113,114@114,115@115,116@116,117@117,118@118,119@119,120@120,121@121,122@122,123@123,124@124,125@125,126@126,127@127, Which is 800+ bytes long, let's switch do dynamic allocations.
Good point. I'll allocate a dozen bytes or so for each physical core detected, that should be enough.
+ int len = 0; for (idx = 0; idx < RTE_MAX_LCORE; idx++) cores[idx] = -1; @@ -862,8 +867,10 @@ eal_parse_corelist(const char *corelist, int *cores) idx = strtol(corelist, &end, 10); if (errno || end == NULL) return -1; - if (idx < 0 || idx >= RTE_MAX_LCORE) + if (idx < 0) return -1; + if (idx >= RTE_MAX_LCORE) + overflow = true;The code before was intermixing parsing and validation of values. This intermix was not that great. Let's separate those concerns.
I see what you mean (in your comments below). Agreed this would be a good idea.
while (isblank(*end)) end++; if (*end == '-') { @@ -873,10 +880,19 @@ eal_parse_corelist(const char *corelist, int *cores) if (min == RTE_MAX_LCORE) min = idx; for (idx = min; idx <= max; idx++) { - if (cores[idx] == -1) { - cores[idx] = count; - count++; + if (idx < RTE_MAX_LCORE) { + if (cores[idx] == -1) + cores[idx] = count; } + count++; + if (count == 1) + len = len + snprintf(&lcores[len], + MAX_LCORES_STRING - len, + "%d@%d", count-1, idx); + else + len = len + snprintf(&lcores[len], + MAX_LCORES_STRING - len, + ",%d@%d", count-1, idx);Always appending a , is easier to read, then after the loop, you just need to trim the last ,.
Sure.
} min = RTE_MAX_LCORE; } else @@ -886,6 +902,13 @@ eal_parse_corelist(const char *corelist, int *cores) if (count == 0) return -1; + if (overflow) { + RTE_LOG(ERR, EAL, "Error = One of the %d cores provided exceeds RTE_MAX_LCORE (%d)\n", + count, RTE_MAX_LCORE); + RTE_LOG(ERR, EAL, "Please use --lcores instead, e.g. --lcores %s\n", + lcores); + return -1; + } return 0;I'd rework both -c and -l parsing to fill a common data structure, then validate and generate the suggestion in common helpers.
OK, I'll take a look.
Something like: https://github.com/david-marchand/dpdk/commit/lcores This probably needs some time to look at to enhance style and carefully check for mem leaks. Tested with max_lcores = 4 (for my 8 cores laptop): $ for opt in "-c 0x" "-c 0x0" "-c 0x1" "-c 0xf" "-c 0x10" "-c 0x1f" "-c 0x11" "-c 0x30" "-l 0" "-l 0-3" "-l 0-3,2" "-l 4" "-l 0-4" "-l 0,4" "-l 4,5" do echo $opt echo quit | build/app/dpdk-testpmd $opt --log-level=lib.eal:debug --no-huge -m 20 -a 0:0.0 -- --total-num-mbufs=2048 -ia |& grep -E '(ready|RTE_MAX_LCORE|Please use|No lcore|Too many)' echo done -c 0x EAL: No lcore in coremask: 0x -c 0x0 EAL: No lcore in coremask: 0x0 -c 0x1 EAL: Main lcore 0 is ready (tid=7f03956d1c00;cpuset=[0]) -c 0xf EAL: Main lcore 0 is ready (tid=7fe464461c00;cpuset=[0]) EAL: lcore 1 is ready (tid=7fe45f924700;cpuset=[1]) EAL: lcore 2 is ready (tid=7fe45f123700;cpuset=[2]) EAL: lcore 3 is ready (tid=7fe45e922700;cpuset=[3]) -c 0x10 EAL: lcore 4 >= RTE_MAX_LCORE (4) EAL: Please use --lcores 0@4 -c 0x1f EAL: Too many lcores in coremask: 0x1f -c 0x11 EAL: lcore 4 >= RTE_MAX_LCORE (4) EAL: Please use --lcores 0@0,1@4 -c 0x30 EAL: lcore 4 >= RTE_MAX_LCORE (4) EAL: lcore 5 >= RTE_MAX_LCORE (4) EAL: Please use --lcores 0@4,1@5 -l 0 EAL: Main lcore 0 is ready (tid=7f833b17ac00;cpuset=[0]) -l 0-3 EAL: Main lcore 0 is ready (tid=7f9ff5216c00;cpuset=[0]) EAL: lcore 2 is ready (tid=7f9fefed8700;cpuset=[2]) EAL: lcore 3 is ready (tid=7f9fef6d7700;cpuset=[3]) EAL: lcore 1 is ready (tid=7f9ff06d9700;cpuset=[1]) -l 0-3,2 EAL: Main lcore 0 is ready (tid=7f106b937c00;cpuset=[0]) EAL: lcore 1 is ready (tid=7f1066dfa700;cpuset=[1]) EAL: lcore 2 is ready (tid=7f10665f9700;cpuset=[2]) EAL: lcore 3 is ready (tid=7f1065df8700;cpuset=[3]) -l 4 EAL: lcore 4 >= RTE_MAX_LCORE (4) EAL: Please use --lcores 0@4 -l 0-4 EAL: Too many lcores in core list: 0-4 -l 0,4 EAL: lcore 4 >= RTE_MAX_LCORE (4) EAL: Please use --lcores 0@0,1@4 -l 4,5 EAL: lcore 4 >= RTE_MAX_LCORE (4) EAL: lcore 5 >= RTE_MAX_LCORE (4) EAL: Please use --lcores 0@4,1@5
