vswitchd: Support changing the type of a bridge port.

Until now, if the type of a bridge port changed in the database, then
ovs-vswitchd would report an error and keep it the same type.  This commit
changes the behavior to something more reasonable: the old datapath port is
deleted and replaced by a new datapath port of the correct type.

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 23ebae3bfb56356805b147d937b811bad0180e25..c27dd8d432804e9dd4784a12a69c111f15798113 100644 (file)
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1247,6 +1247,42 @@ ofproto_is_alive(const struct ofproto *p)
     return !hmap_is_empty(&p->controllers);
 }
 
+/* Deletes port number 'odp_port' from the datapath for 'ofproto'.
+ *
+ * This is almost the same as calling dpif_port_del() directly on the
+ * datapath, but it also makes 'ofproto' close its open netdev for the port
+ * (if any).  This makes it possible to create a new netdev of a different
+ * type under the same name, which otherwise the netdev library would refuse
+ * to do because of the conflict.  (The netdev would eventually get closed on
+ * the next trip through ofproto_run(), but this interface is more direct.)
+ *
+ * The caller must be prepared for a callback to its port_changed_cb hook
+ * function.
+ *
+ * Returns 0 if successful, otherwise a positive errno. */
+int
+ofproto_port_del(struct ofproto *ofproto, uint16_t odp_port)
+{
+    struct ofport *ofport = get_port(ofproto, odp_port);
+    const char *name = ofport ? (char *) ofport->opp.name : "<unknown>";
+    int error;
+
+    error = dpif_port_del(ofproto->dpif, odp_port);
+    if (error) {
+        VLOG_ERR("%s: failed to remove port %"PRIu16" (%s) interface (%s)",
+                 dpif_name(ofproto->dpif), odp_port, name, strerror(error));
+    } else if (ofport) {
+        /* 'name' is ofport->opp.name and update_port() is going to destroy
+         * 'ofport'.  Just in case update_port() refers to 'name' after it
+         * destroys 'ofport', make a copy of it around the update_port()
+         * call. */
+        char *devname = xstrdup(name);
+        update_port(ofproto, devname);
+        free(devname);
+    }
+    return error;
+}
+
 int
 ofproto_send_packet(struct ofproto *p, const flow_t *flow,
                     const union ofp_action *actions, size_t n_actions,

diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
index 22484519e9d59cd89bf9257a6234eebe13800355..e39c6c0d93ffd107b3f6e1b180f87d6d93c1f947 100644 (file)
--- a/ofproto/ofproto.h
+++ b/ofproto/ofproto.h
@@ -99,6 +99,8 @@ int ofproto_run2(struct ofproto *, bool revalidate_all);
 void ofproto_wait(struct ofproto *);
 bool ofproto_is_alive(const struct ofproto *);
 
+int ofproto_port_del(struct ofproto *, uint16_t odp_port);
+
 /* Configuration. */
 void ofproto_set_datapath_id(struct ofproto *, uint64_t datapath_id);
 void ofproto_set_controllers(struct ofproto *,

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index e42d41f49b67dac7d3b2640a093ba0f74c8ca2fa..9a306ab4e884ecdba2b40863ff7f7add1af43389 100644 (file)
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -667,38 +667,58 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
         shash_init(&cur_ifaces);
         for (i = 0; i < n_dpif_ports; i++) {
             const char *name = dpif_ports[i].devname;
-            shash_add_once(&cur_ifaces, name, NULL);
+            shash_add_once(&cur_ifaces, name, &dpif_ports[i]);
         }
-        free(dpif_ports);
 
         /* Get the set of interfaces we want on this datapath. */
         bridge_get_all_ifaces(br, &want_ifaces);
 
+        hmap_clear(&br->ifaces);
         SHASH_FOR_EACH (node, &want_ifaces) {
             const char *if_name = node->name;
             struct iface *iface = node->data;
-
-            if (shash_find(&cur_ifaces, if_name)) {
-                /* Already exists on the datapath.  If we have it open,
-                 * reconfigure it; otherwise we'll open it later. */
-                if (iface && iface->netdev) {
-                    reconfigure_iface_netdev(iface);
+            bool internal = !iface || !strcmp(iface->type, "internal");
+            struct odp_port *dpif_port = shash_find_data(&cur_ifaces, if_name);
+            int error;
+
+            /* If we have a port or a netdev already, and it's not the type we
+             * want, then delete the port (if any) and close the netdev (if
+             * any). */
+            if (internal
+                ? dpif_port && !(dpif_port->flags & ODP_PORT_INTERNAL)
+                : (iface->netdev
+                   && strcmp(iface->type, netdev_get_type(iface->netdev))))
+            {
+                if (dpif_port) {
+                    error = ofproto_port_del(br->ofproto, dpif_port->port);
+                    if (error) {
+                        continue;
+                    }
+                    dpif_port = NULL;
                 }
-            } else {
-                bool internal = !strcmp(iface->type, "internal");
-                int error;
+                if (iface) {
+                    netdev_close(iface->netdev);
+                    iface->netdev = NULL;
+                }
+            }
 
-                /* Create interface if it doesn't already exist. */
-                if (!internal) {
+            /* If it's not an internal port, open (possibly create) the
+             * netdev. */
+            if (!internal) {
+                if (!iface->netdev) {
                     error = create_iface_netdev(iface);
                     if (error) {
                         VLOG_WARN("could not create iface %s: %s", iface->name,
                                   strerror(error));
+                        continue;
                     }
-                    continue;
+                } else {
+                    reconfigure_iface_netdev(iface);
                 }
+            }
 
-                /* Add to datapath. */
+            /* If it's not part of the datapath, add it. */
+            if (!dpif_port) {
                 error = dpif_port_add(br->dpif, if_name,
                                       internal ? ODP_PORT_INTERNAL : 0, NULL);
                 if (error == EFBIG) {
@@ -708,9 +728,25 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
                 } else if (error) {
                     VLOG_ERR("failed to add %s interface to %s: %s",
                              if_name, dpif_name(br->dpif), strerror(error));
+                    continue;
                 }
             }
+
+            /* If it's an internal port, open the netdev. */
+            if (internal) {
+                if (iface && !iface->netdev) {
+                    error = create_iface_netdev(iface);
+                    if (error) {
+                        VLOG_WARN("could not create iface %s: %s", iface->name,
+                                  strerror(error));
+                        continue;
+                    }
+                }
+            } else {
+                assert(iface->netdev != NULL);
+            }
         }
+        free(dpif_ports);
         shash_destroy(&cur_ifaces);
         shash_destroy(&want_ifaces);
     }