bridge: Clear out all Interface fields when an interface cannot be created.

When an Interface record is invalid (for example, when the interface that
it specifies does not exist and cannot be created), ovs-vswitchd would
leave any pre-existing data in its columns, except that it would set the
ofport column to -1 to indicate the error.  This was sometimes confusing
because, for example, the lacp_current field could still be set to "true"
if LACP has previously been active and up-to-date.

This commit changes ovs-vswitchd to reset all such data to its default
values when an interface is invalid.

Bug #7450.
Reported-by: Duffie Cooley <dcooley@nicira.com>
Bug #7491.
Reported-by: Ethan Jackson <ethan@nicira.com>
Release Notes #7500.
Reported-by: Keith Amidon <keith@nicira.com>

diff --git a/AUTHORS b/AUTHORS
index da82f10..20a53d3 100644 (file)
--- a/AUTHORS
+++ b/AUTHORS
@@ -61,6 +61,7 @@ Bryan Osoro             bosoro@nicira.com
 Cedric Hobbs            cedric@nicira.com
 Dave Walker             DaveWalker@ubuntu.com
 Derek Cormier           derek.cormier@lab.ntt.co.jp
+Duffie Cooley           dcooley@nicira.com
 DK Moon                 dkmoon@nicira.com
 Gaetano Catalli         gaetano.catalli@gmail.com
 George Shuklin          amarao@desunote.ru

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index a576844..46d0618 100644 (file)
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -195,6 +195,7 @@ static struct iface *iface_from_ofp_port(const struct bridge *,
                                          uint16_t ofp_port);
 static void iface_set_mac(struct iface *);
 static void iface_set_ofport(const struct ovsrec_interface *, int64_t ofport);
+static void iface_clear_db_record(const struct ovsrec_interface *if_cfg);
 static void iface_configure_qos(struct iface *, const struct ovsrec_qos *);
 static void iface_configure_cfm(struct iface *);
 static void iface_refresh_cfm_stats(struct iface *);
@@ -939,7 +940,7 @@ bridge_add_ofproto_ports(struct bridge *br)
                     /* We already reported a related error, don't bother
                      * duplicating it. */
                 }
-                iface_set_ofport(iface->cfg, -1);
+                iface_clear_db_record(iface->cfg);
                 iface_destroy(iface);
             }
         }
@@ -2162,7 +2163,7 @@ port_add_ifaces(struct port *port)
             && !shash_add_once(&new_ifaces, cfg->name, cfg)) {
             VLOG_WARN("port %s: %s specified twice as port interface",
                       port->name, cfg->name);
-            iface_set_ofport(cfg, -1);
+            iface_clear_db_record(cfg);
         }
     }
 
@@ -2515,6 +2516,29 @@ iface_set_ofport(const struct ovsrec_interface *if_cfg, int64_t ofport)
     }
 }
 
+/* Clears all of the fields in 'if_cfg' that indicate interface status, and
+ * sets the "ofport" field to -1.
+ *
+ * This is appropriate when 'if_cfg''s interface cannot be created or is
+ * otherwise invalid. */
+static void
+iface_clear_db_record(const struct ovsrec_interface *if_cfg)
+{
+    if (!ovsdb_idl_row_is_synthetic(&if_cfg->header_)) {
+        iface_set_ofport(if_cfg, -1);
+        ovsrec_interface_set_status(if_cfg, NULL, NULL, 0);
+        ovsrec_interface_set_admin_state(if_cfg, NULL);
+        ovsrec_interface_set_duplex(if_cfg, NULL);
+        ovsrec_interface_set_link_speed(if_cfg, NULL, 0);
+        ovsrec_interface_set_link_state(if_cfg, NULL);
+        ovsrec_interface_set_mtu(if_cfg, NULL, 0);
+        ovsrec_interface_set_cfm_fault(if_cfg, NULL, 0);
+        ovsrec_interface_set_cfm_remote_mpids(if_cfg, NULL, 0);
+        ovsrec_interface_set_lacp_current(if_cfg, NULL, 0);
+        ovsrec_interface_set_statistics(if_cfg, NULL, NULL, 0);
+    }
+}
+
 /* Adds the 'n' key-value pairs in 'keys' in 'values' to 'shash'.
  *
  * The value strings in '*shash' are taken directly from values[], not copied,