On 11 Sep 2025, at 15:33, Kevin Traynor wrote:
> OVS currently uses other_config:dpdk-lcore-mask <coremask> directly
> in DPDK rte_eal_init() with '-c <coremask>' argument.
>
> '-c' argument is now deprecated from DPDK and will be removed in
> DPDK 25.11, so OVS will no longer be able to use the '-c <coremask>'
> argument.
>
> Convert dpdk-lcore-mask core mask to a core list that can be used with
> '--lcores' and add some tests.
>
> The core list is validated to prevent invalid cores being passed to
> DPDK rte_eal_init().
>
> Using the '--lcores' argument also adds compability for using a core
compability -> compatibility
> in the core mask that is greater than the max lcore, similar to commit
> fe53b478f86e ("dpdk: Fix main lcore on systems with many cores.")
>
> Signed-off-by: Kevin Traynor <[email protected]>
Hi Kevin, thanks for working on this series. Some small comments below, which,
if there are no other comments, could be applied during commit.
With changes applied on commit; Acked-by: Eelco Chaudron <[email protected]>
Cheers,
Eelco
> ---
> lib/dpdk.c | 78 +++++++++++++++++++++++++++++++++++++++++---
> tests/ovs-macros.at | 16 +++++++++
> tests/pmd.at | 15 ---------
> tests/system-dpdk.at | 72 ++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 162 insertions(+), 19 deletions(-)
>
> diff --git a/lib/dpdk.c b/lib/dpdk.c
> index a6f82dd8a..4df480fde 100644
> --- a/lib/dpdk.c
> +++ b/lib/dpdk.c
> @@ -65,4 +65,60 @@ args_contains(const struct svec *args, const char *value)
> }
>
> +static int
> +compare_core_ids(const void *a_, const void *b_)
> +{
> + const unsigned *a = a_;
> + const unsigned *b = b_;
> +
> + return *a < *b ? -1 : *a > *b;
> +}
> +
> +/* Converts a hexadecimal core mask to DPDK lcore list format.
> + * Returns NULL if no valid bits are set in the mask. */
> +static char *
> +cmask_to_lcore_list(const char *cmask)
> +{
> + struct ds ds = DS_EMPTY_INITIALIZER;
> + struct ovs_numa_dump *lcore_dump;
> + struct ovs_numa_info_core *core;
> + unsigned int *core_ids = NULL;
> + unsigned int num_cores;
> + unsigned int i = 0;
> +
> + lcore_dump = ovs_numa_dump_cores_with_cmask(cmask);
> +
> + num_cores = ovs_numa_dump_count(lcore_dump);
> + if (num_cores == 0) {
> + ovs_numa_dump_destroy(lcore_dump);
> + return NULL;
> + }
> + core_ids = xmalloc(num_cores * sizeof *core_ids);
> +
> + FOR_EACH_CORE_ON_DUMP (core, lcore_dump) {
> + core_ids[i++] = core->core_id;
> + }
> +
> + /* ovs_numa_dump uses a hash map, so sort by core id. */
> + qsort(core_ids, num_cores, sizeof *core_ids, compare_core_ids);
> +
> + if (num_cores > RTE_MAX_LCORE) {
> + VLOG_INFO("Truncating DPDK lcore list as dpdk-lcore-mask has >"
> + " %u lcores", RTE_MAX_LCORE);
> + num_cores = RTE_MAX_LCORE;
> + }
> +
> + for (i = 0; i < num_cores; i++) {
> + ds_put_format(&ds, "%d@%u,", i, core_ids[i]);
This should probably %u instead of %d, as i is an unsigned int?
> + }
> + if (ds.length > 0) {
> + /* Remove last comma. */
> + ds_truncate(&ds, ds.length - 1);
> + }
nit: Maybe use ds_chomp(&ds, ‘,’) to avoid the addition if {}
> +
> + ovs_numa_dump_destroy(lcore_dump);
> + free(core_ids);
> + return ds_steal_cstr(&ds);
> +}
> +
> static void
> construct_dpdk_options(const struct smap *ovs_other_config, struct svec
> *args)
> @@ -71,8 +127,9 @@ construct_dpdk_options(const struct smap
> *ovs_other_config, struct svec *args)
> const char *ovs_configuration;
> const char *dpdk_option;
> + char *(*param_conversion) (const char *);
> } opts[] = {
> - {"dpdk-lcore-mask", "-c" },
> - {"dpdk-hugepage-dir", "--huge-dir" },
> - {"dpdk-socket-limit", "--socket-limit"},
> + {"dpdk-lcore-mask", "--lcores", cmask_to_lcore_list},
> + {"dpdk-hugepage-dir", "--huge-dir", NULL},
> + {"dpdk-socket-limit", "--socket-limit", NULL},
> };
>
> @@ -86,9 +143,22 @@ construct_dpdk_options(const struct smap
> *ovs_other_config, struct svec *args)
> if (value) {
> if (!args_contains(args, opts[i].dpdk_option)) {
> + char *dpdk_val = NULL;
> +
> + if (opts[i].param_conversion) {
> + dpdk_val = (opts[i].param_conversion)(value);
> + if (!dpdk_val) {
> + VLOG_WARN("Ignoring database defined option '%s'"
> + " due to invalid value '%s'",
> + opts[i].ovs_configuration, value);
> + continue;
> + }
> + value = dpdk_val;
> + }
> svec_add(args, opts[i].dpdk_option);
> svec_add(args, value);
> + free(dpdk_val);
> } else {
> VLOG_WARN("Ignoring database defined option '%s' due to "
> - "dpdk-extra config", opts[i].dpdk_option);
> + "dpdk-extra config", opts[i].ovs_configuration);
> }
> }
> diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at
> index 132a00541..c73312fd8 100644
> --- a/tests/ovs-macros.at
> +++ b/tests/ovs-macros.at
> @@ -417,2 +417,18 @@ m4_define([XT6_ACCEPT],
> IPTABLES_ACCEPT([ip6tables], [$1])
> fi])
> +
> +dnl CHECK_CPU_DISCOVERED([n_cpu])
> +dnl
> +dnl Waits until CPUs are discovered and checks if the number of discovered
> CPUs
> +dnl is greater or equal to 'n_cpu'. Without the 'n_cpu' parameter checks
> that
> +dnl at least one CPU is discovered.
> +m4_define([CHECK_CPU_DISCOVERED], [
> + PATTERN="Discovered [[0-9]]* NUMA nodes and [[0-9]]* CPU cores"
> + OVS_WAIT_UNTIL([grep "$PATTERN" ovs-vswitchd.log])
> + N_CPU=$(grep "$PATTERN" ovs-vswitchd.log \
> + | sed -e 's/.* \([[0-9]]*\) CPU cores/\1/')
> + if [[ -z "$1" ]]
> + then AT_CHECK([test "$N_CPU" -gt "0"])
> + else AT_SKIP_IF([test "$N_CPU" -lt "$1"])
> + fi
> +])
> diff --git a/tests/pmd.at b/tests/pmd.at
> index 35a44b4df..8254ac3b0 100644
> --- a/tests/pmd.at
> +++ b/tests/pmd.at
> @@ -27,19 +27,4 @@ flow_dump_prepend_pmd () {
> m4_divert_pop([PREPARE_TESTS])
>
> -dnl CHECK_CPU_DISCOVERED([n_cpu])
> -dnl
> -dnl Waits until CPUs discovered and checks if number of discovered CPUs
> -dnl is greater or equal to 'n_cpu'. Without parameters checks that at
> -dnl least one CPU discovered.
> -m4_define([CHECK_CPU_DISCOVERED], [
> - PATTERN="Discovered [[0-9]]* NUMA nodes and [[0-9]]* CPU cores"
> - OVS_WAIT_UNTIL([grep "$PATTERN" ovs-vswitchd.log])
> - N_CPU=$(grep "$PATTERN" ovs-vswitchd.log | sed -e 's/.* \([[0-9]]*\) CPU
> cores/\1/')
> - if [[ -z "$1" ]]
> - then AT_CHECK([test "$N_CPU" -gt "0"])
> - else AT_SKIP_IF([test "$N_CPU" -lt "$1"])
> - fi
> -])
> -
> dnl CHECK_PMD_THREADS_CREATED([n_threads], [numa_id], [+line])
> dnl
> diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at
> index e79c75565..393b76c76 100644
> --- a/tests/system-dpdk.at
> +++ b/tests/system-dpdk.at
> @@ -54,4 +54,76 @@ dnl
> --------------------------------------------------------------------------
>
>
> +dnl
> --------------------------------------------------------------------------
> +dnl Check dpdk-lcore-mask conversion for only first bit
> +AT_SETUP([OVS-DPDK - dpdk-lcore-mask conversion - single])
> +AT_KEYWORDS([dpdk])
> +OVS_DPDK_PRE_CHECK()
> +OVS_DPDK_START_OVSDB()
> +OVS_DPDK_START_VSWITCHD([--no-pci])
> +CHECK_CPU_DISCOVERED()
> +AT_CHECK([ovs-vsctl --no-wait set Open_vSwitch .
> other_config:dpdk-lcore-mask=0x1])
> +AT_CHECK([ovs-vsctl --no-wait set Open_vSwitch .
> other_config:dpdk-init=true])
> +expected_lcores="lcores 0@0"
> +OVS_WAIT_UNTIL([grep "$expected_lcores" ovs-vswitchd.log])
> +OVS_DPDK_STOP_VSWITCHD
> +AT_CLEANUP
> +dnl
> --------------------------------------------------------------------------
> +
> +
> +
> +dnl
> --------------------------------------------------------------------------
> +dnl Check dpdk-lcore-mask conversion for only multiple bits
> +AT_SETUP([OVS-DPDK - dpdk-lcore-mask conversion - multi])
> +AT_KEYWORDS([dpdk])
> +OVS_DPDK_PRE_CHECK()
> +OVS_DPDK_START_OVSDB()
> +OVS_DPDK_START_VSWITCHD([--no-pci])
> +CHECK_CPU_DISCOVERED(4)
> +AT_CHECK([ovs-vsctl --no-wait set Open_vSwitch .
> other_config:dpdk-lcore-mask=0xf])
> +AT_CHECK([ovs-vsctl --no-wait set Open_vSwitch .
> other_config:dpdk-init=true])
> +expected_lcores="lcores 0@0,1@1,2@2,3@3"
> +OVS_WAIT_UNTIL([grep "$expected_lcores" ovs-vswitchd.log])
> +OVS_DPDK_STOP_VSWITCHD
> +AT_CLEANUP
> +dnl
> --------------------------------------------------------------------------
> +
> +
> +
> +dnl
> --------------------------------------------------------------------------
> +dnl Check dpdk-lcore-mask conversion for max length string
> +AT_SETUP([OVS-DPDK - dpdk-lcore-mask conversion - non-contig])
> +AT_KEYWORDS([dpdk])
> +OVS_DPDK_PRE_CHECK()
> +OVS_DPDK_START_OVSDB()
> +OVS_DPDK_START_VSWITCHD([--no-pci])
> +CHECK_CPU_DISCOVERED(8)
> +AT_CHECK([ovs-vsctl --no-wait set Open_vSwitch .
> other_config:dpdk-lcore-mask=0xca])
> +AT_CHECK([ovs-vsctl --no-wait set Open_vSwitch .
> other_config:dpdk-init=true])
> +expected_lcores="lcores 0@1,1@3,2@6,3@7"
> +OVS_WAIT_UNTIL([grep "$expected_lcores" ovs-vswitchd.log])
> +OVS_DPDK_STOP_VSWITCHD
> +AT_CLEANUP
> +dnl
> --------------------------------------------------------------------------
> +
> +
> +
> +dnl
> --------------------------------------------------------------------------
> +dnl Check dpdk-lcore-mask conversion for zero
> +AT_SETUP([OVS-DPDK - dpdk-lcore-mask conversion - zeromask])
> +AT_KEYWORDS([dpdk])
> +OVS_DPDK_PRE_CHECK()
> +OVS_DPDK_START_OVSDB()
> +OVS_DPDK_START_VSWITCHD([--no-pci])
> +AT_CHECK([ovs-vsctl --no-wait set Open_vSwitch .
> other_config:dpdk-lcore-mask=0x0])
> +AT_CHECK([ovs-vsctl --no-wait set Open_vSwitch .
> other_config:dpdk-init=true])
> +OVS_WAIT_UNTIL([grep "Ignoring database defined option 'dpdk-lcore-mask' due
> to invalid value '0x0'" ovs-vswitchd.log])
> +expected_lcores="lcores 0@0"
> +OVS_WAIT_UNTIL([grep "$expected_lcores" ovs-vswitchd.log])
> +OVS_DPDK_STOP_VSWITCHD(["/Ignoring database defined option/d"])
> +AT_CLEANUP
> +dnl
> --------------------------------------------------------------------------
> +
> +
> +
> dnl
> --------------------------------------------------------------------------
> dnl Add standard DPDK PHY port
> --
> 2.51.0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
