[v3,1/2] eal: add additional info if core list too long
Checks
Commit Message
If the user requests to use an lcore above 128 using -l,
the eal will exit with "EAL: invalid core list 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.
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-16,130,132" is used, we
see the following additional output on the command line:
EAL: Error = One of the 7 cores provided exceeds RTE_MAX_LCORE (128)
EAL: Please use --lcores instead, e.g.
--lcores 0@12,1@13,2@14,3@15,4@16,5@130,6@132
Signed-off-by: David Hunt <david.hunt@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.
changes in v3
* separated out some of the corelist cheking into separate function
* added extra messages for the different failure conditions.
* changed allocation of the core strings from static to dynamic
* now prints out a message for each core above RTE_MAX_LCORE
---
lib/eal/common/eal_common_options.c | 102 ++++++++++++++++++++++++----
1 file changed, 89 insertions(+), 13 deletions(-)
Comments
On Tue, Sep 21, 2021 at 12:50:14PM +0100, David Hunt wrote:
> If the user requests to use an lcore above 128 using -l,
> the eal will exit with "EAL: invalid core list 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.
>
> 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-16,130,132" is used, we
> see the following additional output on the command line:
>
> EAL: Error = One of the 7 cores provided exceeds RTE_MAX_LCORE (128)
> EAL: Please use --lcores instead, e.g.
Minor suggestion: it would be good to clarify how to use lcores and what is
happening here in the example. Something like: "Please use --lcores
instead, to map lower lcore ids onto higher-numbered cores", could help the
user understand better what is happening.
> --lcores 0@12,1@13,2@14,3@15,4@16,5@130,6@132
>
> Signed-off-by: David Hunt <david.hunt@intel.com>
With some more info to help the user:
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
On 21/9/2021 12:57 PM, Bruce Richardson wrote:
> On Tue, Sep 21, 2021 at 12:50:14PM +0100, David Hunt wrote:
>> If the user requests to use an lcore above 128 using -l,
>> the eal will exit with "EAL: invalid core list 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.
>>
>> 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-16,130,132" is used, we
>> see the following additional output on the command line:
>>
>> EAL: Error = One of the 7 cores provided exceeds RTE_MAX_LCORE (128)
>> EAL: Please use --lcores instead, e.g.
> Minor suggestion: it would be good to clarify how to use lcores and what is
> happening here in the example. Something like: "Please use --lcores
> instead, to map lower lcore ids onto higher-numbered cores", could help the
> user understand better what is happening.
Hi Bruce, how about:
EAL: Please use --lcores to map logical cores onto cores > RTE_LCORE_MAX
,e.g. --lcores 0@12,1@13,2@14,3@15,4@16,5@130,6@132
Rgds,
Dave.
>
>> --lcores 0@12,1@13,2@14,3@15,4@16,5@130,6@132
>>
>> Signed-off-by: David Hunt <david.hunt@intel.com>
> With some more info to help the user:
>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
On 21/9/2021 1:04 PM, David Hunt wrote:
>
> On 21/9/2021 12:57 PM, Bruce Richardson wrote:
>> On Tue, Sep 21, 2021 at 12:50:14PM +0100, David Hunt wrote:
>>> If the user requests to use an lcore above 128 using -l,
>>> the eal will exit with "EAL: invalid core list 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.
>>>
>>> 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-16,130,132" is used, we
>>> see the following additional output on the command line:
>>>
>>> EAL: Error = One of the 7 cores provided exceeds RTE_MAX_LCORE (128)
>>> EAL: Please use --lcores instead, e.g.
>> Minor suggestion: it would be good to clarify how to use lcores and
>> what is
>> happening here in the example. Something like: "Please use --lcores
>> instead, to map lower lcore ids onto higher-numbered cores", could
>> help the
>> user understand better what is happening.
>
>
> Hi Bruce, how about:
>
> EAL: Please use --lcores to map logical cores onto cores >
> RTE_LCORE_MAX ,e.g. --lcores 0@12,1@13,2@14,3@15,4@16,5@130,6@132
>
> Rgds,
> Dave.
>
>
>
I think this should do it, as it clarifies the mapping:
EAL: lcore 130 >= RTE_MAX_LCORE (128)
EAL: lcore 132 >= RTE_MAX_LCORE (128)
EAL: to use high physical core ids , please use --lcores to map them to
lcore ids below RTE_LCORE_MAX, e.g.'--lcores
0@12,1@13,2@14,3@15,4@16,5@130,6@132'
Thanks,
Dave.
>>
>>> --lcores 0@12,1@13,2@14,3@15,4@16,5@130,6@132
>>>
>>> Signed-off-by: David Hunt <david.hunt@intel.com>
>> With some more info to help the user:
>>
>> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
On Tue, Sep 21, 2021 at 02:16:35PM +0100, David Hunt wrote:
> On 21/9/2021 1:04 PM, David Hunt wrote:
>
> On 21/9/2021 12:57 PM, Bruce Richardson wrote:
>
> On Tue, Sep 21, 2021 at 12:50:14PM +0100, David Hunt wrote:
>
> If the user requests to use an lcore above 128 using -l,
> the eal will exit with "EAL: invalid core list 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.
> 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-16,130,132" is used, we
> see the following additional output on the command line:
> EAL: Error = One of the 7 cores provided exceeds RTE_MAX_LCORE (128)
> EAL: Please use --lcores instead, e.g.
>
> Minor suggestion: it would be good to clarify how to use lcores and
> what is
> happening here in the example. Something like: "Please use --lcores
> instead, to map lower lcore ids onto higher-numbered cores", could
> help the
> user understand better what is happening.
>
> Hi Bruce, how about:
> EAL: Please use --lcores to map logical cores onto cores >
> RTE_LCORE_MAX ,e.g. --lcores 0@12,1@13,2@14,3@15,4@16,5@130,6@132
> Rgds,
> Dave.
>
> I think this should do it, as it clarifies the mapping:
>
> EAL: lcore 130 >= RTE_MAX_LCORE (128)
> EAL: lcore 132 >= RTE_MAX_LCORE (128)
> EAL: to use high physical core ids , please use --lcores to map them to
> lcore ids below RTE_LCORE_MAX, e.g. '--lcores
> 0@12,1@13,2@14,3@15,4@16,5@130,6@132'
>
Text looks good to me.
Again minor nits: I think the continued lines should be indented,
and you should probably wrap immediately after the "e.g." rather than in
the middle of the parameter set.
/Bruce
On Tue, Sep 21, 2021 at 1:50 PM David Hunt <david.hunt@intel.com> wrote:
> static int
> eal_parse_coremask(const char *coremask, int *cores)
> {
> @@ -839,54 +880,89 @@ eal_parse_service_corelist(const char *corelist)
> static int
> eal_parse_corelist(const char *corelist, int *cores)
> {
> - unsigned count = 0;
> + unsigned int count = 0, k;
> char *end = NULL;
> int min, max;
> int idx;
> + int lcores[RTE_MAX_LCORE];
Static array...
"-l 0-RTE_MAX_LCORE" / "-c 0x1<enough f to fill RTE_MAX_LCORE>" / "-l
0-(RTE_MAX_LCORE-1),0" crash.
Please set RTE_MAX_LCORE to 4 (or something that is smaller than your
system core count) and run the tests I provided in my previous mail.
On 21/9/2021 2:51 PM, David Marchand wrote:
> On Tue, Sep 21, 2021 at 1:50 PM David Hunt <david.hunt@intel.com> wrote:
>> static int
>> eal_parse_coremask(const char *coremask, int *cores)
>> {
>> @@ -839,54 +880,89 @@ eal_parse_service_corelist(const char *corelist)
>> static int
>> eal_parse_corelist(const char *corelist, int *cores)
>> {
>> - unsigned count = 0;
>> + unsigned int count = 0, k;
>> char *end = NULL;
>> int min, max;
>> int idx;
>> + int lcores[RTE_MAX_LCORE];
> Static array...
>
> "-l 0-RTE_MAX_LCORE" / "-c 0x1<enough f to fill RTE_MAX_LCORE>" / "-l
> 0-(RTE_MAX_LCORE-1),0" crash.
>
> Please set RTE_MAX_LCORE to 4 (or something that is smaller than your
> system core count) and run the tests I provided in my previous mail.
>
Hi David,
I did set the lcore_max to 4, and ran the tests you provided, and they
all looked OK.
However, as you have pointed out, there is a problem with the lcores
array, which I have now fixed, I'll push up shortly. I'm not expecting
to populate with more than RTE_MAX_LCORE elements, but there was an edge
case where one extra was being added.
Thanks again,
Dave.
@@ -709,6 +709,47 @@ update_lcore_config(int *cores)
return ret;
}
+static int
+check_core_list(int *lcores, unsigned int count)
+{
+ unsigned int i, j;
+ char *lcorestr;
+ int len = 0;
+ bool overflow = false;
+
+ for (j = 0; j < count; j++) {
+ if (lcores[j] >= RTE_MAX_LCORE) {
+ RTE_LOG(ERR, EAL, "lcore %d >= RTE_MAX_LCORE (%d)\n",
+ lcores[j], RTE_MAX_LCORE);
+ overflow = true;
+ }
+ }
+ if (overflow) {
+ /*
+ * If we've encountered a core that's greater than
+ * RTE_MAX_LCORE, suggest using --lcores option to
+ * map lcores onto physical cores greater than
+ * RTE_MAX_LCORE, then return.
+ */
+ lcorestr = calloc(1, RTE_MAX_LCORE * 10);
+ if (lcorestr == NULL) {
+ RTE_LOG(ERR, EAL, "Unable to allocate lcore string\n");
+ return -ENOMEM;
+ }
+ for (i = 0; i < count; i++)
+ len = len + snprintf(&lcorestr[len],
+ RTE_MAX_LCORE * 10 - len,
+ "%d@%d,", i, lcores[i]);
+ if (len > 0)
+ lcorestr[len-1] = 0;
+ RTE_LOG(ERR, EAL, "Please use --lcores instead, e.g. --lcores %s\n",
+ lcorestr);
+ free(lcorestr);
+ return -1;
+ }
+ return 0;
+}
+
static int
eal_parse_coremask(const char *coremask, int *cores)
{
@@ -839,54 +880,89 @@ eal_parse_service_corelist(const char *corelist)
static int
eal_parse_corelist(const char *corelist, int *cores)
{
- unsigned count = 0;
+ unsigned int count = 0, k;
char *end = NULL;
int min, max;
int idx;
+ int lcores[RTE_MAX_LCORE];
+ char *corelist_copy;
for (idx = 0; idx < RTE_MAX_LCORE; idx++)
cores[idx] = -1;
+ corelist_copy = calloc(1, strlen(corelist)+1);
+ if (corelist_copy == NULL) {
+ RTE_LOG(ERR, EAL, "Unable to allocate corelist copy\n");
+ return -ENOMEM;
+ }
+
+ strlcpy(corelist_copy, corelist, strlen(corelist)+1);
+
/* Remove all blank characters ahead */
while (isblank(*corelist))
corelist++;
/* Get list of cores */
- min = RTE_MAX_LCORE;
+ min = -1;
do {
while (isblank(*corelist))
corelist++;
if (*corelist == '\0')
- return -1;
+ goto err;
errno = 0;
idx = strtol(corelist, &end, 10);
if (errno || end == NULL)
- return -1;
- if (idx < 0 || idx >= RTE_MAX_LCORE)
- return -1;
+ goto err;
+ if (idx < 0)
+ goto err;
while (isblank(*end))
end++;
if (*end == '-') {
min = idx;
} else if ((*end == ',') || (*end == '\0')) {
max = idx;
- if (min == RTE_MAX_LCORE)
+ if (min == -1)
min = idx;
for (idx = min; idx <= max; idx++) {
- if (cores[idx] == -1) {
- cores[idx] = count;
- count++;
+ lcores[count] = idx;
+ count++;
+ if (count > RTE_MAX_LCORE) {
+ RTE_LOG(ERR, EAL, "Too many lcores provided. Cannot exceed %d\n",
+ RTE_MAX_LCORE);
+ goto err;
}
}
- min = RTE_MAX_LCORE;
+ min = -1;
} else
- return -1;
+ goto err;
corelist = end + 1;
} while (*end != '\0');
if (count == 0)
- return -1;
+ goto err;
+
+ if (check_core_list(lcores, count))
+ goto err;
+
+ /*
+ * Now that we've gto 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.
+ */
+
+ for (k = 0; k < count; k++)
+ cores[lcores[k]] = k;
+
+ if (corelist_copy)
+ free(corelist_copy);
+
return 0;
+err:
+ if (corelist_copy)
+ free(corelist_copy);
+
+ return -1;
}
/* Changes the lcore id of the main thread */