ofproto-dpif: Flush MACs for deleted ports from every bridge.

Consider this scenario: two hypervisors HV-1 and HV-2, connected to a
common physical network over SLB bonds.  Two virtual machines VM-1 and VM-2
are both running on HV-1.  Patch ports are in use, so that each VM is not
connected to a bridge with a physical Ethernet port but is actually one
virtual "hop" away across a patch port to a second OVS bridge.  VM-2 is
running a "ping" process directed at VM-1.

Now migrate VM-1 to HV-2.  Suppose that VM-1 fails to send out a gratuitous
ARP following migration, or that the gratuitous ARPs are lost, e.g. because
they are sent before the OpenFlow controller manages to populate the flow
table with rules to allow the VM's traffic

Now we are in a situation where HV-1 has learned that VM-1 is local and
HV-2 has learned that VM-1 is on its bond; both are wrong.  One would
expect the problem to resolve itself as soon the VM-1 sends out its first
packet.  However, SLB bonds (for important reasons documented in
vswitchd/INTERNALS) are very reluctant to learn that a currently local MAC
is actually on the bond: the only ways to learn that the MAC is on the bond
are to receive a gratuitous ARP (which we won't, since they were dropped)
or for the MAC learning entry to expire after 60 seconds. This means that
VM-1 can send out as much ordinary traffic as it wants (even ARP requests
and other broadcasts) but HV-1 will drop all of it at the physical Ethernet
since it believes that VM-1 is local.

(In an ordinary setup with a single bridge, HV-1 would have unlearned the
address for VM-1 when VM-1's port was deleted, but that didn't happen
because HV-1 only learned that VM-1 was on the patch port that leads to the
integration bridge.  The patch port didn't get deleted.)

HV-2 does quickly learn that VM-1 is now local.  SLB bonds are only
reluctant to learn that something they think is local is actually on the
bond, not the reverse.

This commit attempts to work around the problem by flushing the MAC
associated with a port from *every* bridge when a port is deleted.

This commit demonstrates yet another good reason not to use SLB bonds.

Build and unit tested only.
Bug #7978.
Bug #7687.

Signed-off-by: Ben Pfaff <blp@nicira.com>

1 files changed, 46 insertions, 9 deletions

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index c54cc5f4..ab14ca15 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -477,6 +477,7 @@ struct table_dpif {
 };
 
 struct ofproto_dpif {
+    struct hmap_node all_ofproto_dpifs_node; /* In 'all_ofproto_dpifs'. */
     struct ofproto up;
     struct dpif *dpif;
     int max_ports;
@@ -522,6 +523,9 @@ struct ofproto_dpif {
  * for debugging the asynchronous flow_mod implementation.) */
 static bool clogged;
 
+/* All existing ofproto_dpif instances, indexed by ->up.name. */
+static struct hmap all_ofproto_dpifs = HMAP_INITIALIZER(&all_ofproto_dpifs);
+
 static void ofproto_dpif_unixctl_init(void);
 
 static struct ofproto_dpif *
@@ -667,6 +671,9 @@ construct(struct ofproto *ofproto_, int *n_tablesp)
     hmap_init(&ofproto->vlandev_map);
     hmap_init(&ofproto->realdev_vid_map);
 
+    hmap_insert(&all_ofproto_dpifs, &ofproto->all_ofproto_dpifs_node,
+                hash_string(ofproto->up.name, 0));
+
     *n_tablesp = N_TABLES;
     return 0;
 }
@@ -691,6 +698,7 @@ destruct(struct ofproto *ofproto_)
     struct classifier *table;
     int i;
 
+    hmap_remove(&all_ofproto_dpifs, &ofproto->all_ofproto_dpifs_node);
     complete_operations(ofproto);
 
     OFPROTO_FOR_EACH_TABLE (table, &ofproto->up) {
@@ -1388,10 +1396,17 @@ set_queues(struct ofport *ofport_,
 
 /* Bundles. */
 
-/* Expires all MAC learning entries associated with 'port' and forces ofproto
- * to revalidate every flow. */
+/* Expires all MAC learning entries associated with 'bundle' and forces its
+ * ofproto to revalidate every flow.
+ *
+ * Normally MAC learning entries are removed only from the ofproto associated
+ * with 'bundle', but if 'all_ofprotos' is true, then the MAC learning entries
+ * are removed from every ofproto.  When patch ports and SLB bonds are in use
+ * and a VM migration happens and the gratuitous ARPs are somehow lost, this
+ * avoids a MAC_ENTRY_IDLE_TIME delay before the migrated VM can communicate
+ * with the host from which it migrated. */
 static void
-bundle_flush_macs(struct ofbundle *bundle)
+bundle_flush_macs(struct ofbundle *bundle, bool all_ofprotos)
 {
     struct ofproto_dpif *ofproto = bundle->ofproto;
     struct mac_learning *ml = ofproto->ml;
@@ -1400,6 +1415,23 @@ bundle_flush_macs(struct ofbundle *bundle)
     ofproto->need_revalidate = true;
     LIST_FOR_EACH_SAFE (mac, next_mac, lru_node, &ml->lrus) {
         if (mac->port.p == bundle) {
+            if (all_ofprotos) {
+                struct ofproto_dpif *o;
+
+                HMAP_FOR_EACH (o, all_ofproto_dpifs_node, &all_ofproto_dpifs) {
+                    if (o != ofproto) {
+                        struct mac_entry *e;
+
+                        e = mac_learning_lookup(o->ml, mac->mac, mac->vlan,
+                                                NULL);
+                        if (e) {
+                            tag_set_add(&o->revalidate_set, e->tag);
+                            mac_learning_expire(o->ml, e);
+                        }
+                    }
+                }
+            }
+
             mac_learning_expire(ml, mac);
         }
     }
@@ -1533,7 +1565,7 @@ bundle_destroy(struct ofbundle *bundle)
         bundle_del_port(port);
     }
 
-    bundle_flush_macs(bundle);
+    bundle_flush_macs(bundle, true);
     hmap_remove(&ofproto->bundles, &bundle->hmap_node);
     free(bundle->name);
     free(bundle->trunks);
@@ -1721,7 +1753,7 @@ bundle_set(struct ofproto *ofproto_, void *aux,
     /* If we changed something that would affect MAC learning, un-learn
      * everything on this port and force flow revalidation. */
     if (need_flush) {
-        bundle_flush_macs(bundle);
+        bundle_flush_macs(bundle, false);
     }
 
     return 0;
@@ -5464,10 +5496,15 @@ send_netflow_active_timeouts(struct ofproto_dpif *ofproto)
 static struct ofproto_dpif *
 ofproto_dpif_lookup(const char *name)
 {
-    struct ofproto *ofproto = ofproto_lookup(name);
-    return (ofproto && ofproto->ofproto_class == &ofproto_dpif_class
-            ? ofproto_dpif_cast(ofproto)
-            : NULL);
+    struct ofproto_dpif *ofproto;
+
+    HMAP_FOR_EACH_WITH_HASH (ofproto, all_ofproto_dpifs_node,
+                             hash_string(name, 0), &all_ofproto_dpifs) {
+        if (!strcmp(ofproto->up.name, name)) {
+            return ofproto;
+        }
+    }
+    return NULL;
 }
 
 static void