On 15/07/2021 22:37, Rosemarie O'Riorden wrote:
> This change removes the default values for EAL args socket-mem and
> socket-limit. As DPDK supports dynamic memory allocation, there is no
> need to allocate a certain amount of memory on start-up, nor limit the
> amount of memory available, if not requested.
>
> Currently, socket-mem has a default value of 1024 when it is not
> configured by the user, and socket-limit takes on the value of socket-mem,
> 1024, by default. With this change, socket-mem is not configured by default,
> meaning that socket-limit is not either. Neither, either or both options can
> be set.
>
> Removed extra logs that announce this change and fixed documentation.
>
> Reported at: https://bugzilla.redhat.com/show_bug.cgi?id=1949850
> Signed-off-by: Rosemarie O'Riorden <rorio...@redhat.com>
> ---
> Version 2:
> - Removes logs added in patch 1 that were not in v1.
> - Removes code added to lib/dpdk.c since v1 that conflicts with this patch
> series.
>
> Version 3:
> - Fixed reference to "patch 1" in commit message.
>
> Documentation/intro/install/dpdk.rst | 5 +-
> NEWS | 4 +-
> lib/dpdk.c | 74 +---------------------------
> vswitchd/vswitch.xml | 16 +++---
> 4 files changed, 11 insertions(+), 88 deletions(-)
>
> diff --git a/Documentation/intro/install/dpdk.rst
> b/Documentation/intro/install/dpdk.rst
> index d8fa931fa..96843af73 100644
> --- a/Documentation/intro/install/dpdk.rst
> +++ b/Documentation/intro/install/dpdk.rst
> @@ -290,9 +290,8 @@ listed below. Defaults will be provided for all values
> not explicitly set.
>
> ``dpdk-socket-mem``
> Comma separated list of memory to pre-allocate from hugepages on specific
> - sockets. If not specified, 1024 MB will be set for each numa node by
> - default. This behavior will change with the 2.17 release, with no default
> - value from OVS. Instead, DPDK default will be used.
> + sockets. If not specified, this option will not be set by default. DPDK
> + default will be used instead.
>
> ``dpdk-hugepage-dir``
> Directory where hugetlbfs is mounted
> diff --git a/NEWS b/NEWS
> index 126f5a927..948f68283 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -29,8 +29,8 @@ Post-v2.15.0
> Available only if DPDK experimantal APIs enabled during the build.
> * Add hardware offload support for VXLAN flows (experimental).
> Available only if DPDK experimantal APIs enabled during the build.
> - * EAL options --socket-mem and --socket-limit to have default values
> - removed with 2.17 release. Logging added to alert users.
> + * EAL option --socket-mem is no longer configured by default upon
> + start-up.
> - ovsdb-tool:
> * New option '--election-timer' to the 'create-cluster' command to set
> the
> leader election timer during cluster creation.
> diff --git a/lib/dpdk.c b/lib/dpdk.c
> index b70c01cf4..3a6990e2f 100644
> --- a/lib/dpdk.c
> +++ b/lib/dpdk.c
> @@ -130,74 +130,12 @@ construct_dpdk_options(const struct smap
> *ovs_other_config, struct svec *args)
> }
> }
>
> -static int
> -compare_numa_node_list(const void *a_, const void *b_)
> -{
> - int a = *(const int *) a_;
> - int b = *(const int *) b_;
> -
> - if (a < b) {
> - return -1;
> - }
> - if (a > b) {
> - return 1;
> - }
> - return 0;
> -}
> -
> -static char *
> -construct_dpdk_socket_mem(void)
> -{
> - const char *def_value = "1024";
> - struct ds dpdk_socket_mem = DS_EMPTY_INITIALIZER;
> -
> - /* Build a list of all numa nodes with at least one core. */
> - struct ovs_numa_dump *dump = ovs_numa_dump_n_cores_per_numa(1);
> - size_t n_numa_nodes = hmap_count(&dump->numas);
> - int *numa_node_list = xcalloc(n_numa_nodes, sizeof *numa_node_list);
> -
> - const struct ovs_numa_info_numa *node;
> - int k = 0, last_node = 0;
> -
> - FOR_EACH_NUMA_ON_DUMP(node, dump) {
> - if (k >= n_numa_nodes) {
> - break;
> - }
> - numa_node_list[k++] = node->numa_id;
> - }
> - qsort(numa_node_list, k, sizeof *numa_node_list, compare_numa_node_list);
> -
> - for (int i = 0; i < n_numa_nodes; i++) {
> - while (numa_node_list[i] > last_node &&
> - numa_node_list[i] != OVS_NUMA_UNSPEC &&
> - numa_node_list[i] <= MAX_NUMA_NODES) {
> - if (last_node == 0) {
> - ds_put_format(&dpdk_socket_mem, "%s", "0");
> - } else {
> - ds_put_format(&dpdk_socket_mem, ",%s", "0");
> - }
> - last_node++;
> - }
> - if (numa_node_list[i] == 0) {
> - ds_put_format(&dpdk_socket_mem, "%s", def_value);
> - } else {
> - ds_put_format(&dpdk_socket_mem, ",%s", def_value);
> - }
> - last_node++;
> - }
> - free(numa_node_list);
> - ovs_numa_dump_destroy(dump);
> - return ds_cstr(&dpdk_socket_mem);
> -}
> -
> #define MAX_DPDK_EXCL_OPTS 10
>
> static void
> construct_dpdk_mutex_options(const struct smap *ovs_other_config,
> struct svec *args)
> {
> - char *default_dpdk_socket_mem = construct_dpdk_socket_mem();
> -
> struct dpdk_exclusive_options_map {
> const char *category;
> const char *ovs_dpdk_options[MAX_DPDK_EXCL_OPTS];
> @@ -208,7 +146,7 @@ construct_dpdk_mutex_options(const struct smap
> *ovs_other_config,
> {"memory type",
> {"dpdk-alloc-mem", "dpdk-socket-mem", NULL,},
> {"-m", "--socket-mem", NULL,},
> - default_dpdk_socket_mem, 1
> + NULL, 0
> },
> };
>
> @@ -217,7 +155,6 @@ construct_dpdk_mutex_options(const struct smap
> *ovs_other_config,
> int found_opts = 0, scan, found_pos = -1;
> const char *found_value;
> struct dpdk_exclusive_options_map *popt = &excl_opts[i];
> - bool using_default = false;
>
> for (scan = 0; scan < MAX_DPDK_EXCL_OPTS
> && popt->ovs_dpdk_options[scan]; ++scan) {
> @@ -234,7 +171,6 @@ construct_dpdk_mutex_options(const struct smap
> *ovs_other_config,
> if (popt->default_option) {
> found_pos = popt->default_option;
> found_value = popt->default_value;
> - using_default = true;
> } else {
> continue;
> }
> @@ -247,12 +183,6 @@ construct_dpdk_mutex_options(const struct smap
> *ovs_other_config,
> }
>
> if (!args_contains(args, popt->eal_dpdk_options[found_pos])) {
> - if (using_default) {
> - VLOG_INFO("Using default value for '%s'. OVS will no longer "
> - "provide a default for this argument starting "
> - "from 2.17 release. DPDK defaults will be used "
> - "instead.", popt->eal_dpdk_options[found_pos]);
> - }
> svec_add(args, popt->eal_dpdk_options[found_pos]);
> svec_add(args, found_value);
> } else {
> @@ -260,8 +190,6 @@ construct_dpdk_mutex_options(const struct smap
> *ovs_other_config,
> "dpdk-extra config",
> popt->eal_dpdk_options[found_pos]);
> }
> }
> -
> - free(default_dpdk_socket_mem);
> }
>
> static void
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index c26ebb796..190e377d3 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -362,13 +362,11 @@
> preallocate 2048MB and socket 3 (no value given) to preallocate
> 0MB.
> </p>
> <p>
> - If dpdk-socket-mem and dpdk-alloc-mem are not specified,
> dpdk-socket-mem
> - will be used and the default value is 1024 for each numa node. If
> - dpdk-socket-mem and dpdk-alloc-mem are specified at same time,
> - dpdk-socket-mem will be used as default. With the 2.17 release,
> - dpdk-socket-mem will no longer be used by default. DPDK defaults
> will
> - be used instead.
> - Changing this value requires restarting the daemon.
> + If dpdk-socket-mem and dpdk-alloc-mem are not specified, neither
> + will be used and there will be no default value for each numa node.
> + DPDK defaults will be used instead. If dpdk-socket-mem and
^
checkpatch is fussy about whitespace and it caught an extra one here
$ ./utilities/checkpatch.py
0002-dpdk-Remove-default-values-for-socket-mem-and-limit.patch
== Checking
"0002-dpdk-Remove-default-values-for-socket-mem-and-limit.patch" ==
WARNING: Line has trailing whitespace
#178 FILE: vswitchd/vswitch.xml:367:
DPDK defaults will be used instead. If dpdk-socket-mem and
Lines checked: 194, Warnings: 1, Errors: 0
> + dpdk-alloc-mem are specified at same time, dpdk-socket-mem will be
> + used as default. Changing this value requires restarting the
> daemon.
> </p>
> </column>
>
> @@ -389,9 +387,7 @@
> <ref column="other_config" key="dpdk-extra"/>. If none of the above
> options specified or <code>--legacy-mem</code> provided in
> <ref column="other_config" key="dpdk-extra"/>, limits will not be
> - applied.
> - With the 2.17 release, the OVS default value will no longer be
> - provided, and DPDK defaults will be used instead.
> + applied. There is no default value from OVS.
> Changing this value requires restarting the daemon.
> </p>
> </column>
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
