netdev: Remove may_create/may_open flags.
authorJesse Gross <jesse@nicira.com>
Tue, 1 Jun 2010 21:20:59 +0000 (14:20 -0700)
committerJesse Gross <jesse@nicira.com>
Wed, 2 Jun 2010 00:27:45 +0000 (17:27 -0700)
The most recent revision of the netdev library added may_create
and may_open flags to explicitly state the intent of the caller as
to whether the device should already be in use.  This was simply
a sanity check for users of the netdev library and the configuration.
At this point the netdev library and its users are well behaved and
should no longer need to be checked.  Additional checks have also
been added for incorrect configuration that mean the netdev library
is no longer the primary line of defense.

These flags themselves create problems because it is not always
easy for a library to know what the state of devices should be.
This is particularly a problem for ovs-openflowd, which expects
ports to be added by ovs-dpctl.  Fixing this either requires that
the checks are so permissive to be useless or ugly hacks to get
around them.  Since they are no longer needed, just remove the
checks.

This commit restores the previous behavior of ovs-openflowd to
not require that ports be specified on the command line or
cleaned up after use.

Bug #2652

CC: Natasha Gude <natasha@nicira.com>
CC: Jean Tourrilhes <jt@hpl.hp.com>
CC: 蒲彦 <yan.p.bjtu@gmail.com>
lib/dhcp-client.c
lib/dpif-netdev.c
lib/netdev.c
lib/netdev.h
ofproto/ofproto.c
utilities/ovs-openflowd.c
vswitchd/bridge.c

