From 85da620e9788b473797b95212b916327974e6942 Mon Sep 17 00:00:00 2001 From: Jesse Gross Date: Wed, 27 Oct 2010 15:29:16 -0700 Subject: [PATCH] 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 --- lib/netdev-provider.h | 6 +++++- lib/netdev-vport.c | 9 +-------- lib/netdev.c | 33 +++++++++++++++++++++++---------- lib/netdev.h | 2 +- ofproto/ofproto-sflow.c | 4 +--- ofproto/ofproto.c | 4 +--- vswitchd/bridge.c | 7 ++++--- 7 files changed, 36 insertions(+), 29 deletions(-) diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h index d4ff6200..170136d4 100644 --- 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 d354dc39..1672a9be 100644 --- 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 993f27a8..a74d5d48 100644 --- 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 133d8ee6..6635a55a 100644 --- 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 784e5528..f886b1b0 100644 --- 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 f44d8a2c..2ec7c8c4 100644 --- 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 532458a6..41fcba52 100644 --- 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); } -- 2.30.2