Re: [ovs-dev] [PATCH v10 5/6] netdev-dpdk: Check dpdk-extra when reading db

2016-03-28 Thread Daniele Di Proietto


On 09/03/2016 09:38, "Aaron Conole"  wrote:

>A previous patch introduced the ability to pass arbitrary EAL command
>line options via the dpdk_extras database entry. This commit enhances
>that by warning the user when such a configuration is detected and
>prefering the value in the database.
>
>Suggested-by: Sean K Mooney 
>Signed-off-by: Aaron Conole 
>Tested-by: Sean K Mooney 
>Tested-by: Kevin Traynor 
>Acked-by: Panu Matilainen 
>Acked-by: Flavio Leitner 
>---
>v9:
>* Added as suggested by Sean K Mooney
>
>v10:
>* no change
>
> lib/netdev-dpdk.c | 66
>+--
> 1 file changed, 55 insertions(+), 11 deletions(-)
>
>diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>index 4d3720f..2ca519d 100644
>--- a/lib/netdev-dpdk.c
>+++ b/lib/netdev-dpdk.c
>@@ -2756,6 +2756,17 @@ dpdk_option_extend(char ***argv, int argc, const
>char *option,
> (*argv)[argc+1] = xstrdup(value);
> }
> 
>+static char **
>+move_argv(char ***argv, size_t cur_size, char **src_argv, size_t
>src_argc)
>+{
>+char **newargv = grow_argv(argv, cur_size, src_argc);
>+while(src_argc--) {

Space between while and (

>+newargv[cur_size+src_argc] = src_argv[src_argc];
>+src_argv[src_argc] = 0;
>+}
>+return newargv;
>+}
>+
> static int
> extra_dpdk_args(const char *ovs_cfg, char ***argv, int argc)
> {
>@@ -2773,9 +2784,21 @@ extra_dpdk_args(const char *ovs_cfg, char ***argv,
>int argc)
> return ret;
> }
> 
>+static bool
>+argv_contains(char **argv_haystack, const size_t argc_haystack,
>+  const char *needle)
>+{
>+for(size_t i = 0; i < argc_haystack; ++i) {

Space between for and (

>+if (!strcmp(argv_haystack[i], needle))
>+return true;
>+}
>+return false;
>+}
>+
> static int
> construct_dpdk_options(const struct ovsrec_open_vswitch *ovs_cfg,
>-   char ***argv, const int initial_size)
>+   char ***argv, const int initial_size,
>+   char **extra_args, const size_t extra_argc)
> {
> struct dpdk_options_map {
> const char *ovs_configuration;
>@@ -2797,8 +2820,13 @@ construct_dpdk_options(const struct
>ovsrec_open_vswitch *ovs_cfg,
> lookup = opts[i].default_value;
> 
> if(lookup) {

Space between if and (

>-dpdk_option_extend(argv, ret, opts[i].dpdk_option, lookup);
>-ret += 2;
>+if (!argv_contains(extra_args, extra_argc,
>opts[i].dpdk_option)) {
>+dpdk_option_extend(argv, ret, opts[i].dpdk_option,
>lookup);
>+ret += 2;
>+} else {
>+VLOG_WARN("Ignoring database defined option '%s' due to "
>+  "dpdk_extras config", opts[i].dpdk_option);
>+}
> }
> }
> 
>@@ -2807,7 +2835,8 @@ construct_dpdk_options(const struct
>ovsrec_open_vswitch *ovs_cfg,
> 
> static int
> construct_dpdk_mutex_options(const struct ovsrec_open_vswitch *ovs_cfg,
>- char ***argv, const int initial_size)
>+ char ***argv, const int initial_size,
>+ char **extra_args, const size_t extra_argc)
> {
> struct dpdk_exclusive_options_map {
> const char *category;
>@@ -2855,9 +2884,15 @@ construct_dpdk_mutex_options(const struct
>ovsrec_open_vswitch *ovs_cfg,
> ovs_abort(0, "Unable to cope with DPDK settings.");
> }
> 
>-dpdk_option_extend(argv, ret, popt->eal_dpdk_options[found_pos],
>-   found_value);
>-ret += 2;
>+if (!argv_contains(extra_args, extra_argc,
>+   popt->eal_dpdk_options[found_pos])) {
>+dpdk_option_extend(argv, ret,
>popt->eal_dpdk_options[found_pos],
>+   found_value);
>+ret += 2;
>+} else {
>+VLOG_WARN("Ignoring database defined option '%s' due to"
>+  " dpdk_extras config",
>popt->eal_dpdk_options[found_pos]);
>+}
> }
> 
> return ret;
>@@ -2868,14 +2903,23 @@ 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);
>+char **extra_args = NULL;
>+int i;
>+size_t extra_argc = 0;
> 
> extra_configuration = smap_get(_cfg->other_config, "dpdk-extra");
> if (extra_configuration) {
>-i = extra_dpdk_args(extra_configuration, argv, i);
>+extra_argc = extra_dpdk_args(extra_configuration, _args,
>0);
> }
>-return i;
>+
>+i = construct_dpdk_options(ovs_cfg, argv, argc, extra_args,
>extra_argc);
>+i = 

[ovs-dev] [PATCH v10 5/6] netdev-dpdk: Check dpdk-extra when reading db

2016-03-09 Thread Aaron Conole
A previous patch introduced the ability to pass arbitrary EAL command
line options via the dpdk_extras database entry. This commit enhances
that by warning the user when such a configuration is detected and
prefering the value in the database.

Suggested-by: Sean K Mooney 
Signed-off-by: Aaron Conole 
Tested-by: Sean K Mooney 
Tested-by: Kevin Traynor 
Acked-by: Panu Matilainen 
Acked-by: Flavio Leitner 
---
v9:
* Added as suggested by Sean K Mooney

v10:
* no change

 lib/netdev-dpdk.c | 66 +--
 1 file changed, 55 insertions(+), 11 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 4d3720f..2ca519d 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2756,6 +2756,17 @@ dpdk_option_extend(char ***argv, int argc, const char 
*option,
 (*argv)[argc+1] = xstrdup(value);
 }
 
+static char **
+move_argv(char ***argv, size_t cur_size, char **src_argv, size_t src_argc)
+{
+char **newargv = grow_argv(argv, cur_size, src_argc);
+while(src_argc--) {
+newargv[cur_size+src_argc] = src_argv[src_argc];
+src_argv[src_argc] = 0;
+}
+return newargv;
+}
+
 static int
 extra_dpdk_args(const char *ovs_cfg, char ***argv, int argc)
 {
@@ -2773,9 +2784,21 @@ extra_dpdk_args(const char *ovs_cfg, char ***argv, int 
argc)
 return ret;
 }
 
+static bool
+argv_contains(char **argv_haystack, const size_t argc_haystack,
+  const char *needle)
+{
+for(size_t i = 0; i < argc_haystack; ++i) {
+if (!strcmp(argv_haystack[i], needle))
+return true;
+}
+return false;
+}
+
 static int
 construct_dpdk_options(const struct ovsrec_open_vswitch *ovs_cfg,
-   char ***argv, const int initial_size)
+   char ***argv, const int initial_size,
+   char **extra_args, const size_t extra_argc)
 {
 struct dpdk_options_map {
 const char *ovs_configuration;
@@ -2797,8 +2820,13 @@ construct_dpdk_options(const struct ovsrec_open_vswitch 
*ovs_cfg,
 lookup = opts[i].default_value;
 
 if(lookup) {
-dpdk_option_extend(argv, ret, opts[i].dpdk_option, lookup);
-ret += 2;
+if (!argv_contains(extra_args, extra_argc, opts[i].dpdk_option)) {
+dpdk_option_extend(argv, ret, opts[i].dpdk_option, lookup);
+ret += 2;
+} else {
+VLOG_WARN("Ignoring database defined option '%s' due to "
+  "dpdk_extras config", opts[i].dpdk_option);
+}
 }
 }
 
@@ -2807,7 +2835,8 @@ construct_dpdk_options(const struct ovsrec_open_vswitch 
*ovs_cfg,
 
 static int
 construct_dpdk_mutex_options(const struct ovsrec_open_vswitch *ovs_cfg,
- char ***argv, const int initial_size)
+ char ***argv, const int initial_size,
+ char **extra_args, const size_t extra_argc)
 {
 struct dpdk_exclusive_options_map {
 const char *category;
@@ -2855,9 +2884,15 @@ construct_dpdk_mutex_options(const struct 
ovsrec_open_vswitch *ovs_cfg,
 ovs_abort(0, "Unable to cope with DPDK settings.");
 }
 
-dpdk_option_extend(argv, ret, popt->eal_dpdk_options[found_pos],
-   found_value);
-ret += 2;
+if (!argv_contains(extra_args, extra_argc,
+   popt->eal_dpdk_options[found_pos])) {
+dpdk_option_extend(argv, ret, popt->eal_dpdk_options[found_pos],
+   found_value);
+ret += 2;
+} else {
+VLOG_WARN("Ignoring database defined option '%s' due to"
+  " dpdk_extras config", 
popt->eal_dpdk_options[found_pos]);
+}
 }
 
 return ret;
@@ -2868,14 +2903,23 @@ 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);
+char **extra_args = NULL;
+int i;
+size_t extra_argc = 0;
 
 extra_configuration = smap_get(_cfg->other_config, "dpdk-extra");
 if (extra_configuration) {
-i = extra_dpdk_args(extra_configuration, argv, i);
+extra_argc = extra_dpdk_args(extra_configuration, _args, 0);
 }
-return i;
+
+i = construct_dpdk_options(ovs_cfg, argv, argc, extra_args, extra_argc);
+i = construct_dpdk_mutex_options(ovs_cfg, argv, i, extra_args, extra_argc);
+
+if (extra_configuration) {
+*argv = move_argv(argv, i, extra_args, extra_argc);
+}
+
+return i + extra_argc;
 }
 
 static char **dpdk_argv;
-- 
2.5.0

___
dev mailing list