On 31.01.2019 17:17, Ilya Maximets wrote:
> On 31.01.2019 15:39, Ian Stokes wrote:
>> On 1/29/2019 8:11 AM, Ilya Maximets wrote:
>>> Since 18.05 release, DPDK moved to dynamic memory model in which
>>> hugepages could be allocated on demand. At the same time '--socket-mem'
>>> option was re-defined as a size of pre-allocated memory, i.e. memory
>>> that should be allocated at startup and could not be freed.
>>> So, DPDK with a new memory model could allocate more hugepage memory
>>> than specified in '--socket-mem' or '-m' options.
>>>
>>> This change adds new configurable 'other_config:dpdk-socket-limit'
>>> which could be used to limit the ammount of memory DPDK could use.
>>> It uses new DPDK option '--socket-limit'.
>>> Ex.:
>>>
>>> ovs-vsctl set Open_vSwitch . other_config:dpdk-socket-limit="1024,1024"
>>>
>>> Also, in order to preserve old behaviour, if '--socket-limit' is not
>>> specified, it will be defaulted to the amount of memory specified by
>>> '--socket-mem' option, i.e. OVS will not be able to allocate more.
>>> This is needed, for example, to disallow OVS to allocate more memory
>>> than reserved for it by Nova in OpenStack installations.
>>>
>>> Signed-off-by: Ilya Maximets <i.maxim...@samsung.com>
>>
>> Thanks for this Ilya. Given that this will affect 2.11 as well, it probably
>> makes sense to back port it (as well as the svec patch in this series) to
>> OVS 2.11 so as to ensure uniform memory behavior between 2.10 to 2.11.
>
> Yes. This probably makes sense.
>
>>
>> One comment below also.
>>
>>> ---
>>> NEWS | 3 +++
>>> lib/dpdk.c | 21 +++++++++++++++++++--
>>> vswitchd/vswitch.xml | 22 ++++++++++++++++++++++
>>> 3 files changed, 44 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/NEWS b/NEWS
>>> index 4985dbaac..a64b9d94d 100644
>>> --- a/NEWS
>>> +++ b/NEWS
>>> @@ -1,5 +1,8 @@
>>> Post-v2.11.0
>>> ---------------------
>>> + - DPDK:
>>> + * New option 'other_config:dpdk-socket-limit' to limit amount of
>>> + hugepage memory that can be used by DPDK.
>>> v2.11.0 - xx xxx xxxx
>>> diff --git a/lib/dpdk.c b/lib/dpdk.c
>>> index 0546191a6..53b74fba4 100644
>>> --- a/lib/dpdk.c
>>> +++ b/lib/dpdk.c
>>> @@ -100,8 +100,9 @@ construct_dpdk_options(const struct smap
>>> *ovs_other_config, struct svec *args)
>>> bool default_enabled;
>>> const char *default_value;
>>> } opts[] = {
>>> - {"dpdk-lcore-mask", "-c", false, NULL},
>>> - {"dpdk-hugepage-dir", "--huge-dir", false, NULL},
>>> + {"dpdk-lcore-mask", "-c", false, NULL},
>>> + {"dpdk-hugepage-dir", "--huge-dir", false, NULL},
>>> + {"dpdk-socket-limit", "--socket-limit", false, NULL},
>>> };
>>> int i;
>>> @@ -318,6 +319,22 @@ dpdk_init__(const struct smap *ovs_other_config)
>>> svec_add(&args, ovs_get_program_name());
>>> construct_dpdk_args(ovs_other_config, &args);
>>> + if (!args_contains(&args, "--legacy-mem")
>>> + && !args_contains(&args, "--socket-limit")) {
>>> + const char *arg;
>>> + size_t i;
>>> +
>>> + SVEC_FOR_EACH (i, arg, &args) {
>>> + if (!strcmp(arg, "--socket-mem")) {
>>> + break;
>>> + }
>>> + }
>>> + if (i < args.n - 1) {
>>> + svec_add(&args, "--socket-limit");
>>> + svec_add(&args, args.names[i + 1]);
>>> + }
>>> + }
>>> +
>>> if (args_contains(&args, "-c") || args_contains(&args, "-l")) {
>>> auto_determine = false;
>>> }
>>> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
>>> index 88edb5d35..1db4e8694 100644
>>> --- a/vswitchd/vswitch.xml
>>> +++ b/vswitchd/vswitch.xml
>>> @@ -333,6 +333,28 @@
>>> </p>
>>> </column>
>>> + <column name="other_config" key="dpdk-socket-limit"
>>> + type='{"type": "string"}'>
>>> + <p>
>>> + Limits the maximum amount of memory that can be used from the
>>> + hugepage pool, on a per-socket basis.
>>> + </p>
>>> + <p>
>>> + The specifier is a comma-separated list of memory limits per
>>> socket.
>>> + <code>0</code> will disable the limit for a particular socket.
>>> + </p>
>>> + <p>
>>> + If not specified, OVS will configure limits equal to the amount
>>> of
>>> + preallocated memory specified by <ref column="other_config"
>>> + key="dpdk-socket-mem"/> or <code>--socket-mem</code> in
>>> + <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.
>>> + Changing this value requires restarting the daemon.
>>> + </p>
>>> + </column>
>>> +
>>
>> As the number for memory configuration options grow in OVS DPDK, do you
>> think it makes sense to expand Documentation/topics/dpdk/memory.rst to
>> provide more details and usage regarding these options?
>
> I'm not sure. Some options already described in
> Documentation/intro/install/dpdk.rst,
> all the options described in man pages (vswitch.xml). Documentation becomes
> more and
> more fragmented.
>
>>
>> For instance when testing I spotted there's an eal error message for when
>> both --legacy and --socket-lim values are set. It's handled at the DPDK
>> level so I don't think we need to specifically check in OVS code but would
>> it be worth mentioning that dpdk-socket-limit will not be used if
>> --legacy-mem is also passed.
>
> I mentioned the --legacy-mem case in man pages (vswitch.xml) and I added
> the check to OVS code to not provide socket-limit default value in case of
> --legacy-mem just to avoid case where OVS could not be started with it.
> I'd like to think that user who provides '--legacy-mem' in extra arguments
> knows what is he/she doing.
>
>>
>> I don't want to get into the specifics of how DPDK memory works in the OVS
>> docs, but a few usecases of how OVS can be configured with these different
>> options could be of use for new users. What are your thoughts? It doesn't
>> have to be part of this series, I can follow it up with a separate patch.
>
> I'm not against the extended documentation about memory options. It could
> be added separately if you think that it's needed.
>
> In general, while working on this patch set, I came up to the thoughts about
> removing all the separate options like 'dpdk-lcore-mask', 'dpdk-socket-mem',
> 'dpdk-hugepage-dir', 'dpdk-socket-limit' keeping only options that changes
> OVS behaviour, the options that we're not passing to rte_eal_init().
>
> For example, we will keep vhost related stuff, 'dpdk-init', "pre-port-memory".
> Instead of all the removed options we could have just
> 'other_config:dpdk-options'
> (or 'dpdk-args'). It'll be the string like 'dpdk-extra' that we have now,
> but with all the options. And we'll scan this argument string and push some
> default values in case of missing mandatory options. And the user will be
> responsible for consistency of this line that will be checked by dpdk itself.
>
> Above schema will allow us to not care much about documenting all the possible
> DPDK arguments and the fragmentation of these docs. We already have too much
> options, some of them conflicts with each other (like socket-mem and
> alloc-mem),
> some of them like 'lcore-mask' even not recommended to use ('-l' is
> preferable).
> And the situation will become only worse with time.
> Also, Current parsing infrastructure in dpdk.c is not that extendable.
>
> IMHO, it's better to leave all the options checking to DPDK itself and only
> focus on providing some sane defaults if some necessary options missing in the
> user input. For example, use following arguments if nothing passed to
> dpdk-options:
>
> "-l <first core from aff. mask> --socket-mem 1024,<for all sockets>
> --socket-limit <same>"
Just realized that we could provide only '-l' by default because socket-mem is
not necessary with dpdk 18.11.
>
> All the other things could be determined by DPDK itself or not necessary.
>
> We will have to document that '-c' or '-l' options will affect only dpdk lcore
> threads (not PMD), document that OVS will provide '--socket-limit' with the
> same restrictions as in this patch. But all the docs could be short and be in
> a single place pointing to DPDK docs for extended information.
>
> What do you think?
>
> Best regards, Ilya Maximets.
>
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
