[ovs-dev] [lacp 1/2] lacp: Remove custom transmission intervals.

Ethan Jackson ethan at nicira.com
Tue Apr 17 00:20:03 UTC 2012


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 at nicira.com>
---
 lib/lacp.c                 |   52 +++++++++-----------------------------------
 lib/lacp.h                 |    9 +-------
 vswitchd/bridge.c          |   14 +-----------
 vswitchd/vswitch.ovsschema |    4 ++--
 vswitchd/vswitch.xml       |   21 +++++-------------
 5 files changed, 20 insertions(+), 80 deletions(-)

diff --git a/lib/lacp.c b/lib/lacp.c
index 9eac4fe..0bfbf4c 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,8 +559,6 @@ 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;
@@ -588,9 +566,7 @@ slave_set_expired(struct slave *slave)
     /* 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 +580,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 +769,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
--- 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 22a3706..ca24d0d 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -2821,7 +2821,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)) {
@@ -2863,18 +2862,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.ovsschema b/vswitchd/vswitch.ovsschema
index c7e1ac9..7a51a0d 100644
--- a/vswitchd/vswitch.ovsschema
+++ b/vswitchd/vswitch.ovsschema
@@ -1,6 +1,6 @@
 {"name": "Open_vSwitch",
- "version": "6.9.0",
- "cksum": "617116616 16682",
+ "version": "6.9.1",
+ "cksum": "3226481229 16682",
  "tables": {
    "Open_vSwitch": {
      "columns": {
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 1128db9..2522ef5 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -897,21 +897,12 @@
         <column name="other_config" key="lacp-time">
           <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.
+            Possible values are <code>fast</code>, <code>slow</code>.  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>
 
-- 
1.7.9.6




More information about the dev mailing list