bridge: Don't learn from inadmissible flows when revising learning table.

Various kinds of flows are inadmissible and must be dropped.  Most notably,
OVS drops packets received on a bond whose destinations are ones that OVS
has already learned on a different port.  As the comment says:

         /* Drop all packets for which we have learned a different input
          * port, because we probably sent the packet on one slave and got
          * it back on the other.  Broadcast ARP replies are an exception
          * to this rule: the host has moved to another switch. */

As an important side effect of dropping these packets, OVS does not use
them for MAC learning when it sets up the corresponding flows.

However, OVS also periodically scans the datapath flow table and uses
information about flow activity to update its learning tables.  (Otherwise,
learning table entries could expire because no new flows were being set up,
even though active flows existed.)  This process, implemented in
bridge_account_flow_ofhook_cb(), did not check for admissibility, so
packets received on a bond could be used for learning even though another
port had already been learned.

This commit fixes the problem by making bridge_account_flow_ofhook_cb()
check for admissibility.

QA notes: Reproducing this problem requires some care and some luck.  One
way is to have two VMs with network interfaces on a single bonded network.
Both bonded interfaces must be up (otherwise packets sent out on one slave
will never be received on the other).  The problem will also not occur if
the physical switch that the bond slaves are plugged into has learned the
MAC address of the VMs involved (because the physical switch will then,
again, drop the packets without sending them back in on the other slave).
Finally, there needs to be some luck in timing and perhaps with the OVS
internal hash function also.

(One way to reproduce it reliably is to plug a pair of Ethernet ports into
each other with a cable, without an intermediate switch, and then use that
pair of ports as a bond.  Then every packet sent out on one will
immediately be received on the other, triggering the problem fairly often.
If this doesn't work at first, try changing the Ethernet address used on
one side or the other.)

To verify that the problem being observed is the one fixed by this commit,
turn on bridge debugging with "ovs-appctl vlog/set bridge:file" and look
for "bridge xapi2: learned that 00:01:02:03:04:06 is on port bond0 in VLAN
0" where 00:01:02:03:04:06 is a VM's Ethernet address and bond0 is the
name of the bond in the ovs-vswitchd log file.

Testing: I ran the "loopback bond" test above with and without this commit,
twice each in case I was just lucky.

CC: Henrik Amren <henrik@nicira.com>
Bug #2366.
Bug NIC-64.
Bug NIC-69.

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index d33944a6eff7fd817370f2572d290c57dbe2d94d..2d6052569951606f8de5ffc347848baab8e970ff 100644 (file)
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -2380,18 +2380,16 @@ bridge_account_flow_ofhook_cb(const flow_t *flow,
                               void *br_)
 {
     struct bridge *br = br_;
-    struct port *in_port;
     const union odp_action *a;
+    struct port *in_port;
+    tag_type tags = 0;
+    int vlan;
 
     /* 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);
-    if (in_port) {
-        int vlan = flow_get_vlan(br, flow, in_port, false);
-         if (vlan >= 0) {
-            update_learning_table(br, flow, vlan, in_port);
-        }
+    if (is_admissible(br, flow, false, &tags, &vlan, &in_port)) {
+        update_learning_table(br, flow, vlan, in_port);
     }
 
     if (!br->has_bonded_ports) {