* [RFC PATCH 0/3] allow easier use of high lcore-ids
@ 2025-03-13 11:38 Bruce Richardson
2025-03-13 11:38 ` [RFC PATCH 1/3] eal: centralize core parameter parsing Bruce Richardson
` (3 more replies)
0 siblings, 4 replies; 23+ messages in thread
From: Bruce Richardson @ 2025-03-13 11:38 UTC (permalink / raw)
To: dev; +Cc: Bruce Richardson
Traditionally, DPDK has had a direct mapping of internal lcore-ids, to
the actual core numbers in use. With higher core count servers becoming
more prevalent the issue becomes one of increasing memory footprint when
using such a scheme, due to the need to have all arrays dimensioned for
all cores on the system, whether or not those cores are in use by the
app.
Therefore, the decision was made in the past to not expand the
build-time RTE_MAX_LCORE value beyond 128. Instead, it was recommended
that users use the "--lcores" EAL parameter to take the high-numbered
cores they wish to use and map them to lcore-ids within the 0 - 128
range. While this works, this is a little clunky as it means that
instead of just passing, for example, "-l 130-139", the user must
instead pass "--lcores 0@130,1@131,2@132,3@133,...."
This patchset attempts to simplify the situation by adding a new flag to
do this mapping automatically. To use cores 130-139 and map them to ids
0-9 internally, the EAL args now become: "-l 130-139 --map-lcore-ids".
Adding this new parameter required some rework of the existing arg
parsing code, because in current DPDK the args are parsed and checked in
the order they appear on the commandline. This means that using the
example above, the core parameter 130-139 will be rejected immediately
before the "map-lcore-ids" parameter is seen. To work around this, the
core (and service core) parameters are not parsed when seen, instead
they are only saved off and parsed after all arguments are parsed. The
"-l" and "-c" parameters are converted into "--lcores" arguments, so all
assigning of lcore ids is done there in all cases.
TODOs and requests-for-feedback:
- is there a suitable single-letter abbreviation we could use for this
mapping option. For example, if using "x" it would mean we could use
e.g. "-xl 130-139" for core options.
- still printfs in the code. This is to make it clearer for anyone
testing what is happening.
- doc updates - will be done if feedback is positive to move from RFC to
proper patch.
Bruce Richardson (3):
eal: centralize core parameter parsing
eal: convert core masks and lists to core sets
eal: allow automatic mapping of high lcore ids
drivers/event/dlb2/dlb2_priv.h | 2 -
drivers/event/dlb2/pf/base/dlb2_resource.c | 48 +--
lib/eal/common/eal_common_options.c | 338 +++++++--------------
lib/eal/common/eal_options.h | 2 +
lib/eal/include/rte_eal.h | 2 +-
5 files changed, 126 insertions(+), 266 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 23+ messages in thread
* [RFC PATCH 1/3] eal: centralize core parameter parsing
2025-03-13 11:38 [RFC PATCH 0/3] allow easier use of high lcore-ids Bruce Richardson
@ 2025-03-13 11:38 ` Bruce Richardson
2025-03-13 11:38 ` [RFC PATCH 2/3] eal: convert core masks and lists to core sets Bruce Richardson
` (2 subsequent siblings)
3 siblings, 0 replies; 23+ messages in thread
From: Bruce Richardson @ 2025-03-13 11:38 UTC (permalink / raw)
To: dev; +Cc: Bruce Richardson
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};
/* 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,
}
+
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.
+ * 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.43.0
^ permalink raw reply [flat|nested] 23+ messages in thread
* [RFC PATCH 2/3] eal: convert core masks and lists to core sets
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 ` 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
3 siblings, 0 replies; 23+ messages in thread
From: Bruce Richardson @ 2025-03-13 11:38 UTC (permalink / raw)
To: dev; +Cc: Bruce Richardson
Rather than directly parsing and working with the core mask and core
list parameters, convert them into core maps, and centralise all core
parsing there. This allows future work to adjust the mappings of cores
when generating that mapping parameter.
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
NOTE: this does change the behaviour of the internal API rte_eal_parse
core_mask, used by dlb driver. However, since this is an internal API
changing behaviour should be allowed AFAIK.
---
drivers/event/dlb2/dlb2_priv.h | 2 -
drivers/event/dlb2/pf/base/dlb2_resource.c | 48 ++---
lib/eal/common/eal_common_options.c | 193 ++++-----------------
3 files changed, 53 insertions(+), 190 deletions(-)
diff --git a/drivers/event/dlb2/dlb2_priv.h b/drivers/event/dlb2/dlb2_priv.h
index 52da31ed31..63c0bea155 100644
--- a/drivers/event/dlb2/dlb2_priv.h
+++ b/drivers/event/dlb2/dlb2_priv.h
@@ -737,8 +737,6 @@ void dlb2_event_build_hcws(struct dlb2_port *qm_port,
uint8_t *sched_type,
uint8_t *queue_id);
-/* Extern functions */
-extern int rte_eal_parse_coremask(const char *coremask, int *cores);
/* Extern globals */
extern struct process_local_port_data dlb2_port[][DLB2_NUM_PORT_TYPES];
diff --git a/drivers/event/dlb2/pf/base/dlb2_resource.c b/drivers/event/dlb2/pf/base/dlb2_resource.c
index 3004902118..c4bc248afc 100644
--- a/drivers/event/dlb2/pf/base/dlb2_resource.c
+++ b/drivers/event/dlb2/pf/base/dlb2_resource.c
@@ -922,49 +922,31 @@ dlb2_resource_probe(struct dlb2_hw *hw, const void *probe_args)
{
const struct dlb2_devargs *args = (const struct dlb2_devargs *)probe_args;
const char *mask = args ? args->producer_coremask : NULL;
- int cpu = 0, cnt = 0, cores[RTE_MAX_LCORE], i;
+ int cpu = 0;
if (args) {
mask = (const char *)args->producer_coremask;
}
- if (mask && rte_eal_parse_coremask(mask, cores)) {
- DLB2_HW_ERR(hw, ": Invalid producer coremask=%s\n", mask);
- return -1;
- }
-
+ cpu = rte_get_next_lcore(-1, 1, 0);
hw->num_prod_cores = 0;
- for (i = 0; i < RTE_MAX_LCORE; i++) {
- bool is_pcore = (mask && cores[i] != -1);
-
- if (rte_lcore_is_enabled(i)) {
- if (is_pcore) {
- /*
- * Populate the producer cores from parsed
- * coremask
- */
- hw->prod_core_list[cores[i]] = i;
- hw->num_prod_cores++;
-
- } else if ((++cnt == DLB2_EAL_PROBE_CORE ||
- rte_lcore_count() < DLB2_EAL_PROBE_CORE)) {
- /*
- * If no producer coremask is provided, use the
- * second EAL core to probe
- */
- cpu = i;
- break;
- }
- } else if (is_pcore) {
- DLB2_HW_ERR(hw, "Producer coremask(%s) must be a subset of EAL coremask\n",
- mask);
+ if (mask) {
+ int n = rte_eal_parse_coremask(mask, hw->prod_core_list);
+ if (n <= 0) {
+ DLB2_HW_ERR(hw, ": Invalid producer coremask=%s\n", mask);
return -1;
}
+ hw->num_prod_cores = n;
+ cpu = hw->prod_core_list[0];
+ for (u8 i = 0; i < hw->num_prod_cores; i++) {
+ if (!rte_lcore_is_enabled(hw->prod_core_list[i])) {
+ DLB2_HW_ERR(hw, "Producer coremask(%s) must be a subset of EAL coremask\n",
+ mask);
+ return -1;
+ }
+ }
}
- /* Use the first core in producer coremask to probe */
- if (hw->num_prod_cores)
- cpu = hw->prod_core_list[0];
dlb2_get_pp_allocation(hw, cpu, DLB2_LDB_PORT);
dlb2_get_pp_allocation(hw, cpu, DLB2_DIR_PORT);
diff --git a/lib/eal/common/eal_common_options.c b/lib/eal/common/eal_common_options.c
index 55c49a923f..807ba760ce 100644
--- a/lib/eal/common/eal_common_options.c
+++ b/lib/eal/common/eal_common_options.c
@@ -690,33 +690,6 @@ eal_parse_service_coremask(const char *coremask)
return 0;
}
-static int
-update_lcore_config(int *cores)
-{
- struct rte_config *cfg = rte_eal_get_configuration();
- unsigned int count = 0;
- unsigned int i;
- int ret = 0;
-
- for (i = 0; i < RTE_MAX_LCORE; i++) {
- if (cores[i] != -1) {
- if (eal_cpu_detected(i) == 0) {
- EAL_LOG(ERR, "lcore %u unavailable", i);
- ret = -1;
- continue;
- }
- cfg->lcore_role[i] = ROLE_RTE;
- count++;
- } else {
- cfg->lcore_role[i] = ROLE_OFF;
- }
- lcore_config[i].core_index = cores[i];
- }
- if (!ret)
- cfg->lcore_count = count;
- return ret;
-}
-
static int
check_core_list(int *lcores, unsigned int count)
{
@@ -759,7 +732,6 @@ int
rte_eal_parse_coremask(const char *coremask, int *cores)
{
const char *coremask_orig = coremask;
- int lcores[RTE_MAX_LCORE];
unsigned int count = 0;
int i, j, idx;
int val;
@@ -803,30 +775,19 @@ rte_eal_parse_coremask(const char *coremask, int *cores)
RTE_MAX_LCORE);
return -1;
}
- lcores[count++] = idx;
+ cores[count++] = idx;
}
}
}
if (count == 0) {
- EAL_LOG(ERR, "No lcores in coremask: [%s]",
- coremask_orig);
+ EAL_LOG(ERR, "No lcores in coremask: [%s]", coremask_orig);
return -1;
}
- if (check_core_list(lcores, count))
+ if (check_core_list(cores, count) != 0)
return -1;
- /*
- * Now that we've got 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.
- */
- do {
- count--;
- cores[lcores[count]] = count;
- } while (count != 0);
-
- return 0;
+ return count;
}
static int
@@ -911,7 +872,6 @@ static int
eal_parse_corelist(const char *corelist, int *cores)
{
unsigned int count = 0, i;
- int lcores[RTE_MAX_LCORE];
char *end = NULL;
int min, max;
int idx;
@@ -949,7 +909,7 @@ eal_parse_corelist(const char *corelist, int *cores)
/* Check if this idx is already present */
for (i = 0; i < count; i++) {
- if (lcores[i] == idx)
+ if (cores[i] == idx)
dup = true;
}
if (dup)
@@ -959,7 +919,7 @@ eal_parse_corelist(const char *corelist, int *cores)
RTE_MAX_LCORE);
return -1;
}
- lcores[count++] = idx;
+ cores[count++] = idx;
}
min = -1;
} else
@@ -967,23 +927,15 @@ eal_parse_corelist(const char *corelist, int *cores)
corelist = end + 1;
} while (*end != '\0');
- if (count == 0)
+ if (count == 0) {
+ EAL_LOG(ERR, "No lcores in corelist");
return -1;
+ }
- if (check_core_list(lcores, count))
+ if (check_core_list(cores, count))
return -1;
- /*
- * Now that we've got 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.
- */
- do {
- count--;
- cores[lcores[count]] = count;
- } while (count != 0);
-
- return 0;
+ return count;
}
/* Changes the lcore id of the main thread */
@@ -1500,75 +1452,6 @@ eal_parse_base_virtaddr(const char *arg)
return 0;
}
-/* caller is responsible for freeing the returned string */
-static char *
-available_cores(void)
-{
- char *str = NULL;
- int previous;
- int sequence;
- char *tmp;
- int idx;
-
- /* find the first available cpu */
- for (idx = 0; idx < RTE_MAX_LCORE; idx++) {
- if (eal_cpu_detected(idx) == 0)
- continue;
- break;
- }
- if (idx >= RTE_MAX_LCORE)
- return NULL;
-
- /* first sequence */
- if (asprintf(&str, "%d", idx) < 0)
- return NULL;
- previous = idx;
- sequence = 0;
-
- for (idx++ ; idx < RTE_MAX_LCORE; idx++) {
- if (eal_cpu_detected(idx) == 0)
- continue;
-
- if (idx == previous + 1) {
- previous = idx;
- sequence = 1;
- continue;
- }
-
- /* finish current sequence */
- if (sequence) {
- if (asprintf(&tmp, "%s-%d", str, previous) < 0) {
- free(str);
- return NULL;
- }
- free(str);
- str = tmp;
- }
-
- /* new sequence */
- if (asprintf(&tmp, "%s,%d", str, idx) < 0) {
- free(str);
- return NULL;
- }
- free(str);
- str = tmp;
- previous = idx;
- sequence = 0;
- }
-
- /* finish last sequence */
- if (sequence) {
- if (asprintf(&tmp, "%s-%d", str, previous) < 0) {
- free(str);
- return NULL;
- }
- free(str);
- str = tmp;
- }
-
- return str;
-}
-
#define HUGE_UNLINK_NEVER "never"
static int
@@ -1981,43 +1864,43 @@ eal_adjust_config(struct internal_config *internal_cfg)
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) {
+ if (lcore_options.core_mask_opt || lcore_options.core_list_opt) {
int lcore_indexes[RTE_MAX_LCORE];
+ int nb_indexes = lcore_options.core_list_opt ?
+ eal_parse_corelist(lcore_options.core_list_opt, lcore_indexes) :
+ rte_eal_parse_coremask(lcore_options.core_mask_opt, lcore_indexes);
- if (eal_parse_corelist(lcore_options.core_list_opt, lcore_indexes) < 0) {
- EAL_LOG(ERR, "invalid core list syntax");
+ if (nb_indexes < 0)
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;
+ char *core_map_opt = malloc(RTE_MAX_LCORE * 10);
+ size_t core_map_len = 0;
+ for (i = 0; i < nb_indexes; i++) {
+ if (!eal_cpu_detected(lcore_indexes[i])) {
+ EAL_LOG(ERR, "core %d not present", lcore_indexes[i]);
+ return -1;
+ }
+ int n = snprintf(core_map_opt + core_map_len,
+ (RTE_MAX_LCORE * 10) - core_map_len,
+ "%s%d", i == 0 ? "" : ",", lcore_indexes[i]);
+ if (n < 0 || (size_t)n >= (RTE_MAX_LCORE * 10) - core_map_len) {
+ EAL_LOG(ERR, "core map string too long");
+ return -1;
+ }
+ core_map_len += n;
}
- } else if (lcore_options.core_map_opt) {
+ lcore_options.core_map_opt = core_map_opt;
+ printf("Generated core map: %s\n", lcore_options.core_map_opt);
+ }
+
+ 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;
}
+ if (lcore_options.core_mask_opt || lcore_options.core_list_opt)
+ free(RTE_CAST_PTR(void *,lcore_options.core_map_opt));
} else {
eal_auto_detect_cores(cfg);
}
--
2.43.0
^ permalink raw reply [flat|nested] 23+ messages in thread
* [RFC PATCH 3/3] eal: allow automatic mapping of high lcore ids
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 ` Bruce Richardson
2025-03-24 17:30 ` [PATCH v2 0/3] allow easier use of high lcore-ids Bruce Richardson
3 siblings, 0 replies; 23+ messages in thread
From: Bruce Richardson @ 2025-03-13 11:38 UTC (permalink / raw)
To: dev; +Cc: Bruce Richardson
To use cores with IDs greater than RTE_MAX_LCORE the user must provide a
mapping of those higher core numbers to ids within the 0 - RTE_MAX_LCORE
range. This can be awkward to do manually when more than a few lcores
are involved, so instead we can provide an extra option to EAL to do
this manually.
This patch therefore introduces the "map-lcore-ids" flag, which
automatically maps all provided lcore numbers to start at zero.
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
drivers/event/dlb2/pf/base/dlb2_resource.c | 2 +-
lib/eal/common/eal_common_options.c | 32 ++++++++++++++--------
lib/eal/common/eal_options.h | 2 ++
lib/eal/include/rte_eal.h | 2 +-
4 files changed, 25 insertions(+), 13 deletions(-)
diff --git a/drivers/event/dlb2/pf/base/dlb2_resource.c b/drivers/event/dlb2/pf/base/dlb2_resource.c
index c4bc248afc..541a16db24 100644
--- a/drivers/event/dlb2/pf/base/dlb2_resource.c
+++ b/drivers/event/dlb2/pf/base/dlb2_resource.c
@@ -931,7 +931,7 @@ dlb2_resource_probe(struct dlb2_hw *hw, const void *probe_args)
cpu = rte_get_next_lcore(-1, 1, 0);
hw->num_prod_cores = 0;
if (mask) {
- int n = rte_eal_parse_coremask(mask, hw->prod_core_list);
+ int n = rte_eal_parse_coremask(mask, hw->prod_core_list, true);
if (n <= 0) {
DLB2_HW_ERR(hw, ": Invalid producer coremask=%s\n", mask);
return -1;
diff --git a/lib/eal/common/eal_common_options.c b/lib/eal/common/eal_common_options.c
index 807ba760ce..107b9eaad5 100644
--- a/lib/eal/common/eal_common_options.c
+++ b/lib/eal/common/eal_common_options.c
@@ -106,6 +106,7 @@ eal_long_options[] = {
{OPT_NO_TELEMETRY, 0, NULL, OPT_NO_TELEMETRY_NUM },
{OPT_FORCE_MAX_SIMD_BITWIDTH, 1, NULL, OPT_FORCE_MAX_SIMD_BITWIDTH_NUM},
{OPT_HUGE_WORKER_STACK, 2, NULL, OPT_HUGE_WORKER_STACK_NUM },
+ {OPT_MAP_LCORE_IDS, 0, NULL, OPT_MAP_LCORE_IDS_NUM },
{0, 0, NULL, 0 }
};
@@ -152,6 +153,7 @@ TAILQ_HEAD_INITIALIZER(devopt_list);
static int main_lcore_parsed;
static int mem_parsed;
static struct lcore_options {
+ bool map_lcore_ids;
const char *core_mask_opt;
const char *core_list_opt;
const char *core_map_opt;
@@ -723,13 +725,14 @@ check_core_list(int *lcores, unsigned int count)
if (len > 0)
lcorestr[len - 1] = 0;
EAL_LOG(ERR, "To use high physical core ids, "
- "please use --lcores to map them to lcore ids below RTE_MAX_LCORE, "
+ "please use --map-lcore-ids flag to map core ids automatically, "
+ "or use --lcores to map them manually to lcore ids below RTE_MAX_LCORE, "
"e.g. --lcores %s", lcorestr);
return -1;
}
int
-rte_eal_parse_coremask(const char *coremask, int *cores)
+rte_eal_parse_coremask(const char *coremask, int *cores, bool no_check)
{
const char *coremask_orig = coremask;
unsigned int count = 0;
@@ -784,7 +787,7 @@ rte_eal_parse_coremask(const char *coremask, int *cores)
return -1;
}
- if (check_core_list(cores, count) != 0)
+ if (!no_check && check_core_list(cores, count) != 0)
return -1;
return count;
@@ -869,7 +872,7 @@ eal_parse_service_corelist(const char *corelist)
}
static int
-eal_parse_corelist(const char *corelist, int *cores)
+eal_parse_corelist(const char *corelist, int *cores, bool no_check)
{
unsigned int count = 0, i;
char *end = NULL;
@@ -932,7 +935,7 @@ eal_parse_corelist(const char *corelist, int *cores)
return -1;
}
- if (check_core_list(cores, count))
+ if (!no_check && check_core_list(cores, count))
return -1;
return count;
@@ -1767,6 +1770,9 @@ eal_parse_common_option(int opt, const char *optarg,
return -1;
}
break;
+ case OPT_MAP_LCORE_IDS_NUM:
+ lcore_options.map_lcore_ids = true;
+ break;
/* don't know what to do, leave this to caller */
default:
@@ -1866,10 +1872,11 @@ eal_adjust_config(struct internal_config *internal_cfg)
if (lcore_options.core_mask_opt || lcore_options.core_list_opt) {
- int lcore_indexes[RTE_MAX_LCORE];
+ bool map_ids = lcore_options.map_lcore_ids;
+ int idxs[RTE_MAX_LCORE];
int nb_indexes = lcore_options.core_list_opt ?
- eal_parse_corelist(lcore_options.core_list_opt, lcore_indexes) :
- rte_eal_parse_coremask(lcore_options.core_mask_opt, lcore_indexes);
+ eal_parse_corelist(lcore_options.core_list_opt, idxs, map_ids) :
+ rte_eal_parse_coremask(lcore_options.core_mask_opt, idxs, map_ids);
if (nb_indexes < 0)
return -1;
@@ -1877,13 +1884,16 @@ eal_adjust_config(struct internal_config *internal_cfg)
char *core_map_opt = malloc(RTE_MAX_LCORE * 10);
size_t core_map_len = 0;
for (i = 0; i < nb_indexes; i++) {
- if (!eal_cpu_detected(lcore_indexes[i])) {
- EAL_LOG(ERR, "core %d not present", lcore_indexes[i]);
+ if (!eal_cpu_detected(idxs[i])) {
+ EAL_LOG(ERR, "core %d not present", idxs[i]);
return -1;
}
int n = snprintf(core_map_opt + core_map_len,
(RTE_MAX_LCORE * 10) - core_map_len,
- "%s%d", i == 0 ? "" : ",", lcore_indexes[i]);
+ "%s%d@%d",
+ i == 0 ? "" : ",",
+ map_ids ? i: idxs[i],
+ idxs[i]);
if (n < 0 || (size_t)n >= (RTE_MAX_LCORE * 10) - core_map_len) {
EAL_LOG(ERR, "core map string too long");
return -1;
diff --git a/lib/eal/common/eal_options.h b/lib/eal/common/eal_options.h
index 95fb4f6108..384c2d016e 100644
--- a/lib/eal/common/eal_options.h
+++ b/lib/eal/common/eal_options.h
@@ -93,6 +93,8 @@ enum {
OPT_FORCE_MAX_SIMD_BITWIDTH_NUM,
#define OPT_HUGE_WORKER_STACK "huge-worker-stack"
OPT_HUGE_WORKER_STACK_NUM,
+#define OPT_MAP_LCORE_IDS "map-lcore-ids"
+ OPT_MAP_LCORE_IDS_NUM,
OPT_LONG_MAX_NUM
};
diff --git a/lib/eal/include/rte_eal.h b/lib/eal/include/rte_eal.h
index c826e143f1..a53fb29c45 100644
--- a/lib/eal/include/rte_eal.h
+++ b/lib/eal/include/rte_eal.h
@@ -508,7 +508,7 @@ rte_eal_get_runtime_dir(void);
*/
__rte_internal
int
-rte_eal_parse_coremask(const char *coremask, int *cores);
+rte_eal_parse_coremask(const char *coremask, int *cores, bool nocheck);
#ifdef __cplusplus
}
--
2.43.0
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 0/3] allow easier use of high lcore-ids
2025-03-13 11:38 [RFC PATCH 0/3] allow easier use of high lcore-ids Bruce Richardson
` (2 preceding siblings ...)
2025-03-13 11:38 ` [RFC PATCH 3/3] eal: allow automatic mapping of high lcore ids Bruce Richardson
@ 2025-03-24 17:30 ` Bruce Richardson
2025-03-24 17:30 ` [PATCH v2 1/3] eal: centralize core parameter parsing Bruce Richardson
` (3 more replies)
3 siblings, 4 replies; 23+ messages in thread
From: Bruce Richardson @ 2025-03-24 17:30 UTC (permalink / raw)
To: dev; +Cc: Bruce Richardson
Traditionally, DPDK has had a direct mapping of internal lcore-ids, to
the actual core numbers in use. With higher core count servers becoming
more prevalent the issue becomes one of increasing memory footprint when
using such a scheme, due to the need to have all arrays dimensioned for
all cores on the system, whether or not those cores are in use by the
app.
Therefore, the decision was made in the past to not expand the
build-time RTE_MAX_LCORE value beyond 128. Instead, it was recommended
that users use the "--lcores" EAL parameter to take the high-numbered
cores they wish to use and map them to lcore-ids within the 0 - 128
range. While this works, this is a little clunky as it means that
instead of just passing, for example, "-l 130-139", the user must
instead pass "--lcores 0@130,1@131,2@132,3@133,...."
This patchset attempts to simplify the situation by adding a new flag to
do this mapping automatically. To use cores 130-139 and map them to ids
0-9 internally, the EAL args now become: "-l 130-139 --map-lcore-ids",
or using the shorter "-M" version of the flag: "-Ml 130-139".
Adding this new parameter required some rework of the existing arg
parsing code, because in current DPDK the args are parsed and checked in
the order they appear on the commandline. This means that using the
example above, the core parameter 130-139 will be rejected immediately
before the "map-lcore-ids" parameter is seen. To work around this, the
core (and service core) parameters are not parsed when seen, instead
they are only saved off and parsed after all arguments are parsed. The
"-l" and "-c" parameters are converted into "--lcores" arguments, so all
assigning of lcore ids is done there in all cases.
RFC->v2:
* converted printf to DEBUG log
* added "-M" as shorter version of flag
* added documentation
* renamed internal API that was changed to avoid any potential hidden
runtime issues.
Bruce Richardson (3):
eal: centralize core parameter parsing
eal: convert core masks and lists to core sets
eal: allow automatic mapping of high lcore ids
doc/guides/linux_gsg/eal_args.include.rst | 26 +-
drivers/event/dlb2/dlb2_priv.h | 2 -
drivers/event/dlb2/pf/base/dlb2_resource.c | 48 +--
lib/eal/common/eal_common_options.c | 340 +++++++--------------
lib/eal/common/eal_options.h | 3 +
lib/eal/include/rte_eal.h | 14 +-
lib/eal/version.map | 2 +-
7 files changed, 161 insertions(+), 274 deletions(-)
--
2.45.2
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 1/3] eal: centralize core parameter parsing
2025-03-24 17:30 ` [PATCH v2 0/3] allow easier use of high lcore-ids Bruce Richardson
@ 2025-03-24 17:30 ` Bruce Richardson
2025-04-07 6:58 ` David Marchand
2025-03-24 17:30 ` [PATCH v2 2/3] eal: convert core masks and lists to core sets Bruce Richardson
` (2 subsequent siblings)
3 siblings, 1 reply; 23+ messages in thread
From: Bruce Richardson @ 2025-03-24 17:30 UTC (permalink / raw)
To: dev; +Cc: Bruce Richardson, Tyler Retzlaff
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};
/* 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,
}
+
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.
+ * 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
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 2/3] eal: convert core masks and lists to core sets
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-03-24 17:30 ` 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
3 siblings, 1 reply; 23+ messages in thread
From: Bruce Richardson @ 2025-03-24 17:30 UTC (permalink / raw)
To: dev; +Cc: Bruce Richardson, Pravin Pathak, Tyler Retzlaff
Rather than directly parsing and working with the core mask and core
list parameters, convert them into core maps, and centralise all core
parsing there. This allows future work to adjust the mappings of cores
when generating that mapping parameter.
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
NOTE: this patch changes the behaviour of the exported, but internal API
rte_eal_parse_core_mask, used by dlb driver. Since this is an internal
API changing behaviour is allowed. However, to prevent issues - in case
any external app is using the API - we rename the function to
rte_eal_expand_coremask.
---
drivers/event/dlb2/dlb2_priv.h | 2 -
drivers/event/dlb2/pf/base/dlb2_resource.c | 48 ++---
lib/eal/common/eal_common_options.c | 195 +++++----------------
lib/eal/include/rte_eal.h | 12 +-
lib/eal/version.map | 2 +-
5 files changed, 60 insertions(+), 199 deletions(-)
diff --git a/drivers/event/dlb2/dlb2_priv.h b/drivers/event/dlb2/dlb2_priv.h
index 52da31ed31..63c0bea155 100644
--- a/drivers/event/dlb2/dlb2_priv.h
+++ b/drivers/event/dlb2/dlb2_priv.h
@@ -737,8 +737,6 @@ void dlb2_event_build_hcws(struct dlb2_port *qm_port,
uint8_t *sched_type,
uint8_t *queue_id);
-/* Extern functions */
-extern int rte_eal_parse_coremask(const char *coremask, int *cores);
/* Extern globals */
extern struct process_local_port_data dlb2_port[][DLB2_NUM_PORT_TYPES];
diff --git a/drivers/event/dlb2/pf/base/dlb2_resource.c b/drivers/event/dlb2/pf/base/dlb2_resource.c
index 3004902118..9b087b03b5 100644
--- a/drivers/event/dlb2/pf/base/dlb2_resource.c
+++ b/drivers/event/dlb2/pf/base/dlb2_resource.c
@@ -922,49 +922,31 @@ dlb2_resource_probe(struct dlb2_hw *hw, const void *probe_args)
{
const struct dlb2_devargs *args = (const struct dlb2_devargs *)probe_args;
const char *mask = args ? args->producer_coremask : NULL;
- int cpu = 0, cnt = 0, cores[RTE_MAX_LCORE], i;
+ int cpu = 0;
if (args) {
mask = (const char *)args->producer_coremask;
}
- if (mask && rte_eal_parse_coremask(mask, cores)) {
- DLB2_HW_ERR(hw, ": Invalid producer coremask=%s\n", mask);
- return -1;
- }
-
+ cpu = rte_get_next_lcore(-1, 1, 0);
hw->num_prod_cores = 0;
- for (i = 0; i < RTE_MAX_LCORE; i++) {
- bool is_pcore = (mask && cores[i] != -1);
-
- if (rte_lcore_is_enabled(i)) {
- if (is_pcore) {
- /*
- * Populate the producer cores from parsed
- * coremask
- */
- hw->prod_core_list[cores[i]] = i;
- hw->num_prod_cores++;
-
- } else if ((++cnt == DLB2_EAL_PROBE_CORE ||
- rte_lcore_count() < DLB2_EAL_PROBE_CORE)) {
- /*
- * If no producer coremask is provided, use the
- * second EAL core to probe
- */
- cpu = i;
- break;
- }
- } else if (is_pcore) {
- DLB2_HW_ERR(hw, "Producer coremask(%s) must be a subset of EAL coremask\n",
- mask);
+ if (mask) {
+ int n = rte_eal_expand_coremask(mask, hw->prod_core_list);
+ if (n <= 0) {
+ DLB2_HW_ERR(hw, ": Invalid producer coremask=%s\n", mask);
return -1;
}
+ hw->num_prod_cores = n;
+ cpu = hw->prod_core_list[0];
+ for (u8 i = 0; i < hw->num_prod_cores; i++) {
+ if (!rte_lcore_is_enabled(hw->prod_core_list[i])) {
+ DLB2_HW_ERR(hw, "Producer coremask(%s) must be a subset of EAL coremask\n",
+ mask);
+ return -1;
+ }
+ }
}
- /* Use the first core in producer coremask to probe */
- if (hw->num_prod_cores)
- cpu = hw->prod_core_list[0];
dlb2_get_pp_allocation(hw, cpu, DLB2_LDB_PORT);
dlb2_get_pp_allocation(hw, cpu, DLB2_DIR_PORT);
diff --git a/lib/eal/common/eal_common_options.c b/lib/eal/common/eal_common_options.c
index 55c49a923f..22ea6b24e4 100644
--- a/lib/eal/common/eal_common_options.c
+++ b/lib/eal/common/eal_common_options.c
@@ -690,33 +690,6 @@ eal_parse_service_coremask(const char *coremask)
return 0;
}
-static int
-update_lcore_config(int *cores)
-{
- struct rte_config *cfg = rte_eal_get_configuration();
- unsigned int count = 0;
- unsigned int i;
- int ret = 0;
-
- for (i = 0; i < RTE_MAX_LCORE; i++) {
- if (cores[i] != -1) {
- if (eal_cpu_detected(i) == 0) {
- EAL_LOG(ERR, "lcore %u unavailable", i);
- ret = -1;
- continue;
- }
- cfg->lcore_role[i] = ROLE_RTE;
- count++;
- } else {
- cfg->lcore_role[i] = ROLE_OFF;
- }
- lcore_config[i].core_index = cores[i];
- }
- if (!ret)
- cfg->lcore_count = count;
- return ret;
-}
-
static int
check_core_list(int *lcores, unsigned int count)
{
@@ -756,10 +729,9 @@ check_core_list(int *lcores, unsigned int count)
}
int
-rte_eal_parse_coremask(const char *coremask, int *cores)
+rte_eal_expand_coremask(const char *coremask, int *cores)
{
const char *coremask_orig = coremask;
- int lcores[RTE_MAX_LCORE];
unsigned int count = 0;
int i, j, idx;
int val;
@@ -803,30 +775,19 @@ rte_eal_parse_coremask(const char *coremask, int *cores)
RTE_MAX_LCORE);
return -1;
}
- lcores[count++] = idx;
+ cores[count++] = idx;
}
}
}
if (count == 0) {
- EAL_LOG(ERR, "No lcores in coremask: [%s]",
- coremask_orig);
+ EAL_LOG(ERR, "No lcores in coremask: [%s]", coremask_orig);
return -1;
}
- if (check_core_list(lcores, count))
+ if (check_core_list(cores, count) != 0)
return -1;
- /*
- * Now that we've got 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.
- */
- do {
- count--;
- cores[lcores[count]] = count;
- } while (count != 0);
-
- return 0;
+ return count;
}
static int
@@ -911,7 +872,6 @@ static int
eal_parse_corelist(const char *corelist, int *cores)
{
unsigned int count = 0, i;
- int lcores[RTE_MAX_LCORE];
char *end = NULL;
int min, max;
int idx;
@@ -949,7 +909,7 @@ eal_parse_corelist(const char *corelist, int *cores)
/* Check if this idx is already present */
for (i = 0; i < count; i++) {
- if (lcores[i] == idx)
+ if (cores[i] == idx)
dup = true;
}
if (dup)
@@ -959,7 +919,7 @@ eal_parse_corelist(const char *corelist, int *cores)
RTE_MAX_LCORE);
return -1;
}
- lcores[count++] = idx;
+ cores[count++] = idx;
}
min = -1;
} else
@@ -967,23 +927,15 @@ eal_parse_corelist(const char *corelist, int *cores)
corelist = end + 1;
} while (*end != '\0');
- if (count == 0)
+ if (count == 0) {
+ EAL_LOG(ERR, "No lcores in corelist");
return -1;
+ }
- if (check_core_list(lcores, count))
+ if (check_core_list(cores, count))
return -1;
- /*
- * Now that we've got 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.
- */
- do {
- count--;
- cores[lcores[count]] = count;
- } while (count != 0);
-
- return 0;
+ return count;
}
/* Changes the lcore id of the main thread */
@@ -1500,75 +1452,6 @@ eal_parse_base_virtaddr(const char *arg)
return 0;
}
-/* caller is responsible for freeing the returned string */
-static char *
-available_cores(void)
-{
- char *str = NULL;
- int previous;
- int sequence;
- char *tmp;
- int idx;
-
- /* find the first available cpu */
- for (idx = 0; idx < RTE_MAX_LCORE; idx++) {
- if (eal_cpu_detected(idx) == 0)
- continue;
- break;
- }
- if (idx >= RTE_MAX_LCORE)
- return NULL;
-
- /* first sequence */
- if (asprintf(&str, "%d", idx) < 0)
- return NULL;
- previous = idx;
- sequence = 0;
-
- for (idx++ ; idx < RTE_MAX_LCORE; idx++) {
- if (eal_cpu_detected(idx) == 0)
- continue;
-
- if (idx == previous + 1) {
- previous = idx;
- sequence = 1;
- continue;
- }
-
- /* finish current sequence */
- if (sequence) {
- if (asprintf(&tmp, "%s-%d", str, previous) < 0) {
- free(str);
- return NULL;
- }
- free(str);
- str = tmp;
- }
-
- /* new sequence */
- if (asprintf(&tmp, "%s,%d", str, idx) < 0) {
- free(str);
- return NULL;
- }
- free(str);
- str = tmp;
- previous = idx;
- sequence = 0;
- }
-
- /* finish last sequence */
- if (sequence) {
- if (asprintf(&tmp, "%s-%d", str, previous) < 0) {
- free(str);
- return NULL;
- }
- free(str);
- str = tmp;
- }
-
- return str;
-}
-
#define HUGE_UNLINK_NEVER "never"
static int
@@ -1981,43 +1864,43 @@ eal_adjust_config(struct internal_config *internal_cfg)
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) {
+ if (lcore_options.core_mask_opt || lcore_options.core_list_opt) {
int lcore_indexes[RTE_MAX_LCORE];
+ int nb_indexes = lcore_options.core_list_opt ?
+ eal_parse_corelist(lcore_options.core_list_opt, lcore_indexes) :
+ rte_eal_expand_coremask(lcore_options.core_mask_opt, lcore_indexes);
- if (eal_parse_corelist(lcore_options.core_list_opt, lcore_indexes) < 0) {
- EAL_LOG(ERR, "invalid core list syntax");
+ if (nb_indexes < 0)
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;
+ char *core_map_opt = malloc(RTE_MAX_LCORE * 10);
+ size_t core_map_len = 0;
+ for (i = 0; i < nb_indexes; i++) {
+ if (!eal_cpu_detected(lcore_indexes[i])) {
+ EAL_LOG(ERR, "core %d not present", lcore_indexes[i]);
+ return -1;
+ }
+ int n = snprintf(core_map_opt + core_map_len,
+ (RTE_MAX_LCORE * 10) - core_map_len,
+ "%s%d", i == 0 ? "" : ",", lcore_indexes[i]);
+ if (n < 0 || (size_t)n >= (RTE_MAX_LCORE * 10) - core_map_len) {
+ EAL_LOG(ERR, "core map string too long");
+ return -1;
+ }
+ core_map_len += n;
}
- } else if (lcore_options.core_map_opt) {
+ lcore_options.core_map_opt = core_map_opt;
+ EAL_LOG(DEBUG, "Generated core map: '%s'", lcore_options.core_map_opt);
+ }
+
+ 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;
}
+ if (lcore_options.core_mask_opt || lcore_options.core_list_opt)
+ free(RTE_CAST_PTR(void *, lcore_options.core_map_opt));
} else {
eal_auto_detect_cores(cfg);
}
diff --git a/lib/eal/include/rte_eal.h b/lib/eal/include/rte_eal.h
index c826e143f1..013075487c 100644
--- a/lib/eal/include/rte_eal.h
+++ b/lib/eal/include/rte_eal.h
@@ -493,22 +493,20 @@ rte_eal_get_runtime_dir(void);
/**
* Convert a string describing a mask of core ids into an array of core ids.
*
- * On success, the passed array is filled with the orders of the core ids
- * present in the mask (-1 indicating that a core id is absent).
- * For example, passing a 0xa coremask results in cores[1] = 0, cores[3] = 1,
- * and the rest of the array is set to -1.
+ * On success, the passed array is filled with the core ids present in the mask.
*
* @param coremask
* A string describing a mask of core ids.
* @param cores
- * An array where to store the core ids orders.
+ * The output array to store the core ids.
* This array must be at least RTE_MAX_LCORE large.
* @return
- * 0 on success, -1 if the string content was invalid.
+ * The number of cores in the coremask, and in the returned "cores" array,
+ * -1 if the string content was invalid.
*/
__rte_internal
int
-rte_eal_parse_coremask(const char *coremask, int *cores);
+rte_eal_expand_coremask(const char *coremask, int *cores);
#ifdef __cplusplus
}
diff --git a/lib/eal/version.map b/lib/eal/version.map
index a20c713eb1..b51051ee38 100644
--- a/lib/eal/version.map
+++ b/lib/eal/version.map
@@ -405,8 +405,8 @@ INTERNAL {
rte_bus_register;
rte_bus_unregister;
+ rte_eal_expand_coremask;
rte_eal_get_baseaddr;
- rte_eal_parse_coremask;
rte_firmware_read;
rte_intr_allow_others;
rte_intr_cap_multiple;
--
2.45.2
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 3/3] eal: allow automatic mapping of high lcore ids
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-03-24 17:30 ` [PATCH v2 2/3] eal: convert core masks and lists to core sets Bruce Richardson
@ 2025-03-24 17:30 ` Bruce Richardson
2025-04-01 14:06 ` [PATCH v2 0/3] allow easier use of high lcore-ids Bruce Richardson
3 siblings, 0 replies; 23+ messages in thread
From: Bruce Richardson @ 2025-03-24 17:30 UTC (permalink / raw)
To: dev; +Cc: Bruce Richardson, Pravin Pathak, Tyler Retzlaff
To use cores with IDs greater than RTE_MAX_LCORE the user must provide a
mapping of those higher core numbers to ids within the 0 - RTE_MAX_LCORE
range. This can be awkward to do manually when more than a few lcores
are involved, so instead we can provide an extra option to EAL to do
this manually.
This patch therefore introduces the "map-lcore-ids" flag, which
automatically maps all provided lcore numbers to start at zero.
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
doc/guides/linux_gsg/eal_args.include.rst | 26 ++++++++++++++++-
drivers/event/dlb2/pf/base/dlb2_resource.c | 2 +-
lib/eal/common/eal_common_options.c | 34 +++++++++++++++-------
lib/eal/common/eal_options.h | 3 ++
lib/eal/include/rte_eal.h | 4 ++-
5 files changed, 55 insertions(+), 14 deletions(-)
diff --git a/doc/guides/linux_gsg/eal_args.include.rst b/doc/guides/linux_gsg/eal_args.include.rst
index 9cfbf7de84..1d9525f73c 100644
--- a/doc/guides/linux_gsg/eal_args.include.rst
+++ b/doc/guides/linux_gsg/eal_args.include.rst
@@ -13,7 +13,7 @@ Lcore-related options
List of cores to run on
The argument format is ``<c1>[-c2][,c3[-c4],...]``
- where ``c1``, ``c2``, etc are core indexes between 0 and 128.
+ where ``c1``, ``c2``, etc are core indexes between 0 and RTE_MAX_LCORE (default 128).
* ``--lcores <core map>``
@@ -29,10 +29,34 @@ Lcore-related options
The grouping ``()`` can be omitted for single element group.
The ``@`` can be omitted if cpus and lcores have the same value.
+.. Note::
+ When using ``--lcores`` parameter to map lcore ids to cpus,
+ the lcore ids must be in the range 0 to ``RTE_MAX_LCORE`` (default 128),
+ but the cpu ids need not be.
+
+ This allows an application to use cpus with ids greater than ``RTE_MAX_LCORE``,
+ so long as the total number of lcores is less than or equal to ``RTE_MAX_LCORE``.
+
.. Note::
At a given instance only one core option ``--lcores``, ``-l`` or ``-c`` can
be used.
+* ``--map-lcore-ids``, ``-M``
+
+ Automatically map the cpus given in a coremask using ``-c`` parameter,
+ or in a core list using ``-l`` parameter, to internal lcore-ids starting at 0.
+ (If ``--lcores`` parameter is provided, this parameter is ignored.)
+
+ This can provide a shorter way, compared to using ``--lcores`` parameter,
+ to use cpus with ids greater than ``RTE_MAX_LCORE`` in an application.
+
+ For example, ``-c 0x3000 --map-lcore-ids`` will cause cpus 12 and 13 to be used,
+ with application lcore ids of 0 and 1 respectively.
+ Similarly, ``-Ml 94,95,31-33`` is equivalent to ``--lcores=0@94,1@95,2@31,3@32,4@33``,
+ where the application lcore ids are 0-4,
+ mapped to the physical cpus 94, 95, 31, 32 and 33 respectively.
+
+
* ``--main-lcore <core ID>``
Core ID that is used as main.
diff --git a/drivers/event/dlb2/pf/base/dlb2_resource.c b/drivers/event/dlb2/pf/base/dlb2_resource.c
index 9b087b03b5..d38aa5115e 100644
--- a/drivers/event/dlb2/pf/base/dlb2_resource.c
+++ b/drivers/event/dlb2/pf/base/dlb2_resource.c
@@ -931,7 +931,7 @@ dlb2_resource_probe(struct dlb2_hw *hw, const void *probe_args)
cpu = rte_get_next_lcore(-1, 1, 0);
hw->num_prod_cores = 0;
if (mask) {
- int n = rte_eal_expand_coremask(mask, hw->prod_core_list);
+ int n = rte_eal_expand_coremask(mask, hw->prod_core_list, true);
if (n <= 0) {
DLB2_HW_ERR(hw, ": Invalid producer coremask=%s\n", mask);
return -1;
diff --git a/lib/eal/common/eal_common_options.c b/lib/eal/common/eal_common_options.c
index 22ea6b24e4..25061ffb9b 100644
--- a/lib/eal/common/eal_common_options.c
+++ b/lib/eal/common/eal_common_options.c
@@ -56,6 +56,7 @@ eal_short_options[] =
"d:" /* driver */
"h" /* help */
"l:" /* corelist */
+ "M" /* map lcore ids to zero*/
"S:" /* service corelist */
"m:" /* memory size */
"n:" /* memory channels */
@@ -106,6 +107,7 @@ eal_long_options[] = {
{OPT_NO_TELEMETRY, 0, NULL, OPT_NO_TELEMETRY_NUM },
{OPT_FORCE_MAX_SIMD_BITWIDTH, 1, NULL, OPT_FORCE_MAX_SIMD_BITWIDTH_NUM},
{OPT_HUGE_WORKER_STACK, 2, NULL, OPT_HUGE_WORKER_STACK_NUM },
+ {OPT_MAP_LCORE_IDS, 0, NULL, OPT_MAP_LCORE_IDS_NUM },
{0, 0, NULL, 0 }
};
@@ -152,6 +154,7 @@ TAILQ_HEAD_INITIALIZER(devopt_list);
static int main_lcore_parsed;
static int mem_parsed;
static struct lcore_options {
+ bool map_lcore_ids;
const char *core_mask_opt;
const char *core_list_opt;
const char *core_map_opt;
@@ -723,13 +726,14 @@ check_core_list(int *lcores, unsigned int count)
if (len > 0)
lcorestr[len - 1] = 0;
EAL_LOG(ERR, "To use high physical core ids, "
- "please use --lcores to map them to lcore ids below RTE_MAX_LCORE, "
+ "please use --map-lcore-ids/-M flag to map core ids automatically, "
+ "or use --lcores to map them manually to lcore ids below RTE_MAX_LCORE, "
"e.g. --lcores %s", lcorestr);
return -1;
}
int
-rte_eal_expand_coremask(const char *coremask, int *cores)
+rte_eal_expand_coremask(const char *coremask, int *cores, bool no_check)
{
const char *coremask_orig = coremask;
unsigned int count = 0;
@@ -784,7 +788,7 @@ rte_eal_expand_coremask(const char *coremask, int *cores)
return -1;
}
- if (check_core_list(cores, count) != 0)
+ if (!no_check && check_core_list(cores, count) != 0)
return -1;
return count;
@@ -869,7 +873,7 @@ eal_parse_service_corelist(const char *corelist)
}
static int
-eal_parse_corelist(const char *corelist, int *cores)
+eal_parse_corelist(const char *corelist, int *cores, bool no_check)
{
unsigned int count = 0, i;
char *end = NULL;
@@ -932,7 +936,7 @@ eal_parse_corelist(const char *corelist, int *cores)
return -1;
}
- if (check_core_list(cores, count))
+ if (!no_check && check_core_list(cores, count))
return -1;
return count;
@@ -1558,6 +1562,10 @@ eal_parse_common_option(int opt, const char *optarg,
case 'l':
lcore_options.core_list_opt = optarg;
break;
+ /* map lcore ids */
+ case 'M':
+ lcore_options.map_lcore_ids = true;
+ break;
/* service coremask */
case 's':
lcore_options.service_mask_opt = optarg;
@@ -1866,10 +1874,11 @@ eal_adjust_config(struct internal_config *internal_cfg)
if (lcore_options.core_mask_opt || lcore_options.core_list_opt) {
- int lcore_indexes[RTE_MAX_LCORE];
+ bool map_ids = lcore_options.map_lcore_ids;
+ int idxs[RTE_MAX_LCORE];
int nb_indexes = lcore_options.core_list_opt ?
- eal_parse_corelist(lcore_options.core_list_opt, lcore_indexes) :
- rte_eal_expand_coremask(lcore_options.core_mask_opt, lcore_indexes);
+ eal_parse_corelist(lcore_options.core_list_opt, idxs, map_ids) :
+ rte_eal_expand_coremask(lcore_options.core_mask_opt, idxs, map_ids);
if (nb_indexes < 0)
return -1;
@@ -1877,13 +1886,16 @@ eal_adjust_config(struct internal_config *internal_cfg)
char *core_map_opt = malloc(RTE_MAX_LCORE * 10);
size_t core_map_len = 0;
for (i = 0; i < nb_indexes; i++) {
- if (!eal_cpu_detected(lcore_indexes[i])) {
- EAL_LOG(ERR, "core %d not present", lcore_indexes[i]);
+ if (!eal_cpu_detected(idxs[i])) {
+ EAL_LOG(ERR, "core %d not present", idxs[i]);
return -1;
}
int n = snprintf(core_map_opt + core_map_len,
(RTE_MAX_LCORE * 10) - core_map_len,
- "%s%d", i == 0 ? "" : ",", lcore_indexes[i]);
+ "%s%d@%d",
+ i == 0 ? "" : ",",
+ map_ids ? i : idxs[i],
+ idxs[i]);
if (n < 0 || (size_t)n >= (RTE_MAX_LCORE * 10) - core_map_len) {
EAL_LOG(ERR, "core map string too long");
return -1;
diff --git a/lib/eal/common/eal_options.h b/lib/eal/common/eal_options.h
index 95fb4f6108..1344bce9ba 100644
--- a/lib/eal/common/eal_options.h
+++ b/lib/eal/common/eal_options.h
@@ -17,6 +17,9 @@ enum {
OPT_DEV_ALLOW_NUM = 'a',
#define OPT_DEV_BLOCK "block"
OPT_DEV_BLOCK_NUM = 'b',
+#define OPT_MAP_LCORE_IDS "map-lcore-ids"
+ OPT_MAP_LCORE_IDS_NUM = 'M',
+
/* first long only option value must be >= 256, so that we won't
* conflict with short options */
diff --git a/lib/eal/include/rte_eal.h b/lib/eal/include/rte_eal.h
index 013075487c..2f7a7dcfbe 100644
--- a/lib/eal/include/rte_eal.h
+++ b/lib/eal/include/rte_eal.h
@@ -500,13 +500,15 @@ rte_eal_get_runtime_dir(void);
* @param cores
* The output array to store the core ids.
* This array must be at least RTE_MAX_LCORE large.
+ * @param nocheck
+ * If true, skip checking that the core ids are between 0..RTE_MAX_LCORE
* @return
* The number of cores in the coremask, and in the returned "cores" array,
* -1 if the string content was invalid.
*/
__rte_internal
int
-rte_eal_expand_coremask(const char *coremask, int *cores);
+rte_eal_expand_coremask(const char *coremask, int *cores, bool nocheck);
#ifdef __cplusplus
}
--
2.45.2
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 0/3] allow easier use of high lcore-ids
2025-03-24 17:30 ` [PATCH v2 0/3] allow easier use of high lcore-ids Bruce Richardson
` (2 preceding siblings ...)
2025-03-24 17:30 ` [PATCH v2 3/3] eal: allow automatic mapping of high lcore ids Bruce Richardson
@ 2025-04-01 14:06 ` Bruce Richardson
2025-04-07 7:04 ` David Marchand
3 siblings, 1 reply; 23+ messages in thread
From: Bruce Richardson @ 2025-04-01 14:06 UTC (permalink / raw)
To: dev
On Mon, Mar 24, 2025 at 05:30:26PM +0000, Bruce Richardson wrote:
> Traditionally, DPDK has had a direct mapping of internal lcore-ids, to
> the actual core numbers in use. With higher core count servers becoming
> more prevalent the issue becomes one of increasing memory footprint when
> using such a scheme, due to the need to have all arrays dimensioned for
> all cores on the system, whether or not those cores are in use by the
> app.
>
> Therefore, the decision was made in the past to not expand the
> build-time RTE_MAX_LCORE value beyond 128. Instead, it was recommended
> that users use the "--lcores" EAL parameter to take the high-numbered
> cores they wish to use and map them to lcore-ids within the 0 - 128
> range. While this works, this is a little clunky as it means that
> instead of just passing, for example, "-l 130-139", the user must
> instead pass "--lcores 0@130,1@131,2@132,3@133,...."
>
> This patchset attempts to simplify the situation by adding a new flag to
> do this mapping automatically. To use cores 130-139 and map them to ids
> 0-9 internally, the EAL args now become: "-l 130-139 --map-lcore-ids",
> or using the shorter "-M" version of the flag: "-Ml 130-139".
>
> Adding this new parameter required some rework of the existing arg
> parsing code, because in current DPDK the args are parsed and checked in
> the order they appear on the commandline. This means that using the
> example above, the core parameter 130-139 will be rejected immediately
> before the "map-lcore-ids" parameter is seen. To work around this, the
> core (and service core) parameters are not parsed when seen, instead
> they are only saved off and parsed after all arguments are parsed. The
> "-l" and "-c" parameters are converted into "--lcores" arguments, so all
> assigning of lcore ids is done there in all cases.
>
> RFC->v2:
> * converted printf to DEBUG log
> * added "-M" as shorter version of flag
> * added documentation
> * renamed internal API that was changed to avoid any potential hidden
> runtime issues.
>
> Bruce Richardson (3):
> eal: centralize core parameter parsing
> eal: convert core masks and lists to core sets
> eal: allow automatic mapping of high lcore ids
>
Ping for review.
At a high level, does this feature seem useful to users?
/Bruce
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 1/3] eal: centralize core parameter parsing
2025-03-24 17:30 ` [PATCH v2 1/3] eal: centralize core parameter parsing Bruce Richardson
@ 2025-04-07 6:58 ` David Marchand
0 siblings, 0 replies; 23+ messages in thread
From: David Marchand @ 2025-04-07 6:58 UTC (permalink / raw)
To: Bruce Richardson; +Cc: dev, Tyler Retzlaff
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 2/3] eal: convert core masks and lists to core sets
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
0 siblings, 0 replies; 23+ messages in thread
From: David Marchand @ 2025-04-07 6:59 UTC (permalink / raw)
To: Bruce Richardson; +Cc: dev, Pravin Pathak, Tyler Retzlaff
On Mon, Mar 24, 2025 at 6:31 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> Rather than directly parsing and working with the core mask and core
> list parameters, convert them into core maps, and centralise all core
> parsing there. This allows future work to adjust the mappings of cores
> when generating that mapping parameter.
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>
> NOTE: this patch changes the behaviour of the exported, but internal API
> rte_eal_parse_core_mask, used by dlb driver. Since this is an internal
> API changing behaviour is allowed. However, to prevent issues - in case
> any external app is using the API - we rename the function to
> rte_eal_expand_coremask.
An external app is not supposed to call this but I agree a build error
is more explicit.
> ---
> drivers/event/dlb2/dlb2_priv.h | 2 -
> drivers/event/dlb2/pf/base/dlb2_resource.c | 48 ++---
> lib/eal/common/eal_common_options.c | 195 +++++----------------
> lib/eal/include/rte_eal.h | 12 +-
> lib/eal/version.map | 2 +-
> 5 files changed, 60 insertions(+), 199 deletions(-)
>
> diff --git a/drivers/event/dlb2/dlb2_priv.h b/drivers/event/dlb2/dlb2_priv.h
> index 52da31ed31..63c0bea155 100644
> --- a/drivers/event/dlb2/dlb2_priv.h
> +++ b/drivers/event/dlb2/dlb2_priv.h
> @@ -737,8 +737,6 @@ void dlb2_event_build_hcws(struct dlb2_port *qm_port,
> uint8_t *sched_type,
> uint8_t *queue_id);
>
> -/* Extern functions */
> -extern int rte_eal_parse_coremask(const char *coremask, int *cores);
Ugly...
>
> /* Extern globals */
> extern struct process_local_port_data dlb2_port[][DLB2_NUM_PORT_TYPES];
> diff --git a/drivers/event/dlb2/pf/base/dlb2_resource.c b/drivers/event/dlb2/pf/base/dlb2_resource.c
> index 3004902118..9b087b03b5 100644
> --- a/drivers/event/dlb2/pf/base/dlb2_resource.c
> +++ b/drivers/event/dlb2/pf/base/dlb2_resource.c
> @@ -922,49 +922,31 @@ dlb2_resource_probe(struct dlb2_hw *hw, const void *probe_args)
> {
> const struct dlb2_devargs *args = (const struct dlb2_devargs *)probe_args;
> const char *mask = args ? args->producer_coremask : NULL;
> - int cpu = 0, cnt = 0, cores[RTE_MAX_LCORE], i;
> + int cpu = 0;
>
> if (args) {
> mask = (const char *)args->producer_coremask;
> }
I have some trouble with this code:
const char *mask = args ? args->producer_coremask : NULL;
...
if (args) {
mask = (const char *)args->producer_coremask;
}
It seems redundant...
>
> - if (mask && rte_eal_parse_coremask(mask, cores)) {
> - DLB2_HW_ERR(hw, ": Invalid producer coremask=%s\n", mask);
> - return -1;
> - }
> -
> + cpu = rte_get_next_lcore(-1, 1, 0);
> hw->num_prod_cores = 0;
> - for (i = 0; i < RTE_MAX_LCORE; i++) {
> - bool is_pcore = (mask && cores[i] != -1);
> -
> - if (rte_lcore_is_enabled(i)) {
> - if (is_pcore) {
> - /*
> - * Populate the producer cores from parsed
> - * coremask
> - */
> - hw->prod_core_list[cores[i]] = i;
> - hw->num_prod_cores++;
> -
> - } else if ((++cnt == DLB2_EAL_PROBE_CORE ||
> - rte_lcore_count() < DLB2_EAL_PROBE_CORE)) {
> - /*
> - * If no producer coremask is provided, use the
> - * second EAL core to probe
> - */
> - cpu = i;
> - break;
> - }
> - } else if (is_pcore) {
> - DLB2_HW_ERR(hw, "Producer coremask(%s) must be a subset of EAL coremask\n",
> - mask);
> + if (mask) {
> + int n = rte_eal_expand_coremask(mask, hw->prod_core_list);
> + if (n <= 0) {
> + DLB2_HW_ERR(hw, ": Invalid producer coremask=%s\n", mask);
> return -1;
> }
> + hw->num_prod_cores = n;
> + cpu = hw->prod_core_list[0];
>
> + for (u8 i = 0; i < hw->num_prod_cores; i++) {
> + if (!rte_lcore_is_enabled(hw->prod_core_list[i])) {
> + DLB2_HW_ERR(hw, "Producer coremask(%s) must be a subset of EAL coremask\n",
> + mask);
> + return -1;
> + }
> + }
> }
> - /* Use the first core in producer coremask to probe */
> - if (hw->num_prod_cores)
> - cpu = hw->prod_core_list[0];
>
> dlb2_get_pp_allocation(hw, cpu, DLB2_LDB_PORT);
> dlb2_get_pp_allocation(hw, cpu, DLB2_DIR_PORT);
> diff --git a/lib/eal/common/eal_common_options.c b/lib/eal/common/eal_common_options.c
> index 55c49a923f..22ea6b24e4 100644
> --- a/lib/eal/common/eal_common_options.c
> +++ b/lib/eal/common/eal_common_options.c
> @@ -690,33 +690,6 @@ eal_parse_service_coremask(const char *coremask)
> return 0;
> }
>
> -static int
> -update_lcore_config(int *cores)
> -{
> - struct rte_config *cfg = rte_eal_get_configuration();
> - unsigned int count = 0;
> - unsigned int i;
> - int ret = 0;
> -
> - for (i = 0; i < RTE_MAX_LCORE; i++) {
> - if (cores[i] != -1) {
> - if (eal_cpu_detected(i) == 0) {
> - EAL_LOG(ERR, "lcore %u unavailable", i);
> - ret = -1;
> - continue;
> - }
> - cfg->lcore_role[i] = ROLE_RTE;
> - count++;
> - } else {
> - cfg->lcore_role[i] = ROLE_OFF;
> - }
> - lcore_config[i].core_index = cores[i];
> - }
> - if (!ret)
> - cfg->lcore_count = count;
> - return ret;
> -}
> -
> static int
> check_core_list(int *lcores, unsigned int count)
> {
> @@ -756,10 +729,9 @@ check_core_list(int *lcores, unsigned int count)
> }
>
> int
> -rte_eal_parse_coremask(const char *coremask, int *cores)
> +rte_eal_expand_coremask(const char *coremask, int *cores)
> {
> const char *coremask_orig = coremask;
> - int lcores[RTE_MAX_LCORE];
> unsigned int count = 0;
> int i, j, idx;
> int val;
> @@ -803,30 +775,19 @@ rte_eal_parse_coremask(const char *coremask, int *cores)
> RTE_MAX_LCORE);
> return -1;
> }
> - lcores[count++] = idx;
> + cores[count++] = idx;
> }
> }
> }
> if (count == 0) {
> - EAL_LOG(ERR, "No lcores in coremask: [%s]",
> - coremask_orig);
> + EAL_LOG(ERR, "No lcores in coremask: [%s]", coremask_orig);
> return -1;
> }
>
> - if (check_core_list(lcores, count))
> + if (check_core_list(cores, count) != 0)
> return -1;
>
> - /*
> - * Now that we've got 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.
> - */
> - do {
> - count--;
> - cores[lcores[count]] = count;
> - } while (count != 0);
> -
> - return 0;
> + return count;
> }
>
> static int
> @@ -911,7 +872,6 @@ static int
> eal_parse_corelist(const char *corelist, int *cores)
> {
> unsigned int count = 0, i;
> - int lcores[RTE_MAX_LCORE];
> char *end = NULL;
> int min, max;
> int idx;
> @@ -949,7 +909,7 @@ eal_parse_corelist(const char *corelist, int *cores)
>
> /* Check if this idx is already present */
> for (i = 0; i < count; i++) {
> - if (lcores[i] == idx)
> + if (cores[i] == idx)
> dup = true;
> }
> if (dup)
> @@ -959,7 +919,7 @@ eal_parse_corelist(const char *corelist, int *cores)
> RTE_MAX_LCORE);
> return -1;
> }
> - lcores[count++] = idx;
> + cores[count++] = idx;
> }
> min = -1;
> } else
> @@ -967,23 +927,15 @@ eal_parse_corelist(const char *corelist, int *cores)
> corelist = end + 1;
> } while (*end != '\0');
>
> - if (count == 0)
> + if (count == 0) {
> + EAL_LOG(ERR, "No lcores in corelist");
> return -1;
> + }
>
> - if (check_core_list(lcores, count))
> + if (check_core_list(cores, count))
> return -1;
>
> - /*
> - * Now that we've got 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.
> - */
> - do {
> - count--;
> - cores[lcores[count]] = count;
> - } while (count != 0);
> -
> - return 0;
> + return count;
> }
>
> /* Changes the lcore id of the main thread */
> @@ -1500,75 +1452,6 @@ eal_parse_base_virtaddr(const char *arg)
> return 0;
> }
>
> -/* caller is responsible for freeing the returned string */
> -static char *
> -available_cores(void)
> -{
> - char *str = NULL;
> - int previous;
> - int sequence;
> - char *tmp;
> - int idx;
> -
> - /* find the first available cpu */
> - for (idx = 0; idx < RTE_MAX_LCORE; idx++) {
> - if (eal_cpu_detected(idx) == 0)
> - continue;
> - break;
> - }
> - if (idx >= RTE_MAX_LCORE)
> - return NULL;
> -
> - /* first sequence */
> - if (asprintf(&str, "%d", idx) < 0)
> - return NULL;
> - previous = idx;
> - sequence = 0;
> -
> - for (idx++ ; idx < RTE_MAX_LCORE; idx++) {
> - if (eal_cpu_detected(idx) == 0)
> - continue;
> -
> - if (idx == previous + 1) {
> - previous = idx;
> - sequence = 1;
> - continue;
> - }
> -
> - /* finish current sequence */
> - if (sequence) {
> - if (asprintf(&tmp, "%s-%d", str, previous) < 0) {
> - free(str);
> - return NULL;
> - }
> - free(str);
> - str = tmp;
> - }
> -
> - /* new sequence */
> - if (asprintf(&tmp, "%s,%d", str, idx) < 0) {
> - free(str);
> - return NULL;
> - }
> - free(str);
> - str = tmp;
> - previous = idx;
> - sequence = 0;
> - }
> -
> - /* finish last sequence */
> - if (sequence) {
> - if (asprintf(&tmp, "%s-%d", str, previous) < 0) {
> - free(str);
> - return NULL;
> - }
> - free(str);
> - str = tmp;
> - }
> -
> - return str;
> -}
> -
Not sure why you remove this helper, it was supposed to help users
with better logs.
> #define HUGE_UNLINK_NEVER "never"
>
> static int
> @@ -1981,43 +1864,43 @@ eal_adjust_config(struct internal_config *internal_cfg)
> 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) {
> + if (lcore_options.core_mask_opt || lcore_options.core_list_opt) {
> int lcore_indexes[RTE_MAX_LCORE];
> + int nb_indexes = lcore_options.core_list_opt ?
> + eal_parse_corelist(lcore_options.core_list_opt, lcore_indexes) :
> + rte_eal_expand_coremask(lcore_options.core_mask_opt, lcore_indexes);
>
> - if (eal_parse_corelist(lcore_options.core_list_opt, lcore_indexes) < 0) {
> - EAL_LOG(ERR, "invalid core list syntax");
> + if (nb_indexes < 0)
> 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;
> + char *core_map_opt = malloc(RTE_MAX_LCORE * 10);
> + size_t core_map_len = 0;
> + for (i = 0; i < nb_indexes; i++) {
> + if (!eal_cpu_detected(lcore_indexes[i])) {
> + EAL_LOG(ERR, "core %d not present", lcore_indexes[i]);
core_map_opt is leaked (idem in next return).
> + return -1;
I would not return here, but instead provide a full report of which
cores are invalid before returning an error.
> + }
> + int n = snprintf(core_map_opt + core_map_len,
> + (RTE_MAX_LCORE * 10) - core_map_len,
> + "%s%d", i == 0 ? "" : ",", lcore_indexes[i]);
> + if (n < 0 || (size_t)n >= (RTE_MAX_LCORE * 10) - core_map_len) {
> + EAL_LOG(ERR, "core map string too long");
> + return -1;
Or use asprintf.
> + }
> + core_map_len += n;
> }
> - } else if (lcore_options.core_map_opt) {
> + lcore_options.core_map_opt = core_map_opt;
> + EAL_LOG(DEBUG, "Generated core map: '%s'", lcore_options.core_map_opt);
> + }
> +
> + 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;
> }
> + if (lcore_options.core_mask_opt || lcore_options.core_list_opt)
> + free(RTE_CAST_PTR(void *, lcore_options.core_map_opt));
> } else {
> eal_auto_detect_cores(cfg);
> }
> diff --git a/lib/eal/include/rte_eal.h b/lib/eal/include/rte_eal.h
> index c826e143f1..013075487c 100644
> --- a/lib/eal/include/rte_eal.h
> +++ b/lib/eal/include/rte_eal.h
> @@ -493,22 +493,20 @@ rte_eal_get_runtime_dir(void);
> /**
> * Convert a string describing a mask of core ids into an array of core ids.
> *
> - * On success, the passed array is filled with the orders of the core ids
> - * present in the mask (-1 indicating that a core id is absent).
> - * For example, passing a 0xa coremask results in cores[1] = 0, cores[3] = 1,
> - * and the rest of the array is set to -1.
> + * On success, the passed array is filled with the core ids present in the mask.
> *
> * @param coremask
> * A string describing a mask of core ids.
> * @param cores
> - * An array where to store the core ids orders.
> + * The output array to store the core ids.
> * This array must be at least RTE_MAX_LCORE large.
> * @return
> - * 0 on success, -1 if the string content was invalid.
> + * The number of cores in the coremask, and in the returned "cores" array,
> + * -1 if the string content was invalid.
> */
> __rte_internal
> int
> -rte_eal_parse_coremask(const char *coremask, int *cores);
> +rte_eal_expand_coremask(const char *coremask, int *cores);
>
> #ifdef __cplusplus
> }
> diff --git a/lib/eal/version.map b/lib/eal/version.map
> index a20c713eb1..b51051ee38 100644
> --- a/lib/eal/version.map
> +++ b/lib/eal/version.map
> @@ -405,8 +405,8 @@ INTERNAL {
>
> rte_bus_register;
> rte_bus_unregister;
> + rte_eal_expand_coremask;
> rte_eal_get_baseaddr;
> - rte_eal_parse_coremask;
> rte_firmware_read;
> rte_intr_allow_others;
> rte_intr_cap_multiple;
> --
> 2.45.2
>
--
David Marchand
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 0/3] allow easier use of high lcore-ids
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
0 siblings, 1 reply; 23+ messages in thread
From: David Marchand @ 2025-04-07 7:04 UTC (permalink / raw)
To: Bruce Richardson; +Cc: dev
Hello Bruce,
On Tue, Apr 1, 2025 at 4:08 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Mon, Mar 24, 2025 at 05:30:26PM +0000, Bruce Richardson wrote:
> > Traditionally, DPDK has had a direct mapping of internal lcore-ids, to
> > the actual core numbers in use. With higher core count servers becoming
> > more prevalent the issue becomes one of increasing memory footprint when
> > using such a scheme, due to the need to have all arrays dimensioned for
> > all cores on the system, whether or not those cores are in use by the
> > app.
> >
> > Therefore, the decision was made in the past to not expand the
> > build-time RTE_MAX_LCORE value beyond 128. Instead, it was recommended
> > that users use the "--lcores" EAL parameter to take the high-numbered
> > cores they wish to use and map them to lcore-ids within the 0 - 128
> > range. While this works, this is a little clunky as it means that
> > instead of just passing, for example, "-l 130-139", the user must
> > instead pass "--lcores 0@130,1@131,2@132,3@133,...."
> >
> > This patchset attempts to simplify the situation by adding a new flag to
> > do this mapping automatically. To use cores 130-139 and map them to ids
> > 0-9 internally, the EAL args now become: "-l 130-139 --map-lcore-ids",
> > or using the shorter "-M" version of the flag: "-Ml 130-139".
> >
> > Adding this new parameter required some rework of the existing arg
> > parsing code, because in current DPDK the args are parsed and checked in
> > the order they appear on the commandline. This means that using the
> > example above, the core parameter 130-139 will be rejected immediately
> > before the "map-lcore-ids" parameter is seen. To work around this, the
> > core (and service core) parameters are not parsed when seen, instead
> > they are only saved off and parsed after all arguments are parsed. The
> > "-l" and "-c" parameters are converted into "--lcores" arguments, so all
> > assigning of lcore ids is done there in all cases.
> >
> > RFC->v2:
> > * converted printf to DEBUG log
> > * added "-M" as shorter version of flag
> > * added documentation
> > * renamed internal API that was changed to avoid any potential hidden
> > runtime issues.
> >
> > Bruce Richardson (3):
> > eal: centralize core parameter parsing
> > eal: convert core masks and lists to core sets
> > eal: allow automatic mapping of high lcore ids
> >
> Ping for review.
>
> At a high level, does this feature seem useful to users?
This seems useful, though I am not I would touch the existing options.
I would have gone with a simple -L option (taking the same kind of
input than -l but with new behavior), and not combine a flag with
existing options.
I scanned through the series, not much to say.
Maybe add a unit test for new cmdline option.
--
David Marchand
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 0/3] allow easier use of high lcore-ids
2025-04-07 7:04 ` David Marchand
@ 2025-04-07 9:48 ` Bruce Richardson
2025-04-07 10:15 ` Morten Brørup
0 siblings, 1 reply; 23+ messages in thread
From: Bruce Richardson @ 2025-04-07 9:48 UTC (permalink / raw)
To: David Marchand; +Cc: dev
On Mon, Apr 07, 2025 at 09:04:05AM +0200, David Marchand wrote:
> Hello Bruce,
>
> On Tue, Apr 1, 2025 at 4:08 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > On Mon, Mar 24, 2025 at 05:30:26PM +0000, Bruce Richardson wrote:
> > > Traditionally, DPDK has had a direct mapping of internal lcore-ids, to
> > > the actual core numbers in use. With higher core count servers becoming
> > > more prevalent the issue becomes one of increasing memory footprint when
> > > using such a scheme, due to the need to have all arrays dimensioned for
> > > all cores on the system, whether or not those cores are in use by the
> > > app.
> > >
> > > Therefore, the decision was made in the past to not expand the
> > > build-time RTE_MAX_LCORE value beyond 128. Instead, it was recommended
> > > that users use the "--lcores" EAL parameter to take the high-numbered
> > > cores they wish to use and map them to lcore-ids within the 0 - 128
> > > range. While this works, this is a little clunky as it means that
> > > instead of just passing, for example, "-l 130-139", the user must
> > > instead pass "--lcores 0@130,1@131,2@132,3@133,...."
> > >
> > > This patchset attempts to simplify the situation by adding a new flag to
> > > do this mapping automatically. To use cores 130-139 and map them to ids
> > > 0-9 internally, the EAL args now become: "-l 130-139 --map-lcore-ids",
> > > or using the shorter "-M" version of the flag: "-Ml 130-139".
> > >
> > > Adding this new parameter required some rework of the existing arg
> > > parsing code, because in current DPDK the args are parsed and checked in
> > > the order they appear on the commandline. This means that using the
> > > example above, the core parameter 130-139 will be rejected immediately
> > > before the "map-lcore-ids" parameter is seen. To work around this, the
> > > core (and service core) parameters are not parsed when seen, instead
> > > they are only saved off and parsed after all arguments are parsed. The
> > > "-l" and "-c" parameters are converted into "--lcores" arguments, so all
> > > assigning of lcore ids is done there in all cases.
> > >
> > > RFC->v2:
> > > * converted printf to DEBUG log
> > > * added "-M" as shorter version of flag
> > > * added documentation
> > > * renamed internal API that was changed to avoid any potential hidden
> > > runtime issues.
> > >
> > > Bruce Richardson (3):
> > > eal: centralize core parameter parsing
> > > eal: convert core masks and lists to core sets
> > > eal: allow automatic mapping of high lcore ids
> > >
> > Ping for review.
> >
> > At a high level, does this feature seem useful to users?
>
> This seems useful, though I am not I would touch the existing options.
> I would have gone with a simple -L option (taking the same kind of
> input than -l but with new behavior), and not combine a flag with
> existing options.
>
That would be an easier patchset to do up. However, it would then mean that
we have no less than 4 different ways to specify the cores to use: "-c",
"-l", "-L", "--lcores" - and therefore need 4 different sets of parsing
options for them, and more checks to ensure we have only one of the 4
specified in any run. That's why I chose the modifier option, and to try
and consolidate the core setup a bit.
However, if having a completely new option is preferred, I am happy enough
to do up a different patchset for that.
> I scanned through the series, not much to say.
> Maybe add a unit test for new cmdline option.
>
Sure. Once it's decided what approach (if any) to take, I'll do up a new
patchset, taking into account any relevant feedback on this set.
/Bruce
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH v2 0/3] allow easier use of high lcore-ids
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 15:14 ` Stephen Hemminger
0 siblings, 2 replies; 23+ messages in thread
From: Morten Brørup @ 2025-04-07 10:15 UTC (permalink / raw)
To: Bruce Richardson, David Marchand; +Cc: dev
> From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> Sent: Monday, 7 April 2025 11.49
>
> On Mon, Apr 07, 2025 at 09:04:05AM +0200, David Marchand wrote:
> > Hello Bruce,
> >
> > On Tue, Apr 1, 2025 at 4:08 PM Bruce Richardson
> > <bruce.richardson@intel.com> wrote:
> > >
> > > On Mon, Mar 24, 2025 at 05:30:26PM +0000, Bruce Richardson wrote:
> > > > Traditionally, DPDK has had a direct mapping of internal lcore-
> ids, to
> > > > the actual core numbers in use. With higher core count servers
> becoming
> > > > more prevalent the issue becomes one of increasing memory
> footprint when
> > > > using such a scheme, due to the need to have all arrays
> dimensioned for
> > > > all cores on the system, whether or not those cores are in use by
> the
> > > > app.
> > > >
> > > > Therefore, the decision was made in the past to not expand the
> > > > build-time RTE_MAX_LCORE value beyond 128. Instead, it was
> recommended
> > > > that users use the "--lcores" EAL parameter to take the high-
> numbered
> > > > cores they wish to use and map them to lcore-ids within the 0 -
> 128
> > > > range. While this works, this is a little clunky as it means that
> > > > instead of just passing, for example, "-l 130-139", the user must
> > > > instead pass "--lcores 0@130,1@131,2@132,3@133,...."
> > > >
> > > > This patchset attempts to simplify the situation by adding a new
> flag to
> > > > do this mapping automatically. To use cores 130-139 and map them
> to ids
> > > > 0-9 internally, the EAL args now become: "-l 130-139 --map-lcore-
> ids",
> > > > or using the shorter "-M" version of the flag: "-Ml 130-139".
> > > >
> > > > Adding this new parameter required some rework of the existing
> arg
> > > > parsing code, because in current DPDK the args are parsed and
> checked in
> > > > the order they appear on the commandline. This means that using
> the
> > > > example above, the core parameter 130-139 will be rejected
> immediately
> > > > before the "map-lcore-ids" parameter is seen. To work around
> this, the
> > > > core (and service core) parameters are not parsed when seen,
> instead
> > > > they are only saved off and parsed after all arguments are
> parsed. The
> > > > "-l" and "-c" parameters are converted into "--lcores" arguments,
> so all
> > > > assigning of lcore ids is done there in all cases.
> > > >
> > > > RFC->v2:
> > > > * converted printf to DEBUG log
> > > > * added "-M" as shorter version of flag
> > > > * added documentation
> > > > * renamed internal API that was changed to avoid any potential
> hidden
> > > > runtime issues.
> > > >
> > > > Bruce Richardson (3):
> > > > eal: centralize core parameter parsing
> > > > eal: convert core masks and lists to core sets
> > > > eal: allow automatic mapping of high lcore ids
> > > >
> > > Ping for review.
> > >
> > > At a high level, does this feature seem useful to users?
> >
> > This seems useful, though I am not I would touch the existing
> options.
> > I would have gone with a simple -L option (taking the same kind of
> > input than -l but with new behavior), and not combine a flag with
> > existing options.
> >
>
> That would be an easier patchset to do up. However, it would then mean
> that
> we have no less than 4 different ways to specify the cores to use: "-
> c",
> "-l", "-L", "--lcores" - and therefore need 4 different sets of parsing
> options for them, and more checks to ensure we have only one of the 4
> specified in any run. That's why I chose the modifier option, and to
> try
> and consolidate the core setup a bit.
>
> However, if having a completely new option is preferred, I am happy
> enough
> to do up a different patchset for that.
>
> > I scanned through the series, not much to say.
> > Maybe add a unit test for new cmdline option.
> >
> Sure. Once it's decided what approach (if any) to take, I'll do up a
> new
> patchset, taking into account any relevant feedback on this set.
>
> /Bruce
Changing the EAL parameter parser to a "two pass parser" makes sense.
I think checking for existence of more than one lcore specification options should suffice; we don't need to accept multiple lcore specification options and check for conflicts.
When remapping, do we need to support gaps in the "lcore" (logical cores) array, e.g. for secondary processes, or can it be continuous from 0 to the number of specified lcores?
And are new EAL parameters for this really beneficial?
Doesn't e.g. "-l 0-9@130-139,100@140" suffice?
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 0/3] allow easier use of high lcore-ids
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 15:14 ` Stephen Hemminger
1 sibling, 1 reply; 23+ messages in thread
From: Bruce Richardson @ 2025-04-07 10:40 UTC (permalink / raw)
To: Morten Brørup; +Cc: David Marchand, dev
On Mon, Apr 07, 2025 at 12:15:13PM +0200, Morten Brørup wrote:
> > From: Bruce Richardson [mailto:bruce.richardson@intel.com] Sent:
> > Monday, 7 April 2025 11.49
> >
> > On Mon, Apr 07, 2025 at 09:04:05AM +0200, David Marchand wrote:
> > > Hello Bruce,
> > >
> > > On Tue, Apr 1, 2025 at 4:08 PM Bruce Richardson
> > > <bruce.richardson@intel.com> wrote:
> > > >
> > > > On Mon, Mar 24, 2025 at 05:30:26PM +0000, Bruce Richardson wrote:
> > > > > Traditionally, DPDK has had a direct mapping of internal lcore-
> > ids, to
> > > > > the actual core numbers in use. With higher core count servers
> > becoming
> > > > > more prevalent the issue becomes one of increasing memory
> > footprint when
> > > > > using such a scheme, due to the need to have all arrays
> > dimensioned for
> > > > > all cores on the system, whether or not those cores are in use by
> > the
> > > > > app.
> > > > >
> > > > > Therefore, the decision was made in the past to not expand the
> > > > > build-time RTE_MAX_LCORE value beyond 128. Instead, it was
> > recommended
> > > > > that users use the "--lcores" EAL parameter to take the high-
> > numbered
> > > > > cores they wish to use and map them to lcore-ids within the 0 -
> > 128
> > > > > range. While this works, this is a little clunky as it means that
> > > > > instead of just passing, for example, "-l 130-139", the user must
> > > > > instead pass "--lcores 0@130,1@131,2@132,3@133,...."
> > > > >
> > > > > This patchset attempts to simplify the situation by adding a new
> > flag to
> > > > > do this mapping automatically. To use cores 130-139 and map them
> > to ids
> > > > > 0-9 internally, the EAL args now become: "-l 130-139 --map-lcore-
> > ids",
> > > > > or using the shorter "-M" version of the flag: "-Ml 130-139".
> > > > >
> > > > > Adding this new parameter required some rework of the existing
> > arg
> > > > > parsing code, because in current DPDK the args are parsed and
> > checked in
> > > > > the order they appear on the commandline. This means that using
> > the
> > > > > example above, the core parameter 130-139 will be rejected
> > immediately
> > > > > before the "map-lcore-ids" parameter is seen. To work around
> > this, the
> > > > > core (and service core) parameters are not parsed when seen,
> > instead
> > > > > they are only saved off and parsed after all arguments are
> > parsed. The
> > > > > "-l" and "-c" parameters are converted into "--lcores" arguments,
> > so all
> > > > > assigning of lcore ids is done there in all cases.
> > > > >
> > > > > RFC->v2: * converted printf to DEBUG log * added "-M" as shorter
> > > > > version of flag * added documentation * renamed internal API that
> > > > > was changed to avoid any potential
> > hidden
> > > > > runtime issues.
> > > > >
> > > > > Bruce Richardson (3): eal: centralize core parameter parsing eal:
> > > > > convert core masks and lists to core sets eal: allow automatic
> > > > > mapping of high lcore ids
> > > > >
> > > > Ping for review.
> > > >
> > > > At a high level, does this feature seem useful to users?
> > >
> > > This seems useful, though I am not I would touch the existing
> > options.
> > > I would have gone with a simple -L option (taking the same kind of
> > > input than -l but with new behavior), and not combine a flag with
> > > existing options.
> > >
> >
> > That would be an easier patchset to do up. However, it would then mean
> > that we have no less than 4 different ways to specify the cores to use:
> > "- c", "-l", "-L", "--lcores" - and therefore need 4 different sets of
> > parsing options for them, and more checks to ensure we have only one of
> > the 4 specified in any run. That's why I chose the modifier option, and
> > to try and consolidate the core setup a bit.
> >
> > However, if having a completely new option is preferred, I am happy
> > enough to do up a different patchset for that.
> >
> > > I scanned through the series, not much to say. Maybe add a unit test
> > > for new cmdline option.
> > >
> > Sure. Once it's decided what approach (if any) to take, I'll do up a
> > new patchset, taking into account any relevant feedback on this set.
> >
> > /Bruce
>
> Changing the EAL parameter parser to a "two pass parser" makes sense. I
> think checking for existence of more than one lcore specification options
> should suffice; we don't need to accept multiple lcore specification
> options and check for conflicts.
>
> When remapping, do we need to support gaps in the "lcore" (logical cores)
> array, e.g. for secondary processes, or can it be continuous from 0 to
> the number of specified lcores?
>
> And are new EAL parameters for this really beneficial? Doesn't e.g. "-l
> 0-9@130-139,100@140" suffice?
>
Actually, I believe "0-9@130-139"[1] is not the same as
"0@130,1@131,2@132,...". The latter affinities one thread to one core ten
times over, while the former affinitizes 10 threads to 10 cores - leaving
those threads free to move about within the 10 cores specified.
Just to confirm, I tweaked our helloworld example to print the cpu affinity
of each core when printing.
./build/examples/dpdk-helloworld --no-pci --lcores '(0-3)@(30-33)'
EAL: Detected CPU lcores: 96
EAL: Detected NUMA nodes: 2
EAL: Detected static linkage of DPDK
EAL: Multi-process socket /run/user/11304126/dpdk/rte/mp_socket
EAL: Selected IOVA mode 'VA'
EAL: VFIO support initialized
hello from core 1, with thread affinity for cores: 30 31 32 33
hello from core 3, with thread affinity for cores: 30 31 32 33
hello from core 2, with thread affinity for cores: 30 31 32 33
hello from core 0, with thread affinity for cores: 30 31 32 33
./build/examples/dpdk-helloworld --no-pci --lcores '0@30,1@31,2@32,3@33'
EAL: Detected CPU lcores: 96
EAL: Detected NUMA nodes: 2
EAL: Detected static linkage of DPDK
EAL: Multi-process socket /run/user/11304126/dpdk/rte/mp_socket
EAL: Selected IOVA mode 'VA'
EAL: VFIO support initialized
hello from core 1, with thread affinity for cores: 31 hello from core 3, with thread affinity for cores:
hello from core 2, with thread affinity for cores: 32
hello from core 0, with thread affinity for cores: 30
33
Regards,
/Bruce
[1] This actually needs to be "(0-9)@(130-139)", and with "--lcores", not
just "-l", there are actually different flags with different behaviours
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH v2 0/3] allow easier use of high lcore-ids
2025-04-07 10:40 ` Bruce Richardson
@ 2025-04-07 11:32 ` Morten Brørup
2025-04-07 11:56 ` Bruce Richardson
0 siblings, 1 reply; 23+ messages in thread
From: Morten Brørup @ 2025-04-07 11:32 UTC (permalink / raw)
To: Bruce Richardson; +Cc: David Marchand, dev
> From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> Sent: Monday, 7 April 2025 12.41
>
> On Mon, Apr 07, 2025 at 12:15:13PM +0200, Morten Brørup wrote:
> > > From: Bruce Richardson [mailto:bruce.richardson@intel.com] Sent:
> > > Monday, 7 April 2025 11.49
> > >
> > > On Mon, Apr 07, 2025 at 09:04:05AM +0200, David Marchand wrote:
> > > > Hello Bruce,
> > > >
> > > > On Tue, Apr 1, 2025 at 4:08 PM Bruce Richardson
> > > > <bruce.richardson@intel.com> wrote:
> > > > >
> > > > > On Mon, Mar 24, 2025 at 05:30:26PM +0000, Bruce Richardson
> wrote:
> > > > > > Traditionally, DPDK has had a direct mapping of internal
> lcore-
> > > ids, to
> > > > > > the actual core numbers in use. With higher core count
> servers
> > > becoming
> > > > > > more prevalent the issue becomes one of increasing memory
> > > footprint when
> > > > > > using such a scheme, due to the need to have all arrays
> > > dimensioned for
> > > > > > all cores on the system, whether or not those cores are in
> use by
> > > the
> > > > > > app.
> > > > > >
> > > > > > Therefore, the decision was made in the past to not expand
> the
> > > > > > build-time RTE_MAX_LCORE value beyond 128. Instead, it was
> > > recommended
> > > > > > that users use the "--lcores" EAL parameter to take the high-
> > > numbered
> > > > > > cores they wish to use and map them to lcore-ids within the 0
> -
> > > 128
> > > > > > range. While this works, this is a little clunky as it means
> that
> > > > > > instead of just passing, for example, "-l 130-139", the user
> must
> > > > > > instead pass "--lcores 0@130,1@131,2@132,3@133,...."
> > > > > >
> > > > > > This patchset attempts to simplify the situation by adding a
> new
> > > flag to
> > > > > > do this mapping automatically. To use cores 130-139 and map
> them
> > > to ids
> > > > > > 0-9 internally, the EAL args now become: "-l 130-139 --map-
> lcore-
> > > ids",
> > > > > > or using the shorter "-M" version of the flag: "-Ml 130-139".
> > > > > >
> > > > > > Adding this new parameter required some rework of the
> existing
> > > arg
> > > > > > parsing code, because in current DPDK the args are parsed and
> > > checked in
> > > > > > the order they appear on the commandline. This means that
> using
> > > the
> > > > > > example above, the core parameter 130-139 will be rejected
> > > immediately
> > > > > > before the "map-lcore-ids" parameter is seen. To work around
> > > this, the
> > > > > > core (and service core) parameters are not parsed when seen,
> > > instead
> > > > > > they are only saved off and parsed after all arguments are
> > > parsed. The
> > > > > > "-l" and "-c" parameters are converted into "--lcores"
> arguments,
> > > so all
> > > > > > assigning of lcore ids is done there in all cases.
> > > > > >
> > > > > > RFC->v2: * converted printf to DEBUG log * added "-M" as
> shorter
> > > > > > version of flag * added documentation * renamed internal API
> that
> > > > > > was changed to avoid any potential
> > > hidden
> > > > > > runtime issues.
> > > > > >
> > > > > > Bruce Richardson (3): eal: centralize core parameter parsing
> eal:
> > > > > > convert core masks and lists to core sets eal: allow
> automatic
> > > > > > mapping of high lcore ids
> > > > > >
> > > > > Ping for review.
> > > > >
> > > > > At a high level, does this feature seem useful to users?
> > > >
> > > > This seems useful, though I am not I would touch the existing
> > > options.
> > > > I would have gone with a simple -L option (taking the same kind
> of
> > > > input than -l but with new behavior), and not combine a flag with
> > > > existing options.
> > > >
> > >
> > > That would be an easier patchset to do up. However, it would then
> mean
> > > that we have no less than 4 different ways to specify the cores to
> use:
> > > "- c", "-l", "-L", "--lcores" - and therefore need 4 different sets
> of
> > > parsing options for them, and more checks to ensure we have only
> one of
> > > the 4 specified in any run. That's why I chose the modifier option,
> and
> > > to try and consolidate the core setup a bit.
> > >
> > > However, if having a completely new option is preferred, I am happy
> > > enough to do up a different patchset for that.
> > >
> > > > I scanned through the series, not much to say. Maybe add a unit
> test
> > > > for new cmdline option.
> > > >
> > > Sure. Once it's decided what approach (if any) to take, I'll do up
> a
> > > new patchset, taking into account any relevant feedback on this
> set.
> > >
> > > /Bruce
> >
> > Changing the EAL parameter parser to a "two pass parser" makes sense.
> I
> > think checking for existence of more than one lcore specification
> options
> > should suffice; we don't need to accept multiple lcore specification
> > options and check for conflicts.
> >
> > When remapping, do we need to support gaps in the "lcore" (logical
> cores)
> > array, e.g. for secondary processes, or can it be continuous from 0
> to
> > the number of specified lcores?
> >
> > And are new EAL parameters for this really beneficial? Doesn't e.g.
> "-l
> > 0-9@130-139,100@140" suffice?
> >
> Actually, I believe "0-9@130-139"[1] is not the same as
> "0@130,1@131,2@132,...". The latter affinities one thread to one core
> ten
> times over, while the former affinitizes 10 threads to 10 cores -
> leaving
> those threads free to move about within the 10 cores specified.
Interesting. The documentation [GSG] isn't clear to me about this; a example there could help clarify.
[GSG]: https://doc.dpdk.org/guides/linux_gsg/linux_eal_parameters.html#lcore-related-options
If users are manually passing lcore parameters to the EAL, then I see why some sort of remapping shorthand is useful.
IMO, if the mappings are relatively exotic, it should be acceptable requiring an external script to build a long list of mapping parameters and then invoke the application with those script-generated EAL parameters.
This would reduce the scope to support relatively simple, common mappings.
Could we expand the --lcores syntax to support common mappings?
E.g. "0-9@130+" to do what I thought.
The lack of "()" treats the entries individually (not as a group), and the "+" indicates auto-increment.
A more advanced example:
"0-9@(130-131)+", meaning lcore 0 gets cpus 130-131, lcore 1 gets cpus 132-133, etc.
>
> Just to confirm, I tweaked our helloworld example to print the cpu
> affinity
> of each core when printing.
>
> ./build/examples/dpdk-helloworld --no-pci --lcores '(0-3)@(30-33)'
> EAL: Detected CPU lcores: 96
> EAL: Detected NUMA nodes: 2
> EAL: Detected static linkage of DPDK
> EAL: Multi-process socket /run/user/11304126/dpdk/rte/mp_socket
> EAL: Selected IOVA mode 'VA'
> EAL: VFIO support initialized
> hello from core 1, with thread affinity for cores: 30 31 32 33
> hello from core 3, with thread affinity for cores: 30 31 32 33
> hello from core 2, with thread affinity for cores: 30 31 32 33
> hello from core 0, with thread affinity for cores: 30 31 32 33
>
> ./build/examples/dpdk-helloworld --no-pci --lcores
> '0@30,1@31,2@32,3@33'
> EAL: Detected CPU lcores: 96
> EAL: Detected NUMA nodes: 2
> EAL: Detected static linkage of DPDK
> EAL: Multi-process socket /run/user/11304126/dpdk/rte/mp_socket
> EAL: Selected IOVA mode 'VA'
> EAL: VFIO support initialized
> hello from core 1, with thread affinity for cores: 31 hello from core
> 3, with thread affinity for cores:
> hello from core 2, with thread affinity for cores: 32
> hello from core 0, with thread affinity for cores: 30
> 33
>
> Regards,
> /Bruce
>
> [1] This actually needs to be "(0-9)@(130-139)", and with "--lcores",
> not
> just "-l", there are actually different flags with different behaviours
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 0/3] allow easier use of high lcore-ids
2025-04-07 11:32 ` Morten Brørup
@ 2025-04-07 11:56 ` Bruce Richardson
2025-04-07 12:25 ` Morten Brørup
0 siblings, 1 reply; 23+ messages in thread
From: Bruce Richardson @ 2025-04-07 11:56 UTC (permalink / raw)
To: Morten Brørup; +Cc: David Marchand, dev
On Mon, Apr 07, 2025 at 01:32:59PM +0200, Morten Brørup wrote:
> > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > Sent: Monday, 7 April 2025 12.41
> >
> > On Mon, Apr 07, 2025 at 12:15:13PM +0200, Morten Brørup wrote:
> > > > From: Bruce Richardson [mailto:bruce.richardson@intel.com] Sent:
> > > > Monday, 7 April 2025 11.49
> > > >
> > > > On Mon, Apr 07, 2025 at 09:04:05AM +0200, David Marchand wrote:
> > > > > Hello Bruce,
> > > > >
> > > > > On Tue, Apr 1, 2025 at 4:08 PM Bruce Richardson
> > > > > <bruce.richardson@intel.com> wrote:
> > > > > >
> > > > > > On Mon, Mar 24, 2025 at 05:30:26PM +0000, Bruce Richardson
> > wrote:
> > > > > > > Traditionally, DPDK has had a direct mapping of internal
> > lcore-
> > > > ids, to
> > > > > > > the actual core numbers in use. With higher core count
> > servers
> > > > becoming
> > > > > > > more prevalent the issue becomes one of increasing memory
> > > > footprint when
> > > > > > > using such a scheme, due to the need to have all arrays
> > > > dimensioned for
> > > > > > > all cores on the system, whether or not those cores are in
> > use by
> > > > the
> > > > > > > app.
> > > > > > >
> > > > > > > Therefore, the decision was made in the past to not expand
> > the
> > > > > > > build-time RTE_MAX_LCORE value beyond 128. Instead, it was
> > > > recommended
> > > > > > > that users use the "--lcores" EAL parameter to take the high-
> > > > numbered
> > > > > > > cores they wish to use and map them to lcore-ids within the 0
> > -
> > > > 128
> > > > > > > range. While this works, this is a little clunky as it means
> > that
> > > > > > > instead of just passing, for example, "-l 130-139", the user
> > must
> > > > > > > instead pass "--lcores 0@130,1@131,2@132,3@133,...."
> > > > > > >
> > > > > > > This patchset attempts to simplify the situation by adding a
> > new
> > > > flag to
> > > > > > > do this mapping automatically. To use cores 130-139 and map
> > them
> > > > to ids
> > > > > > > 0-9 internally, the EAL args now become: "-l 130-139 --map-
> > lcore-
> > > > ids",
> > > > > > > or using the shorter "-M" version of the flag: "-Ml 130-139".
> > > > > > >
> > > > > > > Adding this new parameter required some rework of the
> > existing
> > > > arg
> > > > > > > parsing code, because in current DPDK the args are parsed and
> > > > checked in
> > > > > > > the order they appear on the commandline. This means that
> > using
> > > > the
> > > > > > > example above, the core parameter 130-139 will be rejected
> > > > immediately
> > > > > > > before the "map-lcore-ids" parameter is seen. To work around
> > > > this, the
> > > > > > > core (and service core) parameters are not parsed when seen,
> > > > instead
> > > > > > > they are only saved off and parsed after all arguments are
> > > > parsed. The
> > > > > > > "-l" and "-c" parameters are converted into "--lcores"
> > arguments,
> > > > so all
> > > > > > > assigning of lcore ids is done there in all cases.
> > > > > > >
> > > > > > > RFC->v2: * converted printf to DEBUG log * added "-M" as
> > shorter
> > > > > > > version of flag * added documentation * renamed internal API
> > that
> > > > > > > was changed to avoid any potential
> > > > hidden
> > > > > > > runtime issues.
> > > > > > >
> > > > > > > Bruce Richardson (3): eal: centralize core parameter parsing
> > eal:
> > > > > > > convert core masks and lists to core sets eal: allow
> > automatic
> > > > > > > mapping of high lcore ids
> > > > > > >
> > > > > > Ping for review.
> > > > > >
> > > > > > At a high level, does this feature seem useful to users?
> > > > >
> > > > > This seems useful, though I am not I would touch the existing
> > > > options.
> > > > > I would have gone with a simple -L option (taking the same kind
> > of
> > > > > input than -l but with new behavior), and not combine a flag with
> > > > > existing options.
> > > > >
> > > >
> > > > That would be an easier patchset to do up. However, it would then
> > mean
> > > > that we have no less than 4 different ways to specify the cores to
> > use:
> > > > "- c", "-l", "-L", "--lcores" - and therefore need 4 different sets
> > of
> > > > parsing options for them, and more checks to ensure we have only
> > one of
> > > > the 4 specified in any run. That's why I chose the modifier option,
> > and
> > > > to try and consolidate the core setup a bit.
> > > >
> > > > However, if having a completely new option is preferred, I am happy
> > > > enough to do up a different patchset for that.
> > > >
> > > > > I scanned through the series, not much to say. Maybe add a unit
> > test
> > > > > for new cmdline option.
> > > > >
> > > > Sure. Once it's decided what approach (if any) to take, I'll do up
> > a
> > > > new patchset, taking into account any relevant feedback on this
> > set.
> > > >
> > > > /Bruce
> > >
> > > Changing the EAL parameter parser to a "two pass parser" makes sense.
> > I
> > > think checking for existence of more than one lcore specification
> > options
> > > should suffice; we don't need to accept multiple lcore specification
> > > options and check for conflicts.
> > >
> > > When remapping, do we need to support gaps in the "lcore" (logical
> > cores)
> > > array, e.g. for secondary processes, or can it be continuous from 0
> > to
> > > the number of specified lcores?
> > >
> > > And are new EAL parameters for this really beneficial? Doesn't e.g.
> > "-l
> > > 0-9@130-139,100@140" suffice?
> > >
> > Actually, I believe "0-9@130-139"[1] is not the same as
> > "0@130,1@131,2@132,...". The latter affinities one thread to one core
> > ten
> > times over, while the former affinitizes 10 threads to 10 cores -
> > leaving
> > those threads free to move about within the 10 cores specified.
>
> Interesting. The documentation [GSG] isn't clear to me about this; a example there could help clarify.
>
> [GSG]: https://doc.dpdk.org/guides/linux_gsg/linux_eal_parameters.html#lcore-related-options
>
Yep, agreed.
> If users are manually passing lcore parameters to the EAL, then I see why some sort of remapping shorthand is useful.
> IMO, if the mappings are relatively exotic, it should be acceptable requiring an external script to build a long list of mapping parameters and then invoke the application with those script-generated EAL parameters.
> This would reduce the scope to support relatively simple, common mappings.
>
> Could we expand the --lcores syntax to support common mappings?
>
> E.g. "0-9@130+" to do what I thought.
> The lack of "()" treats the entries individually (not as a group), and the "+" indicates auto-increment.
>
> A more advanced example:
> "0-9@(130-131)+", meaning lcore 0 gets cpus 130-131, lcore 1 gets cpus 132-133, etc.
>
My issues with the above syntax idea is:
* I think it's overly complicating the lcores parameter adding in the
support for the "+" symbol - especially in the advanced case example you
provide. I worry about code maintainability here.
* More significantly for me, I think it's also getting things backwards in
that it is focusing more on the lcore ids visible to the app, rather than
the physical cores to be used. For the example above of 0-9@130+, what I
would expect is that the user is mainly thinking about the cores he wants
to use 130-139, which are then to be mapped to lower ids. If we had the
syntax reversed where the physical cores were first, I'd say it would
make more sense, e.g. 130-139@0+
* finally, as I found out last month, there are systems on which the cores
are spread across numa-nodes on odd/even boundaries, so to have an app
running on socket 0, you need to give the core ids as a list i.e.
0,2,4,6, and cannot use ranges. [This also reenforces the point above
too, where again it's the internal ids we need to generalize, not the
physical cpus]
My thinking on the matter, and I'm happy to be corrected if I'm wrong here,
is that end-users are unlikely to significantly care what the actual lcore
ids are internally in the program, so long as they work and are unique.
What does matter is the physical cpus on which the code is to run.
Therefore, my approach to this is to find the simplest possible solution
whereby the user can just provide a list of cores and tell EAL to just map
them to reasonable values. For the "reasonable" values, I would imagine
that for the vast majority of cases starting "0" is what is wanted. For
anything beyond that, we already have the existing --lcores syntax to be
used.
My 2c.
/Bruce
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH v2 0/3] allow easier use of high lcore-ids
2025-04-07 11:56 ` Bruce Richardson
@ 2025-04-07 12:25 ` Morten Brørup
2025-04-07 12:41 ` Bruce Richardson
0 siblings, 1 reply; 23+ messages in thread
From: Morten Brørup @ 2025-04-07 12:25 UTC (permalink / raw)
To: Bruce Richardson; +Cc: David Marchand, dev
> From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> Sent: Monday, 7 April 2025 13.56
>
> On Mon, Apr 07, 2025 at 01:32:59PM +0200, Morten Brørup wrote:
> > > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > > Sent: Monday, 7 April 2025 12.41
> > >
> > > On Mon, Apr 07, 2025 at 12:15:13PM +0200, Morten Brørup wrote:
> > > > > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> Sent:
> > > > > Monday, 7 April 2025 11.49
> > > > >
> > > > > On Mon, Apr 07, 2025 at 09:04:05AM +0200, David Marchand wrote:
> > > > > > Hello Bruce,
> > > > > >
> > > > > > On Tue, Apr 1, 2025 at 4:08 PM Bruce Richardson
> > > > > > <bruce.richardson@intel.com> wrote:
> > > > > > >
> > > > > > > On Mon, Mar 24, 2025 at 05:30:26PM +0000, Bruce Richardson
> > > wrote:
> > > > > > > > Traditionally, DPDK has had a direct mapping of internal
> > > lcore-
> > > > > ids, to
> > > > > > > > the actual core numbers in use. With higher core count
> > > servers
> > > > > becoming
> > > > > > > > more prevalent the issue becomes one of increasing memory
> > > > > footprint when
> > > > > > > > using such a scheme, due to the need to have all arrays
> > > > > dimensioned for
> > > > > > > > all cores on the system, whether or not those cores are
> in
> > > use by
> > > > > the
> > > > > > > > app.
> > > > > > > >
> > > > > > > > Therefore, the decision was made in the past to not
> expand
> > > the
> > > > > > > > build-time RTE_MAX_LCORE value beyond 128. Instead, it
> was
> > > > > recommended
> > > > > > > > that users use the "--lcores" EAL parameter to take the
> high-
> > > > > numbered
> > > > > > > > cores they wish to use and map them to lcore-ids within
> the 0
> > > -
> > > > > 128
> > > > > > > > range. While this works, this is a little clunky as it
> means
> > > that
> > > > > > > > instead of just passing, for example, "-l 130-139", the
> user
> > > must
> > > > > > > > instead pass "--lcores 0@130,1@131,2@132,3@133,...."
> > > > > > > >
> > > > > > > > This patchset attempts to simplify the situation by
> adding a
> > > new
> > > > > flag to
> > > > > > > > do this mapping automatically. To use cores 130-139 and
> map
> > > them
> > > > > to ids
> > > > > > > > 0-9 internally, the EAL args now become: "-l 130-139 --
> map-
> > > lcore-
> > > > > ids",
> > > > > > > > or using the shorter "-M" version of the flag: "-Ml 130-
> 139".
> > > > > > > >
> > > > > > > > Adding this new parameter required some rework of the
> > > existing
> > > > > arg
> > > > > > > > parsing code, because in current DPDK the args are parsed
> and
> > > > > checked in
> > > > > > > > the order they appear on the commandline. This means that
> > > using
> > > > > the
> > > > > > > > example above, the core parameter 130-139 will be
> rejected
> > > > > immediately
> > > > > > > > before the "map-lcore-ids" parameter is seen. To work
> around
> > > > > this, the
> > > > > > > > core (and service core) parameters are not parsed when
> seen,
> > > > > instead
> > > > > > > > they are only saved off and parsed after all arguments
> are
> > > > > parsed. The
> > > > > > > > "-l" and "-c" parameters are converted into "--lcores"
> > > arguments,
> > > > > so all
> > > > > > > > assigning of lcore ids is done there in all cases.
> > > > > > > >
> > > > > > > > RFC->v2: * converted printf to DEBUG log * added "-M" as
> > > shorter
> > > > > > > > version of flag * added documentation * renamed internal
> API
> > > that
> > > > > > > > was changed to avoid any potential
> > > > > hidden
> > > > > > > > runtime issues.
> > > > > > > >
> > > > > > > > Bruce Richardson (3): eal: centralize core parameter
> parsing
> > > eal:
> > > > > > > > convert core masks and lists to core sets eal: allow
> > > automatic
> > > > > > > > mapping of high lcore ids
> > > > > > > >
> > > > > > > Ping for review.
> > > > > > >
> > > > > > > At a high level, does this feature seem useful to users?
> > > > > >
> > > > > > This seems useful, though I am not I would touch the existing
> > > > > options.
> > > > > > I would have gone with a simple -L option (taking the same
> kind
> > > of
> > > > > > input than -l but with new behavior), and not combine a flag
> with
> > > > > > existing options.
> > > > > >
> > > > >
> > > > > That would be an easier patchset to do up. However, it would
> then
> > > mean
> > > > > that we have no less than 4 different ways to specify the cores
> to
> > > use:
> > > > > "- c", "-l", "-L", "--lcores" - and therefore need 4 different
> sets
> > > of
> > > > > parsing options for them, and more checks to ensure we have
> only
> > > one of
> > > > > the 4 specified in any run. That's why I chose the modifier
> option,
> > > and
> > > > > to try and consolidate the core setup a bit.
> > > > >
> > > > > However, if having a completely new option is preferred, I am
> happy
> > > > > enough to do up a different patchset for that.
> > > > >
> > > > > > I scanned through the series, not much to say. Maybe add a
> unit
> > > test
> > > > > > for new cmdline option.
> > > > > >
> > > > > Sure. Once it's decided what approach (if any) to take, I'll do
> up
> > > a
> > > > > new patchset, taking into account any relevant feedback on this
> > > set.
> > > > >
> > > > > /Bruce
> > > >
> > > > Changing the EAL parameter parser to a "two pass parser" makes
> sense.
> > > I
> > > > think checking for existence of more than one lcore specification
> > > options
> > > > should suffice; we don't need to accept multiple lcore
> specification
> > > > options and check for conflicts.
> > > >
> > > > When remapping, do we need to support gaps in the "lcore"
> (logical
> > > cores)
> > > > array, e.g. for secondary processes, or can it be continuous from
> 0
> > > to
> > > > the number of specified lcores?
> > > >
> > > > And are new EAL parameters for this really beneficial? Doesn't
> e.g.
> > > "-l
> > > > 0-9@130-139,100@140" suffice?
> > > >
> > > Actually, I believe "0-9@130-139"[1] is not the same as
> > > "0@130,1@131,2@132,...". The latter affinities one thread to one
> core
> > > ten
> > > times over, while the former affinitizes 10 threads to 10 cores -
> > > leaving
> > > those threads free to move about within the 10 cores specified.
> >
> > Interesting. The documentation [GSG] isn't clear to me about this; a
> example there could help clarify.
> >
> > [GSG]:
> https://doc.dpdk.org/guides/linux_gsg/linux_eal_parameters.html#lcore-
> related-options
> >
>
> Yep, agreed.
>
> > If users are manually passing lcore parameters to the EAL, then I see
> why some sort of remapping shorthand is useful.
> > IMO, if the mappings are relatively exotic, it should be acceptable
> requiring an external script to build a long list of mapping parameters
> and then invoke the application with those script-generated EAL
> parameters.
> > This would reduce the scope to support relatively simple, common
> mappings.
> >
> > Could we expand the --lcores syntax to support common mappings?
> >
> > E.g. "0-9@130+" to do what I thought.
> > The lack of "()" treats the entries individually (not as a group),
> and the "+" indicates auto-increment.
> >
> > A more advanced example:
> > "0-9@(130-131)+", meaning lcore 0 gets cpus 130-131, lcore 1 gets
> cpus 132-133, etc.
> >
>
> My issues with the above syntax idea is:
> * I think it's overly complicating the lcores parameter adding in the
> support for the "+" symbol - especially in the advanced case example
> you
> provide. I worry about code maintainability here.
> * More significantly for me, I think it's also getting things backwards
> in
> that it is focusing more on the lcore ids visible to the app, rather
> than
> the physical cores to be used. For the example above of 0-9@130+,
> what I
> would expect is that the user is mainly thinking about the cores he
> wants
> to use 130-139, which are then to be mapped to lower ids. If we had
> the
> syntax reversed where the physical cores were first, I'd say it would
> make more sense, e.g. 130-139@0+
I 100 % agree on the syntax being backwards.
A good reason for introducing a new parameter, rather than expanding on "--lcores".
We should consider deprecating the old (backwards) syntax, so users don’t get confused about one EAL parameter being "backwards" of the other.
> * finally, as I found out last month, there are systems on which the
> cores
> are spread across numa-nodes on odd/even boundaries, so to have an
> app
> running on socket 0, you need to give the core ids as a list i.e.
> 0,2,4,6, and cannot use ranges. [This also reenforces the point above
> too, where again it's the internal ids we need to generalize, not the
> physical cpus]
For this, we could use "/2" (like in subnets), or "+2" as increment parameter.
>
> My thinking on the matter, and I'm happy to be corrected if I'm wrong
> here,
> is that end-users are unlikely to significantly care what the actual
> lcore
> ids are internally in the program, so long as they work and are unique.
Generally yes.
Note that we should allow the same physical CPU core to be assigned to multiple lcores...
If a CPU core is shared between multiple worker lcores, then it could be problematic, but with a mix of lcore roles, this might be handy for some applications. E.g. a virtual machine with only one CPU core could use that single CPU core as both main and worker lcore, or as main and service lcore.
Sharing an lcore between threads requires the developer to take special care of e.g. spinlocks, but the EAL parameter parser should not prohibit it. It might log a notice, though.
> What does matter is the physical cpus on which the code is to run.
> Therefore, my approach to this is to find the simplest possible
> solution
> whereby the user can just provide a list of cores and tell EAL to just
> map
> them to reasonable values. For the "reasonable" values, I would imagine
> that for the vast majority of cases starting "0" is what is wanted. For
> anything beyond that, we already have the existing --lcores syntax to
> be
> used.
Agree with all of the above. :-)
>
> My 2c.
>
> /Bruce
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 0/3] allow easier use of high lcore-ids
2025-04-07 12:25 ` Morten Brørup
@ 2025-04-07 12:41 ` Bruce Richardson
2025-04-07 13:18 ` Morten Brørup
0 siblings, 1 reply; 23+ messages in thread
From: Bruce Richardson @ 2025-04-07 12:41 UTC (permalink / raw)
To: Morten Brørup; +Cc: David Marchand, dev
On Mon, Apr 07, 2025 at 02:25:46PM +0200, Morten Brørup wrote:
> > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > Sent: Monday, 7 April 2025 13.56
> >
> > On Mon, Apr 07, 2025 at 01:32:59PM +0200, Morten Brørup wrote:
> > > > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > > > Sent: Monday, 7 April 2025 12.41
> > > >
> > > > On Mon, Apr 07, 2025 at 12:15:13PM +0200, Morten Brørup wrote:
> > > > > > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > Sent:
> > > > > > Monday, 7 April 2025 11.49
> > > > > >
> > > > > > On Mon, Apr 07, 2025 at 09:04:05AM +0200, David Marchand wrote:
> > > > > > > Hello Bruce,
> > > > > > >
> > > > > > > On Tue, Apr 1, 2025 at 4:08 PM Bruce Richardson
> > > > > > > <bruce.richardson@intel.com> wrote:
> > > > > > > >
> > > > > > > > On Mon, Mar 24, 2025 at 05:30:26PM +0000, Bruce Richardson
> > > > wrote:
> > > > > > > > > Traditionally, DPDK has had a direct mapping of internal
> > > > lcore-
> > > > > > ids, to
> > > > > > > > > the actual core numbers in use. With higher core count
> > > > servers
> > > > > > becoming
> > > > > > > > > more prevalent the issue becomes one of increasing memory
> > > > > > footprint when
> > > > > > > > > using such a scheme, due to the need to have all arrays
> > > > > > dimensioned for
> > > > > > > > > all cores on the system, whether or not those cores are
> > in
> > > > use by
> > > > > > the
> > > > > > > > > app.
> > > > > > > > >
> > > > > > > > > Therefore, the decision was made in the past to not
> > expand
> > > > the
> > > > > > > > > build-time RTE_MAX_LCORE value beyond 128. Instead, it
> > was
> > > > > > recommended
> > > > > > > > > that users use the "--lcores" EAL parameter to take the
> > high-
> > > > > > numbered
> > > > > > > > > cores they wish to use and map them to lcore-ids within
> > the 0
> > > > -
> > > > > > 128
> > > > > > > > > range. While this works, this is a little clunky as it
> > means
> > > > that
> > > > > > > > > instead of just passing, for example, "-l 130-139", the
> > user
> > > > must
> > > > > > > > > instead pass "--lcores 0@130,1@131,2@132,3@133,...."
> > > > > > > > >
> > > > > > > > > This patchset attempts to simplify the situation by
> > adding a
> > > > new
> > > > > > flag to
> > > > > > > > > do this mapping automatically. To use cores 130-139 and
> > map
> > > > them
> > > > > > to ids
> > > > > > > > > 0-9 internally, the EAL args now become: "-l 130-139 --
> > map-
> > > > lcore-
> > > > > > ids",
> > > > > > > > > or using the shorter "-M" version of the flag: "-Ml 130-
> > 139".
> > > > > > > > >
> > > > > > > > > Adding this new parameter required some rework of the
> > > > existing
> > > > > > arg
> > > > > > > > > parsing code, because in current DPDK the args are parsed
> > and
> > > > > > checked in
> > > > > > > > > the order they appear on the commandline. This means that
> > > > using
> > > > > > the
> > > > > > > > > example above, the core parameter 130-139 will be
> > rejected
> > > > > > immediately
> > > > > > > > > before the "map-lcore-ids" parameter is seen. To work
> > around
> > > > > > this, the
> > > > > > > > > core (and service core) parameters are not parsed when
> > seen,
> > > > > > instead
> > > > > > > > > they are only saved off and parsed after all arguments
> > are
> > > > > > parsed. The
> > > > > > > > > "-l" and "-c" parameters are converted into "--lcores"
> > > > arguments,
> > > > > > so all
> > > > > > > > > assigning of lcore ids is done there in all cases.
> > > > > > > > >
> > > > > > > > > RFC->v2: * converted printf to DEBUG log * added "-M" as
> > > > shorter
> > > > > > > > > version of flag * added documentation * renamed internal
> > API
> > > > that
> > > > > > > > > was changed to avoid any potential
> > > > > > hidden
> > > > > > > > > runtime issues.
> > > > > > > > >
> > > > > > > > > Bruce Richardson (3): eal: centralize core parameter
> > parsing
> > > > eal:
> > > > > > > > > convert core masks and lists to core sets eal: allow
> > > > automatic
> > > > > > > > > mapping of high lcore ids
> > > > > > > > >
> > > > > > > > Ping for review.
> > > > > > > >
> > > > > > > > At a high level, does this feature seem useful to users?
> > > > > > >
> > > > > > > This seems useful, though I am not I would touch the existing
> > > > > > options.
> > > > > > > I would have gone with a simple -L option (taking the same
> > kind
> > > > of
> > > > > > > input than -l but with new behavior), and not combine a flag
> > with
> > > > > > > existing options.
> > > > > > >
> > > > > >
> > > > > > That would be an easier patchset to do up. However, it would
> > then
> > > > mean
> > > > > > that we have no less than 4 different ways to specify the cores
> > to
> > > > use:
> > > > > > "- c", "-l", "-L", "--lcores" - and therefore need 4 different
> > sets
> > > > of
> > > > > > parsing options for them, and more checks to ensure we have
> > only
> > > > one of
> > > > > > the 4 specified in any run. That's why I chose the modifier
> > option,
> > > > and
> > > > > > to try and consolidate the core setup a bit.
> > > > > >
> > > > > > However, if having a completely new option is preferred, I am
> > happy
> > > > > > enough to do up a different patchset for that.
> > > > > >
> > > > > > > I scanned through the series, not much to say. Maybe add a
> > unit
> > > > test
> > > > > > > for new cmdline option.
> > > > > > >
> > > > > > Sure. Once it's decided what approach (if any) to take, I'll do
> > up
> > > > a
> > > > > > new patchset, taking into account any relevant feedback on this
> > > > set.
> > > > > >
> > > > > > /Bruce
> > > > >
> > > > > Changing the EAL parameter parser to a "two pass parser" makes
> > sense.
> > > > I
> > > > > think checking for existence of more than one lcore specification
> > > > options
> > > > > should suffice; we don't need to accept multiple lcore
> > specification
> > > > > options and check for conflicts.
> > > > >
> > > > > When remapping, do we need to support gaps in the "lcore"
> > (logical
> > > > cores)
> > > > > array, e.g. for secondary processes, or can it be continuous from
> > 0
> > > > to
> > > > > the number of specified lcores?
> > > > >
> > > > > And are new EAL parameters for this really beneficial? Doesn't
> > e.g.
> > > > "-l
> > > > > 0-9@130-139,100@140" suffice?
> > > > >
> > > > Actually, I believe "0-9@130-139"[1] is not the same as
> > > > "0@130,1@131,2@132,...". The latter affinities one thread to one
> > core
> > > > ten
> > > > times over, while the former affinitizes 10 threads to 10 cores -
> > > > leaving
> > > > those threads free to move about within the 10 cores specified.
> > >
> > > Interesting. The documentation [GSG] isn't clear to me about this; a
> > example there could help clarify.
> > >
> > > [GSG]:
> > https://doc.dpdk.org/guides/linux_gsg/linux_eal_parameters.html#lcore-
> > related-options
> > >
> >
> > Yep, agreed.
> >
> > > If users are manually passing lcore parameters to the EAL, then I see
> > why some sort of remapping shorthand is useful.
> > > IMO, if the mappings are relatively exotic, it should be acceptable
> > requiring an external script to build a long list of mapping parameters
> > and then invoke the application with those script-generated EAL
> > parameters.
> > > This would reduce the scope to support relatively simple, common
> > mappings.
> > >
> > > Could we expand the --lcores syntax to support common mappings?
> > >
> > > E.g. "0-9@130+" to do what I thought.
> > > The lack of "()" treats the entries individually (not as a group),
> > and the "+" indicates auto-increment.
> > >
> > > A more advanced example:
> > > "0-9@(130-131)+", meaning lcore 0 gets cpus 130-131, lcore 1 gets
> > cpus 132-133, etc.
> > >
> >
> > My issues with the above syntax idea is:
> > * I think it's overly complicating the lcores parameter adding in the
> > support for the "+" symbol - especially in the advanced case example
> > you
> > provide. I worry about code maintainability here.
> > * More significantly for me, I think it's also getting things backwards
> > in
> > that it is focusing more on the lcore ids visible to the app, rather
> > than
> > the physical cores to be used. For the example above of 0-9@130+,
> > what I
> > would expect is that the user is mainly thinking about the cores he
> > wants
> > to use 130-139, which are then to be mapped to lower ids. If we had
> > the
> > syntax reversed where the physical cores were first, I'd say it would
> > make more sense, e.g. 130-139@0+
>
> I 100 % agree on the syntax being backwards.
> A good reason for introducing a new parameter, rather than expanding on "--lcores".
>
Yes and no.
I agree with not expanding on --lcores, but I also don't think any new
parameter added should attempt to reproduce all of what lcores does. I
would leave --lcores as-is, as the power-tool for lcore config e.g. what
you talk about below for mapping multiple lcores to the same physical cpu.
> We should consider deprecating the old (backwards) syntax, so users don’t get confused about one EAL parameter being "backwards" of the other.
>
I disagree with this. I would be ok with deprecating the old "-c" coremask
syntax - I think the time is long past when we should be dealing with
masks. However, removing "-l" and "--lcores" flag is, to me anyway, too big
and jarring a change for end users for us to consider.
> > * finally, as I found out last month, there are systems on which the
> > cores
> > are spread across numa-nodes on odd/even boundaries, so to have an
> > app
> > running on socket 0, you need to give the core ids as a list i.e.
> > 0,2,4,6, and cannot use ranges. [This also reenforces the point above
> > too, where again it's the internal ids we need to generalize, not the
> > physical cpus]
>
> For this, we could use "/2" (like in subnets), or "+2" as increment parameter.
>
> >
> > My thinking on the matter, and I'm happy to be corrected if I'm wrong
> > here,
> > is that end-users are unlikely to significantly care what the actual
> > lcore
> > ids are internally in the program, so long as they work and are unique.
>
> Generally yes.
> Note that we should allow the same physical CPU core to be assigned to multiple lcores...
> If a CPU core is shared between multiple worker lcores, then it could be problematic, but with a mix of lcore roles, this might be handy for some applications. E.g. a virtual machine with only one CPU core could use that single CPU core as both main and worker lcore, or as main and service lcore.
> Sharing an lcore between threads requires the developer to take special care of e.g. spinlocks, but the EAL parameter parser should not prohibit it. It might log a notice, though.
>
This is already taken care of via --lcores, so I don't see the need to
reimplement it using a new flag also. Any new flags we add should be kept
deliberately simple.
> > What does matter is the physical cpus on which the code is to run.
> > Therefore, my approach to this is to find the simplest possible
> > solution
> > whereby the user can just provide a list of cores and tell EAL to just
> > map
> > them to reasonable values. For the "reasonable" values, I would imagine
> > that for the vast majority of cases starting "0" is what is wanted. For
> > anything beyond that, we already have the existing --lcores syntax to
> > be
> > used.
>
> Agree with all of the above. :-)
>
Great. That still leaves us with the problem of what exactly to do as the
best solution. Here are some alternatives that I see:
1. Add a modifier flag for -l and -c parameters to auto-remap the lcore ids
to zero, so user is just specifying physical CPU id's.
2. Add a new flag for specifying physical cpu ids, which auto-remaps the
cores specified to zero.
2a. To simplify our code and user experience we could at the same time:
* deprecate "-c" flag for coremasks
* make "-l" and "--lcores" the same flag just in long and short
versions. This should not break anything as "-l" parameter is
just as subset of what "--lcores" provides.
* that would leave us with effectively two core flag paths:
- -l/--lcores, behaviour as now, full explicit control
- -L/--lcores-remapped, takes simplified core list (only "," and
"-" supported as with "-l" now), and maps them to zero-based.
Third options? Any other feedback?
/Bruce
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH v2 0/3] allow easier use of high lcore-ids
2025-04-07 12:41 ` Bruce Richardson
@ 2025-04-07 13:18 ` Morten Brørup
2025-04-07 13:24 ` Bruce Richardson
0 siblings, 1 reply; 23+ messages in thread
From: Morten Brørup @ 2025-04-07 13:18 UTC (permalink / raw)
To: Bruce Richardson; +Cc: David Marchand, dev
> From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> Sent: Monday, 7 April 2025 14.42
>
> On Mon, Apr 07, 2025 at 02:25:46PM +0200, Morten Brørup wrote:
> > > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > > Sent: Monday, 7 April 2025 13.56
> > >
> > > On Mon, Apr 07, 2025 at 01:32:59PM +0200, Morten Brørup wrote:
> > > > > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > > > > Sent: Monday, 7 April 2025 12.41
> > > > >
> > > > > On Mon, Apr 07, 2025 at 12:15:13PM +0200, Morten Brørup wrote:
> > > > > > > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > > Sent:
> > > > > > > Monday, 7 April 2025 11.49
> > > > > > >
> > > > > > > On Mon, Apr 07, 2025 at 09:04:05AM +0200, David Marchand
> wrote:
> > > > > > > > Hello Bruce,
> > > > > > > >
> > > > > > > > On Tue, Apr 1, 2025 at 4:08 PM Bruce Richardson
> > > > > > > > <bruce.richardson@intel.com> wrote:
> > > > > > > > >
> > > > > > > > > On Mon, Mar 24, 2025 at 05:30:26PM +0000, Bruce
> Richardson
> > > > > wrote:
> > > > > > > > > > Traditionally, DPDK has had a direct mapping of
> internal
> > > > > lcore-
> > > > > > > ids, to
> > > > > > > > > > the actual core numbers in use. With higher core
> count
> > > > > servers
> > > > > > > becoming
> > > > > > > > > > more prevalent the issue becomes one of increasing
> memory
> > > > > > > footprint when
> > > > > > > > > > using such a scheme, due to the need to have all
> arrays
> > > > > > > dimensioned for
> > > > > > > > > > all cores on the system, whether or not those cores
> are
> > > in
> > > > > use by
> > > > > > > the
> > > > > > > > > > app.
> > > > > > > > > >
> > > > > > > > > > Therefore, the decision was made in the past to not
> > > expand
> > > > > the
> > > > > > > > > > build-time RTE_MAX_LCORE value beyond 128. Instead,
> it
> > > was
> > > > > > > recommended
> > > > > > > > > > that users use the "--lcores" EAL parameter to take
> the
> > > high-
> > > > > > > numbered
> > > > > > > > > > cores they wish to use and map them to lcore-ids
> within
> > > the 0
> > > > > -
> > > > > > > 128
> > > > > > > > > > range. While this works, this is a little clunky as
> it
> > > means
> > > > > that
> > > > > > > > > > instead of just passing, for example, "-l 130-139",
> the
> > > user
> > > > > must
> > > > > > > > > > instead pass "--lcores 0@130,1@131,2@132,3@133,...."
> > > > > > > > > >
> > > > > > > > > > This patchset attempts to simplify the situation by
> > > adding a
> > > > > new
> > > > > > > flag to
> > > > > > > > > > do this mapping automatically. To use cores 130-139
> and
> > > map
> > > > > them
> > > > > > > to ids
> > > > > > > > > > 0-9 internally, the EAL args now become: "-l 130-139
> --
> > > map-
> > > > > lcore-
> > > > > > > ids",
> > > > > > > > > > or using the shorter "-M" version of the flag: "-Ml
> 130-
> > > 139".
> > > > > > > > > >
> > > > > > > > > > Adding this new parameter required some rework of the
> > > > > existing
> > > > > > > arg
> > > > > > > > > > parsing code, because in current DPDK the args are
> parsed
> > > and
> > > > > > > checked in
> > > > > > > > > > the order they appear on the commandline. This means
> that
> > > > > using
> > > > > > > the
> > > > > > > > > > example above, the core parameter 130-139 will be
> > > rejected
> > > > > > > immediately
> > > > > > > > > > before the "map-lcore-ids" parameter is seen. To work
> > > around
> > > > > > > this, the
> > > > > > > > > > core (and service core) parameters are not parsed
> when
> > > seen,
> > > > > > > instead
> > > > > > > > > > they are only saved off and parsed after all
> arguments
> > > are
> > > > > > > parsed. The
> > > > > > > > > > "-l" and "-c" parameters are converted into "--
> lcores"
> > > > > arguments,
> > > > > > > so all
> > > > > > > > > > assigning of lcore ids is done there in all cases.
> > > > > > > > > >
> > > > > > > > > > RFC->v2: * converted printf to DEBUG log * added "-M"
> as
> > > > > shorter
> > > > > > > > > > version of flag * added documentation * renamed
> internal
> > > API
> > > > > that
> > > > > > > > > > was changed to avoid any potential
> > > > > > > hidden
> > > > > > > > > > runtime issues.
> > > > > > > > > >
> > > > > > > > > > Bruce Richardson (3): eal: centralize core parameter
> > > parsing
> > > > > eal:
> > > > > > > > > > convert core masks and lists to core sets eal: allow
> > > > > automatic
> > > > > > > > > > mapping of high lcore ids
> > > > > > > > > >
> > > > > > > > > Ping for review.
> > > > > > > > >
> > > > > > > > > At a high level, does this feature seem useful to
> users?
> > > > > > > >
> > > > > > > > This seems useful, though I am not I would touch the
> existing
> > > > > > > options.
> > > > > > > > I would have gone with a simple -L option (taking the
> same
> > > kind
> > > > > of
> > > > > > > > input than -l but with new behavior), and not combine a
> flag
> > > with
> > > > > > > > existing options.
> > > > > > > >
> > > > > > >
> > > > > > > That would be an easier patchset to do up. However, it
> would
> > > then
> > > > > mean
> > > > > > > that we have no less than 4 different ways to specify the
> cores
> > > to
> > > > > use:
> > > > > > > "- c", "-l", "-L", "--lcores" - and therefore need 4
> different
> > > sets
> > > > > of
> > > > > > > parsing options for them, and more checks to ensure we have
> > > only
> > > > > one of
> > > > > > > the 4 specified in any run. That's why I chose the modifier
> > > option,
> > > > > and
> > > > > > > to try and consolidate the core setup a bit.
> > > > > > >
> > > > > > > However, if having a completely new option is preferred, I
> am
> > > happy
> > > > > > > enough to do up a different patchset for that.
> > > > > > >
> > > > > > > > I scanned through the series, not much to say. Maybe add
> a
> > > unit
> > > > > test
> > > > > > > > for new cmdline option.
> > > > > > > >
> > > > > > > Sure. Once it's decided what approach (if any) to take,
> I'll do
> > > up
> > > > > a
> > > > > > > new patchset, taking into account any relevant feedback on
> this
> > > > > set.
> > > > > > >
> > > > > > > /Bruce
> > > > > >
> > > > > > Changing the EAL parameter parser to a "two pass parser"
> makes
> > > sense.
> > > > > I
> > > > > > think checking for existence of more than one lcore
> specification
> > > > > options
> > > > > > should suffice; we don't need to accept multiple lcore
> > > specification
> > > > > > options and check for conflicts.
> > > > > >
> > > > > > When remapping, do we need to support gaps in the "lcore"
> > > (logical
> > > > > cores)
> > > > > > array, e.g. for secondary processes, or can it be continuous
> from
> > > 0
> > > > > to
> > > > > > the number of specified lcores?
> > > > > >
> > > > > > And are new EAL parameters for this really beneficial?
> Doesn't
> > > e.g.
> > > > > "-l
> > > > > > 0-9@130-139,100@140" suffice?
> > > > > >
> > > > > Actually, I believe "0-9@130-139"[1] is not the same as
> > > > > "0@130,1@131,2@132,...". The latter affinities one thread to
> one
> > > core
> > > > > ten
> > > > > times over, while the former affinitizes 10 threads to 10 cores
> -
> > > > > leaving
> > > > > those threads free to move about within the 10 cores specified.
> > > >
> > > > Interesting. The documentation [GSG] isn't clear to me about
> this; a
> > > example there could help clarify.
> > > >
> > > > [GSG]:
> > >
> https://doc.dpdk.org/guides/linux_gsg/linux_eal_parameters.html#lcore-
> > > related-options
> > > >
> > >
> > > Yep, agreed.
> > >
> > > > If users are manually passing lcore parameters to the EAL, then I
> see
> > > why some sort of remapping shorthand is useful.
> > > > IMO, if the mappings are relatively exotic, it should be
> acceptable
> > > requiring an external script to build a long list of mapping
> parameters
> > > and then invoke the application with those script-generated EAL
> > > parameters.
> > > > This would reduce the scope to support relatively simple, common
> > > mappings.
> > > >
> > > > Could we expand the --lcores syntax to support common mappings?
> > > >
> > > > E.g. "0-9@130+" to do what I thought.
> > > > The lack of "()" treats the entries individually (not as a
> group),
> > > and the "+" indicates auto-increment.
> > > >
> > > > A more advanced example:
> > > > "0-9@(130-131)+", meaning lcore 0 gets cpus 130-131, lcore 1 gets
> > > cpus 132-133, etc.
> > > >
> > >
> > > My issues with the above syntax idea is:
> > > * I think it's overly complicating the lcores parameter adding in
> the
> > > support for the "+" symbol - especially in the advanced case
> example
> > > you
> > > provide. I worry about code maintainability here.
> > > * More significantly for me, I think it's also getting things
> backwards
> > > in
> > > that it is focusing more on the lcore ids visible to the app,
> rather
> > > than
> > > the physical cores to be used. For the example above of 0-9@130+,
> > > what I
> > > would expect is that the user is mainly thinking about the cores
> he
> > > wants
> > > to use 130-139, which are then to be mapped to lower ids. If we
> had
> > > the
> > > syntax reversed where the physical cores were first, I'd say it
> would
> > > make more sense, e.g. 130-139@0+
> >
> > I 100 % agree on the syntax being backwards.
> > A good reason for introducing a new parameter, rather than expanding
> on "--lcores".
> >
>
> Yes and no.
> I agree with not expanding on --lcores, but I also don't think any new
> parameter added should attempt to reproduce all of what lcores does. I
> would leave --lcores as-is, as the power-tool for lcore config e.g.
> what
> you talk about below for mapping multiple lcores to the same physical
> cpu.
>
> > We should consider deprecating the old (backwards) syntax, so users
> don’t get confused about one EAL parameter being "backwards" of the
> other.
> >
>
> I disagree with this. I would be ok with deprecating the old "-c"
> coremask
> syntax - I think the time is long past when we should be dealing with
> masks. However, removing "-l" and "--lcores" flag is, to me anyway, too
> big
> and jarring a change for end users for us to consider.
>
> > > * finally, as I found out last month, there are systems on which
> the
> > > cores
> > > are spread across numa-nodes on odd/even boundaries, so to have
> an
> > > app
> > > running on socket 0, you need to give the core ids as a list i.e.
> > > 0,2,4,6, and cannot use ranges. [This also reenforces the point
> above
> > > too, where again it's the internal ids we need to generalize, not
> the
> > > physical cpus]
> >
> > For this, we could use "/2" (like in subnets), or "+2" as increment
> parameter.
> >
> > >
> > > My thinking on the matter, and I'm happy to be corrected if I'm
> wrong
> > > here,
> > > is that end-users are unlikely to significantly care what the
> actual
> > > lcore
> > > ids are internally in the program, so long as they work and are
> unique.
> >
> > Generally yes.
> > Note that we should allow the same physical CPU core to be assigned
> to multiple lcores...
> > If a CPU core is shared between multiple worker lcores, then it could
> be problematic, but with a mix of lcore roles, this might be handy for
> some applications. E.g. a virtual machine with only one CPU core could
> use that single CPU core as both main and worker lcore, or as main and
> service lcore.
> > Sharing an lcore between threads requires the developer to take
> special care of e.g. spinlocks, but the EAL parameter parser should not
> prohibit it. It might log a notice, though.
> >
>
> This is already taken care of via --lcores, so I don't see the need to
> reimplement it using a new flag also. Any new flags we add should be
> kept
> deliberately simple.
>
> > > What does matter is the physical cpus on which the code is to run.
> > > Therefore, my approach to this is to find the simplest possible
> > > solution
> > > whereby the user can just provide a list of cores and tell EAL to
> just
> > > map
> > > them to reasonable values. For the "reasonable" values, I would
> imagine
> > > that for the vast majority of cases starting "0" is what is wanted.
> For
> > > anything beyond that, we already have the existing --lcores syntax
> to
> > > be
> > > used.
> >
> > Agree with all of the above. :-)
> >
>
> Great. That still leaves us with the problem of what exactly to do as
> the
> best solution. Here are some alternatives that I see:
>
> 1. Add a modifier flag for -l and -c parameters to auto-remap the lcore
> ids
> to zero, so user is just specifying physical CPU id's.
> 2. Add a new flag for specifying physical cpu ids, which auto-remaps
> the
> cores specified to zero.
> 2a. To simplify our code and user experience we could at the same
> time:
> * deprecate "-c" flag for coremasks
> * make "-l" and "--lcores" the same flag just in long and short
> versions. This should not break anything as "-l" parameter is
> just as subset of what "--lcores" provides.
> * that would leave us with effectively two core flag paths:
> - -l/--lcores, behaviour as now, full explicit control
> - -L/--lcores-remapped, takes simplified core list (only ","
> and
> "-" supported as with "-l" now), and maps them to zero-
> based.
+1 for 2a.
>
> Third options? Any other feedback?
If the internal lcore ids are effectively hidden by passing physical CPU ids, any related options should take physical CPU ids too. I.e. the options for choosing main lcore and service lcores also need variants with other names.
The service cores are passed as a core mask; this should be deprecated (like "-c"). And superseded by a parameter taking a core list.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 0/3] allow easier use of high lcore-ids
2025-04-07 13:18 ` Morten Brørup
@ 2025-04-07 13:24 ` Bruce Richardson
0 siblings, 0 replies; 23+ messages in thread
From: Bruce Richardson @ 2025-04-07 13:24 UTC (permalink / raw)
To: Morten Brørup; +Cc: David Marchand, dev
On Mon, Apr 07, 2025 at 03:18:36PM +0200, Morten Brørup wrote:
> > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > Sent: Monday, 7 April 2025 14.42
> >
> > On Mon, Apr 07, 2025 at 02:25:46PM +0200, Morten Brørup wrote:
> > > > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > > > Sent: Monday, 7 April 2025 13.56
> > > >
> > > > On Mon, Apr 07, 2025 at 01:32:59PM +0200, Morten Brørup wrote:
> > > > > > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > > > > > Sent: Monday, 7 April 2025 12.41
> > > > > >
> > > > > > On Mon, Apr 07, 2025 at 12:15:13PM +0200, Morten Brørup wrote:
> > > > > > > > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > > > Sent:
> > > > > > > > Monday, 7 April 2025 11.49
> > > > > > > >
> > > > > > > > On Mon, Apr 07, 2025 at 09:04:05AM +0200, David Marchand
> > wrote:
> > > > > > > > > Hello Bruce,
> > > > > > > > >
> > > > > > > > > On Tue, Apr 1, 2025 at 4:08 PM Bruce Richardson
> > > > > > > > > <bruce.richardson@intel.com> wrote:
> > > > > > > > > >
> > > > > > > > > > On Mon, Mar 24, 2025 at 05:30:26PM +0000, Bruce
> > Richardson
> > > > > > wrote:
> > > > > > > > > > > Traditionally, DPDK has had a direct mapping of
> > internal
> > > > > > lcore-
> > > > > > > > ids, to
> > > > > > > > > > > the actual core numbers in use. With higher core
> > count
> > > > > > servers
> > > > > > > > becoming
> > > > > > > > > > > more prevalent the issue becomes one of increasing
> > memory
> > > > > > > > footprint when
> > > > > > > > > > > using such a scheme, due to the need to have all
> > arrays
> > > > > > > > dimensioned for
> > > > > > > > > > > all cores on the system, whether or not those cores
> > are
> > > > in
> > > > > > use by
> > > > > > > > the
> > > > > > > > > > > app.
> > > > > > > > > > >
> > > > > > > > > > > Therefore, the decision was made in the past to not
> > > > expand
> > > > > > the
> > > > > > > > > > > build-time RTE_MAX_LCORE value beyond 128. Instead,
> > it
> > > > was
> > > > > > > > recommended
> > > > > > > > > > > that users use the "--lcores" EAL parameter to take
> > the
> > > > high-
> > > > > > > > numbered
> > > > > > > > > > > cores they wish to use and map them to lcore-ids
> > within
> > > > the 0
> > > > > > -
> > > > > > > > 128
> > > > > > > > > > > range. While this works, this is a little clunky as
> > it
> > > > means
> > > > > > that
> > > > > > > > > > > instead of just passing, for example, "-l 130-139",
> > the
> > > > user
> > > > > > must
> > > > > > > > > > > instead pass "--lcores 0@130,1@131,2@132,3@133,...."
> > > > > > > > > > >
> > > > > > > > > > > This patchset attempts to simplify the situation by
> > > > adding a
> > > > > > new
> > > > > > > > flag to
> > > > > > > > > > > do this mapping automatically. To use cores 130-139
> > and
> > > > map
> > > > > > them
> > > > > > > > to ids
> > > > > > > > > > > 0-9 internally, the EAL args now become: "-l 130-139
> > --
> > > > map-
> > > > > > lcore-
> > > > > > > > ids",
> > > > > > > > > > > or using the shorter "-M" version of the flag: "-Ml
> > 130-
> > > > 139".
> > > > > > > > > > >
> > > > > > > > > > > Adding this new parameter required some rework of the
> > > > > > existing
> > > > > > > > arg
> > > > > > > > > > > parsing code, because in current DPDK the args are
> > parsed
> > > > and
> > > > > > > > checked in
> > > > > > > > > > > the order they appear on the commandline. This means
> > that
> > > > > > using
> > > > > > > > the
> > > > > > > > > > > example above, the core parameter 130-139 will be
> > > > rejected
> > > > > > > > immediately
> > > > > > > > > > > before the "map-lcore-ids" parameter is seen. To work
> > > > around
> > > > > > > > this, the
> > > > > > > > > > > core (and service core) parameters are not parsed
> > when
> > > > seen,
> > > > > > > > instead
> > > > > > > > > > > they are only saved off and parsed after all
> > arguments
> > > > are
> > > > > > > > parsed. The
> > > > > > > > > > > "-l" and "-c" parameters are converted into "--
> > lcores"
> > > > > > arguments,
> > > > > > > > so all
> > > > > > > > > > > assigning of lcore ids is done there in all cases.
> > > > > > > > > > >
> > > > > > > > > > > RFC->v2: * converted printf to DEBUG log * added "-M"
> > as
> > > > > > shorter
> > > > > > > > > > > version of flag * added documentation * renamed
> > internal
> > > > API
> > > > > > that
> > > > > > > > > > > was changed to avoid any potential
> > > > > > > > hidden
> > > > > > > > > > > runtime issues.
> > > > > > > > > > >
> > > > > > > > > > > Bruce Richardson (3): eal: centralize core parameter
> > > > parsing
> > > > > > eal:
> > > > > > > > > > > convert core masks and lists to core sets eal: allow
> > > > > > automatic
> > > > > > > > > > > mapping of high lcore ids
> > > > > > > > > > >
> > > > > > > > > > Ping for review.
> > > > > > > > > >
> > > > > > > > > > At a high level, does this feature seem useful to
> > users?
> > > > > > > > >
> > > > > > > > > This seems useful, though I am not I would touch the
> > existing
> > > > > > > > options.
> > > > > > > > > I would have gone with a simple -L option (taking the
> > same
> > > > kind
> > > > > > of
> > > > > > > > > input than -l but with new behavior), and not combine a
> > flag
> > > > with
> > > > > > > > > existing options.
> > > > > > > > >
> > > > > > > >
> > > > > > > > That would be an easier patchset to do up. However, it
> > would
> > > > then
> > > > > > mean
> > > > > > > > that we have no less than 4 different ways to specify the
> > cores
> > > > to
> > > > > > use:
> > > > > > > > "- c", "-l", "-L", "--lcores" - and therefore need 4
> > different
> > > > sets
> > > > > > of
> > > > > > > > parsing options for them, and more checks to ensure we have
> > > > only
> > > > > > one of
> > > > > > > > the 4 specified in any run. That's why I chose the modifier
> > > > option,
> > > > > > and
> > > > > > > > to try and consolidate the core setup a bit.
> > > > > > > >
> > > > > > > > However, if having a completely new option is preferred, I
> > am
> > > > happy
> > > > > > > > enough to do up a different patchset for that.
> > > > > > > >
> > > > > > > > > I scanned through the series, not much to say. Maybe add
> > a
> > > > unit
> > > > > > test
> > > > > > > > > for new cmdline option.
> > > > > > > > >
> > > > > > > > Sure. Once it's decided what approach (if any) to take,
> > I'll do
> > > > up
> > > > > > a
> > > > > > > > new patchset, taking into account any relevant feedback on
> > this
> > > > > > set.
> > > > > > > >
> > > > > > > > /Bruce
> > > > > > >
> > > > > > > Changing the EAL parameter parser to a "two pass parser"
> > makes
> > > > sense.
> > > > > > I
> > > > > > > think checking for existence of more than one lcore
> > specification
> > > > > > options
> > > > > > > should suffice; we don't need to accept multiple lcore
> > > > specification
> > > > > > > options and check for conflicts.
> > > > > > >
> > > > > > > When remapping, do we need to support gaps in the "lcore"
> > > > (logical
> > > > > > cores)
> > > > > > > array, e.g. for secondary processes, or can it be continuous
> > from
> > > > 0
> > > > > > to
> > > > > > > the number of specified lcores?
> > > > > > >
> > > > > > > And are new EAL parameters for this really beneficial?
> > Doesn't
> > > > e.g.
> > > > > > "-l
> > > > > > > 0-9@130-139,100@140" suffice?
> > > > > > >
> > > > > > Actually, I believe "0-9@130-139"[1] is not the same as
> > > > > > "0@130,1@131,2@132,...". The latter affinities one thread to
> > one
> > > > core
> > > > > > ten
> > > > > > times over, while the former affinitizes 10 threads to 10 cores
> > -
> > > > > > leaving
> > > > > > those threads free to move about within the 10 cores specified.
> > > > >
> > > > > Interesting. The documentation [GSG] isn't clear to me about
> > this; a
> > > > example there could help clarify.
> > > > >
> > > > > [GSG]:
> > > >
> > https://doc.dpdk.org/guides/linux_gsg/linux_eal_parameters.html#lcore-
> > > > related-options
> > > > >
> > > >
> > > > Yep, agreed.
> > > >
> > > > > If users are manually passing lcore parameters to the EAL, then I
> > see
> > > > why some sort of remapping shorthand is useful.
> > > > > IMO, if the mappings are relatively exotic, it should be
> > acceptable
> > > > requiring an external script to build a long list of mapping
> > parameters
> > > > and then invoke the application with those script-generated EAL
> > > > parameters.
> > > > > This would reduce the scope to support relatively simple, common
> > > > mappings.
> > > > >
> > > > > Could we expand the --lcores syntax to support common mappings?
> > > > >
> > > > > E.g. "0-9@130+" to do what I thought.
> > > > > The lack of "()" treats the entries individually (not as a
> > group),
> > > > and the "+" indicates auto-increment.
> > > > >
> > > > > A more advanced example:
> > > > > "0-9@(130-131)+", meaning lcore 0 gets cpus 130-131, lcore 1 gets
> > > > cpus 132-133, etc.
> > > > >
> > > >
> > > > My issues with the above syntax idea is:
> > > > * I think it's overly complicating the lcores parameter adding in
> > the
> > > > support for the "+" symbol - especially in the advanced case
> > example
> > > > you
> > > > provide. I worry about code maintainability here.
> > > > * More significantly for me, I think it's also getting things
> > backwards
> > > > in
> > > > that it is focusing more on the lcore ids visible to the app,
> > rather
> > > > than
> > > > the physical cores to be used. For the example above of 0-9@130+,
> > > > what I
> > > > would expect is that the user is mainly thinking about the cores
> > he
> > > > wants
> > > > to use 130-139, which are then to be mapped to lower ids. If we
> > had
> > > > the
> > > > syntax reversed where the physical cores were first, I'd say it
> > would
> > > > make more sense, e.g. 130-139@0+
> > >
> > > I 100 % agree on the syntax being backwards.
> > > A good reason for introducing a new parameter, rather than expanding
> > on "--lcores".
> > >
> >
> > Yes and no.
> > I agree with not expanding on --lcores, but I also don't think any new
> > parameter added should attempt to reproduce all of what lcores does. I
> > would leave --lcores as-is, as the power-tool for lcore config e.g.
> > what
> > you talk about below for mapping multiple lcores to the same physical
> > cpu.
> >
> > > We should consider deprecating the old (backwards) syntax, so users
> > don’t get confused about one EAL parameter being "backwards" of the
> > other.
> > >
> >
> > I disagree with this. I would be ok with deprecating the old "-c"
> > coremask
> > syntax - I think the time is long past when we should be dealing with
> > masks. However, removing "-l" and "--lcores" flag is, to me anyway, too
> > big
> > and jarring a change for end users for us to consider.
> >
> > > > * finally, as I found out last month, there are systems on which
> > the
> > > > cores
> > > > are spread across numa-nodes on odd/even boundaries, so to have
> > an
> > > > app
> > > > running on socket 0, you need to give the core ids as a list i.e.
> > > > 0,2,4,6, and cannot use ranges. [This also reenforces the point
> > above
> > > > too, where again it's the internal ids we need to generalize, not
> > the
> > > > physical cpus]
> > >
> > > For this, we could use "/2" (like in subnets), or "+2" as increment
> > parameter.
> > >
> > > >
> > > > My thinking on the matter, and I'm happy to be corrected if I'm
> > wrong
> > > > here,
> > > > is that end-users are unlikely to significantly care what the
> > actual
> > > > lcore
> > > > ids are internally in the program, so long as they work and are
> > unique.
> > >
> > > Generally yes.
> > > Note that we should allow the same physical CPU core to be assigned
> > to multiple lcores...
> > > If a CPU core is shared between multiple worker lcores, then it could
> > be problematic, but with a mix of lcore roles, this might be handy for
> > some applications. E.g. a virtual machine with only one CPU core could
> > use that single CPU core as both main and worker lcore, or as main and
> > service lcore.
> > > Sharing an lcore between threads requires the developer to take
> > special care of e.g. spinlocks, but the EAL parameter parser should not
> > prohibit it. It might log a notice, though.
> > >
> >
> > This is already taken care of via --lcores, so I don't see the need to
> > reimplement it using a new flag also. Any new flags we add should be
> > kept
> > deliberately simple.
> >
> > > > What does matter is the physical cpus on which the code is to run.
> > > > Therefore, my approach to this is to find the simplest possible
> > > > solution
> > > > whereby the user can just provide a list of cores and tell EAL to
> > just
> > > > map
> > > > them to reasonable values. For the "reasonable" values, I would
> > imagine
> > > > that for the vast majority of cases starting "0" is what is wanted.
> > For
> > > > anything beyond that, we already have the existing --lcores syntax
> > to
> > > > be
> > > > used.
> > >
> > > Agree with all of the above. :-)
> > >
> >
> > Great. That still leaves us with the problem of what exactly to do as
> > the
> > best solution. Here are some alternatives that I see:
> >
> > 1. Add a modifier flag for -l and -c parameters to auto-remap the lcore
> > ids
> > to zero, so user is just specifying physical CPU id's.
> > 2. Add a new flag for specifying physical cpu ids, which auto-remaps
> > the
> > cores specified to zero.
> > 2a. To simplify our code and user experience we could at the same
> > time:
> > * deprecate "-c" flag for coremasks
> > * make "-l" and "--lcores" the same flag just in long and short
> > versions. This should not break anything as "-l" parameter is
> > just as subset of what "--lcores" provides.
> > * that would leave us with effectively two core flag paths:
> > - -l/--lcores, behaviour as now, full explicit control
> > - -L/--lcores-remapped, takes simplified core list (only ","
> > and
> > "-" supported as with "-l" now), and maps them to zero-
> > based.
>
> +1 for 2a.
>
> >
> > Third options? Any other feedback?
>
> If the internal lcore ids are effectively hidden by passing physical CPU ids, any related options should take physical CPU ids too. I.e. the options for choosing main lcore and service lcores also need variants with other names.
Yes, I suppose so. That may be tricky, but we'll have to see what we can
do.
> The service cores are passed as a core mask; this should be deprecated (like "-c"). And superseded by a parameter taking a core list.
>
Yes to deprecate -s. I believe -S already exists to take a service core
list, so that can be marked off the list.
/Bruce
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 0/3] allow easier use of high lcore-ids
2025-04-07 10:15 ` Morten Brørup
2025-04-07 10:40 ` Bruce Richardson
@ 2025-04-07 15:14 ` Stephen Hemminger
2025-04-07 15:38 ` Bruce Richardson
1 sibling, 1 reply; 23+ messages in thread
From: Stephen Hemminger @ 2025-04-07 15:14 UTC (permalink / raw)
To: Morten Brørup; +Cc: Bruce Richardson, David Marchand, dev
On Mon, 7 Apr 2025 12:15:13 +0200
Morten Brørup <mb@smartsharesystems.com> wrote:
> > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > Sent: Monday, 7 April 2025 11.49
> >
> > On Mon, Apr 07, 2025 at 09:04:05AM +0200, David Marchand wrote:
> > > Hello Bruce,
> > >
> > > On Tue, Apr 1, 2025 at 4:08 PM Bruce Richardson
> > > <bruce.richardson@intel.com> wrote:
> > > >
> > > > On Mon, Mar 24, 2025 at 05:30:26PM +0000, Bruce Richardson wrote:
> > > > > Traditionally, DPDK has had a direct mapping of internal lcore-
> > ids, to
> > > > > the actual core numbers in use. With higher core count servers
> > becoming
> > > > > more prevalent the issue becomes one of increasing memory
> > footprint when
> > > > > using such a scheme, due to the need to have all arrays
> > dimensioned for
> > > > > all cores on the system, whether or not those cores are in use by
> > the
> > > > > app.
> > > > >
> > > > > Therefore, the decision was made in the past to not expand the
> > > > > build-time RTE_MAX_LCORE value beyond 128. Instead, it was
> > recommended
> > > > > that users use the "--lcores" EAL parameter to take the high-
> > numbered
> > > > > cores they wish to use and map them to lcore-ids within the 0 -
> > 128
> > > > > range. While this works, this is a little clunky as it means that
> > > > > instead of just passing, for example, "-l 130-139", the user must
> > > > > instead pass "--lcores 0@130,1@131,2@132,3@133,...."
> > > > >
> > > > > This patchset attempts to simplify the situation by adding a new
> > flag to
> > > > > do this mapping automatically. To use cores 130-139 and map them
> > to ids
> > > > > 0-9 internally, the EAL args now become: "-l 130-139 --map-lcore-
> > ids",
> > > > > or using the shorter "-M" version of the flag: "-Ml 130-139".
> > > > >
> > > > > Adding this new parameter required some rework of the existing
> > arg
> > > > > parsing code, because in current DPDK the args are parsed and
> > checked in
> > > > > the order they appear on the commandline. This means that using
> > the
> > > > > example above, the core parameter 130-139 will be rejected
> > immediately
> > > > > before the "map-lcore-ids" parameter is seen. To work around
> > this, the
> > > > > core (and service core) parameters are not parsed when seen,
> > instead
> > > > > they are only saved off and parsed after all arguments are
> > parsed. The
> > > > > "-l" and "-c" parameters are converted into "--lcores" arguments,
> > so all
> > > > > assigning of lcore ids is done there in all cases.
> > > > >
> > > > > RFC->v2:
> > > > > * converted printf to DEBUG log
> > > > > * added "-M" as shorter version of flag
> > > > > * added documentation
> > > > > * renamed internal API that was changed to avoid any potential
> > hidden
> > > > > runtime issues.
> > > > >
> > > > > Bruce Richardson (3):
> > > > > eal: centralize core parameter parsing
> > > > > eal: convert core masks and lists to core sets
> > > > > eal: allow automatic mapping of high lcore ids
> > > > >
> > > > Ping for review.
> > > >
> > > > At a high level, does this feature seem useful to users?
> > >
> > > This seems useful, though I am not I would touch the existing
> > options.
> > > I would have gone with a simple -L option (taking the same kind of
> > > input than -l but with new behavior), and not combine a flag with
> > > existing options.
> > >
> >
> > That would be an easier patchset to do up. However, it would then mean
> > that
> > we have no less than 4 different ways to specify the cores to use: "-
> > c",
> > "-l", "-L", "--lcores" - and therefore need 4 different sets of parsing
> > options for them, and more checks to ensure we have only one of the 4
> > specified in any run. That's why I chose the modifier option, and to
> > try
> > and consolidate the core setup a bit.
> >
> > However, if having a completely new option is preferred, I am happy
> > enough
> > to do up a different patchset for that.
> >
> > > I scanned through the series, not much to say.
> > > Maybe add a unit test for new cmdline option.
> > >
> > Sure. Once it's decided what approach (if any) to take, I'll do up a
> > new
> > patchset, taking into account any relevant feedback on this set.
> >
> > /Bruce
>
> Changing the EAL parameter parser to a "two pass parser" makes sense.
> I think checking for existence of more than one lcore specification options should suffice; we don't need to accept multiple lcore specification options and check for conflicts.
There already is a first pass to catch log parameters, could the offset arg be handled there?
> When remapping, do we need to support gaps in the "lcore" (logical cores) array, e.g. for secondary processes, or can it be continuous from 0 to the number of specified lcores?
>
> And are new EAL parameters for this really beneficial?
> Doesn't e.g. "-l 0-9@130-139,100@140" suffice?
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 0/3] allow easier use of high lcore-ids
2025-04-07 15:14 ` Stephen Hemminger
@ 2025-04-07 15:38 ` Bruce Richardson
0 siblings, 0 replies; 23+ messages in thread
From: Bruce Richardson @ 2025-04-07 15:38 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Morten Brørup, David Marchand, dev
On Mon, Apr 07, 2025 at 08:14:50AM -0700, Stephen Hemminger wrote:
> On Mon, 7 Apr 2025 12:15:13 +0200
> Morten Brørup <mb@smartsharesystems.com> wrote:
>
> > > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > > Sent: Monday, 7 April 2025 11.49
> > >
> > > On Mon, Apr 07, 2025 at 09:04:05AM +0200, David Marchand wrote:
> > > > Hello Bruce,
> > > >
> > > > On Tue, Apr 1, 2025 at 4:08 PM Bruce Richardson
> > > > <bruce.richardson@intel.com> wrote:
> > > > >
> > > > > On Mon, Mar 24, 2025 at 05:30:26PM +0000, Bruce Richardson wrote:
> > > > > > Traditionally, DPDK has had a direct mapping of internal lcore-
> > > ids, to
> > > > > > the actual core numbers in use. With higher core count servers
> > > becoming
> > > > > > more prevalent the issue becomes one of increasing memory
> > > footprint when
> > > > > > using such a scheme, due to the need to have all arrays
> > > dimensioned for
> > > > > > all cores on the system, whether or not those cores are in use by
> > > the
> > > > > > app.
> > > > > >
> > > > > > Therefore, the decision was made in the past to not expand the
> > > > > > build-time RTE_MAX_LCORE value beyond 128. Instead, it was
> > > recommended
> > > > > > that users use the "--lcores" EAL parameter to take the high-
> > > numbered
> > > > > > cores they wish to use and map them to lcore-ids within the 0 -
> > > 128
> > > > > > range. While this works, this is a little clunky as it means that
> > > > > > instead of just passing, for example, "-l 130-139", the user must
> > > > > > instead pass "--lcores 0@130,1@131,2@132,3@133,...."
> > > > > >
> > > > > > This patchset attempts to simplify the situation by adding a new
> > > flag to
> > > > > > do this mapping automatically. To use cores 130-139 and map them
> > > to ids
> > > > > > 0-9 internally, the EAL args now become: "-l 130-139 --map-lcore-
> > > ids",
> > > > > > or using the shorter "-M" version of the flag: "-Ml 130-139".
> > > > > >
> > > > > > Adding this new parameter required some rework of the existing
> > > arg
> > > > > > parsing code, because in current DPDK the args are parsed and
> > > checked in
> > > > > > the order they appear on the commandline. This means that using
> > > the
> > > > > > example above, the core parameter 130-139 will be rejected
> > > immediately
> > > > > > before the "map-lcore-ids" parameter is seen. To work around
> > > this, the
> > > > > > core (and service core) parameters are not parsed when seen,
> > > instead
> > > > > > they are only saved off and parsed after all arguments are
> > > parsed. The
> > > > > > "-l" and "-c" parameters are converted into "--lcores" arguments,
> > > so all
> > > > > > assigning of lcore ids is done there in all cases.
> > > > > >
> > > > > > RFC->v2:
> > > > > > * converted printf to DEBUG log
> > > > > > * added "-M" as shorter version of flag
> > > > > > * added documentation
> > > > > > * renamed internal API that was changed to avoid any potential
> > > hidden
> > > > > > runtime issues.
> > > > > >
> > > > > > Bruce Richardson (3):
> > > > > > eal: centralize core parameter parsing
> > > > > > eal: convert core masks and lists to core sets
> > > > > > eal: allow automatic mapping of high lcore ids
> > > > > >
> > > > > Ping for review.
> > > > >
> > > > > At a high level, does this feature seem useful to users?
> > > >
> > > > This seems useful, though I am not I would touch the existing
> > > options.
> > > > I would have gone with a simple -L option (taking the same kind of
> > > > input than -l but with new behavior), and not combine a flag with
> > > > existing options.
> > > >
> > >
> > > That would be an easier patchset to do up. However, it would then mean
> > > that
> > > we have no less than 4 different ways to specify the cores to use: "-
> > > c",
> > > "-l", "-L", "--lcores" - and therefore need 4 different sets of parsing
> > > options for them, and more checks to ensure we have only one of the 4
> > > specified in any run. That's why I chose the modifier option, and to
> > > try
> > > and consolidate the core setup a bit.
> > >
> > > However, if having a completely new option is preferred, I am happy
> > > enough
> > > to do up a different patchset for that.
> > >
> > > > I scanned through the series, not much to say.
> > > > Maybe add a unit test for new cmdline option.
> > > >
> > > Sure. Once it's decided what approach (if any) to take, I'll do up a
> > > new
> > > patchset, taking into account any relevant feedback on this set.
> > >
> > > /Bruce
> >
> > Changing the EAL parameter parser to a "two pass parser" makes sense.
> > I think checking for existence of more than one lcore specification options should suffice; we don't need to accept multiple lcore specification options and check for conflicts.
>
> There already is a first pass to catch log parameters, could the offset arg be handled there?
>
It could, but I'd rather not get into further handling of args in a
two-pass setup. If we go that way, we might be better to do a completely
"delayed-parsing" setup, where we use getopt to put all arguments into a
structure with named pointers for each arg type. Thereafter we do the
actual processing of args from the structure itself, allowing us to do all
arg processing in a fixed/known order. Unfortunately, that would be a
significant change in how things are done.
Also, from the discussion on this thread, there seems to be some support
for having a completely new cmdline arg that takes the core list and always
remaps them, rather than using a modifier to existing args. Your opinions
on the relative benefits/drawbacks of the two approaches are welcome! :-)
/Bruce
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2025-04-07 15:39 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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
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).