From c17f0d5e2e4dde4345ec383a99a3b70f6c353caa Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Mon, 21 Mar 2011 13:25:02 -0700 Subject: [PATCH] bridge: Get rid of "port_ifidx" in struct iface. Fix bonding hash. This is a first step toward changing the array of ifaces in struct port to a more suitable data structure. As a side effect this fixes a bonding problem that I noticed via code inspection. Before this commit, each bond_entry specified an interface via index. If an iface was deleted, however, this shifted all of the iface indexes, and the code didn't compensate for that. This commit fixes the problem by using pointers to ifaces instead, which don't shift around. --- vswitchd/bridge.c | 66 ++++++++++++++++++++++++++++------------------- 1 file changed, 40 insertions(+), 26 deletions(-) diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index c1a0945b..277e940c 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -99,7 +99,6 @@ static void dst_set_free(struct dst_set *); struct iface { /* These members are always valid. */ struct port *port; /* Containing port. */ - size_t port_ifidx; /* Index within containing port. */ char *name; /* Host network device name. */ tag_type tag; /* Tag associated with this interface. */ long long delay_expires; /* Time after which 'enabled' may change. */ @@ -120,7 +119,7 @@ struct iface { #define BOND_MASK 0xff struct bond_entry { - int iface_idx; /* Index of assigned iface, or -1 if none. */ + struct iface *iface; /* Assigned iface, or NULL if none. */ uint64_t tx_bytes; /* Count of bytes recently transmitted. */ tag_type tag; /* Tag for bond_entry<->iface association. */ }; @@ -2226,21 +2225,18 @@ choose_output_iface(const struct port *port, const struct flow *flow, } } else { struct bond_entry *e = lookup_bond_entry(port, flow, vlan); - if (e->iface_idx < 0 || e->iface_idx >= port->n_ifaces - || !port->ifaces[e->iface_idx]->enabled) { + if (!e->iface || !e->iface->enabled) { /* XXX select interface properly. The current interface selection * is only good for testing the rebalancing code. */ - iface = bond_choose_iface(port); - if (!iface) { - e->iface_idx = -1; + e->iface = bond_choose_iface(port); + if (!e->iface) { *tags |= port->no_ifaces_tag; return false; } - e->iface_idx = iface->port_ifidx; e->tag = tag_create_random(); } *tags |= e->tag; - iface = port->ifaces[e->iface_idx]; + iface = e->iface; } *dp_ifidx = iface->dp_ifidx; *tags |= iface->tag; /* Currently only used for bonding. */ @@ -3107,8 +3103,8 @@ compare_bond_entries(const void *a_, const void *b_) const struct bond_entry *const *bp = b_; const struct bond_entry *a = *ap; const struct bond_entry *b = *bp; - if (a->iface_idx != b->iface_idx) { - return a->iface_idx > b->iface_idx ? 1 : -1; + if (a->iface != b->iface) { + return a->iface > b->iface ? 1 : -1; } else if (a->tx_bytes != b->tx_bytes) { return a->tx_bytes > b->tx_bytes ? 1 : -1; } else { @@ -3235,7 +3231,7 @@ bond_shift_load(struct slave_balance *from, struct slave_balance *to, /* Arrange for flows to be revalidated. */ ofproto_revalidate(port->bridge->ofproto, hash->tag); - hash->iface_idx = to->iface->port_ifidx; + hash->iface = to->iface; hash->tag = tag_create_random(); } @@ -3275,13 +3271,19 @@ bond_rebalance_port(struct port *port) qsort(hashes, BOND_MASK + 1, sizeof *hashes, compare_bond_entries); for (i = 0; i <= BOND_MASK; i++) { e = hashes[i]; - if (e->iface_idx >= 0 && e->iface_idx < port->n_ifaces) { - b = &bals[e->iface_idx]; - b->tx_bytes += e->tx_bytes; - if (!b->hashes) { - b->hashes = &hashes[i]; + if (!e->iface) { + continue; + } + + for (b = bals; b < &bals[n_bals]; b++) { + if (b->iface == e->iface) { + b->tx_bytes += e->tx_bytes; + if (!b->hashes) { + b->hashes = &hashes[i]; + } + b->n_hashes++; + break; } - b->n_hashes++; } } qsort(bals, n_bals, sizeof *bals, compare_slave_balance); @@ -3383,7 +3385,7 @@ bond_rebalance_port(struct port *port) for (e = &port->bond_hash[0]; e <= &port->bond_hash[BOND_MASK]; e++) { e->tx_bytes /= 2; if (!e->tx_bytes) { - e->iface_idx = -1; + e->iface = NULL; } } @@ -3571,7 +3573,7 @@ bond_unixctl_show(struct unixctl_conn *conn, int hash = be - port->bond_hash; struct mac_entry *me; - if (be->iface_idx != j) { + if (be->iface != iface) { continue; } @@ -3656,7 +3658,7 @@ bond_unixctl_migrate(struct unixctl_conn *conn, const char *args_, entry = &port->bond_hash[hash]; ofproto_revalidate(port->bridge->ofproto, entry->tag); - entry->iface_idx = iface->port_ifidx; + entry->iface = iface; entry->tag = tag_create_random(); unixctl_command_reply(conn, 200, "migrated"); } @@ -4265,7 +4267,7 @@ port_update_bonding(struct port *port) 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]; - e->iface_idx = -1; + e->iface = NULL; e->tx_bytes = 0; } port->bond_next_rebalance @@ -4302,7 +4304,6 @@ iface_create(struct port *port, const struct ovsrec_interface *if_cfg) iface = xzalloc(sizeof *iface); iface->port = port; - iface->port_ifidx = port->n_ifaces; iface->name = xstrdup(name); iface->dp_ifidx = -1; iface->tag = tag_create_random(); @@ -4335,7 +4336,16 @@ iface_destroy(struct iface *iface) struct port *port = iface->port; struct bridge *br = port->bridge; bool del_active = port->active_iface == iface; - struct iface *del; + size_t i; + + if (port->bond_hash) { + struct bond_entry *e; + for (e = port->bond_hash; e <= &port->bond_hash[BOND_MASK]; e++) { + if (e->iface == iface) { + e->iface = NULL; + } + } + } if (iface->port->lacp) { lacp_slave_unregister(iface->port->lacp, iface); @@ -4351,8 +4361,12 @@ iface_destroy(struct iface *iface) hmap_remove(&br->ifaces, &iface->dp_ifidx_node); } - del = port->ifaces[iface->port_ifidx] = port->ifaces[--port->n_ifaces]; - del->port_ifidx = iface->port_ifidx; + for (i = 0; i < port->n_ifaces; i++) { + if (iface == port->ifaces[i]) { + port->ifaces[i] = port->ifaces[--port->n_ifaces]; + break; + } + } netdev_close(iface->netdev); -- 2.30.2