X-Git-Url: http://git.onelab.eu/?a=blobdiff_plain;f=lib%2Fbond.c;h=03696b6a035aea0152c5a37afe1f1e57e23b1510;hb=f61d8d2931806eae5b25ed8bdd5ff4cf43afe776;hp=62f38c9ae09eac2a110774b381a8540b45c617e4;hpb=09a5d390eec472e7837c6accb6196770a78f5bd1;p=sliver-openvswitch.git diff --git a/lib/bond.c b/lib/bond.c index 62f38c9ae..03696b6a0 100644 --- a/lib/bond.c +++ b/lib/bond.c @@ -21,6 +21,7 @@ #include #include #include +#include #include "coverage.h" #include "dynamic-string.h" @@ -50,7 +51,7 @@ VLOG_DEFINE_THIS_MODULE(bond); struct bond_entry { struct bond_slave *slave; /* Assigned slave, NULL if unassigned. */ uint64_t tx_bytes; /* Count of bytes recently transmitted. */ - tag_type tag; /* Tag for entry<->slave association. */ + tag_type tag; /* Tag for entry<->facet association. */ struct list list_node; /* In bond_slave's 'entries' list. */ }; @@ -74,9 +75,6 @@ struct bond_slave { struct list bal_node; /* In bond_rebalance()'s 'bals' list. */ struct list entries; /* 'struct bond_entry's assigned here. */ uint64_t tx_bytes; /* Sum across 'tx_bytes' of entries. */ - - /* BM_STABLE specific bonding info. */ - uint32_t stb_id; /* ID used for 'stb_slaves' ordering. */ }; /* A bond, that is, a set of network devices grouped to improve performance or @@ -103,9 +101,6 @@ struct bond { long long int next_rebalance; /* Next rebalancing time. */ bool send_learning_packets; - /* BM_STABLE specific bonding info. */ - tag_type stb_tag; /* Tag associated with this bond. */ - /* Legacy compatibility. */ long long int next_fake_iface_update; /* LLONG_MAX if disabled. */ @@ -113,28 +108,40 @@ struct bond { * where we can't otherwise provide revalidation feedback to the client. * That's only unixctl commands now; I hope no other cases will arise. */ struct tag_set unixctl_tags; + + atomic_int ref_cnt; }; -static struct hmap all_bonds = HMAP_INITIALIZER(&all_bonds); +static struct ovs_rwlock rwlock = OVS_RWLOCK_INITIALIZER; +static struct hmap all_bonds__ = HMAP_INITIALIZER(&all_bonds__); +static struct hmap *const all_bonds OVS_GUARDED_BY(rwlock) = &all_bonds__; -static void bond_entry_reset(struct bond *); -static struct bond_slave *bond_slave_lookup(struct bond *, const void *slave_); +static void bond_entry_reset(struct bond *) OVS_REQ_WRLOCK(rwlock); +static struct bond_slave *bond_slave_lookup(struct bond *, const void *slave_) + OVS_REQ_RDLOCK(rwlock); static void bond_enable_slave(struct bond_slave *, bool enable, - struct tag_set *); -static void bond_link_status_update(struct bond_slave *, struct tag_set *); -static void bond_choose_active_slave(struct bond *, struct tag_set *); + struct tag_set *) OVS_REQ_WRLOCK(rwlock); +static void bond_link_status_update(struct bond_slave *, struct tag_set *) + OVS_REQ_WRLOCK(rwlock); +static void bond_choose_active_slave(struct bond *, struct tag_set *) + OVS_REQ_WRLOCK(rwlock);; static unsigned int bond_hash_src(const uint8_t mac[ETH_ADDR_LEN], uint16_t vlan, uint32_t basis); static unsigned int bond_hash_tcp(const struct flow *, uint16_t vlan, uint32_t basis); static struct bond_entry *lookup_bond_entry(const struct bond *, const struct flow *, - uint16_t vlan); -static tag_type bond_get_active_slave_tag(const struct bond *); + uint16_t vlan) + OVS_REQ_RDLOCK(rwlock); +static tag_type bond_get_active_slave_tag(const struct bond *) + OVS_REQ_RDLOCK(rwlock); static struct bond_slave *choose_output_slave(const struct bond *, const struct flow *, - uint16_t vlan, tag_type *tags); -static void bond_update_fake_slave_stats(struct bond *); + struct flow_wildcards *, + uint16_t vlan, tag_type *tags) + OVS_REQ_RDLOCK(rwlock); +static void bond_update_fake_slave_stats(struct bond *) + OVS_REQ_RDLOCK(rwlock); /* Attempts to parse 's' as the name of a bond balancing mode. If successful, * stores the mode in '*balance' and returns true. Otherwise returns false @@ -146,8 +153,6 @@ bond_mode_from_string(enum bond_mode *balance, const char *s) *balance = BM_TCP; } else if (!strcmp(s, bond_mode_to_string(BM_SLB))) { *balance = BM_SLB; - } else if (!strcmp(s, bond_mode_to_string(BM_STABLE))) { - *balance = BM_STABLE; } else if (!strcmp(s, bond_mode_to_string(BM_AB))) { *balance = BM_AB; } else { @@ -164,8 +169,6 @@ bond_mode_to_string(enum bond_mode balance) { return "balance-tcp"; case BM_SLB: return "balance-slb"; - case BM_STABLE: - return "stable"; case BM_AB: return "active-backup"; } @@ -186,8 +189,8 @@ bond_create(const struct bond_settings *s) bond = xzalloc(sizeof *bond); hmap_init(&bond->slaves); bond->no_slaves_tag = tag_create_random(); - bond->stb_tag = tag_create_random(); bond->next_fake_iface_update = LLONG_MAX; + atomic_init(&bond->ref_cnt, 1); bond_reconfigure(bond, s); @@ -196,17 +199,39 @@ bond_create(const struct bond_settings *s) return bond; } +struct bond * +bond_ref(const struct bond *bond_) +{ + struct bond *bond = CONST_CAST(struct bond *, bond_); + + if (bond) { + int orig; + atomic_add(&bond->ref_cnt, 1, &orig); + ovs_assert(orig > 0); + } + return bond; +} + /* Frees 'bond'. */ void -bond_destroy(struct bond *bond) +bond_unref(struct bond *bond) { struct bond_slave *slave, *next_slave; + int orig; if (!bond) { return; } - hmap_remove(&all_bonds, &bond->hmap_node); + atomic_sub(&bond->ref_cnt, 1, &orig); + ovs_assert(orig > 0); + if (orig != 1) { + return; + } + + ovs_rwlock_wrlock(&rwlock); + hmap_remove(all_bonds, &bond->hmap_node); + ovs_rwlock_unlock(&rwlock); HMAP_FOR_EACH_SAFE (slave, next_slave, hmap_node, &bond->slaves) { hmap_remove(&bond->slaves, &slave->hmap_node); @@ -235,13 +260,14 @@ bond_reconfigure(struct bond *bond, const struct bond_settings *s) { bool revalidate = false; + ovs_rwlock_wrlock(&rwlock); if (!bond->name || strcmp(bond->name, s->name)) { if (bond->name) { - hmap_remove(&all_bonds, &bond->hmap_node); + hmap_remove(all_bonds, &bond->hmap_node); free(bond->name); } bond->name = xstrdup(s->name); - hmap_insert(&all_bonds, &bond->hmap_node, hash_string(bond->name, 0)); + hmap_insert(all_bonds, &bond->hmap_node, hash_string(bond->name, 0)); } bond->updelay = s->up_delay; @@ -255,12 +281,6 @@ bond_reconfigure(struct bond *bond, const struct bond_settings *s) if (bond->balance != s->balance) { bond->balance = s->balance; revalidate = true; - - if (bond->balance == BM_STABLE) { - VLOG_WARN_ONCE("Stable bond mode is deprecated and may be removed" - " in February 2013. Please email" - " dev@openvswitch.org with concerns."); - } } if (bond->basis != s->basis) { @@ -285,11 +305,13 @@ bond_reconfigure(struct bond *bond, const struct bond_settings *s) bond_entry_reset(bond); } + ovs_rwlock_unlock(&rwlock); return revalidate; } static void bond_slave_set_netdev__(struct bond_slave *slave, struct netdev *netdev) + OVS_REQ_WRLOCK(rwlock) { if (slave->netdev != netdev) { slave->netdev = netdev; @@ -302,20 +324,17 @@ bond_slave_set_netdev__(struct bond_slave *slave, struct netdev *netdev) * bond. If 'slave_' already exists within 'bond' then this function * reconfigures the existing slave. * - * 'stb_id' is used in BM_STABLE bonds to guarantee consistent slave choices - * across restarts and distributed vswitch instances. It should be unique per - * slave, and preferably consistent across restarts and reconfigurations. - * * 'netdev' must be the network device that 'slave_' represents. It is owned * by the client, so the client must not close it before either unregistering * 'slave_' or destroying 'bond'. */ void -bond_slave_register(struct bond *bond, void *slave_, uint32_t stb_id, - struct netdev *netdev) +bond_slave_register(struct bond *bond, void *slave_, struct netdev *netdev) { - struct bond_slave *slave = bond_slave_lookup(bond, slave_); + struct bond_slave *slave; + ovs_rwlock_wrlock(&rwlock); + slave = bond_slave_lookup(bond, slave_); if (!slave) { slave = xzalloc(sizeof *slave); @@ -330,15 +349,11 @@ bond_slave_register(struct bond *bond, void *slave_, uint32_t stb_id, bond_enable_slave(slave, netdev_get_carrier(netdev), NULL); } - if (slave->stb_id != stb_id) { - slave->stb_id = stb_id; - bond->bond_revalidate = true; - } - bond_slave_set_netdev__(slave, netdev); free(slave->name); slave->name = xstrdup(netdev_get_name(netdev)); + ovs_rwlock_unlock(&rwlock); } /* Updates the network device to be used with 'slave_' to 'netdev'. @@ -349,10 +364,14 @@ bond_slave_register(struct bond *bond, void *slave_, uint32_t stb_id, void bond_slave_set_netdev(struct bond *bond, void *slave_, struct netdev *netdev) { - struct bond_slave *slave = bond_slave_lookup(bond, slave_); + struct bond_slave *slave; + + ovs_rwlock_wrlock(&rwlock); + slave = bond_slave_lookup(bond, slave_); if (slave) { bond_slave_set_netdev__(slave, netdev); } + ovs_rwlock_unlock(&rwlock); } /* Unregisters 'slave_' from 'bond'. If 'bond' does not contain such a slave @@ -362,11 +381,13 @@ bond_slave_set_netdev(struct bond *bond, void *slave_, struct netdev *netdev) void bond_slave_unregister(struct bond *bond, const void *slave_) { - struct bond_slave *slave = bond_slave_lookup(bond, slave_); + struct bond_slave *slave; bool del_active; + ovs_rwlock_wrlock(&rwlock); + slave = bond_slave_lookup(bond, slave_); if (!slave) { - return; + goto out; } bond_enable_slave(slave, false, NULL); @@ -394,6 +415,8 @@ bond_slave_unregister(struct bond *bond, const void *slave_) bond_choose_active_slave(bond, &tags); bond->send_learning_packets = true; } +out: + ovs_rwlock_unlock(&rwlock); } /* Should be called on each slave in 'bond' before bond_run() to indicate @@ -404,7 +427,9 @@ bond_slave_unregister(struct bond *bond, const void *slave_) void bond_slave_set_may_enable(struct bond *bond, void *slave_, bool may_enable) { + ovs_rwlock_wrlock(&rwlock); bond_slave_lookup(bond, slave_)->may_enable = may_enable; + ovs_rwlock_unlock(&rwlock); } /* Performs periodic maintenance on 'bond'. The caller must provide 'tags' to @@ -416,6 +441,7 @@ bond_run(struct bond *bond, struct tag_set *tags, enum lacp_status lacp_status) { struct bond_slave *slave; + ovs_rwlock_wrlock(&rwlock); if (bond->lacp_status != lacp_status) { bond->lacp_status = lacp_status; bond->bond_revalidate = true; @@ -437,17 +463,12 @@ bond_run(struct bond *bond, struct tag_set *tags, enum lacp_status lacp_status) } if (bond->bond_revalidate) { - bond->bond_revalidate = false; + struct bond_slave *slave; + bond->bond_revalidate = false; bond_entry_reset(bond); - if (bond->balance != BM_STABLE) { - struct bond_slave *slave; - - HMAP_FOR_EACH (slave, hmap_node, &bond->slaves) { - tag_set_add(tags, slave->tag); - } - } else { - tag_set_add(tags, bond->stb_tag); + HMAP_FOR_EACH (slave, hmap_node, &bond->slaves) { + tag_set_add(tags, slave->tag); } tag_set_add(tags, bond->no_slaves_tag); } @@ -455,6 +476,7 @@ bond_run(struct bond *bond, struct tag_set *tags, enum lacp_status lacp_status) /* Invalidate any tags required by */ tag_set_union(tags, &bond->unixctl_tags); tag_set_init(&bond->unixctl_tags); + ovs_rwlock_unlock(&rwlock); } /* Causes poll_block() to wake up when 'bond' needs something to be done. */ @@ -463,6 +485,7 @@ bond_wait(struct bond *bond) { struct bond_slave *slave; + ovs_rwlock_rdlock(&rwlock); HMAP_FOR_EACH (slave, hmap_node, &bond->slaves) { if (slave->delay_expires != LLONG_MAX) { poll_timer_wait_until(slave->delay_expires); @@ -481,6 +504,7 @@ bond_wait(struct bond *bond) if (!tag_set_is_empty(&bond->unixctl_tags)) { poll_immediate_wake(); } + ovs_rwlock_unlock(&rwlock); /* We don't wait for bond->next_rebalance because rebalancing can only run * at a flow account checkpoint. ofproto does checkpointing on its own @@ -512,8 +536,12 @@ may_send_learning_packets(const struct bond *bond) bool bond_should_send_learning_packets(struct bond *bond) { - bool send = bond->send_learning_packets && may_send_learning_packets(bond); + bool send; + + ovs_rwlock_wrlock(&rwlock); + send = bond->send_learning_packets && may_send_learning_packets(bond); bond->send_learning_packets = false; + ovs_rwlock_unlock(&rwlock); return send; } @@ -532,11 +560,11 @@ bond_compose_learning_packet(struct bond *bond, tag_type tags = 0; struct flow flow; - assert(may_send_learning_packets(bond)); - + ovs_rwlock_rdlock(&rwlock); + ovs_assert(may_send_learning_packets(bond)); memset(&flow, 0, sizeof flow); memcpy(flow.dl_src, eth_src, ETH_ADDR_LEN); - slave = choose_output_slave(bond, &flow, vlan, &tags); + slave = choose_output_slave(bond, &flow, NULL, vlan, &tags); packet = ofpbuf_new(0); compose_rarp(packet, eth_src); @@ -545,6 +573,7 @@ bond_compose_learning_packet(struct bond *bond, } *port_aux = slave->aux; + ovs_rwlock_unlock(&rwlock); return packet; } @@ -567,7 +596,14 @@ enum bond_verdict bond_check_admissibility(struct bond *bond, const void *slave_, const uint8_t eth_dst[ETH_ADDR_LEN], tag_type *tags) { - struct bond_slave *slave = bond_slave_lookup(bond, slave_); + enum bond_verdict verdict = BV_DROP; + struct bond_slave *slave; + + ovs_rwlock_rdlock(&rwlock); + slave = bond_slave_lookup(bond, slave_); + if (!slave) { + goto out; + } /* LACP bonds have very loose admissibility restrictions because we can * assume the remote switch is aware of the bond and will "do the right @@ -578,16 +614,20 @@ bond_check_admissibility(struct bond *bond, const void *slave_, * If LACP is configured, but LACP negotiations have been unsuccessful, we * drop all incoming traffic. */ switch (bond->lacp_status) { - case LACP_NEGOTIATED: return slave->enabled ? BV_ACCEPT : BV_DROP; - case LACP_CONFIGURED: return BV_DROP; - case LACP_DISABLED: break; + case LACP_NEGOTIATED: + verdict = slave->enabled ? BV_ACCEPT : BV_DROP; + goto out; + case LACP_CONFIGURED: + goto out; + case LACP_DISABLED: + break; } /* Drop all multicast packets on inactive slaves. */ if (eth_addr_is_multicast(eth_dst)) { *tags |= bond_get_active_slave_tag(bond); - if (bond->active_slave != bond_slave_lookup(bond, slave_)) { - return BV_DROP; + if (bond->active_slave != slave) { + goto out; } } @@ -602,15 +642,16 @@ bond_check_admissibility(struct bond *bond, const void *slave_, VLOG_DBG_RL(&rl, "active-backup bond received packet on backup" " slave (%s) destined for " ETH_ADDR_FMT, slave->name, ETH_ADDR_ARGS(eth_dst)); - return BV_DROP; + goto out; } - return BV_ACCEPT; + verdict = BV_ACCEPT; + goto out; case BM_TCP: /* TCP balanced bonds require successful LACP negotiated. Based on the * above check, LACP is off on this bond. Therfore, we drop all * incoming traffic. */ - return BV_DROP; + goto out; case BM_SLB: /* Drop all packets for which we have learned a different input port, @@ -619,13 +660,15 @@ bond_check_admissibility(struct bond *bond, const void *slave_, * the host has moved to another switch. The exception to the * exception is if we locked the learning table to avoid reflections on * bond slaves. */ - return BV_DROP_IF_MOVED; - - case BM_STABLE: - return BV_ACCEPT; + verdict = BV_DROP_IF_MOVED; + goto out; } NOT_REACHED(); +out: + ovs_rwlock_unlock(&rwlock); + return verdict; + } /* Returns the slave (registered on 'bond' by bond_slave_register()) to which @@ -639,25 +682,35 @@ bond_check_admissibility(struct bond *bond, const void *slave_, * packet belongs to (so for an access port it will be the access port's VLAN). * * Adds a tag to '*tags' that associates the flow with the returned slave. + * + * If 'wc' is non-NULL, bitwise-OR's 'wc' with the set of bits that were + * significant in the selection. At some point earlier, 'wc' should + * have been initialized (e.g., by flow_wildcards_init_catchall()). */ void * bond_choose_output_slave(struct bond *bond, const struct flow *flow, - uint16_t vlan, tag_type *tags) + struct flow_wildcards *wc, uint16_t vlan, + tag_type *tags) { - struct bond_slave *slave = choose_output_slave(bond, flow, vlan, tags); + struct bond_slave *slave; + void *result = NULL; + + ovs_rwlock_rdlock(&rwlock); + slave = choose_output_slave(bond, flow, wc, vlan, tags); if (slave) { - *tags |= bond->balance == BM_STABLE ? bond->stb_tag : slave->tag; - return slave->aux; + *tags |= slave->tag; + result = slave->aux; } else { *tags |= bond->no_slaves_tag; - return NULL; } + ovs_rwlock_unlock(&rwlock); + return result; } /* Rebalancing. */ static bool -bond_is_balanced(const struct bond *bond) +bond_is_balanced(const struct bond *bond) OVS_REQ_RDLOCK(rwlock) { return bond->rebalance_interval && (bond->balance == BM_SLB || bond->balance == BM_TCP); @@ -668,13 +721,15 @@ void bond_account(struct bond *bond, const struct flow *flow, uint16_t vlan, uint64_t n_bytes) { + ovs_rwlock_wrlock(&rwlock); if (bond_is_balanced(bond)) { lookup_bond_entry(bond, flow, vlan)->tx_bytes += n_bytes; } + ovs_rwlock_unlock(&rwlock); } static struct bond_slave * -bond_slave_from_bal_node(struct list *bal) +bond_slave_from_bal_node(struct list *bal) OVS_REQ_RDLOCK(rwlock) { return CONTAINER_OF(bal, struct bond_slave, bal_node); } @@ -772,8 +827,12 @@ choose_entry_to_migrate(const struct bond_slave *from, uint64_t to_tx_bytes) delta = e->tx_bytes; old_ratio = (double)from->tx_bytes / to_tx_bytes; new_ratio = (double)(from->tx_bytes - delta) / (to_tx_bytes + delta); - if (old_ratio - new_ratio > 0.1) { - /* Would decrease the ratio, move it. */ + if (old_ratio - new_ratio > 0.1 + && fabs(new_ratio - 1.0) < fabs(old_ratio - 1.0)) { + /* We're aiming for an ideal ratio of 1, meaning both the 'from' + and 'to' slave have the same load. Therefore, we only move an + entry if it decreases the load on 'from', and brings us closer + to equal traffic load. */ return e; } } @@ -816,7 +875,9 @@ bond_rebalance(struct bond *bond, struct tag_set *tags) struct bond_entry *e; struct list bals; + ovs_rwlock_wrlock(&rwlock); if (!bond_is_balanced(bond) || time_msec() < bond->next_rebalance) { + ovs_rwlock_unlock(&rwlock); return; } bond->next_rebalance = time_msec() + bond->rebalance_interval; @@ -892,17 +953,18 @@ bond_rebalance(struct bond *bond, struct tag_set *tags) e->slave = NULL; } } + ovs_rwlock_unlock(&rwlock); } /* Bonding unixctl user interface functions. */ static struct bond * -bond_find(const char *name) +bond_find(const char *name) OVS_REQ_RDLOCK(rwlock) { struct bond *bond; HMAP_FOR_EACH_WITH_HASH (bond, hmap_node, hash_string(name, 0), - &all_bonds) { + all_bonds) { if (!strcmp(bond->name, name)) { return bond; } @@ -933,7 +995,8 @@ bond_unixctl_list(struct unixctl_conn *conn, ds_put_cstr(&ds, "bond\ttype\tslaves\n"); - HMAP_FOR_EACH (bond, hmap_node, &all_bonds) { + ovs_rwlock_rdlock(&rwlock); + HMAP_FOR_EACH (bond, hmap_node, all_bonds) { const struct bond_slave *slave; size_t i; @@ -949,12 +1012,14 @@ bond_unixctl_list(struct unixctl_conn *conn, } ds_put_char(&ds, '\n'); } + ovs_rwlock_unlock(&rwlock); unixctl_command_reply(conn, ds_cstr(&ds)); ds_destroy(&ds); } static void bond_print_details(struct ds *ds, const struct bond *bond) + OVS_REQ_RDLOCK(rwlock) { struct shash slave_shash = SHASH_INITIALIZER(&slave_shash); const struct shash_node **sorted_slaves = NULL; @@ -1046,24 +1111,28 @@ bond_unixctl_show(struct unixctl_conn *conn, { struct ds ds = DS_EMPTY_INITIALIZER; + ovs_rwlock_rdlock(&rwlock); if (argc > 1) { const struct bond *bond = bond_find(argv[1]); if (!bond) { unixctl_command_reply_error(conn, "no such bond"); - return; + goto out; } bond_print_details(&ds, bond); } else { const struct bond *bond; - HMAP_FOR_EACH (bond, hmap_node, &all_bonds) { + HMAP_FOR_EACH (bond, hmap_node, all_bonds) { bond_print_details(&ds, bond); } } unixctl_command_reply(conn, ds_cstr(&ds)); ds_destroy(&ds); + +out: + ovs_rwlock_unlock(&rwlock); } static void @@ -1079,33 +1148,34 @@ bond_unixctl_migrate(struct unixctl_conn *conn, struct bond_entry *entry; int hash; + ovs_rwlock_wrlock(&rwlock); bond = bond_find(bond_s); if (!bond) { unixctl_command_reply_error(conn, "no such bond"); - return; + goto out; } if (bond->balance != BM_SLB) { unixctl_command_reply_error(conn, "not an SLB bond"); - return; + goto out; } if (strspn(hash_s, "0123456789") == strlen(hash_s)) { hash = atoi(hash_s) & BOND_MASK; } else { unixctl_command_reply_error(conn, "bad hash"); - return; + goto out; } slave = bond_lookup_slave(bond, slave_s); if (!slave) { unixctl_command_reply_error(conn, "no such slave"); - return; + goto out; } if (!slave->enabled) { unixctl_command_reply_error(conn, "cannot migrate to disabled slave"); - return; + goto out; } entry = &bond->hash[hash]; @@ -1113,6 +1183,9 @@ bond_unixctl_migrate(struct unixctl_conn *conn, entry->slave = slave; entry->tag = tag_create_random(); unixctl_command_reply(conn, "migrated"); + +out: + ovs_rwlock_unlock(&rwlock); } static void @@ -1125,21 +1198,22 @@ bond_unixctl_set_active_slave(struct unixctl_conn *conn, struct bond *bond; struct bond_slave *slave; + ovs_rwlock_wrlock(&rwlock); bond = bond_find(bond_s); if (!bond) { unixctl_command_reply_error(conn, "no such bond"); - return; + goto out; } slave = bond_lookup_slave(bond, slave_s); if (!slave) { unixctl_command_reply_error(conn, "no such slave"); - return; + goto out; } if (!slave->enabled) { unixctl_command_reply_error(conn, "cannot make disabled slave active"); - return; + goto out; } if (bond->active_slave != slave) { @@ -1153,6 +1227,8 @@ bond_unixctl_set_active_slave(struct unixctl_conn *conn, } else { unixctl_command_reply(conn, "no change"); } +out: + ovs_rwlock_unlock(&rwlock); } static void @@ -1163,20 +1239,24 @@ enable_slave(struct unixctl_conn *conn, const char *argv[], bool enable) struct bond *bond; struct bond_slave *slave; + ovs_rwlock_wrlock(&rwlock); bond = bond_find(bond_s); if (!bond) { unixctl_command_reply_error(conn, "no such bond"); - return; + goto out; } slave = bond_lookup_slave(bond, slave_s); if (!slave) { unixctl_command_reply_error(conn, "no such slave"); - return; + goto out; } bond_enable_slave(slave, enable, &bond->unixctl_tags); unixctl_command_reply(conn, enable ? "enabled" : "disabled"); + +out: + ovs_rwlock_unlock(&rwlock); } static void @@ -1292,23 +1372,18 @@ bond_slave_lookup(struct bond *bond, const void *slave_) static void bond_enable_slave(struct bond_slave *slave, bool enable, struct tag_set *tags) { - struct bond *bond = slave->bond; slave->delay_expires = LLONG_MAX; if (enable != slave->enabled) { slave->enabled = enable; if (!slave->enabled) { - VLOG_WARN("interface %s: disabled", slave->name); + VLOG_INFO("interface %s: disabled", slave->name); if (tags) { tag_set_add(tags, slave->tag); } } else { - VLOG_WARN("interface %s: enabled", slave->name); + VLOG_INFO("interface %s: enabled", slave->name); slave->tag = tag_create_random(); } - - if (bond->balance == BM_STABLE) { - bond->bond_revalidate = true; - } } } @@ -1368,7 +1443,7 @@ bond_hash_tcp(const struct flow *flow, uint16_t vlan, uint32_t basis) static unsigned int bond_hash(const struct bond *bond, const struct flow *flow, uint16_t vlan) { - assert(bond->balance == BM_TCP || bond->balance == BM_SLB); + ovs_assert(bond->balance == BM_TCP || bond->balance == BM_SLB); return (bond->balance == BM_TCP ? bond_hash_tcp(flow, vlan, bond->basis) @@ -1382,38 +1457,9 @@ lookup_bond_entry(const struct bond *bond, const struct flow *flow, return &bond->hash[bond_hash(bond, flow, vlan) & BOND_MASK]; } -/* This function uses Highest Random Weight hashing to choose an output slave. - * This approach only reassigns a minimal number of flows when slaves are - * enabled or disabled. Unfortunately, it has O(n) performance against the - * number of slaves. There exist algorithms which are O(1), but have slightly - * more complex implementations and require the use of memory. This may need - * to be reimplemented if it becomes a performance bottleneck. */ -static struct bond_slave * -choose_stb_slave(const struct bond *bond, uint32_t flow_hash) -{ - struct bond_slave *best, *slave; - uint32_t best_hash; - - best = NULL; - best_hash = 0; - HMAP_FOR_EACH (slave, hmap_node, &bond->slaves) { - if (slave->enabled) { - uint32_t hash; - - hash = hash_2words(flow_hash, slave->stb_id); - if (!best || hash > best_hash) { - best = slave; - best_hash = hash; - } - } - } - - return best; -} - static struct bond_slave * choose_output_slave(const struct bond *bond, const struct flow *flow, - uint16_t vlan, tag_type *tags) + struct flow_wildcards *wc, uint16_t vlan, tag_type *tags) { struct bond_entry *e; @@ -1427,18 +1473,18 @@ choose_output_slave(const struct bond *bond, const struct flow *flow, case BM_AB: return bond->active_slave; - case BM_STABLE: - return choose_stb_slave(bond, bond_hash_tcp(flow, vlan, bond->basis)); - case BM_TCP: if (bond->lacp_status != LACP_NEGOTIATED) { /* Must have LACP negotiations for TCP balanced bonds. */ return NULL; } + if (wc) { + flow_mask_hash_fields(flow, wc, NX_HASH_FIELDS_SYMMETRIC_L4); + } /* Fall Through. */ case BM_SLB: - if (!bond_is_balanced(bond)) { - return choose_stb_slave(bond, bond_hash(bond, flow, vlan)); + if (wc) { + flow_mask_hash_fields(flow, wc, NX_HASH_FIELDS_ETH_SRC); } e = lookup_bond_entry(bond, flow, vlan); if (!e->slave || !e->slave->enabled) { @@ -1507,7 +1553,7 @@ bond_choose_active_slave(struct bond *bond, struct tag_set *tags) bond->send_learning_packets = true; } else if (old_active_slave) { - VLOG_WARN_RL(&rl, "bond %s: all interfaces disabled", bond->name); + VLOG_INFO_RL(&rl, "bond %s: all interfaces disabled", bond->name); } }