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 9eac4fe9f47ef5beba4766ec6b9e2960ed62cf17..89e70b75682e58f6e36c554f7a3b82c8ce22da35 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 293fc454d2f0d7ca1f9ae857ccf361949927b86a..d408298431833b5e0f992321332439dd89ea8d08 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 74cba8742e61d1128e31dbda06dc1f824b9f1a1f..c7349a4dc18602cf13243d2635e34753836c637c 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 ef019746cee4489b7392fb30e0b0dc6aabe1607c..61f472ad593bf6efc92ad045e08715e89833bb35 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>