[ovs-dev] [PATCH] lacp: Select a may-enable IF as the lead IF

[Torgny Lindberg]

A reboot of one switch in an MC-LAG bond makes all bond links
to go down, causing a total connectivity loss for 3 seconds.

Packet capture shows that spurious LACP PDUs are sent to OVS with
a different MAC address (partner system id) during the final
stages of the MC-LAG switch reboot.

The current code selects a lead interface based on information
in the LACP PDU, regardless of its synchronization state. If a
non-synchronized interface is selected as the OVS lead interface
then all other interfaces are forced down as their stored partner
system id differs and the bond ends up with no working interface.
The bond recovers within three seconds after the last spurious
message.

To avoid the problem, this commit requires a lead interface
to be synchronized. In case no synchronized interface exists,
the selection of lead interface is done as in the current code.

Signed-off-by: Torgny Lindberg <torgny.lindberg at ericsson.com>
---
 lib/lacp.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/lib/lacp.c b/lib/lacp.c
index ad6ef8e..a5a169f 100644
--- a/lib/lacp.c
+++ b/lib/lacp.c
@@ -594,13 +594,14 @@ lacp_wait(struct lacp *lacp) OVS_EXCLUDED(mutex)
 static void
 lacp_update_attached(struct lacp *lacp) OVS_REQUIRES(mutex)
 {
-    struct slave *lead, *slave;
-    struct lacp_info lead_pri;
+    struct slave *lead, *enabled_lead, *slave;
+    struct lacp_info lead_pri, enabled_lead_pri;
     static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 10);
 
     lacp->update = false;
 
     lead = NULL;
+    enabled_lead = NULL;
     HMAP_FOR_EACH (slave, node, &lacp->slaves) {
         struct lacp_info pri;
 
@@ -628,6 +629,21 @@ lacp_update_attached(struct lacp *lacp) OVS_REQUIRES(mutex)
             lead = slave;
             lead_pri = pri;
         }
+
+        if (slave_may_enable__(slave)) {
+            if (!enabled_lead ||
+                memcmp(&pri, &enabled_lead_pri, sizeof pri) < 0) {
+                enabled_lead = slave;
+                enabled_lead_pri = pri;
+            }
+        }
+    }
+
+    /* Select the lead interface from available synchronized and
+     * enabled interfaces to avoid unwanted changes in partner
+     * system id that brings down all other links. */
+    if (enabled_lead) {
+        lead = enabled_lead;
     }
 
     lacp->negotiated = lead != NULL;
-- 
1.9.1