Re: [ovs-dev] [PATCH v2 2/3] dpdk: Remove default values for socket-mem and limit.

2021-07-15 Thread Kevin Traynor
On 13/07/2021 20:15, 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 added in patch 1 that announce this change.
> 
> Reported at: https://bugzilla.redhat.com/show_bug.cgi?id=1949850
> Signed-off-by: Rosemarie O'Riorden 
> ---
> 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.
> 
>  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.

Fine for now, but the NEWS in 2 and 3 will need to rebase to post 2.16
changes when the time is right.

> - 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 ed57067ee..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(>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(_socket_mem, "%s", "0");
> -} else {
> -ds_put_format(_socket_mem, ",%s", "0");
> -}
> -last_node++;
> -}
> -if (numa_node_list[i] == 0) {
> -ds_put_format(_socket_mem, "%s", def_value);
> -} else {
> -ds_put_format(_socket_mem, ",%s", def_value);
> -}
> -last_node++;
> -}

This code had a short stay :'|

> -free(numa_node_list);
> -ovs_numa_dump_destroy(dump);
> -return ds_cstr(_socket_mem);
> -}
> -

Re: [ovs-dev] [PATCH v2 2/3] dpdk: Remove default values for socket-mem and limit.

2021-07-13 Thread 0-day Robot
Bleep bloop.  Greetings Rosemarie O'Riorden, I am a robot and I have tried out 
your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line has trailing whitespace
#201 FILE: vswitchd/vswitch.xml:367:
  DPDK defaults will be used instead. If dpdk-socket-mem and 

WARNING: Line is 81 characters long (recommended limit is 79)
#202 FILE: vswitchd/vswitch.xml:368:
  dpdk-alloc-mem are specified at same time, dpdk-socket-mem will be 
used

Lines checked: 220, Warnings: 2, Errors: 0


Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH v2 2/3] dpdk: Remove default values for socket-mem and limit.

2021-07-13 Thread Rosemarie O'Riorden
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 added in patch 1 that announce this change.

Reported at: https://bugzilla.redhat.com/show_bug.cgi?id=1949850
Signed-off-by: Rosemarie O'Riorden 
---
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.

 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 ed57067ee..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(>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(_socket_mem, "%s", "0");
-} else {
-ds_put_format(_socket_mem, ",%s", "0");
-}
-last_node++;
-}
-if (numa_node_list[i] == 0) {
-ds_put_format(_socket_mem, "%s", def_value);
-} else {
-ds_put_format(_socket_mem, ",%s", def_value);
-}
-last_node++;
-}
-free(numa_node_list);
-ovs_numa_dump_destroy(dump);
-return ds_cstr(_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