From: Ben Pfaff <blp@nicira.com>
Date: Tue, 28 Sep 2010 18:57:40 +0000 (-0700)
Subject: vswitchd: Better tolerate changes in datapath ports.
X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=784032d000c5be2cbbcf1a4779f398be503029c4;p=openvswitch

vswitchd: Better tolerate changes in datapath ports.

Until now, a command that removed and added ports in a single change to
the database, e.g.:
	ovs-vsctl del-port br0 vif1.0 -- add-port br0 vif2.0
typically failed, because of this sequence of events:

1. Bridge code removes vif1.0 from br0.
2. Bridge code adds vif2.0 to br0.
3. ofproto_run() receives kernel notification that vif1.0 was deleted, so
   it notifies the bridge by calling back to bridge_port_changed_ofhook_cb,
   which sees that it has an interface with the specified port number, and
   deletes it.  Oops--this is where the problem occurs.  For completeness:
4. ofproto_run() receives kernel notification that vif2.0 was added, so
   it notifies the bridge by calling back to ,
   which sees that it has no interface with the specified port number, and
   does nothing.

This commit fixes the problem by making bridge_port_changed_ofhook_cb() not
care about ports being dropped.  This is a corner case that we shouldn't
work too hard to care about, since it can only happen if an administrator
is meddling with datapaths using ovs-dpctl, and the consequences are simply
that packets directed to that device will take longer to be rerouted to
another device (it will take a while for the MAC learning table to time out
the entry).  Basically, the admin gets what he deserves.

Thanks to Jesse Gross for identifying the problem.

Bug #3671.
---

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 16621a9c..73e6d687 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -2563,29 +2563,20 @@ bridge_port_changed_ofhook_cb(enum ofp_port_reason reason,
     struct iface *iface;
     struct port *port;
 
+    if (reason == OFPPR_DELETE || !br->has_bonded_ports) {
+        return;
+    }
+
     iface = iface_from_dp_ifidx(br, ofp_port_to_odp_port(opp->port_no));
     if (!iface) {
         return;
     }
     port = iface->port;
 
-    if (reason == OFPPR_DELETE) {
-        VLOG_WARN("bridge %s: interface %s deleted unexpectedly",
-                  br->name, iface->name);
-        iface_destroy(iface);
-        if (!port->n_ifaces) {
-            VLOG_WARN("bridge %s: port %s has no interfaces, dropping",
-                      br->name, port->name);
-            port_destroy(port);
-        }
-
-        bridge_flush(br);
-    } else {
-        if (port->n_ifaces > 1) {
-            bool up = !(opp->state & OFPPS_LINK_DOWN);
-            bond_link_status_update(iface, up);
-            port_update_bond_compat(port);
-        }
+    if (port->n_ifaces > 1) {
+        bool up = !(opp->state & OFPPS_LINK_DOWN);
+        bond_link_status_update(iface, up);
+        port_update_bond_compat(port);
     }
 }