[ovs-dev] [PATCH v10 4/6] netdev-dpdk: Allow arbitrary eal arguments
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.");
+}
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)
+{
+int ret = argc;
+char *release_tok = xstrdup(ovs_cfg);
+char *tok = release_tok, *endptr = NULL;
+
+for(tok = strtok_r(release_tok, " ", &endptr); tok != NULL;
+tok = strtok_r(NULL, " ", &endptr)) {
+char **newarg = grow_argv(argv, ret, 1);
+*argv = newarg;
+(*argv)[ret++] = xstrdup(tok);
+}
+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<
+
+
+ Specifies additional eal command line arguments for DPDK.
+
+
+ The default is empty. Changing this value requires restarting the
+ forwarding path.
+
+
+
--
2.5.0
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v10 4/6] netdev-dpdk: Allow arbitrary eal arguments
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);
>-
