On 7/9/21 2:43 PM, Kevin Traynor wrote: > On 09/07/2021 13:24, Ilya Maximets wrote: >> On 6/30/21 10:48 PM, Rosemarie O'Riorden wrote: >>> From: Rosemarie O'Riorden <rorio...@redhat.com> >>> >>> Currently, there is a default value of 1024 for socket-mem if not >>> configured. socket-limit automatically takes on the value of socket-mem >>> unless otherwise specified. With these changes, memory allocation will >>> be dynamically managed by DPDK, meaning that by default, no memory will >>> be pre-allocated on startup, and there will be no limit to how much >>> memory can be used. Either or both of these values can be set by the >>> user. >>> >>> The EAL arguments will look like this: >>> >>> - dpdk-socket-mem=<not set>, dpdk-socket-limit=<not set> >>> current: "--scket-mem=1024,1024 --socket-limit=1024,1024" >>> patch 1: "" >>> patch 2: "" >>> >>> - dpdk-socket-mem=<MEM>, dpdk-socket-limit=<not set> >>> current: "--scket-mem=MEM --socket-limit=MEM" >>> patch 1: "--scket-mem=MEM --socket-limit=MEM" >>> patch 2: "--scket-mem=MEM" >>> >>> - dpdk-socket-mem=<not set>, dpdk-socket-limit=<LIMIT> >>> current: "--scket-mem=1024,1024 --socket-limit=LIMIT" >>> patch 1: "--socket-limit=LIMIT" >>> patch 2: "--socket-limit=LIMIT" >>> >>> - dpdk-socket-mem=<MEM>, dpdk-socket-limit=<LIMIT> >>> current: "--scket-mem=MEM --socket-limit=LIMIT" >>> patch 1: "--scket-mem=MEM --socket-limit=LIMIT" >>> patch 2: "--scket-mem=MEM --socket-limit=LIMIT" >>> >>> Rosemarie O'Riorden (2): >>> dpdk: Remove default values for socket-mem and limit. >>> dpdk: Stop configuring socket-limit with the value of socket-mem. >>> >>> Documentation/intro/install/dpdk.rst | 3 +-- >>> NEWS | 4 ++++ >>> lib/dpdk.c | 6 +----- >>> vswitchd/vswitch.xml | 13 ++++++------- >>> 4 files changed, 12 insertions(+), 14 deletions(-) >>> >> >> Hi, Ian. On a last upsteam meeting you mentioned that we, probably, >> need to follow a deprecation procedure here and my initial reaction >> was that it's not necessary. However, thinking more about this, >> it seems that it will be a good thing to do. >> >> So, what I'm suggesting is to make a 3-patch series: >> >> 1. First patch adds INFO level messages to the log in case OVS adds >> default socket-mem or if it adds default socket-limit. >> Something like this: >> >> diff --git a/lib/dpdk.c b/lib/dpdk.c >> index 0c910092c..ecc630d20 100644 >> --- a/lib/dpdk.c >> +++ b/lib/dpdk.c >> @@ -217,6 +217,7 @@ 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) { >> @@ -233,6 +234,7 @@ 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; >> } >> @@ -245,6 +247,11 @@ 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 default for this argument starting from " >> + "2.17 release.", >> popt->eal_dpdk_options[found_pos]); > > > Maybe can add something like "DPDK defaults will be used instead." so > it's not interpreted that the user will have to provide something as an > alternative to the defaults OVS is removing?
Good point. I agree. Initially, I wanted to add something about dynamic memory model, but, technically, we already using it, so I didn't add anything. Your version makes much more sense. Thanks! > >> + } >> svec_add(args, popt->eal_dpdk_options[found_pos]); >> svec_add(args, found_value); >> } else { >> --- >> >> And something similar for --socket-limit. >> Logging at INFO level to avoid possible CI breakages for OVS and users. >> Warning should be added to the Documentation/intro/install/dpdk.rst >> and vswitch.xml. And the NEWS entry, of course. >> >> 2. Second patch removes extra logging and the default value for socket-mem. >> >> 3. Third one removes extra logging and the default value for socket-limit. >> >> First patch can be applied for 2.16, the rest of the set could be accepted >> right after the branching and will be part of 2.17 release. >> >> Does that sound good to you? >> > > Sounds like a good plan to me anyway. > >> In the meantime, I think, Rosemarie may start working on updated patches. >> >> Best regards, Ilya Maximets. >> > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev