[ovs-dev] [PATCH 1/1] Option to toggle "Require successful LACP negotiation when configured"

Ravi Kondamuru Ravi.Kondamuru at citrix.com
Fri Jun 28 16:58:23 UTC 2013


Commit bdebeece5 (lacp: Require successful LACP negotiations when
configured.) makes successful LACP negotiation mandatory.
This change makes it configurable. The user has the option to
set other-config:lacp-fallback-slb to true if pre 1.5.0 behavior
is needed. The switch will fallback to balance-slb when LACP
negotiation fails or is unsupported by the partner switch. The
default is false, requiring a successful LACP negotiation (which is
the current behavior).

Signed-off-by: Dominic Curran <dominic.curran at citrix.com>
---
 lib/bond.c           |   41 +++++++++++++++++++++++++++--------------
 lib/bond.h           |    1 +
 lib/lacp.c           |   15 +++++++++++++--
 lib/lacp.h           |    1 +
 vswitchd/bridge.c    |   17 +++++++++++++++++
 vswitchd/vswitch.xml |   19 +++++++++++++++++--
 6 files changed, 76 insertions(+), 18 deletions(-)

diff --git a/lib/bond.c b/lib/bond.c
index 68ac068..6365cd0 100644
--- a/lib/bond.c
+++ b/lib/bond.c
@@ -103,6 +103,7 @@ struct bond {
 
     /* Legacy compatibility. */
     long long int next_fake_iface_update; /* LLONG_MAX if disabled. */
+    bool lacp_fallback_slb; /* fallback to balance-slb on lacp failure */
 
     /* Tag set saved for next bond_run().  This tag set is a kluge for cases
      * where we can't otherwise provide revalidation feedback to the client.
@@ -238,6 +239,11 @@ bond_reconfigure(struct bond *bond, const struct bond_settings *s)
     bond->updelay = s->up_delay;
     bond->downdelay = s->down_delay;
 
+    if (bond->lacp_fallback_slb != s->lacp_fallback_slb_cfg) {
+        bond->lacp_fallback_slb = s->lacp_fallback_slb_cfg;
+        revalidate = true;
+    }
+
     if (bond->rebalance_interval != s->rebalance_interval) {
         bond->rebalance_interval = s->rebalance_interval;
         revalidate = true;
@@ -463,8 +469,9 @@ bond_wait(struct bond *bond)
 static bool
 may_send_learning_packets(const struct bond *bond)
 {
-    return bond->lacp_status == LACP_DISABLED
-        && (bond->balance == BM_SLB || bond->balance == BM_AB)
+    return ((bond->lacp_status == LACP_DISABLED
+        && (bond->balance == BM_SLB || bond->balance == BM_AB))
+        || (bond->lacp_fallback_slb && bond->lacp_status == LACP_CONFIGURED))
         && bond->active_slave;
 }
 
@@ -546,10 +553,12 @@ bond_check_admissibility(struct bond *bond, const void *slave_,
      * packets to them.
      *
      * If LACP is configured, but LACP negotiations have been unsuccessful, we
-     * drop all incoming traffic. */
+     * drop all incoming traffic except if lacp_fallback_slb is true. */
     switch (bond->lacp_status) {
     case LACP_NEGOTIATED: return slave->enabled ? BV_ACCEPT : BV_DROP;
-    case LACP_CONFIGURED: return BV_DROP;
+    case LACP_CONFIGURED:
+        if (!bond->lacp_fallback_slb)
+            return BV_DROP;
     case LACP_DISABLED: break;
     }
 
@@ -578,9 +587,11 @@ bond_check_admissibility(struct bond *bond, const void *slave_,
 
     case BM_TCP:
         /* TCP balanced bonds require successful LACP negotiated. Based on the
-         * above check, LACP is off on this bond.  Therfore, we drop all
-         * incoming traffic. */
-        return BV_DROP;
+         * above check, LACP is off or lacp_fallback_slb is true on this bond.
+         * If lacp_fallback_slb is true fall through to BM_SLB case else, we
+         * drop all incoming traffic. */
+       if (!bond->lacp_fallback_slb)
+            return BV_DROP;
 
     case BM_SLB:
         /* Drop all packets for which we have learned a different input port,
@@ -1359,9 +1370,9 @@ choose_output_slave(const struct bond *bond, const struct flow *flow,
 {
     struct bond_entry *e;
 
-    if (bond->lacp_status == LACP_CONFIGURED) {
+    if (bond->lacp_status == LACP_CONFIGURED && !bond->lacp_fallback_slb) {
         /* LACP has been configured on this bond but negotiations were
-         * unsuccussful.  Drop all traffic. */
+         * unsuccussful and lacp_fallback_slb not true.  Drop all traffic. */
         return NULL;
     }
 
@@ -1370,13 +1381,15 @@ choose_output_slave(const struct bond *bond, const struct flow *flow,
         return bond->active_slave;
 
     case BM_TCP:
-        if (bond->lacp_status != LACP_NEGOTIATED) {
-            /* Must have LACP negotiations for TCP balanced bonds. */
+        if (bond->lacp_status == LACP_NEGOTIATED) {
+            if (wc)
+                flow_mask_hash_fields(wc, NX_HASH_FIELDS_SYMMETRIC_L4);
+        } else if (!((bond->lacp_status == LACP_CONFIGURED)
+            && bond->lacp_fallback_slb)) {
+            /* Must have LACP negotiations for TCP balanced bonds or LACP
+             * Configured with lacp_fallback_slb set to true. */
             return NULL;
         }
-        if (wc) {
-            flow_mask_hash_fields(wc, NX_HASH_FIELDS_SYMMETRIC_L4);
-        }
         /* Fall Through. */
     case BM_SLB:
         if (wc) {
diff --git a/lib/bond.h b/lib/bond.h
index 306cf42..f466383 100644
--- a/lib/bond.h
+++ b/lib/bond.h
@@ -54,6 +54,7 @@ struct bond_settings {
 
     /* Legacy compatibility. */
     bool fake_iface;            /* Update fake stats for netdev 'name'? */
+    bool lacp_fallback_slb_cfg;	/* fallback to balance-slb on lacp failure */
 };
 
 /* Program startup. */
diff --git a/lib/lacp.c b/lib/lacp.c
index 8bc115d..e2b6e91 100644
--- a/lib/lacp.c
+++ b/lib/lacp.c
@@ -100,6 +100,7 @@ struct lacp {
     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 fallback_slb;       /* fallback to balance-slb on lacp failure */
 };
 
 struct slave {
@@ -238,6 +239,11 @@ lacp_configure(struct lacp *lacp, const struct lacp_settings *s)
 
     lacp->active = s->active;
     lacp->fast = s->fast;
+
+    if (lacp->fallback_slb != s->fallback_slb_cfg) {
+        lacp->fallback_slb = s->fallback_slb_cfg;
+        lacp->update = true;
+    }
 }
 
 /* Returns true if 'lacp' is configured in active mode, false if 'lacp' is
@@ -366,7 +372,9 @@ slave_may_enable__(struct slave *slave)
 {
     /* The slave may be enabled if it's attached to an aggregator and its
      * partner is synchronized.*/
-    return slave->attached && (slave->partner.state & LACP_STATE_SYNC);
+    return slave->attached && (slave->partner.state & LACP_STATE_SYNC
+        || (slave->lacp && slave->lacp->fallback_slb
+        && slave->status == LACP_DEFAULTED));
 }
 
 /* This function should be called before enabling 'slave_' to send or receive
@@ -483,6 +491,8 @@ lacp_update_attached(struct lacp *lacp)
         }
 
         if (slave->status == LACP_DEFAULTED) {
+            if (lacp->fallback_slb)
+                slave->attached = true;
             continue;
         }
 
@@ -499,7 +509,8 @@ lacp_update_attached(struct lacp *lacp)
 
     if (lead) {
         HMAP_FOR_EACH (slave, node, &lacp->slaves) {
-            if (lead->partner.key != slave->partner.key
+            if ((lacp->fallback_slb && slave->status == LACP_DEFAULTED)
+                || lead->partner.key != slave->partner.key
                 || !eth_addr_equals(lead->partner.sys_id,
                                     slave->partner.sys_id)) {
                 slave->attached = false;
diff --git a/lib/lacp.h b/lib/lacp.h
index 399b39e..21f2290 100644
--- a/lib/lacp.h
+++ b/lib/lacp.h
@@ -35,6 +35,7 @@ struct lacp_settings {
     uint16_t priority;                /* System priority. */
     bool active;                      /* Active or passive mode? */
     bool fast;                        /* Fast or slow probe interval. */
+    bool fallback_slb_cfg;            /* fallback to BM_SLB on lacp failure */
 };
 
 void lacp_init(void);
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 4ac2b26..4c9e698 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -234,6 +234,7 @@ static struct lacp_settings *port_configure_lacp(struct port *,
                                                  struct lacp_settings *);
 static void port_configure_bond(struct port *, struct bond_settings *);
 static bool port_is_synthetic(const struct port *);
+static bool set_lacp_fallback_slb_cfg(struct port *);
 
 static void reconfigure_system_stats(const struct ovsrec_open_vswitch *);
 static void run_system_stats(void);
@@ -3264,6 +3265,17 @@ enable_lacp(struct port *port, bool *activep)
     }
 }
 
+static bool set_lacp_fallback_slb_cfg(struct port *port)
+{
+    const char *lacp_fallback_s;
+
+    lacp_fallback_s = smap_get(&port->cfg->other_config, "lacp-fallback-slb");
+    if (lacp_fallback_s && !strcmp(lacp_fallback_s, "true"))
+        return true;
+
+    return false;
+}
+
 static struct lacp_settings *
 port_configure_lacp(struct port *port, struct lacp_settings *s)
 {
@@ -3302,6 +3314,9 @@ port_configure_lacp(struct port *port, struct lacp_settings *s)
 
     lacp_time = smap_get(&port->cfg->other_config, "lacp-time");
     s->fast = lacp_time && !strcasecmp(lacp_time, "fast");
+
+    s->fallback_slb_cfg = set_lacp_fallback_slb_cfg(port);
+
     return s;
 }
 
@@ -3390,6 +3405,8 @@ port_configure_bond(struct port *port, struct bond_settings *s)
 
     s->fake_iface = port->cfg->bond_fake_iface;
 
+    s->lacp_fallback_slb_cfg = set_lacp_fallback_slb_cfg(port);
+
     LIST_FOR_EACH (iface, port_elem, &port->ifaces) {
         netdev_set_miimon_interval(iface->netdev, miimon_interval);
     }
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 12780d6..2e894c5 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -925,7 +925,9 @@
 
       <p>
         The following modes require the upstream switch to support 802.3ad with
-        successful LACP negotiation:
+        successful LACP negotiation. If LACP negotiation fails and
+        other-config:lacp-fallback-slb is true, then <code>balance-slb</code>
+        style flow hashing is used:
       </p>
 
       <dl>
@@ -1015,7 +1017,8 @@
           in LACP negotiations initiated by a remote switch, but not allowed to
           initiate such negotiations themselves.  If LACP is enabled on a port
           whose partner switch does not support LACP, the bond will be
-          disabled.  Defaults to <code>off</code> if unset.
+          disabled, unless other-config:lacp-fallback-slb is set to true.
+          Defaults to <code>off</code> if unset.
         </column>
 
         <column name="other_config" key="lacp-system-id">
@@ -1043,6 +1046,18 @@
             rate of once every 30 seconds.
           </p>
         </column>
+
+        <column name="other_config" key="lacp-fallback-slb"
+          type='{"type": "boolean"}'>
+          <p>
+            Determines the behavior of openvswitch bond in LACP mode. If
+            the partner switch does not support LACP, setting this option
+            to <code>true</code> allows openvswitch to fallback to
+            balance-slb. If the option is set to <code>false</code>, the
+            bond will be disabled. In both the cases, once the partner switch
+            is configured to LACP mode then the bond will use LACP.
+          </p>
+        </column>
       </group>
 
       <group title="Rebalancing Configuration">
-- 
1.7.2.5




More information about the dev mailing list