From 0bb5c3ec4dde32bfbdf8b63296d0580f33f829dd Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Fri, 1 Oct 2010 15:59:56 -0700 Subject: [PATCH] ofproto: Get rid of port_changed_cb. Jesse pointed out that port_changed_cb isn't a great interface. It's only around because, earlier, we had a lousy interface for monitoring netdev status, so that we needed to pass along information obtained by ofproto into the bridge. But netdev_monitor is now sufficiently sophisticated that the bridge can set up an independent netdev_monitor without any important loss of efficiency. Since this makes the code cleaner, this commit does so. --- ofproto/ofproto.c | 7 ------ ofproto/ofproto.h | 2 -- vswitchd/bridge.c | 60 +++++++++++++++++++++++------------------------ 3 files changed, 29 insertions(+), 40 deletions(-) diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index c27dd8d4..1b8dfbf0 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -1256,9 +1256,6 @@ ofproto_is_alive(const struct ofproto *p) * to do because of the conflict. (The netdev would eventually get closed on * the next trip through ofproto_run(), but this interface is more direct.) * - * The caller must be prepared for a callback to its port_changed_cb hook - * function. - * * Returns 0 if successful, otherwise a positive errno. */ int ofproto_port_del(struct ofproto *ofproto, uint16_t odp_port) @@ -1517,9 +1514,6 @@ send_port_status(struct ofproto *p, const struct ofport *ofport, hton_ofp_phy_port(&ops->desc); queue_tx(b, ofconn, NULL); } - if (p->ofhooks->port_changed_cb) { - p->ofhooks->port_changed_cb(reason, &ofport->opp, p->aux); - } } static void @@ -4865,7 +4859,6 @@ default_normal_ofhook_cb(const flow_t *flow, const struct ofpbuf *packet, } static const struct ofhooks default_ofhooks = { - NULL, default_normal_ofhook_cb, NULL, NULL diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h index e39c6c0d..7ba4acb4 100644 --- a/ofproto/ofproto.h +++ b/ofproto/ofproto.h @@ -140,8 +140,6 @@ void ofproto_flush_flows(struct ofproto *); /* Hooks for ovs-vswitchd. */ struct ofhooks { - void (*port_changed_cb)(enum ofp_port_reason, const struct ofp_phy_port *, - void *aux); bool (*normal_cb)(const flow_t *, const struct ofpbuf *packet, struct odp_actions *, tag_type *, uint16_t *nf_output_iface, void *aux); diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index 73e6d687..0945e018 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -147,6 +147,7 @@ struct port { long long int bond_next_fake_iface_update; /* Time of next update. */ int bond_rebalance_interval; /* Interval between rebalances, in ms. */ long long int bond_next_rebalance; /* Next rebalancing time. */ + struct netdev_monitor *monitor; /* Tracks carrier up/down status. */ /* Port mirroring info. */ mirror_mask_t src_mirrors; /* Mirrors triggered when packet received. */ @@ -2054,6 +2055,21 @@ bond_run(struct bridge *br) struct port *port = br->ports[i]; if (port->n_ifaces >= 2) { + char *devname; + + /* 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)) { + bond_link_status_update(iface, carrier); + port_update_bond_compat(port); + } + free(devname); + } + for (j = 0; j < port->n_ifaces; j++) { struct iface *iface = port->ifaces[j]; if (time_msec() >= iface->delay_expires) { @@ -2085,6 +2101,7 @@ bond_wait(struct bridge *br) if (port->n_ifaces < 2) { continue; } + netdev_monitor_poll_wait(port->monitor); for (j = 0; j < port->n_ifaces; j++) { struct iface *iface = port->ifaces[j]; if (iface->delay_expires != LLONG_MAX) { @@ -2552,34 +2569,6 @@ done: return true; } -/* Careful: 'opp' is in host byte order and opp->port_no is an OFP port - * number. */ -static void -bridge_port_changed_ofhook_cb(enum ofp_port_reason reason, - const struct ofp_phy_port *opp, - void *br_) -{ - struct bridge *br = br_; - struct iface *iface; - struct port *port; - - if (reason == OFPPR_DELETE || !br->has_bonded_ports) { - return; - } - - iface = iface_from_dp_ifidx(br, ofp_port_to_odp_port(opp->port_no)); - if (!iface) { - return; - } - port = iface->port; - - if (port->n_ifaces > 1) { - bool up = !(opp->state & OFPPS_LINK_DOWN); - bond_link_status_update(iface, up); - port_update_bond_compat(port); - } -} - static bool bridge_normal_ofhook_cb(const flow_t *flow, const struct ofpbuf *packet, struct odp_actions *actions, tag_type *tags, @@ -2653,7 +2642,6 @@ bridge_account_checkpoint_ofhook_cb(void *br_) } static struct ofhooks bridge_ofhooks = { - bridge_port_changed_ofhook_cb, bridge_normal_ofhook_cb, bridge_account_flow_ofhook_cb, bridge_account_checkpoint_ofhook_cb, @@ -3542,6 +3530,7 @@ port_destroy(struct port *port) del = br->ports[port->port_idx] = br->ports[--br->n_ports]; del->port_idx = port->port_idx; + netdev_monitor_destroy(port->monitor); free(port->ifaces); bitmap_free(port->trunks); free(port->name); @@ -3573,6 +3562,10 @@ port_lookup_iface(const struct port *port, const char *name) static void port_update_bonding(struct port *port) { + if (port->monitor) { + netdev_monitor_destroy(port->monitor); + port->monitor = NULL; + } if (port->n_ifaces < 2) { /* Not a bonded port. */ if (port->bond_hash) { @@ -3582,9 +3575,9 @@ port_update_bonding(struct port *port) port->bond_fake_iface = false; } } else { - if (!port->bond_hash) { - size_t i; + size_t i; + if (!port->bond_hash) { port->bond_hash = xcalloc(BOND_MASK + 1, sizeof *port->bond_hash); for (i = 0; i <= BOND_MASK; i++) { struct bond_entry *e = &port->bond_hash[i]; @@ -3602,6 +3595,11 @@ port_update_bonding(struct port *port) } port->bond_compat_is_stale = true; port->bond_fake_iface = port->cfg->bond_fake_iface; + + port->monitor = netdev_monitor_create(); + for (i = 0; i < port->n_ifaces; i++) { + netdev_monitor_add(port->monitor, port->ifaces[i]->netdev); + } } } -- 2.30.2