[dpdk-dev,v1] eal: add error check for core options
Checks
Commit Message
Error information on the current core
usage list,mask and map were incomplete.
Added states to differentiate core usage
and to inform user.
Signed-off-by: Marko Kovacevic <marko.kovacevic@intel.com>
---
doc/guides/testpmd_app_ug/run_app.rst | 4 ++++
lib/librte_eal/common/eal_common_options.c | 33 +++++++++++++++++++++++++++---
2 files changed, 34 insertions(+), 3 deletions(-)
Comments
On 01-Feb-18 3:39 PM, Marko Kovacevic wrote:
> Error information on the current core
> usage list,mask and map were incomplete.
> Added states to differentiate core usage
> and to inform user.
Nitpicking, but line width on commit message is a little on the short
side...
>
> Signed-off-by: Marko Kovacevic <marko.kovacevic@intel.com>
> ---
> doc/guides/testpmd_app_ug/run_app.rst | 4 ++++
> lib/librte_eal/common/eal_common_options.c | 33 +++++++++++++++++++++++++++---
> 2 files changed, 34 insertions(+), 3 deletions(-)
>
> diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
> index 46da1df..26500bf 100644
> --- a/doc/guides/testpmd_app_ug/run_app.rst
> +++ b/doc/guides/testpmd_app_ug/run_app.rst
> @@ -62,6 +62,10 @@ See the DPDK Getting Started Guides for more information on these options.
> The grouping ``()`` can be omitted for single element group.
> The ``@`` can be omitted if cpus and lcores have the same value.
>
> +.. Note::
> + When ``--lcores`` is in use, the options ``-l`` and ``-c`` cannot be used.
> +
> +
> * ``--master-lcore ID``
>
> Core ID that is used as master.
> diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
> index b6d2762..6604c64 100644
> --- a/lib/librte_eal/common/eal_common_options.c
> +++ b/lib/librte_eal/common/eal_common_options.c
> @@ -57,6 +57,9 @@
> #include "eal_filesystem.h"
>
> #define BITS_PER_HEX 4
> +#define LCORE_OPT_LST 1
> +#define LCORE_OPT_MSK 2
> +#define LCORE_OPT_MAP 3
>
> const char
> eal_short_options[] =
> @@ -1028,7 +1031,15 @@ eal_parse_common_option(int opt, const char *optarg,
> RTE_LOG(ERR, EAL, "invalid coremask\n");
> return -1;
> }
> - core_parsed = 1;
> +
> + if (core_parsed) {
> + RTE_LOG(ERR, EAL, "Core Mask Option is ignored, because core (%s) is set!\n",
> + (core_parsed == LCORE_OPT_LST)?"LIST" :
> + (core_parsed == LCORE_OPT_MAP)?"MAP" : "Unknown");
i think "LIST" and "MAP" are terribly undescriptive. It would be better
to put the respective cmdline arguments ("-l" or "-c") there instead.
Same applies to other cases.
Otherwise,
Reviewed-by: Anatoly Burakov <anatoly.burakov@intel.com>
> + return -1;
> + }
> +
> + core_parsed = LCORE_OPT_MSK;
> break;
> /* corelist */
> case 'l':
> @@ -1036,7 +1047,15 @@ eal_parse_common_option(int opt, const char *optarg,
> RTE_LOG(ERR, EAL, "invalid core list\n");
> return -1;
> }
> - core_parsed = 1;
> +
> + if (core_parsed) {
> + RTE_LOG(ERR, EAL, "Core List Option is ignored, because core (%s) is set!\n",
> + (core_parsed == LCORE_OPT_MSK)?"LIST" :
> + (core_parsed == LCORE_OPT_MAP)?"MAP" : "Unknown");
> + return -1;
> + }
> +
> + core_parsed = LCORE_OPT_LST;
> break;
> /* service coremask */
> case 's':
> @@ -1156,7 +1175,15 @@ eal_parse_common_option(int opt, const char *optarg,
> OPT_LCORES "\n");
> return -1;
> }
> - core_parsed = 1;
> +
> + if (core_parsed) {
> + RTE_LOG(ERR, EAL, "Core Map Option is ignored, because core (%s) is set!\n",
> + (core_parsed == LCORE_OPT_LST)?"LIST" :
> + (core_parsed == LCORE_OPT_MSK)?"MASK" : "Unknown");
> + return -1;
> + }
> +
> + core_parsed = LCORE_OPT_MAP;
> break;
>
> /* don't know what to do, leave this to caller */
>
@@ -62,6 +62,10 @@ See the DPDK Getting Started Guides for more information on these options.
The grouping ``()`` can be omitted for single element group.
The ``@`` can be omitted if cpus and lcores have the same value.
+.. Note::
+ When ``--lcores`` is in use, the options ``-l`` and ``-c`` cannot be used.
+
+
* ``--master-lcore ID``
Core ID that is used as master.
@@ -57,6 +57,9 @@
#include "eal_filesystem.h"
#define BITS_PER_HEX 4
+#define LCORE_OPT_LST 1
+#define LCORE_OPT_MSK 2
+#define LCORE_OPT_MAP 3
const char
eal_short_options[] =
@@ -1028,7 +1031,15 @@ eal_parse_common_option(int opt, const char *optarg,
RTE_LOG(ERR, EAL, "invalid coremask\n");
return -1;
}
- core_parsed = 1;
+
+ if (core_parsed) {
+ RTE_LOG(ERR, EAL, "Core Mask Option is ignored, because core (%s) is set!\n",
+ (core_parsed == LCORE_OPT_LST)?"LIST" :
+ (core_parsed == LCORE_OPT_MAP)?"MAP" : "Unknown");
+ return -1;
+ }
+
+ core_parsed = LCORE_OPT_MSK;
break;
/* corelist */
case 'l':
@@ -1036,7 +1047,15 @@ eal_parse_common_option(int opt, const char *optarg,
RTE_LOG(ERR, EAL, "invalid core list\n");
return -1;
}
- core_parsed = 1;
+
+ if (core_parsed) {
+ RTE_LOG(ERR, EAL, "Core List Option is ignored, because core (%s) is set!\n",
+ (core_parsed == LCORE_OPT_MSK)?"LIST" :
+ (core_parsed == LCORE_OPT_MAP)?"MAP" : "Unknown");
+ return -1;
+ }
+
+ core_parsed = LCORE_OPT_LST;
break;
/* service coremask */
case 's':
@@ -1156,7 +1175,15 @@ eal_parse_common_option(int opt, const char *optarg,
OPT_LCORES "\n");
return -1;
}
- core_parsed = 1;
+
+ if (core_parsed) {
+ RTE_LOG(ERR, EAL, "Core Map Option is ignored, because core (%s) is set!\n",
+ (core_parsed == LCORE_OPT_LST)?"LIST" :
+ (core_parsed == LCORE_OPT_MSK)?"MASK" : "Unknown");
+ return -1;
+ }
+
+ core_parsed = LCORE_OPT_MAP;
break;
/* don't know what to do, leave this to caller */