From efa3112b7c340203decf0061e7d1d12be03a150f 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-patch.c | 2 +- lib/netdev-provider.h | 6 +++++- lib/netdev-tunnel.c | 4 ++-- lib/netdev-vport.c | 7 ------- lib/netdev-vport.h | 1 - lib/netdev.c | 33 +++++++++++++++++++++++---------- lib/netdev.h | 2 +- ofproto/ofproto-sflow.c | 4 +--- ofproto/ofproto.c | 4 +--- vswitchd/bridge.c | 2 +- 10 files changed, 35 insertions(+), 30 deletions(-) diff --git a/lib/netdev-patch.c b/lib/netdev-patch.c index 7e8b1990e..d2a064332 100644 --- a/lib/netdev-patch.c +++ b/lib/netdev-patch.c @@ -200,7 +200,7 @@ const struct netdev_class netdev_patch_class = { 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-provider.h b/lib/netdev-provider.h index c0ed4ef60..fb610d751 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-tunnel.c b/lib/netdev-tunnel.c index d0ecd98e2..2c3fca54e 100644 --- a/lib/netdev-tunnel.c +++ b/lib/netdev-tunnel.c @@ -256,7 +256,7 @@ const struct netdev_class netdev_gre_class = { 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, @@ -316,7 +316,7 @@ const struct netdev_class netdev_capwap_class = { 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-vport.c b/lib/netdev-vport.c index 0153ac78d..84dda8143 100644 --- a/lib/netdev-vport.c +++ b/lib/netdev-vport.c @@ -113,13 +113,6 @@ netdev_vport_get_mtu(const struct netdev *netdev, int *mtup) return 0; } -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) { diff --git a/lib/netdev-vport.h b/lib/netdev-vport.h index b4016606f..dc65fe78e 100644 --- a/lib/netdev-vport.h +++ b/lib/netdev-vport.h @@ -27,7 +27,6 @@ int netdev_vport_set_etheraddr(struct netdev *, int netdev_vport_get_etheraddr(const struct netdev *, uint8_t mac[ETH_ADDR_LEN]); int netdev_vport_get_mtu(const struct netdev *, int *mtup); -int netdev_vport_get_carrier(const struct netdev *, bool *carrier); int netdev_vport_get_stats(const struct netdev *, struct netdev_stats *); int netdev_vport_set_stats(struct netdev *, const struct netdev_stats *); int netdev_vport_update_flags(struct netdev *, enum netdev_flags off, diff --git a/lib/netdev.c b/lib/netdev.c index 24c2a88fa..db4bb6ef2 100644 --- a/lib/netdev.c +++ b/lib/netdev.c @@ -909,19 +909,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 cd5c8c300..93997aee7 100644 --- a/lib/netdev.h +++ b/lib/netdev.h @@ -127,7 +127,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 c74c7360e..f86ca3625 100644 --- a/ofproto/ofproto-sflow.c +++ b/ofproto/ofproto-sflow.c @@ -161,10 +161,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 fb13c7be9..37678a4d5 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -1396,7 +1396,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); @@ -1423,8 +1422,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 598b0016d..687487987 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -390,7 +390,7 @@ set_up_iface(const struct ovsrec_interface *iface_cfg, 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); } } else if (iface->netdev) { const char *netdev_type = netdev_get_type(iface->netdev); -- 2.43.0