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