[dpdk-dev] [PATCH v2] eal: add additional info if lcore exceeds max cores

David Hunt david.hunt at intel.com
Mon Sep 20 11:30:33 CEST 2021


Hi David,

On 16/9/2021 1:34 PM, David Marchand wrote:
> On Wed, Sep 15, 2021 at 2:11 PM David Hunt <david.hunt at 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 at 12,1 at 13,2 at 14,3 at 130,4 at 132
>>
>> Signed-off-by: David Hunt <david.hunt at 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 at 0,1 at 1,2 at 2,3 at 3,4 at 4,5 at 5,6 at 6,7 at 7,8 at 8,9 at 9,10 at 10,11 at 11,12 at 12,13 at 13,14 at 14,15 at 15,16 at 16,17 at 17,18 at 18,19 at 19,20 at 20,21 at 21,22 at 22,23 at 23,24 at 24,25 at 25,26 at 26,27 at 27,28 at 28,29 at 29,30 at 30,31 at 31,32 at 32,33 at 33,34 at 34,35 at 35,36 at 36,37 at 37,38 at 38,39 at 39,40 at 40,41 at 41,42 at 42,43 at 43,44 at 44,45 at 45,46 at 46,47 at 47,48 at 48,49 at 49,50 at 50,51 at 51,52 at 52,53 at 53,54 at 54,55 at 55,56 at 56,57 at 57,58 at 58,59 at 59,60 at 60,61 at 61,62 at 62,63 at 63,64 at 64,65 at 65,66 at 66,67 at 67,68 at 68,69 at 69,70 at 70,71 at 71,72 at 72,73 at 73,74 at 74,75 at 75,76 at 76,77 at 77,78 at 78,79 at 79,80 at 80,81 at 81,82 at 82,83 at 83,84 at 84,85 at 85,86 at 86,87 at 87,88 at 88,89 at 89,90 at 90,91 at 91,92 at 92,93 at 93,94 at 94,95 at 95,96 at 96,97 at 97,98 at 98,99 at 99,100 at 100,101 at 101,102 at 102,103 at 103,104 at 104,105 at 105,106 at 106,107 at 107,108 at 108,109 at 109,110 at 110,111 at 111,112 at 112,113 at 113,114 at 114,115 at 115,116 at 116,117 at 117,118 at 118,119 at 119,120 at 120,121 at 121,122 at 122,123 at 123,124 at 124,125 at 125,126 at 126,127 at 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 at 4
>
> -c 0x1f
> EAL: Too many lcores in coremask: 0x1f
>
> -c 0x11
> EAL: lcore 4 >= RTE_MAX_LCORE (4)
> EAL: Please use --lcores 0 at 0,1 at 4
>
> -c 0x30
> EAL: lcore 4 >= RTE_MAX_LCORE (4)
> EAL: lcore 5 >= RTE_MAX_LCORE (4)
> EAL: Please use --lcores 0 at 4,1 at 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 at 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 at 0,1 at 4
>
> -l 4,5
> EAL: lcore 4 >= RTE_MAX_LCORE (4)
> EAL: lcore 5 >= RTE_MAX_LCORE (4)
> EAL: Please use --lcores 0 at 4,1 at 5
>
>


More information about the dev mailing list