bridge: Consistently use miimon status if miimon is configured.
authorBen Pfaff <blp@nicira.com>
Thu, 17 Mar 2011 17:48:05 +0000 (10:48 -0700)
committerBen Pfaff <blp@nicira.com>
Fri, 18 Mar 2011 23:51:14 +0000 (16:51 -0700)
A port can be configured to use miimon reporting as the criterion for
enabling or disabling an interface, but in some cases (such as for reading
the initial link status) the code was reading the carrier status instead.
This commit fixes the problem.

This changes the meaning of the link_status column in the Interface table.
I don't think that the old meaning was useful to the controller in the
case of a bond configured for miimon monitoring, because the controller
could not use it to detect which interfaces the bond considered to be up
or down.

vswitchd/bridge.c
vswitchd/vswitch.xml

index 6b8df98..b4bacc9 100644 (file)
@@ -307,7 +307,8 @@ static void iface_update_qos(struct iface *, const struct ovsrec_qos *);
 static void iface_update_cfm(struct iface *);
 static void iface_refresh_cfm_stats(struct iface *iface);
 static void iface_send_packet(struct iface *, struct ofpbuf *packet);
-static void iface_update_carrier(struct iface *, bool carrier);
+static void iface_update_carrier(struct iface *);
+static bool iface_get_carrier(const struct iface *);
 
 static void shash_from_ovs_idl_map(char **keys, char **values, size_t n,
                                    struct shash *);
@@ -726,7 +727,7 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
                 /* Update 'iface'. */
                 if (iface) {
                     iface->netdev = netdev;
-                    iface->enabled = netdev_get_carrier(iface->netdev);
+                    iface->enabled = iface_get_carrier(iface);
                     iface->up = iface->enabled;
                 }
             } else if (iface && iface->netdev) {
@@ -1194,8 +1195,7 @@ iface_refresh_status(struct iface *iface)
 
 
     ovsrec_interface_set_link_state(iface->cfg,
-                                    netdev_get_carrier(iface->netdev)
-                                    ? "up" : "down");
+                                    iface_get_carrier(iface) ? "up" : "down");
 
     error = netdev_get_mtu(iface->netdev, &mtu);
     if (!error && mtu != INT_MAX) {
@@ -3882,7 +3882,7 @@ port_run(struct port *port)
 
             iface = port_lookup_iface(port, devname);
             if (iface) {
-                iface_update_carrier(iface, netdev_get_carrier(iface->netdev));
+                iface_update_carrier(iface);
             }
             free(devname);
         }
@@ -3890,7 +3890,7 @@ port_run(struct port *port)
 
         for (i = 0; i < port->n_ifaces; i++) {
             struct iface *iface = port->ifaces[i];
-            iface_update_carrier(iface, netdev_get_miimon(iface->netdev));
+            iface_update_carrier(iface);
         }
         port->miimon_next_update = time_msec() + port->miimon_interval;
     }
@@ -4589,8 +4589,9 @@ iface_delete_queues(unsigned int queue_id,
 }
 
 static void
-iface_update_carrier(struct iface *iface, bool carrier)
+iface_update_carrier(struct iface *iface)
 {
+    bool carrier = iface_get_carrier(iface);
     if (carrier == iface->up) {
         return;
     }
@@ -4688,6 +4689,18 @@ iface_update_cfm(struct iface *iface)
         iface->cfm = NULL;
     }
 }
+
+/* Read carrier or miimon status directly from 'iface''s netdev, according to
+ * how 'iface''s port is configured.
+ *
+ * Returns true if 'iface' is up, false otherwise. */
+static bool
+iface_get_carrier(const struct iface *iface)
+{
+    return (iface->port->monitor
+            ? netdev_get_carrier(iface->netdev)
+            : netdev_get_miimon(iface->netdev));
+}
 \f
 /* Port mirroring. */
 
index cea198a..36d08a4 100644 (file)
 
       <column name="link_state">
         <p>
-          The observed state of the physical network link;
-          i.e. whether a carrier is detected by the interface.
+          The observed state of the physical network link.  This is ordinarily
+          the link's carrier status.  If the interface's <ref table="Port"/> is
+          a bond configured for miimon monitoring, it is instead the network
+          link's miimon status.
         </p>
       </column>