index 563a415c52f3f0baab53ac3a2902473a84a30dfa..709d7a368a601fe30071322cb1d6a2a57c55e141 100644 (file)
@@ -160,8 +160,6 @@ dhclient_create(const char *netdev_name,
     memset(&netdev_options, 0, sizeof netdev_options);
     netdev_options.name = netdev_name;
     netdev_options.ethertype = ETH_TYPE_IP;
-    netdev_options.may_create = true;
-    netdev_options.may_open = true;
 
     error = netdev_open(&netdev_options, &netdev);
     /* XXX install socket filter to catch only DHCP packets. */
index c4cc6e98357f594124f7ba14199da3f7a3c58c5b..7b1bb5db22de9db1b521104804be2cf466265721 100644 (file)
@@ -375,11 +375,8 @@ do_add_port(struct dp_netdev *dp, const char *devname, uint16_t flags,
     memset(&netdev_options, 0, sizeof netdev_options);
     netdev_options.name = devname;
     netdev_options.ethertype = NETDEV_ETH_TYPE_ANY;
-    netdev_options.may_create = true;
     if (internal) {
         netdev_options.type = "tap";
-    } else {
-        netdev_options.may_open = true;
     }
 
     error = netdev_open(&netdev_options, &netdev);
index 0b0463b9070d2fb83e403faa7d00f52c8f998065..a15315a79019ca5062a07b7796d0ff3476fad97f 100644 (file)
@@ -265,12 +265,6 @@ create_device(struct netdev_options *options, struct netdev_dev **netdev_devp)
 {
     struct netdev_class *netdev_class;
 
-    if (!options->may_create) {
-        VLOG_WARN("attempted to create a device that may not be created: %s",
-                  options->name);
-        return ENODEV;
-    }
-
     if (!options->type || strlen(options->type) == 0) {
         /* Default to system. */
         options->type = "system";
@@ -297,15 +291,7 @@ create_device(struct netdev_options *options, struct netdev_dev **netdev_devp)
  * 'ethertype' may be a 16-bit Ethernet protocol value in host byte order to
  * capture frames of that type received on the device.  It may also be one of
  * the 'enum netdev_pseudo_ethertype' values to receive frames in one of those
- * categories.
- *
- * If the 'may_create' flag is set then this is allowed to be the first time
- * the device is opened (i.e. the refcount will be 1 after this call).  It
- * may be set to false if the device should have already been created.
- *
- * If the 'may_open' flag is set then the call will succeed even if another
- * caller has already opened it.  It may be to false if the device should not
- * currently be open. */
+ * categories. */
 
 int
 netdev_open(struct netdev_options *options, struct netdev **netdevp)
@@ -330,18 +316,12 @@ netdev_open(struct netdev_options *options, struct netdev **netdevp)
         }
         update_device_args(netdev_dev, options->args);
 
-    } else if (options->may_open) {
-        if (!shash_is_empty(options->args) &&
-            !compare_device_args(netdev_dev, options->args)) {
+    } else if (!shash_is_empty(options->args) &&
+               !compare_device_args(netdev_dev, options->args)) {
 
-            VLOG_WARN("%s: attempted to open already created netdev with "
-                      "different arguments", options->name);
-            return EINVAL;
-        }
-    } else {
-        VLOG_WARN("%s: attempted to create a netdev device with bound name",
-                  options->name);
-        return EEXIST;
+        VLOG_WARN("%s: attempted to open already open netdev with "
+                  "different arguments", options->name);
+        return EINVAL;
     }
 
     error = netdev_dev->netdev_class->open(netdev_dev, options->ethertype, 
@@ -364,11 +344,8 @@ netdev_open_default(const char *name, struct netdev **netdevp)
     struct netdev_options options;
 
     memset(&options, 0, sizeof options);
-
     options.name = name;
     options.ethertype = NETDEV_ETH_TYPE_NONE;
-    options.may_create = true;
-    options.may_open = true;
 
     return netdev_open(&options, netdevp);
 }
index 2a096ee27dd11e2a59545e0a48d2da397fe81cf7..5dca24cf927247b3f8586d060bd2f176880468aa 100644 (file)
@@ -85,8 +85,6 @@ struct netdev_options {
     const char *type;
     const struct shash *args;
     int ethertype;
-    bool may_create;
-    bool may_open;
 };
 
 struct netdev;
index 7cca951b40fcb14289b2d421507acbe777cefa2d..9a91c1cea27df8574fcc467a31a12a2dba7467b1 100644 (file)
@@ -1358,7 +1358,6 @@ make_ofport(const struct odp_port *odp_port)
     memset(&netdev_options, 0, sizeof netdev_options);
     netdev_options.name = odp_port->devname;
     netdev_options.ethertype = NETDEV_ETH_TYPE_NONE;
-    netdev_options.may_open = true;
 
     error = netdev_open(&netdev_options, &netdev);
     if (error) {
index e84a3999cd4fd5f47f11242d325bd9eb7105ed11..ebfc3c33efc84e9fe7af5873f2894f803104d6d3 100644 (file)
@@ -122,14 +122,8 @@ main(int argc, char *argv[])
     if (s.ports.n) {
         const char *port;
         size_t i;
-        struct netdev *netdev;
 
         SVEC_FOR_EACH (i, port, &s.ports) {
-            error = netdev_open_default(port, &netdev);
-            if (error) {
-                ovs_fatal(error, "failed to open %s as a device", port);
-            }
-
             error = dpif_port_add(dpif, port, 0, NULL);
             if (error) {
                 ovs_fatal(error, "failed to add %s as a port", port);
index 61813bb4cb41e33b84f0c704d25ed3dd0eeb5038..931f987c49624a7141cbe4c31c0c91bb3e2242a7 100644 (file)
@@ -396,10 +396,6 @@ set_up_iface(const struct ovsrec_interface *iface_cfg, struct iface *iface,
         }
         netdev_options.args = &options;
         netdev_options.ethertype = NETDEV_ETH_TYPE_NONE;
-        netdev_options.may_create = true;
-        if (iface_is_internal(iface->port->bridge, iface_cfg->name)) {
-            netdev_options.may_open = true;
-        }
 
         error = netdev_open(&netdev_options, &iface->netdev);