[ovs-dev] Issue with OVS lacp-fallback-ab option

[Ben Pfaff]

The problem is that there are basically two separate implementations of
bonds, one that is based on recirculation and one that is not, and the
version that uses recirculation does not implement fallback to
active-backup.

The bonding code is old and not well organized and hard to understand
(and hard to test), so it's hard to add features without breaking
something.  I'm appending a patch that might do the job, for master.  I
think this code needs to be refactored overall, though.

--8<--------------------------cut here-------------------------->8--

diff --git a/ofproto/bond.c b/ofproto/bond.c
index f87cdba7908f..071309e27928 100644
--- a/ofproto/bond.c
+++ b/ofproto/bond.c
@@ -186,6 +186,7 @@ static struct bond_slave *choose_output_slave(const struct bond *,
                                               uint16_t vlan)
     OVS_REQ_RDLOCK(rwlock);
 static void update_recirc_rules__(struct bond *bond);
+static bool bond_is_falling_back_to_ab(const struct bond *);
 
 /* Attempts to parse 's' as the name of a bond balancing mode.  If successful,
  * stores the mode in '*balance' and returns true.  Otherwise returns false
@@ -648,6 +649,13 @@ bond_run(struct bond *bond, enum lacp_status lacp_status)
     if (bond->lacp_status != lacp_status) {
         bond->lacp_status = lacp_status;
         bond->bond_revalidate = true;
+
+        /* Change in LACP status can affect whether the bond is falling back to
+         * active-backup.  Make sure to create or destroy buckets if
+         * necessary.  */
+        if (bond_is_falling_back_to_ab(bond) || !bond->hash) {
+            bond_entry_reset(bond);
+        }
     }
 
     /* Enable slaves based on link status and LACP feedback. */
@@ -756,6 +764,13 @@ bond_compose_learning_packet(struct bond *bond, const struct eth_addr eth_src,
     return packet;
 }
 
+static bool
+bond_is_falling_back_to_ab(const struct bond *bond)
+{
+    return (bond->lacp_fallback_ab && bond->balance == BM_TCP
+            && bond->lacp_status == LACP_CONFIGURED);
+}
+
 /* Checks whether a packet that arrived on 'slave_' within 'bond', with an
  * Ethernet destination address of 'eth_dst', should be admitted.
  *
@@ -927,7 +942,8 @@ bond_recirculation_account(struct bond *bond)
 static bool
 bond_may_recirc(const struct bond *bond)
 {
-    return bond->balance == BM_TCP && bond->recirc_id;
+    return (bond->balance == BM_TCP && bond->recirc_id
+            && !bond_is_falling_back_to_ab(bond));
 }
 
 static void
@@ -1155,7 +1171,8 @@ bond_rebalance(struct bond *bond)
     bool use_recirc;
 
     ovs_rwlock_wrlock(&rwlock);
-    if (!bond_is_balanced(bond) || time_msec() < bond->next_rebalance) {
+    if (!bond_is_balanced(bond) || time_msec() < bond->next_rebalance
+        || bond_is_falling_back_to_ab(bond)) {
         goto done;
     }
     bond->next_rebalance = time_msec() + bond->rebalance_interval;
@@ -1645,7 +1662,7 @@ bond_init(void)
 static void
 bond_entry_reset(struct bond *bond)
 {
-    if (bond->balance != BM_AB) {
+    if (bond->balance != BM_AB && !bond_is_falling_back_to_ab(bond)) {
         size_t hash_len = BOND_BUCKETS * sizeof *bond->hash;
 
         if (!bond->hash) {