From bf83f7c82f28d920487fb00741e5b0b692d93d36 Mon Sep 17 00:00:00 2001 From: Ethan Jackson Date: Mon, 16 Apr 2012 14:55:58 -0700 Subject: [PATCH] lacp: Remove custom transmission intervals. Open vSwitch allowed users to set a custom LACP PDU transmission interval. This turned out to be an ill conceived idea which was more confusing than useful. This patch reverts Open vSwitch to the behavior supported in the LACP specification: two transmission intervals, fast and slow. Signed-off-by: Ethan Jackson --- lib/lacp.c | 55 ++++++++------------------------------------ lib/lacp.h | 9 +------- vswitchd/bridge.c | 14 +---------- vswitchd/vswitch.xml | 23 ++++++------------ 4 files changed, 19 insertions(+), 82 deletions(-) diff --git a/lib/lacp.c b/lib/lacp.c index 9eac4fe9f..89e70b756 100644 --- a/lib/lacp.c +++ b/lib/lacp.c @@ -98,8 +98,7 @@ struct lacp { struct hmap slaves; /* Slaves this LACP object controls. */ struct slave *key_slave; /* Slave whose ID will be the aggregation key. */ - enum lacp_time lacp_time; /* Fast, Slow or Custom LACP time. */ - long long int custom_time; /* LACP_TIME_CUSTOM transmission rate. */ + bool fast; /* True if using fast probe interval. */ bool negotiated; /* True if LACP negotiations were successful. */ bool update; /* True if lacp_update() needs to be called. */ bool heartbeat; /* LACP heartbeat mode. */ @@ -242,10 +241,7 @@ lacp_configure(struct lacp *lacp, const struct lacp_settings *s) } lacp->active = s->active; - lacp->lacp_time = s->lacp_time; - lacp->custom_time = (s->lacp_time == LACP_TIME_CUSTOM - ? MAX(TIME_UPDATE_INTERVAL, s->custom_time) - : 0); + lacp->fast = s->fast; } /* Returns true if 'lacp' is configured in active mode, false if 'lacp' is @@ -274,20 +270,8 @@ lacp_process_packet(struct lacp *lacp, const void *slave_, return; } - switch (lacp->lacp_time) { - case LACP_TIME_FAST: - tx_rate = LACP_FAST_TIME_TX; - break; - case LACP_TIME_SLOW: - tx_rate = LACP_SLOW_TIME_TX; - break; - case LACP_TIME_CUSTOM: - tx_rate = lacp->custom_time; - break; - default: NOT_REACHED(); - } - slave->status = LACP_CURRENT; + tx_rate = lacp->fast ? LACP_FAST_TIME_TX : LACP_SLOW_TIME_TX; timer_set_duration(&slave->rx, LACP_RX_MULTIPLIER * tx_rate); slave->ntt_actor = pdu->partner; @@ -453,13 +437,9 @@ lacp_run(struct lacp *lacp, lacp_send_pdu *send_pdu) compose_lacp_pdu(&actor, &slave->partner, &pdu); send_pdu(slave->aux, &pdu, sizeof pdu); - if (lacp->lacp_time == LACP_TIME_CUSTOM) { - duration = lacp->custom_time; - } else { - duration = (slave->partner.state & LACP_STATE_TIME - ? LACP_FAST_TIME_TX - : LACP_SLOW_TIME_TX); - } + duration = (slave->partner.state & LACP_STATE_TIME + ? LACP_FAST_TIME_TX + : LACP_SLOW_TIME_TX); timer_set_duration(&slave->tx, duration); } @@ -579,18 +559,11 @@ slave_set_defaulted(struct slave *slave) static void slave_set_expired(struct slave *slave) { - struct lacp *lacp = slave->lacp; - slave->status = LACP_EXPIRED; slave->partner.state |= LACP_STATE_TIME; slave->partner.state &= ~LACP_STATE_SYNC; - /* The spec says we should wait LACP_RX_MULTIPLIER * LACP_FAST_TIME_TX. - * This doesn't make sense when using custom times which can be much - * smaller than LACP_FAST_TIME. */ - timer_set_duration(&slave->rx, (lacp->lacp_time == LACP_TIME_CUSTOM - ? lacp->custom_time - : LACP_RX_MULTIPLIER * LACP_FAST_TIME_TX)); + timer_set_duration(&slave->rx, LACP_RX_MULTIPLIER * LACP_FAST_TIME_TX); } static void @@ -604,7 +577,7 @@ slave_get_actor(struct slave *slave, struct lacp_info *actor) state |= LACP_STATE_ACT; } - if (lacp->lacp_time != LACP_TIME_SLOW) { + if (lacp->fast) { state |= LACP_STATE_TIME; } @@ -793,18 +766,10 @@ lacp_print_details(struct ds *ds, struct lacp *lacp) ds_put_cstr(ds, "\n"); ds_put_cstr(ds, "\tlacp_time: "); - switch (lacp->lacp_time) { - case LACP_TIME_FAST: + if (lacp->fast) { ds_put_cstr(ds, "fast\n"); - break; - case LACP_TIME_SLOW: + } else { ds_put_cstr(ds, "slow\n"); - break; - case LACP_TIME_CUSTOM: - ds_put_format(ds, "custom (%lld)\n", lacp->custom_time); - break; - default: - ds_put_cstr(ds, "unknown\n"); } HMAP_FOR_EACH (slave, node, &lacp->slaves) { diff --git a/lib/lacp.h b/lib/lacp.h index 293fc454d..d40829843 100644 --- a/lib/lacp.h +++ b/lib/lacp.h @@ -23,12 +23,6 @@ /* LACP Protocol Implementation. */ -enum lacp_time { - LACP_TIME_FAST, /* LACP fast mode. */ - LACP_TIME_SLOW, /* LACP slow mode. */ - LACP_TIME_CUSTOM /* Nonstandard custom mode. */ -}; - enum lacp_status { LACP_NEGOTIATED, /* Successful LACP negotations. */ LACP_CONFIGURED, /* LACP is enabled but not negotiated. */ @@ -40,8 +34,7 @@ struct lacp_settings { uint8_t id[ETH_ADDR_LEN]; /* System ID. Must be nonzero. */ uint16_t priority; /* System priority. */ bool active; /* Active or passive mode? */ - enum lacp_time lacp_time; /* Probe rate. */ - long long int custom_time; /* Probe interval if LACP_TIME_CUSTOM. */ + bool fast; /* Fast or slow probe interval. */ bool heartbeat; /* Heartbeat mode. */ }; diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index 74cba8742..c7349a4dc 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -2825,7 +2825,6 @@ static struct lacp_settings * port_configure_lacp(struct port *port, struct lacp_settings *s) { const char *lacp_time, *system_id; - long long int custom_time; int priority; if (!enable_lacp(port, &s->active)) { @@ -2867,18 +2866,7 @@ port_configure_lacp(struct port *port, struct lacp_settings *s) lacp_time = ovsrec_port_get_other_config_value(port->cfg, "lacp-time", "slow"); - custom_time = atoi(lacp_time); - if (!strcmp(lacp_time, "fast")) { - s->lacp_time = LACP_TIME_FAST; - } else if (!strcmp(lacp_time, "slow")) { - s->lacp_time = LACP_TIME_SLOW; - } else if (custom_time > 0) { - s->lacp_time = LACP_TIME_CUSTOM; - s->custom_time = custom_time; - } else { - s->lacp_time = LACP_TIME_SLOW; - } - + s->fast = !strcasecmp(lacp_time, "fast"); return s; } diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml index ef019746c..61f472ad5 100644 --- a/vswitchd/vswitch.xml +++ b/vswitchd/vswitch.xml @@ -894,24 +894,15 @@ numerically lower priority. - +

The LACP timing which should be used on this . - Possible values are fast, slow and a - positive number of milliseconds. By default slow is - used. When configured to be fast LACP heartbeats are - requested at a rate of once per second causing connectivity - problems to be detected more quickly. In slow mode, - heartbeats are requested at a rate of once every 30 seconds. -

- -

- Users may manually set a heartbeat transmission rate to increase - the fault detection speed further. When manually set, OVS expects - the partner switch to be configured with the same transmission - rate. Manually setting lacp-time to something other - than fast or slow is not supported by the - LACP specification. + By default slow is used. When configured to be + fast LACP heartbeats are requested at a rate of once + per second causing connectivity problems to be detected more + quickly. In slow mode, heartbeats are requested at a + rate of once every 30 seconds.

-- 2.43.0