bridge: Immediately drop interfaces that can't be opened.

Previously we would keep interfaces around that couldn't be opened
because they might be internal interfaces that are created later.
However, this leads to a race condition if the interface appears
after we try to create it and fails since some operations may
succeed.  Instead, give up on the interface immediately if it can't
be opened and isn't internal (which we control and so won't have
this issue).

Bug #2737

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 6254251..c883cf5 100644 (file)
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -1474,7 +1474,13 @@ bridge_reconfigure_one(const struct ovsrec_open_vswitch *ovs_cfg,
         if (!port) {
             port = port_create(br, node->name);
         }
+
         port_reconfigure(port, node->data);
+        if (!port->n_ifaces) {
+            VLOG_WARN("bridge %s: port %s has no interfaces, dropping",
+                      br->name, port->name);
+            port_destroy(port);
+        }
     }
     shash_destroy(&old_ports);
     shash_destroy(&new_ports);
@@ -3604,6 +3610,19 @@ iface_create(struct port *port, const struct ovsrec_interface *if_cfg)
     iface->netdev = NULL;
     iface->cfg = if_cfg;
 
+    /* Attempt to create the network interface in case it doesn't exist yet. */
+    if (!iface_is_internal(port->bridge, iface->name)) {
+        error = set_up_iface(if_cfg, iface, true);
+        if (error) {
+            VLOG_WARN("could not create iface %s: %s", iface->name,
+                      strerror(error));
+
+            free(iface->name);
+            free(iface);
+            return NULL;
+        }
+    }
+
     if (port->n_ifaces >= port->allocated_ifaces) {
         port->ifaces = x2nrealloc(port->ifaces, &port->allocated_ifaces,
                                   sizeof *port->ifaces);
@@ -3613,16 +3632,6 @@ iface_create(struct port *port, const struct ovsrec_interface *if_cfg)
         port->bridge->has_bonded_ports = true;
     }
 
-    /* Attempt to create the network interface in case it
-     * doesn't exist yet. */
-    if (!iface_is_internal(port->bridge, iface->name)) {
-        error = set_up_iface(if_cfg, iface, true);
-        if (error) {
-            VLOG_WARN("could not create iface %s: %s", iface->name,
-                    strerror(error));
-        }
-    }
-
     VLOG_DBG("attached network device %s to port %s", iface->name, port->name);
 
     bridge_flush(port->bridge);