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 <ethan@nicira.com>

diff --git a/lib/lacp.c b/lib/lacp.c
index 9eac4fe..89e70b7 100644 (file)
--- 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 293fc45..d408298 100644 (file)
--- 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 74cba87..c7349a4 100644 (file)
--- 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 ef01974..61f472a 100644 (file)
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -894,24 +894,15 @@
           numerically lower priority.
         </column>
 
-        <column name="other_config" key="lacp-time">
+        <column name="other_config" key="lacp-time"
+          type='{"type": "string", "enum": ["set", ["fast", "slow"]]}'>
           <p>
             The LACP timing which should be used on this <ref table="Port"/>.
-            Possible values are <code>fast</code>, <code>slow</code> and a
-            positive number of milliseconds.  By default <code>slow</code> is
-            used.  When configured to be <code>fast</code> LACP heartbeats are
-            requested at a rate of once per second causing connectivity
-            problems to be detected more quickly.  In <code>slow</code> mode,
-            heartbeats are requested at a rate of once every 30 seconds.
-          </p>
-
-          <p>
-            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 <code>lacp-time</code> to something other
-            than <code>fast</code> or <code>slow</code> is not supported by the
-            LACP specification.
+            By default <code>slow</code> is used.  When configured to be
+            <code>fast</code> LACP heartbeats are requested at a rate of once
+            per second causing connectivity problems to be detected more
+            quickly.  In <code>slow</code> mode, heartbeats are requested at a
+            rate of once every 30 seconds.
           </p>
         </column>