[ovs-dev] [tags 2/3] bond: Stop using tags.
Ethan Jackson
ethan at nicira.com
Fri Aug 2 02:08:33 UTC 2013
This patch transitions bonding away from using tags as required by
future patches.
Signed-off-by: Ethan Jackson <ethan at nicira.com>
---
lib/bond.c | 156 +++++++++++++-----------------------------
lib/bond.h | 11 ++-
ofproto/ofproto-dpif-xlate.c | 5 +-
ofproto/ofproto-dpif.c | 12 +++-
4 files changed, 62 insertions(+), 122 deletions(-)
diff --git a/lib/bond.c b/lib/bond.c
index 03696b6..a0bb640 100644
--- a/lib/bond.c
+++ b/lib/bond.c
@@ -35,7 +35,6 @@
#include "packets.h"
#include "poll-loop.h"
#include "shash.h"
-#include "tag.h"
#include "timeval.h"
#include "unixctl.h"
#include "vlog.h"
@@ -51,7 +50,6 @@ 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<->facet association. */
struct list list_node; /* In bond_slave's 'entries' list. */
};
@@ -69,7 +67,6 @@ struct bond_slave {
long long delay_expires; /* Time after which 'enabled' may change. */
bool enabled; /* May be chosen for flows? */
bool may_enable; /* Client considers this slave bondable. */
- tag_type tag; /* Tag associated with this slave. */
/* Rebalancing info. Used only by bond_rebalance(). */
struct list bal_node; /* In bond_rebalance()'s 'bals' list. */
@@ -89,7 +86,6 @@ struct bond {
/* Bonding info. */
enum bond_mode balance; /* Balancing mode, one of BM_*. */
struct bond_slave *active_slave;
- tag_type no_slaves_tag; /* Tag for flows when all slaves disabled. */
int updelay, downdelay; /* Delay before slave goes up/down, in ms. */
enum lacp_status lacp_status; /* Status of LACP negotiations. */
bool bond_revalidate; /* True if flows need revalidation. */
@@ -104,11 +100,6 @@ struct bond {
/* Legacy compatibility. */
long long int next_fake_iface_update; /* LLONG_MAX if disabled. */
- /* Tag set saved for next bond_run(). This tag set is a kluge for cases
- * 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;
};
@@ -119,11 +110,11 @@ static struct hmap *const all_bonds OVS_GUARDED_BY(rwlock) = &all_bonds__;
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 *) OVS_REQ_WRLOCK(rwlock);
-static void bond_link_status_update(struct bond_slave *, struct tag_set *)
+static void bond_enable_slave(struct bond_slave *, bool enable)
+ OVS_REQ_WRLOCK(rwlock);
+static void bond_link_status_update(struct bond_slave *)
OVS_REQ_WRLOCK(rwlock);
-static void bond_choose_active_slave(struct bond *, struct tag_set *)
+static void bond_choose_active_slave(struct bond *)
OVS_REQ_WRLOCK(rwlock);;
static unsigned int bond_hash_src(const uint8_t mac[ETH_ADDR_LEN],
uint16_t vlan, uint32_t basis);
@@ -133,12 +124,10 @@ static struct bond_entry *lookup_bond_entry(const struct bond *,
const struct flow *,
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 *,
struct flow_wildcards *,
- uint16_t vlan, tag_type *tags)
+ uint16_t vlan)
OVS_REQ_RDLOCK(rwlock);
static void bond_update_fake_slave_stats(struct bond *)
OVS_REQ_RDLOCK(rwlock);
@@ -188,14 +177,10 @@ bond_create(const struct bond_settings *s)
bond = xzalloc(sizeof *bond);
hmap_init(&bond->slaves);
- bond->no_slaves_tag = tag_create_random();
bond->next_fake_iface_update = LLONG_MAX;
atomic_init(&bond->ref_cnt, 1);
bond_reconfigure(bond, s);
-
- tag_set_init(&bond->unixctl_tags);
-
return bond;
}
@@ -334,6 +319,8 @@ bond_slave_register(struct bond *bond, void *slave_, struct netdev *netdev)
struct bond_slave *slave;
ovs_rwlock_wrlock(&rwlock);
+ bond_entry_reset(bond);
+
slave = bond_slave_lookup(bond, slave_);
if (!slave) {
slave = xzalloc(sizeof *slave);
@@ -346,7 +333,7 @@ bond_slave_register(struct bond *bond, void *slave_, struct netdev *netdev)
bond->bond_revalidate = true;
slave->enabled = false;
- bond_enable_slave(slave, netdev_get_carrier(netdev), NULL);
+ bond_enable_slave(slave, netdev_get_carrier(netdev));
}
bond_slave_set_netdev__(slave, netdev);
@@ -385,12 +372,15 @@ bond_slave_unregister(struct bond *bond, const void *slave_)
bool del_active;
ovs_rwlock_wrlock(&rwlock);
+ bond_entry_reset(bond);
+
slave = bond_slave_lookup(bond, slave_);
if (!slave) {
goto out;
}
- bond_enable_slave(slave, false, NULL);
+ bond->bond_revalidate = true;
+ bond_enable_slave(slave, false);
del_active = bond->active_slave == slave;
if (bond->hash) {
@@ -409,10 +399,7 @@ bond_slave_unregister(struct bond *bond, const void *slave_)
free(slave);
if (del_active) {
- struct tag_set tags;
-
- tag_set_init(&tags);
- bond_choose_active_slave(bond, &tags);
+ bond_choose_active_slave(bond);
bond->send_learning_packets = true;
}
out:
@@ -432,14 +419,16 @@ bond_slave_set_may_enable(struct bond *bond, void *slave_, bool may_enable)
ovs_rwlock_unlock(&rwlock);
}
-/* Performs periodic maintenance on 'bond'. The caller must provide 'tags' to
- * allow tagged flows to be invalidated.
+/* Performs periodic maintenance on 'bond'.
+ *
+ * Returns true if the caller should revalidate its flows.
*
* The caller should check bond_should_send_learning_packets() afterward. */
-void
-bond_run(struct bond *bond, struct tag_set *tags, enum lacp_status lacp_status)
+bool
+bond_run(struct bond *bond, enum lacp_status lacp_status)
{
struct bond_slave *slave;
+ bool revalidate;
ovs_rwlock_wrlock(&rwlock);
if (bond->lacp_status != lacp_status) {
@@ -449,11 +438,11 @@ bond_run(struct bond *bond, struct tag_set *tags, enum lacp_status lacp_status)
/* Enable slaves based on link status and LACP feedback. */
HMAP_FOR_EACH (slave, hmap_node, &bond->slaves) {
- bond_link_status_update(slave, tags);
+ bond_link_status_update(slave);
slave->change_seq = netdev_change_seq(slave->netdev);
}
if (!bond->active_slave || !bond->active_slave->enabled) {
- bond_choose_active_slave(bond, tags);
+ bond_choose_active_slave(bond);
}
/* Update fake bond interface stats. */
@@ -462,21 +451,11 @@ bond_run(struct bond *bond, struct tag_set *tags, enum lacp_status lacp_status)
bond->next_fake_iface_update = time_msec() + 1000;
}
- if (bond->bond_revalidate) {
- struct bond_slave *slave;
-
- bond->bond_revalidate = false;
- bond_entry_reset(bond);
- HMAP_FOR_EACH (slave, hmap_node, &bond->slaves) {
- tag_set_add(tags, slave->tag);
- }
- tag_set_add(tags, bond->no_slaves_tag);
- }
-
- /* Invalidate any tags required by */
- tag_set_union(tags, &bond->unixctl_tags);
- tag_set_init(&bond->unixctl_tags);
+ revalidate = bond->bond_revalidate;
+ bond->bond_revalidate = false;
ovs_rwlock_unlock(&rwlock);
+
+ return revalidate;
}
/* Causes poll_block() to wake up when 'bond' needs something to be done. */
@@ -500,8 +479,7 @@ bond_wait(struct bond *bond)
poll_timer_wait_until(bond->next_fake_iface_update);
}
- /* Ensure that any saved tags get revalidated right away. */
- if (!tag_set_is_empty(&bond->unixctl_tags)) {
+ if (!bond->bond_revalidate) {
poll_immediate_wake();
}
ovs_rwlock_unlock(&rwlock);
@@ -557,14 +535,13 @@ bond_compose_learning_packet(struct bond *bond,
{
struct bond_slave *slave;
struct ofpbuf *packet;
- tag_type tags = 0;
struct flow flow;
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, NULL, vlan, &tags);
+ slave = choose_output_slave(bond, &flow, NULL, vlan);
packet = ofpbuf_new(0);
compose_rarp(packet, eth_src);
@@ -594,7 +571,7 @@ bond_compose_learning_packet(struct bond *bond,
*/
enum bond_verdict
bond_check_admissibility(struct bond *bond, const void *slave_,
- const uint8_t eth_dst[ETH_ADDR_LEN], tag_type *tags)
+ const uint8_t eth_dst[ETH_ADDR_LEN])
{
enum bond_verdict verdict = BV_DROP;
struct bond_slave *slave;
@@ -625,7 +602,6 @@ bond_check_admissibility(struct bond *bond, const void *slave_,
/* 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 != slave) {
goto out;
}
@@ -635,7 +611,6 @@ bond_check_admissibility(struct bond *bond, const void *slave_,
case BM_AB:
/* Drop all packets which arrive on backup slaves. This is similar to
* how Linux bonding handles active-backup bonds. */
- *tags |= bond_get_active_slave_tag(bond);
if (bond->active_slave != slave) {
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
@@ -681,30 +656,20 @@ out:
* nonzero only for trunk ports), whereas 'vlan' is the logical VLAN that the
* 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,
- struct flow_wildcards *wc, uint16_t vlan,
- tag_type *tags)
+ struct flow_wildcards *wc, uint16_t vlan)
{
struct bond_slave *slave;
- void *result = NULL;
ovs_rwlock_rdlock(&rwlock);
- slave = choose_output_slave(bond, flow, wc, vlan, tags);
- if (slave) {
- *tags |= slave->tag;
- result = slave->aux;
- } else {
- *tags |= bond->no_slaves_tag;
- }
+ slave = choose_output_slave(bond, flow, wc, vlan);
ovs_rwlock_unlock(&rwlock);
- return result;
+ return slave;
}
/* Rebalancing. */
@@ -772,8 +737,7 @@ log_bals(struct bond *bond, const struct list *bals)
/* Shifts 'hash' from its current slave to 'to'. */
static void
-bond_shift_load(struct bond_entry *hash, struct bond_slave *to,
- struct tag_set *set)
+bond_shift_load(struct bond_entry *hash, struct bond_slave *to)
{
struct bond_slave *from = hash->slave;
struct bond *bond = from->bond;
@@ -792,9 +756,7 @@ bond_shift_load(struct bond_entry *hash, struct bond_slave *to,
to->tx_bytes += delta;
/* Arrange for flows to be revalidated. */
- tag_set_add(set, hash->tag);
- hash->slave = to;
- hash->tag = tag_create_random();
+ bond->bond_revalidate = true;
}
/* Picks and returns a bond_entry to migrate from 'from' (the most heavily
@@ -869,7 +831,7 @@ reinsert_bal(struct list *bals, struct bond_slave *slave)
* The caller should have called bond_account() for each active flow, to ensure
* that flow data is consistently accounted at this point. */
void
-bond_rebalance(struct bond *bond, struct tag_set *tags)
+bond_rebalance(struct bond *bond)
{
struct bond_slave *slave;
struct bond_entry *e;
@@ -925,7 +887,7 @@ bond_rebalance(struct bond *bond, struct tag_set *tags)
* to move from 'from' to 'to'. */
e = choose_entry_to_migrate(from, to->tx_bytes);
if (e) {
- bond_shift_load(e, to, tags);
+ bond_shift_load(e, to);
/* Delete element from from->entries.
*
@@ -1179,9 +1141,8 @@ bond_unixctl_migrate(struct unixctl_conn *conn,
}
entry = &bond->hash[hash];
- tag_set_add(&bond->unixctl_tags, entry->tag);
+ bond->bond_revalidate = true;
entry->slave = slave;
- entry->tag = tag_create_random();
unixctl_command_reply(conn, "migrated");
out:
@@ -1217,9 +1178,8 @@ bond_unixctl_set_active_slave(struct unixctl_conn *conn,
}
if (bond->active_slave != slave) {
- tag_set_add(&bond->unixctl_tags, bond_get_active_slave_tag(bond));
+ bond->bond_revalidate = true;
bond->active_slave = slave;
- bond->active_slave->tag = tag_create_random();
VLOG_INFO("bond %s: active interface is now %s",
bond->name, slave->name);
bond->send_learning_packets = true;
@@ -1252,7 +1212,7 @@ enable_slave(struct unixctl_conn *conn, const char *argv[], bool enable)
goto out;
}
- bond_enable_slave(slave, enable, &bond->unixctl_tags);
+ bond_enable_slave(slave, enable);
unixctl_command_reply(conn, enable ? "enabled" : "disabled");
out:
@@ -1370,25 +1330,19 @@ bond_slave_lookup(struct bond *bond, const void *slave_)
}
static void
-bond_enable_slave(struct bond_slave *slave, bool enable, struct tag_set *tags)
+bond_enable_slave(struct bond_slave *slave, bool enable)
{
slave->delay_expires = LLONG_MAX;
if (enable != slave->enabled) {
+ slave->bond->bond_revalidate = true;
slave->enabled = enable;
- if (!slave->enabled) {
- VLOG_INFO("interface %s: disabled", slave->name);
- if (tags) {
- tag_set_add(tags, slave->tag);
- }
- } else {
- VLOG_INFO("interface %s: enabled", slave->name);
- slave->tag = tag_create_random();
- }
+ VLOG_INFO("interface %s: %s", slave->name,
+ slave->enabled ? "enabled" : "disabled");
}
}
static void
-bond_link_status_update(struct bond_slave *slave, struct tag_set *tags)
+bond_link_status_update(struct bond_slave *slave)
{
struct bond *bond = slave->bond;
bool up;
@@ -1418,7 +1372,7 @@ bond_link_status_update(struct bond_slave *slave, struct tag_set *tags)
}
if (time_msec() >= slave->delay_expires) {
- bond_enable_slave(slave, up, tags);
+ bond_enable_slave(slave, up);
}
}
@@ -1459,7 +1413,7 @@ lookup_bond_entry(const struct bond *bond, const struct flow *flow,
static struct bond_slave *
choose_output_slave(const struct bond *bond, const struct flow *flow,
- struct flow_wildcards *wc, uint16_t vlan, tag_type *tags)
+ struct flow_wildcards *wc, uint16_t vlan)
{
struct bond_entry *e;
@@ -1493,9 +1447,7 @@ choose_output_slave(const struct bond *bond, const struct flow *flow,
if (!e->slave->enabled) {
e->slave = bond->active_slave;
}
- e->tag = tag_create_random();
}
- *tags |= e->tag;
return e->slave;
default:
@@ -1529,7 +1481,7 @@ bond_choose_slave(const struct bond *bond)
}
static void
-bond_choose_active_slave(struct bond *bond, struct tag_set *tags)
+bond_choose_active_slave(struct bond *bond)
{
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
struct bond_slave *old_active_slave = bond->active_slave;
@@ -1544,11 +1496,7 @@ bond_choose_active_slave(struct bond *bond, struct tag_set *tags)
"remaining %lld ms updelay (since no interface was "
"enabled)", bond->name, bond->active_slave->name,
bond->active_slave->delay_expires - time_msec());
- bond_enable_slave(bond->active_slave, true, tags);
- }
-
- if (!old_active_slave) {
- tag_set_add(tags, bond->no_slaves_tag);
+ bond_enable_slave(bond->active_slave, true);
}
bond->send_learning_packets = true;
@@ -1557,16 +1505,6 @@ bond_choose_active_slave(struct bond *bond, struct tag_set *tags)
}
}
-/* Returns the tag for 'bond''s active slave, or 'bond''s no_slaves_tag if
- * there is no active slave. */
-static tag_type
-bond_get_active_slave_tag(const struct bond *bond)
-{
- return (bond->active_slave
- ? bond->active_slave->tag
- : bond->no_slaves_tag);
-}
-
/* Attempts to make the sum of the bond slaves' statistics appear on the fake
* bond interface. */
static void
diff --git a/lib/bond.h b/lib/bond.h
index 7190935..f80fead 100644
--- a/lib/bond.h
+++ b/lib/bond.h
@@ -21,7 +21,6 @@
#include <stdint.h>
#include "packets.h"
-#include "tag.h"
struct flow;
struct netdev;
@@ -69,7 +68,7 @@ void bond_slave_register(struct bond *, void *slave_, struct netdev *);
void bond_slave_set_netdev(struct bond *, void *slave_, struct netdev *);
void bond_slave_unregister(struct bond *, const void *slave);
-void bond_run(struct bond *, struct tag_set *, enum lacp_status);
+bool bond_run(struct bond *, enum lacp_status);
void bond_wait(struct bond *);
void bond_slave_set_may_enable(struct bond *, void *slave_, bool may_enable);
@@ -87,15 +86,13 @@ enum bond_verdict {
BV_DROP_IF_MOVED /* Drop if we've learned a different port. */
};
enum bond_verdict bond_check_admissibility(struct bond *, const void *slave_,
- const uint8_t eth_dst[ETH_ADDR_LEN],
- tag_type *);
+ const uint8_t dst[ETH_ADDR_LEN]);
void *bond_choose_output_slave(struct bond *, const struct flow *,
- struct flow_wildcards *, uint16_t vlan,
- tag_type *);
+ struct flow_wildcards *, uint16_t vlan);
/* Rebalancing. */
void bond_account(struct bond *, const struct flow *, uint16_t vlan,
uint64_t n_bytes);
-void bond_rebalance(struct bond *, struct tag_set *);
+void bond_rebalance(struct bond *);
#endif /* bond.h */
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 01f1a86..b665b07 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -773,8 +773,7 @@ output_normal(struct xlate_ctx *ctx, const struct xbundle *out_xbundle,
struct ofport_dpif *ofport;
ofport = bond_choose_output_slave(out_xbundle->bond, &ctx->xin->flow,
- &ctx->xout->wc, vid,
- &ctx->xout->tags);
+ &ctx->xout->wc, vid);
xport = ofport ? xport_lookup(ofport) : NULL;
if (!xport) {
@@ -902,7 +901,7 @@ is_admissible(struct xlate_ctx *ctx, struct xport *in_port,
struct mac_entry *mac;
switch (bond_check_admissibility(in_xbundle->bond, in_port->ofport,
- flow->dl_dst, &ctx->xout->tags)) {
+ flow->dl_dst)) {
case BV_ACCEPT:
break;
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index aa05d2a..2cf33b2 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -390,6 +390,7 @@ struct table_dpif {
enum revalidate_reason {
REV_RECONFIGURE = 1, /* Switch configuration changed. */
REV_STP, /* Spanning tree protocol port status change. */
+ REV_BOND, /* Bonding changed. */
REV_PORT_TOGGLED, /* Port enabled or disabled by CFM, LACP, ...*/
REV_FLOW_TABLE, /* Flow table changed. */
REV_MAC_LEARNING, /* Mac learning changed. */
@@ -397,6 +398,7 @@ enum revalidate_reason {
};
COVERAGE_DEFINE(rev_reconfigure);
COVERAGE_DEFINE(rev_stp);
+COVERAGE_DEFINE(rev_bond);
COVERAGE_DEFINE(rev_port_toggled);
COVERAGE_DEFINE(rev_flow_table);
COVERAGE_DEFINE(rev_mac_learning);
@@ -768,6 +770,7 @@ type_run(const char *type)
switch (backer->need_revalidate) {
case REV_RECONFIGURE: COVERAGE_INC(rev_reconfigure); break;
case REV_STP: COVERAGE_INC(rev_stp); break;
+ case REV_BOND: COVERAGE_INC(rev_bond); break;
case REV_PORT_TOGGLED: COVERAGE_INC(rev_port_toggled); break;
case REV_FLOW_TABLE: COVERAGE_INC(rev_flow_table); break;
case REV_MAC_LEARNING: COVERAGE_INC(rev_mac_learning); break;
@@ -2783,8 +2786,11 @@ bundle_run(struct ofbundle *bundle)
bond_slave_set_may_enable(bundle->bond, port, port->may_enable);
}
- bond_run(bundle->bond, &bundle->ofproto->backer->revalidate_set,
- lacp_status(bundle->lacp));
+ if (bond_run(bundle->bond, lacp_status(bundle->lacp))
+ && !bundle->ofproto->backer->need_revalidate) {
+ bundle->ofproto->backer->need_revalidate = REV_BOND;
+ }
+
if (bond_should_send_learning_packets(bundle->bond)) {
bundle_send_learning_packets(bundle);
}
@@ -4163,7 +4169,7 @@ expire(struct dpif_backer *backer)
HMAP_FOR_EACH (bundle, hmap_node, &ofproto->bundles) {
if (bundle->bond) {
- bond_rebalance(bundle->bond, &backer->revalidate_set);
+ bond_rebalance(bundle->bond);
}
}
}
--
1.7.9.5
More information about the dev
mailing list