[ovs-dev] [PATCH v9 5/6] netdev-dpdk: Check dpdk-extra when reading db
Traynor, Kevin
kevin.traynor at intel.com
Mon Feb 15 11:59:12 UTC 2016
> -----Original Message-----
> From: Aaron Conole [mailto:aconole at redhat.com]
> Sent: Thursday, February 11, 2016 8:16 PM
> To: dev at openvswitch.org
> Cc: Flavio Leitner <fbl at sysclose.org>; Panu Matilainen <pmatilai at redhat.com>;
> Traynor, Kevin <kevin.traynor at intel.com>; Wojciechowicz, RobertX
> <robertx.wojciechowicz at intel.com>; Zoltan Kiss <zoltan.kiss at linaro.org>;
> Ansis Atteka <ansisatteka at gmail.com>; Christian Ehrhardt
> <christian.ehrhardt at canonical.com>; Mooney, Sean K <sean.k.mooney at intel.com>
> Subject: [PATCH v9 5/6] netdev-dpdk: Check dpdk-extra when reading db
>
> 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.
hi Aaron,
I think we need a small doc update because of this patch. This is
allowing (with warning) and preferring something that docs from 4/6
say is not allowed.
+ values. You MUST not pass arguments via dpdk-extra if they can be passed
+ via another database parameter.
I tested this functionality for -c and --socket-mem and I see that dpdk_extras is
being preferred.
Tested-by: Kevin Traynor <kevin.traynor at intel.com>
One other thing I noticed is that 1/6 is giving a build warning, as 'cuse_dev_name'
now needs to be wrapped in the #ifdef VHOST_CUSE
lib/netdev-dpdk.c:109:14: warning: 'cuse_dev_name' defined but not used [-Wunused-variable]
static char *cuse_dev_name = NULL; /* Character device cuse_dev_name. */
Not sure if these are big enough to warrant a re-spin or they could be quick follow ups.
Kevin.
>
> Suggested-by: Sean K Mooney <sean.k.mooney at intel.com>
> Signed-off-by: Aaron Conole <aconole at redhat.com>
> ---
> v9:
> * Added as suggested by Sean K Mooney
>
> 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 1d6b907..b376e40 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -2257,6 +2257,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)
> {
> @@ -2274,9 +2285,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;
> @@ -2298,8 +2321,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);
> + }
> }
> }
>
> @@ -2308,7 +2336,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;
> @@ -2356,9 +2385,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;
> @@ -2369,14 +2404,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(&ovs_cfg->other_config, "dpdk-extra");
> if (extra_configuration) {
> - i = extra_dpdk_args(extra_configuration, argv, i);
> + extra_argc = extra_dpdk_args(extra_configuration, &extra_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
More information about the dev
mailing list