[ovs-dev] [PATCH ovn 08/11] dummy: Introduce new --enable-dummy=system option.

Ben Pfaff blp at nicira.com
Mon Jun 15 18:47:32 UTC 2015


I think it would be a little better, but I know of third-party trees
that use --enable-dummy (I don't know whether they use
--enable-dummy=override specifically) for testing.  I don't want to
break their use cases just for a little bit of better naming.

On Mon, Jun 15, 2015 at 11:04:04AM -0700, Alex Wang wrote:
> Do you think it is better to change the optarg to "override-all" and
> "override-system"?
> 
> Acked-by: Alex Wang <alexw at nicira.com>
> 
> On Sun, Jun 14, 2015 at 12:19 PM, Ben Pfaff <blp at nicira.com> wrote:
> 
> > Until now there have been two variants for --enable-dummy:
> >
> >     * --enable-dummy: This adds support for "dummy" dpif and netdev.
> >
> >     * --enable-dummy=override: In addition, this replaces *every* existing
> >       dpif and netdev by the dummy type.
> >
> > The latter is useful for testing but it defeats the possibility of using
> > the userspace native tunneling implementation (because all the tunnel
> > netdevs get replaced by dummy netdevs).  Thus, this commit adds a third
> > variant:
> >
> >     * --enable-dummy=system: This replaces the "system" dpif and netdev
> >       by dummies but leaves the others untouched.
> >
> > Signed-off-by: Ben Pfaff <blp at nicira.com>
> > ---
> >  lib/dpif-netdev.c       | 18 +++++++++++++-----
> >  lib/dummy.c             | 28 ++++++++++++++++++++--------
> >  lib/dummy.h             | 19 +++++++++++++++----
> >  lib/netdev-dummy.c      | 44 +++++++++++++++++++++++++-------------------
> >  vswitchd/ovs-vswitchd.c |  4 ++--
> >  5 files changed, 75 insertions(+), 38 deletions(-)
> >
> > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> > index d306f77..be09bff 100644
> > --- a/lib/dpif-netdev.c
> > +++ b/lib/dpif-netdev.c
> > @@ -3688,21 +3688,29 @@ dpif_dummy_register__(const char *type)
> >      dp_register_provider(class);
> >  }
> >
> > +static void
> > +dpif_dummy_override(const char *type)
> > +{
> > +    if (!dp_unregister_provider(type)) {
> > +        dpif_dummy_register__(type);
> > +    }
> > +}
> > +
> >  void
> > -dpif_dummy_register(bool override)
> > +dpif_dummy_register(enum dummy_level level)
> >  {
> > -    if (override) {
> > +    if (level == DUMMY_OVERRIDE_ALL) {
> >          struct sset types;
> >          const char *type;
> >
> >          sset_init(&types);
> >          dp_enumerate_types(&types);
> >          SSET_FOR_EACH (type, &types) {
> > -            if (!dp_unregister_provider(type)) {
> > -                dpif_dummy_register__(type);
> > -            }
> > +            dpif_dummy_override(type);
> >          }
> >          sset_destroy(&types);
> > +    } else if (level == DUMMY_OVERRIDE_SYSTEM) {
> > +        dpif_dummy_override("system");
> >      }
> >
> >      dpif_dummy_register__("dummy");
> > diff --git a/lib/dummy.c b/lib/dummy.c
> > index 0918037..ef36578 100644
> > --- a/lib/dummy.c
> > +++ b/lib/dummy.c
> > @@ -1,5 +1,5 @@
> >  /*
> > - * Copyright (c) 2010, 2011, 2012, 2013 Nicira, Inc.
> > + * Copyright (c) 2010, 2011, 2012, 2013, 2015 Nicira, Inc.
> >   *
> >   * Licensed under the Apache License, Version 2.0 (the "License");
> >   * you may not use this file except in compliance with the License.
> > @@ -15,23 +15,35 @@
> >   */
> >
> >  #include <config.h>
> > -
> >  #include "dummy.h"
> > +#include <string.h>
> > +#include "util.h"
> >
> >  /* Enables support for "dummy" network devices and dpifs, which are
> > useful for
> >   * testing.  A client program might call this function if it is designed
> >   * specifically for testing or the user enables it on the command line.
> >   *
> > - * If 'override' is false, then "dummy" dpif and netdev classes will be
> > - * created.  If 'override' is true, then in addition all existing dpif and
> > - * netdev classes will be deleted and replaced by dummy classes.
> > + * 'arg' is parsed to determine the override level (see the definition of
> > enum
> > + * dummy_level).
> >   *
> >   * There is no strong reason why dummy devices shouldn't always be
> > enabled. */
> >  void
> > -dummy_enable(bool override)
> > +dummy_enable(const char *arg)
> >  {
> > -    netdev_dummy_register(override);
> > -    dpif_dummy_register(override);
> > +    enum dummy_level level;
> > +
> > +    if (!arg || !arg[0]) {
> > +        level = DUMMY_OVERRIDE_NONE;
> > +    } else if (!strcmp(arg, "system")) {
> > +        level = DUMMY_OVERRIDE_SYSTEM;
> > +    } else if (!strcmp(arg, "override")) {
> > +        level = DUMMY_OVERRIDE_ALL;
> > +    } else {
> > +        ovs_fatal(0, "%s: unknown dummy level", arg);
> > +    }
> > +
> > +    netdev_dummy_register(level);
> > +    dpif_dummy_register(level);
> >      timeval_dummy_register();
> >      vlandev_dummy_enable();
> >  }
> > diff --git a/lib/dummy.h b/lib/dummy.h
> > index b20519a..a94658b 100644
> > --- a/lib/dummy.h
> > +++ b/lib/dummy.h
> > @@ -1,5 +1,5 @@
> >  /*
> > - * Copyright (c) 2010, 2011, 2012, 2013 Nicira, Inc.
> > + * Copyright (c) 2010, 2011, 2012, 2013, 2015 Nicira, Inc.
> >   *
> >   * Licensed under the Apache License, Version 2.0 (the "License");
> >   * you may not use this file except in compliance with the License.
> > @@ -19,12 +19,23 @@
> >
> >  #include <stdbool.h>
> >
> > +/* Degree of dummy support.
> > + *
> > + * Beyond enabling support for dummies, it can be useful to replace some
> > kinds
> > + * of bridges and netdevs, or all kinds, by dummies.  This enum expresses
> > the
> > + * degree to which this should happen. */
> > +enum dummy_level {
> > +    DUMMY_OVERRIDE_NONE,        /* Support dummy but don't force its use.
> > */
> > +    DUMMY_OVERRIDE_SYSTEM,      /* Replace "system" by dummy. */
> > +    DUMMY_OVERRIDE_ALL,         /* Replace all types by dummy. */
> > +};
> > +
> >  /* For client programs to call directly to enable dummy support. */
> > -void dummy_enable(bool override);
> > +void dummy_enable(const char *arg);
> >
> >  /* Implementation details. */
> > -void dpif_dummy_register(bool override);
> > -void netdev_dummy_register(bool override);
> > +void dpif_dummy_register(enum dummy_level);
> > +void netdev_dummy_register(enum dummy_level);
> >  void timeval_dummy_register(void);
> >  void vlandev_dummy_enable(void);
> >
> > diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
> > index ff65689..c8072b6 100644
> > --- a/lib/netdev-dummy.c
> > +++ b/lib/netdev-dummy.c
> > @@ -1426,8 +1426,27 @@ netdev_dummy_ip4addr(struct unixctl_conn *conn, int
> > argc OVS_UNUSED,
> >
> >  }
> >
> > +static void
> > +netdev_dummy_override(const char *type)
> > +{
> > +    if (!netdev_unregister_provider(type)) {
> > +        struct netdev_class *class;
> > +        int error;
> > +
> > +        class = xmemdup(&dummy_class, sizeof dummy_class);
> > +        class->type = xstrdup(type);
> > +        error = netdev_register_provider(class);
> > +        if (error) {
> > +            VLOG_ERR("%s: failed to register netdev provider (%s)",
> > +                     type, ovs_strerror(error));
> > +            free(CONST_CAST(char *, class->type));
> > +            free(class);
> > +        }
> > +    }
> > +}
> > +
> >  void
> > -netdev_dummy_register(bool override)
> > +netdev_dummy_register(enum dummy_level level)
> >  {
> >      unixctl_command_register("netdev-dummy/receive", "name
> > packet|flow...",
> >                               2, INT_MAX, netdev_dummy_receive, NULL);
> > @@ -1441,33 +1460,20 @@ netdev_dummy_register(bool override)
> >                               "[netdev] ipaddr/mask-prefix-len", 2, 2,
> >                               netdev_dummy_ip4addr, NULL);
> >
> > -
> > -    if (override) {
> > +    if (level == DUMMY_OVERRIDE_ALL) {
> >          struct sset types;
> >          const char *type;
> >
> >          sset_init(&types);
> >          netdev_enumerate_types(&types);
> >          SSET_FOR_EACH (type, &types) {
> > -            if (!strcmp(type, "patch")) {
> > -                continue;
> > -            }
> > -            if (!netdev_unregister_provider(type)) {
> > -                struct netdev_class *class;
> > -                int error;
> > -
> > -                class = xmemdup(&dummy_class, sizeof dummy_class);
> > -                class->type = xstrdup(type);
> > -                error = netdev_register_provider(class);
> > -                if (error) {
> > -                    VLOG_ERR("%s: failed to register netdev provider
> > (%s)",
> > -                             type, ovs_strerror(error));
> > -                    free(CONST_CAST(char *, class->type));
> > -                    free(class);
> > -                }
> > +            if (strcmp(type, "patch")) {
> > +                netdev_dummy_override(type);
> >              }
> >          }
> >          sset_destroy(&types);
> > +    } else if (level == DUMMY_OVERRIDE_SYSTEM) {
> > +        netdev_dummy_override("system");
> >      }
> >      netdev_register_provider(&dummy_class);
> >
> > diff --git a/vswitchd/ovs-vswitchd.c b/vswitchd/ovs-vswitchd.c
> > index a1b33da..ba1c6bf 100644
> > --- a/vswitchd/ovs-vswitchd.c
> > +++ b/vswitchd/ovs-vswitchd.c
> > @@ -1,4 +1,4 @@
> > -/* Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
> > +/* Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira,
> > Inc.
> >   *
> >   * Licensed under the Apache License, Version 2.0 (the "License");
> >   * you may not use this file except in compliance with the License.
> > @@ -204,7 +204,7 @@ parse_options(int argc, char *argv[], char
> > **unixctl_pathp)
> >              break;
> >
> >          case OPT_ENABLE_DUMMY:
> > -            dummy_enable(optarg && !strcmp(optarg, "override"));
> > +            dummy_enable(optarg);
> >              break;
> >
> >          case OPT_DISABLE_SYSTEM:
> > --
> > 2.1.3
> >
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
> >



More information about the dev mailing list