bridge: Feed flow stats into learning table.

Update the learning table to keep it current with flows actually
going through the datapath.  This ensures that entries do not age
out when they are actively being used.

Feature #2209

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 1e55ad2..8e9739f 100644 (file)
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -1831,6 +1831,71 @@ compose_actions(struct bridge *br, const flow_t *flow, uint16_t vlan,
     }
 }
 
+/* Returns the effective vlan of a packet, taking into account both the
+ * 802.1Q header and implicitly tagged ports.  A value of 0 indicates that
+ * the packet is untagged and -1 indicates it has an invalid header and
+ * should be dropped. */
+static int flow_get_vlan(struct bridge *br, const flow_t *flow,
+                         struct port *in_port, bool have_packet)
+{
+    /* Note that dl_vlan of 0 and of OFP_VLAN_NONE both mean that the packet
+     * belongs to VLAN 0, so we should treat both cases identically.  (In the
+     * former case, the packet has an 802.1Q header that specifies VLAN 0,
+     * presumably to allow a priority to be specified.  In the latter case, the
+     * packet does not have any 802.1Q header.) */
+    int vlan = ntohs(flow->dl_vlan);
+    if (vlan == OFP_VLAN_NONE) {
+        vlan = 0;
+    }
+    if (in_port->vlan >= 0) {
+        if (vlan) {
+            /* XXX support double tagging? */
+            if (have_packet) {
+                static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+                VLOG_WARN_RL(&rl, "bridge %s: dropping VLAN %"PRIu16" tagged "
+                             "packet received on port %s configured with "
+                             "implicit VLAN %"PRIu16,
+                             br->name, ntohs(flow->dl_vlan),
+                             in_port->name, in_port->vlan);
+            }
+            return -1;
+        }
+        vlan = in_port->vlan;
+    } else {
+        if (!port_includes_vlan(in_port, vlan)) {
+            if (have_packet) {
+                static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+                VLOG_WARN_RL(&rl, "bridge %s: dropping VLAN %d tagged "
+                             "packet received on port %s not configured for "
+                             "trunking VLAN %d",
+                             br->name, vlan, in_port->name, vlan);
+            }
+            return -1;
+        }
+    }
+
+    return vlan;
+}
+
+static void
+update_learning_table(struct bridge *br, const flow_t *flow, int vlan,
+                      struct port *in_port)
+{
+    tag_type rev_tag = mac_learning_learn(br->ml, flow->dl_src,
+                                          vlan, in_port->port_idx);
+    if (rev_tag) {
+        /* The log messages here could actually be useful in debugging,
+         * so keep the rate limit relatively high. */
+        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(30,
+                                                                300);
+        VLOG_DBG_RL(&rl, "bridge %s: learned that "ETH_ADDR_FMT" is "
+                    "on port %s in VLAN %d",
+                    br->name, ETH_ADDR_ARGS(flow->dl_src),
+                    in_port->name, vlan);
+        ofproto_revalidate(br->ofproto, rev_tag);
+    }
+}
+
 static bool
 is_bcast_arp_reply(const flow_t *flow)
 {
@@ -1879,41 +1944,9 @@ process_flow(struct bridge *br, const flow_t *flow,
         return true;
     }
     in_port = in_iface->port;
-
-    /* Figure out what VLAN this packet belongs to.
-     *
-     * Note that dl_vlan of 0 and of OFP_VLAN_NONE both mean that the packet
-     * belongs to VLAN 0, so we should treat both cases identically.  (In the
-     * former case, the packet has an 802.1Q header that specifies VLAN 0,
-     * presumably to allow a priority to be specified.  In the latter case, the
-     * packet does not have any 802.1Q header.) */
-    vlan = ntohs(flow->dl_vlan);
-    if (vlan == OFP_VLAN_NONE) {
-        vlan = 0;
-    }
-    if (in_port->vlan >= 0) {
-        if (vlan) {
-            /* XXX support double tagging? */
-            if (packet != NULL) {
-                static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
-                VLOG_WARN_RL(&rl, "bridge %s: dropping VLAN %"PRIu16" tagged "
-                             "packet received on port %s configured with "
-                             "implicit VLAN %"PRIu16,
-                             br->name, ntohs(flow->dl_vlan),
-                             in_port->name, in_port->vlan);
-            }
-            goto done;
-        }
-        vlan = in_port->vlan;
-    } else {
-        if (!port_includes_vlan(in_port, vlan)) {
-            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
-            VLOG_WARN_RL(&rl, "bridge %s: dropping VLAN %d tagged "
-                         "packet received on port %s not configured for "
-                         "trunking VLAN %d",
-                         br->name, vlan, in_port->name, vlan);
-            goto done;
-        }
+    vlan = flow_get_vlan(br, flow, in_port, !!packet);
+    if (vlan < 0) {
+        goto done;
     }
 
     /* Drop frames for ports that STP wants entirely killed (both for
@@ -1964,19 +1997,7 @@ process_flow(struct bridge *br, const flow_t *flow,
     out_port = FLOOD_PORT;
     /* Learn source MAC (but don't try to learn from revalidation). */
     if (packet) {
-        tag_type rev_tag = mac_learning_learn(br->ml, flow->dl_src,
-                                              vlan, in_port->port_idx);
-        if (rev_tag) {
-            /* The log messages here could actually be useful in debugging,
-             * so keep the rate limit relatively high. */
-            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(30,
-                                                                    300);
-            VLOG_DBG_RL(&rl, "bridge %s: learned that "ETH_ADDR_FMT" is "
-                        "on port %s in VLAN %d",
-                        br->name, ETH_ADDR_ARGS(flow->dl_src),
-                        in_port->name, vlan);
-            ofproto_revalidate(br->ofproto, rev_tag);
-        }
+        update_learning_table(br, flow, vlan, in_port);
     }
 
     /* Determine output port. */
@@ -1984,10 +2005,12 @@ process_flow(struct bridge *br, const flow_t *flow,
                                            tags);
     if (out_port_idx >= 0 && out_port_idx < br->n_ports) {
         out_port = br->ports[out_port_idx];
-    } else if (!packet) {
+    } else if (!packet && !eth_addr_is_multicast(flow->dl_dst)) {
         /* If we are revalidating but don't have a learning entry then
-         * eject the flow.  Installing a flow that floods packets will
-         * prevent us from seeing future packets and learning properly. */
+         * eject the flow.  Installing a flow that floods packets opens
+         * up a window of time where we could learn from a packet reflected
+         * on a bond and blackhole packets before the learning table is
+         * updated to reflect the correct port. */
         return false;
     }
 
@@ -2068,17 +2091,29 @@ bridge_account_flow_ofhook_cb(const flow_t *flow,
                               void *br_)
 {
     struct bridge *br = br_;
+    struct port *in_port;
+    int vlan;
     const union odp_action *a;
 
+    /* Feed information from the active flows back into the learning table
+     * to ensure that table is always in sync with what is actually flowing
+     * through the datapath. */
+    in_port = port_from_dp_ifidx(br, flow->in_port);
+    vlan = flow_get_vlan(br, flow, in_port, false);
+    if (in_port && vlan >= 0) {
+        update_learning_table(br, flow, vlan, in_port);
+    }
+
     if (!br->has_bonded_ports) {
         return;
     }
 
     for (a = actions; a < &actions[n_actions]; a++) {
         if (a->type == ODPAT_OUTPUT) {
-            struct port *port = port_from_dp_ifidx(br, a->output.port);
-            if (port && port->n_ifaces >= 2) {
-                struct bond_entry *e = lookup_bond_entry(port, flow->dl_src);
+            struct port *out_port = port_from_dp_ifidx(br, a->output.port);
+            if (out_port && out_port->n_ifaces >= 2) {
+                struct bond_entry *e = lookup_bond_entry(out_port,
+                                                         flow->dl_src);
                 e->tx_bytes += n_bytes;
             }
         }