On 09/03/2016 10:38, "Aaron Conole" wrote:
>A previous change moved some commonly used arguments from commandline to
>the database, and with it the ability to pass arbitrary arguments to
>EAL. This change allows arbitrary eal arguments to be provided
>via a new db entry 'other_config:dpdk-extra' which will tokenize the
>string and add it to the argument list. The only argument which will not
>be supported with this change is '--no-huge', which appears to break the
>system in other ways.
>
>Signed-off-by: Aaron Conole
>Tested-by: Sean K Mooney
>Tested-by: RobertX Wojciechowicz
>Tested-by: Kevin Traynor
>Acked-by: Panu Matilainen
>Acked-by: Kevin Traynor
>Acked-by: Flavio Leitner
>---
>v4:
>* Added by suggestion of Panu, making socket-mem non-default
>
>v5:
>* Keep the socket-mem as default parameter, and mention that we
> do not support --no-huge
>* Update ovs-dev.py with the new mechanism for passing arbitrary dpdk
> options
>
>v6->v9:
>* No change
>
>v10:
>* INSTALL.DPDK.md - removed the note since a future commit in the series
>makes
> that documentation invalid (and it seems silly to add it here, only to
>remove
> in in the next commit)
>
> INSTALL.DPDK.md | 5 +
> lib/netdev-dpdk.c| 49
>+++--
> utilities/ovs-dev.py | 6 --
> vswitchd/vswitch.xml | 11 +++
> 4 files changed, 55 insertions(+), 16 deletions(-)
>
>diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md
>index 613764f..3b3d3a0 100644
>--- a/INSTALL.DPDK.md
>+++ b/INSTALL.DPDK.md
>@@ -178,6 +178,11 @@ Using the DPDK with ovs-vswitchd:
>* dpdk-hugepage-dir
>Directory where hugetlbfs is mounted
>
>+ * dpdk-extra
>+ Extra arguments to provide to DPDK EAL, as previously specified on the
>+ command line. Do not pass '--no-huge' to the system in this way.
>Support
>+ for running the system without hugepages is nonexistent.
>+
>* cuse-dev-name
>Option to set the vhost_cuse character device name.
>
>diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>index 289f916..4d3720f 100644
>--- a/lib/netdev-dpdk.c
>+++ b/lib/netdev-dpdk.c
>@@ -2739,6 +2739,9 @@ static char **
> grow_argv(char ***argv, size_t cur_siz, size_t grow_by)
> {
> char **new_argv = realloc(*argv, sizeof(char *) * (cur_siz +
>grow_by));
>+if (!new_argv) {
>+ovs_abort(0, "grow_argv() failed to allocate memory.");
>+}
No need to check if we use xrealloc()
> return new_argv;
> }
>
>@@ -2748,16 +2751,29 @@ dpdk_option_extend(char ***argv, int argc, const
>char *option,
> {
> char **newargv = grow_argv(argv, argc, 2);
>
>-if (!newargv) {
>-ovs_abort(0, "grow_argv() failed to allocate memory.");
>-}
>-
> *argv = newargv;
> (*argv)[argc] = xstrdup(option);
> (*argv)[argc+1] = xstrdup(value);
> }
>
> static int
>+extra_dpdk_args(const char *ovs_cfg, char ***argv, int argc)
Would you mind using another name for "ovs_cfg", please?
It's used elsewhere with another meaning
>+{
>+int ret = argc;
>+char *release_tok = xstrdup(ovs_cfg);
>+char *tok = release_tok, *endptr = NULL;
>+
>+for(tok = strtok_r(release_tok, " ", &endptr); tok != NULL;
Space between for and (
>+tok = strtok_r(NULL, " ", &endptr)) {
>+char **newarg = grow_argv(argv, ret, 1);
>+*argv = newarg;
>+(*argv)[ret++] = xstrdup(tok);
I'd use "newarg" instead of "(*argv)"
>+}
>+free(release_tok);
>+return ret;
>+}
>+
>+static int
> construct_dpdk_options(const struct ovsrec_open_vswitch *ovs_cfg,
>char ***argv, const int initial_size)
> {
>@@ -2851,8 +2867,14 @@ static int
> get_dpdk_args(const struct ovsrec_open_vswitch *ovs_cfg, char ***argv,
> int argc)
> {
>+const char *extra_configuration;
> int i = construct_dpdk_options(ovs_cfg, argv, argc);
> i = construct_dpdk_mutex_options(ovs_cfg, argv, i);
>+
>+extra_configuration = smap_get(&ovs_cfg->other_config, "dpdk-extra");
>+if (extra_configuration) {
>+i = extra_dpdk_args(extra_configuration, argv, i);
>+}
> return i;
> }
>
>@@ -2911,17 +2933,15 @@ __dpdk_init(const struct ovsrec_open_vswitch
>*ovs_cfg)
> }
>
> argv = grow_argv(&argv, argc, argc+1);
>-if (!argv) {
>-ovs_abort(0, "Unable to allocate an initial argv.");
>-}
> argv[argc++] = xstrdup("ovs"); /* TODO use prctl to get process name
>*/
> argc_tmp = get_dpdk_args(ovs_cfg, &argv, argc);
>
> while(argc_tmp != argc) {
>-if (!strcmp("-c", argv[argc++])) {
>+if (!strcmp("-c", argv[argc]) || !strcmp("-l", argv[argc])) {
> auto_determine = false;
> break;
> }
>+argc++;
> }
> argc = argc_tmp;
>
>@@ -2936,9 +2956,6 @@ __dpdk_init(const struct ovsrec_open_vswitch
>*ovs_cfg)
> char buf[MAX_BUFSIZ];
> snprintf(buf, MAX_BUFSIZ, "0x%08llX", (1ULL< argv = grow_argv(&argv, argc, 2);
>-