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 <[email protected]>
>>>
>>> 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
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
