From: David Marchand <david.marchand@redhat.com>
To: Bruce Richardson <bruce.richardson@intel.com>
Cc: dev@dpdk.org, Tyler Retzlaff <roretzla@linux.microsoft.com>
Subject: Re: [PATCH v2 1/3] eal: centralize core parameter parsing
Date: Mon, 7 Apr 2025 08:58:17 +0200 [thread overview]
Message-ID: <CAJFAV8yiKs8HJDKCzJwWNDZAufj5B16ov=mX+vpv+qK9xy7vyQ@mail.gmail.com> (raw)
In-Reply-To: <20250324173030.3733517-2-bruce.richardson@intel.com>
On Mon, Mar 24, 2025 at 6:31 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> Rather than parsing the lcore parameters as they are encountered, just
> save off the lcore parameters and then parse them at the end. This
> allows better knowledge of what parameters are available or not when
> parsing.
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
> lib/eal/common/eal_common_options.c | 183 +++++++++++++---------------
> 1 file changed, 84 insertions(+), 99 deletions(-)
>
> diff --git a/lib/eal/common/eal_common_options.c b/lib/eal/common/eal_common_options.c
> index 79db9a47dd..55c49a923f 100644
> --- a/lib/eal/common/eal_common_options.c
> +++ b/lib/eal/common/eal_common_options.c
> @@ -151,7 +151,13 @@ TAILQ_HEAD_INITIALIZER(devopt_list);
>
> static int main_lcore_parsed;
> static int mem_parsed;
> -static int core_parsed;
> +static struct lcore_options {
> + const char *core_mask_opt;
> + const char *core_list_opt;
> + const char *core_map_opt;
> + const char *service_mask_opt;
> + const char *service_list_opt;
> +} lcore_options = {0};
eal_parse_main_lcore() still checks if selected main lcore is a
service lcore. I think this check is useless now.
>
> /* Allow the application to print its usage message too if set */
> static rte_usage_hook_t rte_application_usage_hook;
> @@ -674,7 +680,7 @@ eal_parse_service_coremask(const char *coremask)
> if (count == 0)
> return -1;
>
> - if (core_parsed && taken_lcore_count != count) {
> + if (taken_lcore_count != count) {
> EAL_LOG(WARNING,
> "Not all service cores are in the coremask. "
> "Please ensure -c or -l includes service cores");
> @@ -684,17 +690,6 @@ eal_parse_service_coremask(const char *coremask)
> return 0;
> }
>
> -static int
> -eal_service_cores_parsed(void)
> -{
> - int idx;
> - for (idx = 0; idx < RTE_MAX_LCORE; idx++) {
> - if (lcore_config[idx].core_role == ROLE_SERVICE)
> - return 1;
> - }
> - return 0;
> -}
> -
> static int
> update_lcore_config(int *cores)
> {
> @@ -903,7 +898,7 @@ eal_parse_service_corelist(const char *corelist)
> if (count == 0)
> return -1;
>
> - if (core_parsed && taken_lcore_count != count) {
> + if (taken_lcore_count != count) {
> EAL_LOG(WARNING,
> "Not all service cores were in the coremask. "
> "Please ensure -c or -l includes service cores");
> @@ -1673,83 +1668,20 @@ eal_parse_common_option(int opt, const char *optarg,
> a_used = 1;
> break;
> /* coremask */
> - case 'c': {
> - int lcore_indexes[RTE_MAX_LCORE];
> -
> - if (eal_service_cores_parsed())
> - EAL_LOG(WARNING,
> - "Service cores parsed before dataplane cores. Please ensure -c is before -s or -S");
> - if (rte_eal_parse_coremask(optarg, lcore_indexes) < 0) {
> - EAL_LOG(ERR, "invalid coremask syntax");
> - return -1;
> - }
> - if (update_lcore_config(lcore_indexes) < 0) {
> - char *available = available_cores();
> -
> - EAL_LOG(ERR,
> - "invalid coremask, please check specified cores are part of %s",
> - available);
> - free(available);
> - return -1;
> - }
> -
> - if (core_parsed) {
> - EAL_LOG(ERR, "Option -c is ignored, because (%s) is set!",
> - (core_parsed == LCORE_OPT_LST) ? "-l" :
> - (core_parsed == LCORE_OPT_MAP) ? "--lcores" :
> - "-c");
> - return -1;
> - }
> -
> - core_parsed = LCORE_OPT_MSK;
> + case 'c':
> + lcore_options.core_mask_opt = optarg;
> break;
> - }
> /* corelist */
> - case 'l': {
> - int lcore_indexes[RTE_MAX_LCORE];
> -
> - if (eal_service_cores_parsed())
> - EAL_LOG(WARNING,
> - "Service cores parsed before dataplane cores. Please ensure -l is before -s or -S");
> -
> - if (eal_parse_corelist(optarg, lcore_indexes) < 0) {
> - EAL_LOG(ERR, "invalid core list syntax");
> - return -1;
> - }
> - if (update_lcore_config(lcore_indexes) < 0) {
> - char *available = available_cores();
> -
> - EAL_LOG(ERR,
> - "invalid core list, please check specified cores are part of %s",
> - available);
> - free(available);
> - return -1;
> - }
> -
> - if (core_parsed) {
> - EAL_LOG(ERR, "Option -l is ignored, because (%s) is set!",
> - (core_parsed == LCORE_OPT_MSK) ? "-c" :
> - (core_parsed == LCORE_OPT_MAP) ? "--lcores" :
> - "-l");
> - return -1;
> - }
> -
> - core_parsed = LCORE_OPT_LST;
> + case 'l':
> + lcore_options.core_list_opt = optarg;
> break;
> - }
> /* service coremask */
> case 's':
> - if (eal_parse_service_coremask(optarg) < 0) {
> - EAL_LOG(ERR, "invalid service coremask");
> - return -1;
> - }
> + lcore_options.service_mask_opt = optarg;
> break;
> /* service corelist */
> case 'S':
> - if (eal_parse_service_corelist(optarg) < 0) {
> - EAL_LOG(ERR, "invalid service core list");
> - return -1;
> - }
> + lcore_options.service_list_opt = optarg;
> break;
> /* size of memory */
> case 'm':
> @@ -1918,21 +1850,7 @@ eal_parse_common_option(int opt, const char *optarg,
> #endif /* !RTE_EXEC_ENV_WINDOWS */
>
> case OPT_LCORES_NUM:
> - if (eal_parse_lcores(optarg) < 0) {
> - EAL_LOG(ERR, "invalid parameter for --"
> - OPT_LCORES);
> - return -1;
> - }
> -
> - if (core_parsed) {
> - EAL_LOG(ERR, "Option --lcores is ignored, because (%s) is set!",
> - (core_parsed == LCORE_OPT_LST) ? "-l" :
> - (core_parsed == LCORE_OPT_MSK) ? "-c" :
> - "--lcores");
> - return -1;
> - }
> -
> - core_parsed = LCORE_OPT_MAP;
> + lcore_options.core_map_opt = optarg;
> break;
> case OPT_LEGACY_MEM_NUM:
> conf->legacy_mem = 1;
> @@ -1973,6 +1891,7 @@ eal_parse_common_option(int opt, const char *optarg,
>
> }
>
> +
Unneeded empty line.
> return 0;
>
> ba_conflict:
> @@ -2046,8 +1965,74 @@ eal_adjust_config(struct internal_config *internal_cfg)
> struct internal_config *internal_conf =
> eal_get_internal_configuration();
>
> - if (!core_parsed)
> + /* handle all the various core options, ignoring order of them.
Handle
> + * First check that multiple lcore options (coremask, corelist, coremap) are
> + * not used together. Then check that the service options (coremask, corelist)
> + * are not both set. In both cases error out if multiple options are set.
> + */
> + if ((lcore_options.core_mask_opt && lcore_options.core_list_opt) ||
> + (lcore_options.core_mask_opt && lcore_options.core_map_opt) ||
> + (lcore_options.core_list_opt && lcore_options.core_map_opt)) {
> + EAL_LOG(ERR, "Only one of -c, -l and --"OPT_LCORES" may be given at a time");
> + return -1;
> + }
> + if ((lcore_options.service_mask_opt && lcore_options.service_list_opt)) {
> + EAL_LOG(ERR, "Only one of -s and -S may be given at a time");
> + return -1;
> + }
> +
> + if (lcore_options.core_mask_opt) {
> + int lcore_indexes[RTE_MAX_LCORE];
> +
> + if (rte_eal_parse_coremask(lcore_options.core_mask_opt, lcore_indexes) < 0) {
> + EAL_LOG(ERR, "invalid coremask syntax");
> + return -1;
> + }
> + if (update_lcore_config(lcore_indexes) < 0) {
> + char *available = available_cores();
> +
> + EAL_LOG(ERR,
> + "invalid coremask, please check specified cores are part of %s",
> + available);
> + free(available);
> + return -1;
> + }
> + } else if (lcore_options.core_list_opt) {
> + int lcore_indexes[RTE_MAX_LCORE];
> +
> + if (eal_parse_corelist(lcore_options.core_list_opt, lcore_indexes) < 0) {
> + EAL_LOG(ERR, "invalid core list syntax");
> + return -1;
> + }
> + if (update_lcore_config(lcore_indexes) < 0) {
> + char *available = available_cores();
> +
> + EAL_LOG(ERR,
> + "invalid core list, please check specified cores are part of %s",
> + available);
> + free(available);
> + return -1;
> + }
> + } else if (lcore_options.core_map_opt) {
> + if (eal_parse_lcores(lcore_options.core_map_opt) < 0) {
> + EAL_LOG(ERR, "invalid parameter for --" OPT_LCORES);
> + return -1;
> + }
> + } else {
> eal_auto_detect_cores(cfg);
> + }
> +
> + if (lcore_options.service_mask_opt) {
> + if (eal_parse_service_coremask(lcore_options.service_mask_opt) < 0) {
> + EAL_LOG(ERR, "invalid service coremask");
> + return -1;
> + }
> + } else if (lcore_options.service_list_opt) {
> + if (eal_parse_service_corelist(lcore_options.service_list_opt) < 0) {
> + EAL_LOG(ERR, "invalid service core list");
> + return -1;
> + }
> + }
>
> if (internal_conf->process_type == RTE_PROC_AUTO)
> internal_conf->process_type = eal_proc_type_detect();
> --
> 2.45.2
>
--
David Marchand
next prev parent reply other threads:[~2025-04-07 6:58 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-13 11:38 [RFC PATCH 0/3] allow easier use of high lcore-ids Bruce Richardson
2025-03-13 11:38 ` [RFC PATCH 1/3] eal: centralize core parameter parsing Bruce Richardson
2025-03-13 11:38 ` [RFC PATCH 2/3] eal: convert core masks and lists to core sets Bruce Richardson
2025-03-13 11:38 ` [RFC PATCH 3/3] eal: allow automatic mapping of high lcore ids Bruce Richardson
2025-03-24 17:30 ` [PATCH v2 0/3] allow easier use of high lcore-ids Bruce Richardson
2025-03-24 17:30 ` [PATCH v2 1/3] eal: centralize core parameter parsing Bruce Richardson
2025-04-07 6:58 ` David Marchand [this message]
2025-03-24 17:30 ` [PATCH v2 2/3] eal: convert core masks and lists to core sets Bruce Richardson
2025-04-07 6:59 ` David Marchand
2025-03-24 17:30 ` [PATCH v2 3/3] eal: allow automatic mapping of high lcore ids Bruce Richardson
2025-04-01 14:06 ` [PATCH v2 0/3] allow easier use of high lcore-ids Bruce Richardson
2025-04-07 7:04 ` David Marchand
2025-04-07 9:48 ` Bruce Richardson
2025-04-07 10:15 ` Morten Brørup
2025-04-07 10:40 ` Bruce Richardson
2025-04-07 11:32 ` Morten Brørup
2025-04-07 11:56 ` Bruce Richardson
2025-04-07 12:25 ` Morten Brørup
2025-04-07 12:41 ` Bruce Richardson
2025-04-07 13:18 ` Morten Brørup
2025-04-07 13:24 ` Bruce Richardson
2025-04-07 15:14 ` Stephen Hemminger
2025-04-07 15:38 ` Bruce Richardson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAJFAV8yiKs8HJDKCzJwWNDZAufj5B16ov=mX+vpv+qK9xy7vyQ@mail.gmail.com' \
--to=david.marchand@redhat.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=roretzla@linux.microsoft.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).