Only transmit when the LACP partner needs to be updated instead of
whenever it might need to be updated.
---
lib/lacp.c | 83 ++++++++++++++++++++++++++++++++++-------------------------
1 files changed, 48 insertions(+), 35 deletions(-)
diff --git a/lib/lacp.c b/lib/lacp.c
index b79c96b..020dd9c 100644
--- a/lib/lacp.c
+++ b/lib/lacp.c
@@ -65,8 +65,8 @@ struct slave {
enum slave_status status; /* Slave status. */
bool attached; /* Attached. Traffic may flow. */
bool enabled; /* Enabled. Traffic is flowing. */
- struct lacp_info actor; /* Actor information. */
struct lacp_info partner; /* Partner information. */
+ struct lacp_info ntt_actor; /* Used to decide if we Need To Transmit. */
long long int tx; /* Next message transmission time. */
long long int rx; /* Expected message receive time. */
};
@@ -78,7 +78,7 @@ static void lacp_update_attached(struct lacp *);
static void slave_destroy(struct slave *);
static void slave_set_defaulted(struct slave *);
static void slave_set_expired(struct slave *);
-static void slave_update_actor(struct slave *);
+static void slave_get_actor(struct slave *, struct lacp_info *actor);
static void slave_get_priority(struct slave *, struct lacp_info *priority);
static bool slave_may_tx(const struct slave *);
static struct slave *slave_lookup(const struct lacp *, const void *slave);
@@ -153,12 +153,7 @@ lacp_process_pdu(struct lacp *lacp, const void *slave_,
slave->status = LACP_CURRENT;
slave->rx = time_msec() + LACP_SLOW_TIME_RX;
- /* Check if our partner has incorrect information about our current state.
- * If so update them. */
- slave_update_actor(slave);
- if (memcmp(&slave->actor, &pdu->partner, sizeof pdu->partner)) {
- slave->tx = 0;
- }
+ slave->ntt_actor = pdu->partner;
/* Update our information about our partner if it's out of date. This may
* cause priorities to change so re-calculate attached status of all
@@ -208,7 +203,6 @@ lacp_slave_register(struct lacp *lacp, const void *slave_,
const char *name,
slave->port_id = port_id;
slave->port_priority = port_priority;
- slave->tx = 0;
lacp->update = true;
if (lacp->active || lacp->negotiated) {
@@ -235,7 +229,6 @@ lacp_slave_enable(struct lacp *lacp, void *slave_, bool
enabled)
if (slave->enabled != enabled) {
slave->enabled = enabled;
- slave->tx = 0;
}
}
@@ -246,7 +239,6 @@ lacp_slave_carrier_changed(const struct lacp *lacp, const
void *slave_)
{
struct slave *slave = slave_lookup(lacp, slave_);
- slave->tx = 0;
if (slave->status == LACP_CURRENT || slave->lacp->active) {
slave_set_expired(slave);
}
@@ -282,7 +274,6 @@ lacp_run(struct lacp *lacp)
slave_set_defaulted(slave);
}
}
- slave_update_actor(slave);
}
if (lacp->update) {
@@ -291,12 +282,35 @@ lacp_run(struct lacp *lacp)
HMAP_FOR_EACH (slave, node, &lacp->slaves) {
struct lacp_pdu pdu;
+ struct lacp_info actor;
+ uint8_t state_mask;
+ uint8_t actor_state;
- if (time_msec() < slave->tx || !slave_may_tx(slave)) {
+ slave_get_actor(slave, &actor);
+
+ if (!slave_may_tx(slave)) {
+ continue;
+ }
+
+ /* LACP specification dictates that we transmit whenever the actor and
+ * remote_actor differ in the following fields: Port, Port Priority,
+ * System, System Priority, Aggregation Key, Activity State, Timeout
+ * State, Sync State, and Aggregation State. */
+ state_mask = LACP_STATE_ACT | LACP_STATE_TIME
+ | LACP_STATE_SYNC | LACP_STATE_AGG;
+
+ actor_state = actor.state;
+ actor.state &= state_mask;
+ slave->ntt_actor.state &= state_mask;
+
+ if (time_msec() < slave->tx
+ && !memcmp(&actor, &slave->ntt_actor, sizeof actor)) {
continue;
}
- compose_lacp_pdu(&slave->actor, &slave->partner, &pdu);
+ actor.state = actor_state;
+ slave->ntt_actor = actor;
+ compose_lacp_pdu(&actor, &slave->partner, &pdu);
lacp->send_pdu(slave->aux, &pdu);
slave->tx = time_msec() +
@@ -344,7 +358,7 @@ lacp_update_attached(struct lacp *lacp)
/* XXX: In the future allow users to configure the expected system ID.
* For now just special case loopback. */
- if (eth_addr_equals(slave->partner.sys_id, slave->actor.sys_id)) {
+ if (eth_addr_equals(slave->partner.sys_id, slave->lacp->sys_id)) {
VLOG_WARN_RL(&rl, "slave %s: Loopback detected. Slave is "
"connected to its own bond", slave->name);
slave->attached = false;
@@ -406,7 +420,6 @@ slave_set_defaulted(struct slave *slave)
{
memset(&slave->partner, 0, sizeof slave->partner);
- slave->tx = 0;
slave->lacp->update = true;
slave->status = LACP_DEFAULTED;
}
@@ -419,11 +432,10 @@ slave_set_expired(struct slave *slave)
slave->partner.state &= ~LACP_STATE_SYNC;
slave->rx = time_msec() + LACP_FAST_TIME_RX;
- slave->tx = 0;
}
static void
-slave_update_actor(struct slave *slave)
+slave_get_actor(struct slave *slave, struct lacp_info *actor)
{
uint8_t state = 0;
@@ -451,12 +463,12 @@ slave_update_actor(struct slave *slave)
state |= LACP_STATE_COL | LACP_STATE_DIST;
}
- slave->actor.state = state;
- slave->actor.key = htons(slave->lacp->key_slave->port_id);
- slave->actor.port_priority = htons(slave->port_priority);
- slave->actor.port_id = htons(slave->port_id);
- slave->actor.sys_priority = htons(slave->lacp->sys_priority);
- memcpy(&slave->actor.sys_id, slave->lacp->sys_id, ETH_ADDR_LEN);
+ actor->state = state;
+ actor->key = htons(slave->lacp->key_slave->port_id);
+ actor->port_priority = htons(slave->port_priority);
+ actor->port_id = htons(slave->port_id);
+ actor->sys_priority = htons(slave->lacp->sys_priority);
+ memcpy(&actor->sys_id, slave->lacp->sys_id, ETH_ADDR_LEN);
}
/* Given 'slave', populates 'priority' with data representing its LACP link
@@ -470,15 +482,15 @@ slave_get_priority(struct slave *slave, struct lacp_info
*priority)
/* Choose the lacp_info of the higher priority system by comparing their
* system priorities and mac addresses. */
- actor_priority = ntohs(slave->actor.sys_priority);
+ actor_priority = slave->lacp->sys_priority;
partner_priority = ntohs(slave->partner.sys_priority);
if (actor_priority < partner_priority) {
- *priority = slave->actor;
+ slave_get_actor(slave, priority);
} else if (partner_priority < actor_priority) {
*priority = slave->partner;
- } else if (eth_addr_compare_3way(slave->actor.sys_id,
+ } else if (eth_addr_compare_3way(slave->lacp->sys_id,
slave->partner.sys_id) < 0) {
- *priority = slave->actor;
+ slave_get_actor(slave, priority);
} else {
*priority = slave->partner;
}
@@ -589,8 +601,9 @@ lacp_unixctl_show(struct unixctl_conn *conn,
HMAP_FOR_EACH (slave, node, &lacp->slaves) {
char *status;
+ struct lacp_info actor;
- slave_update_actor(slave);
+ slave_get_actor(slave, &actor);
switch (slave->status) {
case LACP_CURRENT:
status = "current";
@@ -612,17 +625,17 @@ lacp_unixctl_show(struct unixctl_conn *conn,
ds_put_format(&ds, "\tport_priority: %u\n", slave->port_priority);
ds_put_format(&ds, "\n\tactor sys_id: " ETH_ADDR_FMT "\n",
- ETH_ADDR_ARGS(slave->actor.sys_id));
+ ETH_ADDR_ARGS(actor.sys_id));
ds_put_format(&ds, "\tactor sys_priority: %u\n",
- ntohs(slave->actor.sys_priority));
+ ntohs(actor.sys_priority));
ds_put_format(&ds, "\tactor port_id: %u\n",
- ntohs(slave->actor.port_id));
+ ntohs(actor.port_id));
ds_put_format(&ds, "\tactor port_priority: %u\n",
- ntohs(slave->actor.port_priority));
+ ntohs(actor.port_priority));
ds_put_format(&ds, "\tactor key: %u\n",
- ntohs(slave->actor.key));
+ ntohs(actor.key));
ds_put_cstr(&ds, "\tactor state: ");
- ds_put_lacp_state(&ds, slave->actor.state);
+ ds_put_lacp_state(&ds, actor.state);
ds_put_cstr(&ds, "\n\n");
ds_put_format(&ds, "\tpartner sys_id: " ETH_ADDR_FMT "\n",
--
1.7.4.1
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
