netdev: Fix carrier status for down interfaces.

Currently netdev_get_carrier() returns both a carrier status and
an error code.  However, usage of the error code was inconsistent:
most callers either ignored it or didn't perform their task if an
error occured, which prevented bond rebalancing.  This makes the
handling consistent by translating an error into a down status in
the netdev library.

Bug #3959

diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h
index d4ff620..170136d 100644 (file)
--- a/lib/netdev-provider.h
+++ b/lib/netdev-provider.h
@@ -259,7 +259,11 @@ struct netdev_class {
     int (*get_ifindex)(const struct netdev *netdev);
 
     /* Sets 'carrier' to true if carrier is active (link light is on) on
-     * 'netdev'. */
+     * 'netdev'.
+     *
+     * May be null if device does not provide carrier status (will be always
+     * up as long as device is up).
+     */
     int (*get_carrier)(const struct netdev *netdev, bool *carrier);
 
     /* Retrieves current device stats for 'netdev' into 'stats'.

diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index d354dc3..1672a9b 100644 (file)
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -272,13 +272,6 @@ netdev_vport_get_mtu(const struct netdev *netdev, int *mtup)
     return 0;
 }
 
-static int
-netdev_vport_get_carrier(const struct netdev *netdev OVS_UNUSED, bool *carrier)
-{
-    *carrier = true;
-    return 0;
-}
-
 int
 netdev_vport_get_stats(const struct netdev *netdev, struct netdev_stats *stats)
 {
@@ -613,7 +606,7 @@ parse_patch_config(struct vport_info *port, const struct shash *args)
     netdev_vport_get_etheraddr,                             \
     netdev_vport_get_mtu,                                   \
     NULL,                       /* get_ifindex */           \
-    netdev_vport_get_carrier,                               \
+    NULL,                       /* get_carrier */           \
     netdev_vport_get_stats,                                 \
     netdev_vport_set_stats,                                 \
                                                             \

diff --git a/lib/netdev.c b/lib/netdev.c
index 993f27a..a74d5d4 100644 (file)
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -892,19 +892,32 @@ netdev_arp_lookup(const struct netdev *netdev,
     return error;
 }
 
-/* Sets 'carrier' to true if carrier is active (link light is on) on
- * 'netdev'. */
-int
-netdev_get_carrier(const struct netdev *netdev, bool *carrier)
+/* Returns true if carrier is active (link light is on) on 'netdev'. */
+bool
+netdev_get_carrier(const struct netdev *netdev)
 {
-    int error = (netdev_get_dev(netdev)->netdev_class->get_carrier
-                 ? netdev_get_dev(netdev)->netdev_class->get_carrier(netdev,
-                        carrier)
-                 : EOPNOTSUPP);
+    int error;
+    enum netdev_flags flags;
+    bool carrier;
+
+    netdev_get_flags(netdev, &flags);
+    if (!(flags & NETDEV_UP)) {
+        return false;
+    }
+
+    if (!netdev_get_dev(netdev)->netdev_class->get_carrier) {
+        return true;
+    }
+
+    error = netdev_get_dev(netdev)->netdev_class->get_carrier(netdev,
+                                                              &carrier);
     if (error) {
-        *carrier = false;
+        VLOG_DBG("%s: failed to get network device carrier status, assuming "
+                 "down: %s", netdev_get_name(netdev), strerror(error));
+        carrier = false;
     }
-    return error;
+
+    return carrier;
 }
 
 /* Retrieves current device stats for 'netdev'. */

diff --git a/lib/netdev.h b/lib/netdev.h
index 133d8ee..6635a55 100644 (file)
--- a/lib/netdev.h
+++ b/lib/netdev.h
@@ -125,7 +125,7 @@ int netdev_set_etheraddr(struct netdev *, const uint8_t mac[6]);
 int netdev_get_etheraddr(const struct netdev *, uint8_t mac[6]);
 
 /* PHY interface. */
-int netdev_get_carrier(const struct netdev *, bool *carrier);
+bool netdev_get_carrier(const struct netdev *);
 int netdev_get_features(struct netdev *,
                         uint32_t *current, uint32_t *advertised,
                         uint32_t *supported, uint32_t *peer);

diff --git a/ofproto/ofproto-sflow.c b/ofproto/ofproto-sflow.c
index 784e552..f886b1b 100644 (file)
--- a/ofproto/ofproto-sflow.c
+++ b/ofproto/ofproto-sflow.c
@@ -182,10 +182,8 @@ sflow_agent_get_counters(void *os_, SFLPoller *poller,
         counters->ifDirection = 0;
     }
     if (!netdev_get_flags(osp->netdev, &flags) && flags & NETDEV_UP) {
-        bool carrier;
-
         counters->ifStatus = 1; /* ifAdminStatus up. */
-        if (!netdev_get_carrier(osp->netdev, &carrier) && carrier) {
+        if (netdev_get_carrier(osp->netdev)) {
             counters->ifStatus |= 2; /* ifOperStatus us. */
         }
     } else {

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index f44d8a2..2ec7c8c 100644 (file)
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1398,7 +1398,6 @@ make_ofport(const struct odp_port *odp_port)
     enum netdev_flags flags;
     struct ofport *ofport;
     struct netdev *netdev;
-    bool carrier;
     int error;
 
     memset(&netdev_options, 0, sizeof netdev_options);
@@ -1426,8 +1425,7 @@ make_ofport(const struct odp_port *odp_port)
     netdev_get_flags(netdev, &flags);
     ofport->opp.config = flags & NETDEV_UP ? 0 : OFPPC_PORT_DOWN;
 
-    netdev_get_carrier(netdev, &carrier);
-    ofport->opp.state = carrier ? 0 : OFPPS_LINK_DOWN;
+    ofport->opp.state = netdev_get_carrier(netdev) ? 0 : OFPPS_LINK_DOWN;
 
     netdev_get_features(netdev,
                         &ofport->opp.curr, &ofport->opp.advertised,

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 532458a..41fcba5 100644 (file)
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -420,7 +420,7 @@ create_iface_netdev(struct iface *iface)
     error = netdev_open(&netdev_options, &iface->netdev);
 
     if (iface->netdev) {
-        netdev_get_carrier(iface->netdev, &iface->enabled);
+        iface->enabled = netdev_get_carrier(iface->netdev);
     }
 
     shash_destroy(&options);
@@ -2077,10 +2077,11 @@ bond_run(struct bridge *br)
             /* Track carrier going up and down on interfaces. */
             while (!netdev_monitor_poll(port->monitor, &devname)) {
                 struct iface *iface;
-                bool carrier;
 
                 iface = port_lookup_iface(port, devname);
-                if (iface && !netdev_get_carrier(iface->netdev, &carrier)) {
+                if (iface) {
+                    bool carrier = netdev_get_carrier(iface->netdev);
+
                     bond_link_status_update(iface, carrier);
                     port_update_bond_compat(port);
                 }