From ccd6957f394de1cc4cb9b3c0fbb1442ff503ba68 Mon Sep 17 00:00:00 2001
From: Ben Pfaff <blp@nicira.com>
Date: Mon, 29 Dec 2008 16:01:16 -0800
Subject: [PATCH] vswitchd: Revalidate all flows upon bridge configuration
 change.

Otherwise, now-invalid flows can linger, causing trouble.
---
 vswitchd/bridge.c | 58 ++++++++++++++++++++++++++++++++---------------
 1 file changed, 40 insertions(+), 18 deletions(-)

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index a99171fc..d58e635a 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -141,6 +141,7 @@ struct bridge {
     /* Flow tracking. */
     struct ft *ft;
     struct tag_set revalidate_set;
+    bool flush;
 
     /* Flow statistics gathering. */
     struct stats_mgr *stats_mgr;
@@ -352,6 +353,20 @@ bridge_wait(void)
         stats_mgr_wait(br->stats_mgr);
         flowstats_wait(br);
         bond_wait(br);
+        if (!tag_set_is_empty(&br->revalidate_set)) {
+            poll_immediate_wake();
+        }
+    }
+}
+
+/* Forces 'br' to revalidate all of its flows.  This is appropriate when 'br''s
+ * configuration changes.  */
+void
+bridge_flush(struct bridge *br)
+{
+    br->flush = true;
+    if (br->ml) {
+        mac_learning_flush(br->ml);
     }
 }
 
@@ -386,6 +401,8 @@ bridge_create(const char *name)
     port_array_init(&br->ifaces);
 
     br->ft = ft_create();
+    tag_set_init(&br->revalidate_set);
+    br->flush = false;
 
     br->stats_mgr = stats_mgr_create(br->rconn);
 
@@ -659,8 +676,6 @@ bridge_run_one(struct bridge *br)
         }
     }
 
-    tag_set_init(&br->revalidate_set);
-
     /* Now do the things that may want to revalidate flows. */
     for (iteration = 0; iteration < 50 && !bridge_is_backlogged(br);
          iteration++) {
@@ -679,19 +694,22 @@ bridge_run_one(struct bridge *br)
     flowstats_run(br);
     bond_run(br);
 
+    /* Start or restart secchan if necessary. */
+    run_secchan(br);
+
     /* Now revalidate any flows that need it. */
-    if (!tag_set_is_empty(&br->revalidate_set)) {
+    if (br->flush || !tag_set_is_empty(&br->revalidate_set)) {
         struct ft_flow *f, *next;
 
         HMAP_FOR_EACH_SAFE (f, next, struct ft_flow, node, &br->ft->flows) {
-            if (tag_set_intersects(&br->revalidate_set, f->tags)) {
+            if (br->flush
+                || tag_set_intersects(&br->revalidate_set, f->tags)) {
                 revalidate_flow(br, f);
             }
         }
     }
-
-    /* Start or restart secchan if necessary. */
-    run_secchan(br);
+    tag_set_init(&br->revalidate_set);
+    br->flush = false;
 }
 
 static void
@@ -1391,9 +1409,6 @@ process_flow(struct bridge *br, const struct flow *flow,
         uint16_t out_port_idx;
         bool may_learn;
 
-        /* XXX flush learning table entries when port indexes change due to
-         * reconfiguration */
-
         if (!pkt->buf) {
             /* Don't try to learn from revalidation. */
             may_learn = false;
@@ -1978,6 +1993,7 @@ port_create(struct bridge *br, const char *name)
     br->ports[br->n_ports++] = port;
 
     VLOG_WARN("created port %s on bridge %s", port->name, br->name);
+    bridge_flush(br);
 }
 
 static void
@@ -1985,6 +2001,7 @@ port_reconfigure(struct port *port)
 {
     bool bonded = cfg_has_section("bonding.%s", port->name);
     struct svec old_ifaces, new_ifaces;
+    int vlan;
     size_t i;
 
     /* Collect old and new interfaces. */
@@ -2032,18 +2049,16 @@ port_reconfigure(struct port *port)
     }
 
     /* Get VLAN tag. */
-    port->vlan = 0;
+    vlan = 0;
     if (cfg_has("vlan.%s.tag", port->name)) {
         if (!bonded) {
-            int vlan = cfg_get_int(0, "vlan.%s.tag", port->name);
-            if (port->vlan >= 0 && port->vlan <= 4095) {
-                VLOG_DBG("port %s: assigning VLAN tag %d",
-                         port->name, port->vlan);
-                port->vlan = vlan;
+            vlan = cfg_get_int(0, "vlan.%s.tag", port->name);
+            if (vlan >= 0 && vlan <= 4095) {
+                VLOG_DBG("port %s: assigning VLAN tag %d", port->name, vlan);
             } else {
                 VLOG_WARN("port %s: ignoring VLAN tag %d because it is not in "
-                          "the valid range from 0 to 4095",
-                          port->name, port->vlan);
+                          "the valid range from 0 to 4095", port->name, vlan);
+                vlan = 0;
             }
         } else {
             /* It's possible that bonded, VLAN-tagged ports make sense.  Maybe
@@ -2052,6 +2067,10 @@ port_reconfigure(struct port *port)
                       port->name);
         }
     }
+    if (port->vlan != vlan) {
+        port->vlan = vlan;
+        bridge_flush(port->bridge);
+    }
     svec_destroy(&old_ifaces);
     svec_destroy(&new_ifaces);
 }
@@ -2072,6 +2091,7 @@ port_destroy(struct port *port)
         }
         free(port->ifaces);
         free(port);
+        bridge_flush(br);
     }
 }
 
@@ -2128,6 +2148,7 @@ iface_create(struct port *port, const char *name)
     VLOG_DBG("attached network device %s to port %s", iface->name, port->name);
 
     port_update_bonding(port);
+    bridge_flush(port->bridge);
 }
 
 static void
@@ -2140,6 +2161,7 @@ iface_destroy(struct iface *iface)
         free(iface);
 
         port_update_bonding(port);
+        bridge_flush(port->bridge);
     }
 }
 
-- 
2.30.2