OFAgent: Avoid re-wiring ports unnecessarily

Port the following patch to OFAgent.
commit: a9050757d94009c68f06c9f374de33eefbf06aa9
https://review.openstack.org/#/c/63558/

Partial-Bug: 1293265

Change-Id: I35d58bad104b01136c2502c933475ac85a3752a5

diff --git a/neutron/plugins/ofagent/agent/ofa_neutron_agent.py b/neutron/plugins/ofagent/agent/ofa_neutron_agent.py
index c6e65e5b6a42df8f4231eda6e299b8848618bcd2..2cd0a4708f54ccf3250a7cc98c664a6c43e0b3ac 100644 (file)
--- a/neutron/plugins/ofagent/agent/ofa_neutron_agent.py
+++ b/neutron/plugins/ofagent/agent/ofa_neutron_agent.py
@@ -654,19 +654,21 @@ class OFANeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin):
                                       physical_network, segmentation_id)
         lvm = self.local_vlan_map[net_uuid]
         lvm.vif_ports[port.vif_id] = port
-
-        self.int_br.set_db_attribute("Port", port.port_name, "tag",
-                                     str(lvm.vlan))
-        if int(port.ofport) != -1:
-            match = self.int_br.ofparser.OFPMatch(in_port=port.ofport)
-            msg = self.int_br.ofparser.OFPFlowMod(
-                self.int_br.datapath,
-                table_id=ryu_ofp13.OFPTT_ALL,
-                command=ryu_ofp13.OFPFC_DELETE,
-                out_group=ryu_ofp13.OFPG_ANY,
-                out_port=ryu_ofp13.OFPP_ANY,
-                match=match)
-            self.ryu_send_msg(msg)
+        # Do not bind a port if it's already bound
+        cur_tag = self.int_br.db_get_val("Port", port.port_name, "tag")
+        if cur_tag != str(lvm.vlan):
+            self.int_br.set_db_attribute("Port", port.port_name, "tag",
+                                         str(lvm.vlan))
+            if int(port.ofport) != -1:
+                match = self.int_br.ofparser.OFPMatch(in_port=port.ofport)
+                msg = self.int_br.ofparser.OFPFlowMod(
+                    self.int_br.datapath,
+                    table_id=ryu_ofp13.OFPTT_ALL,
+                    command=ryu_ofp13.OFPFC_DELETE,
+                    out_group=ryu_ofp13.OFPG_ANY,
+                    out_port=ryu_ofp13.OFPP_ANY,
+                    match=match)
+                self.ryu_send_msg(msg)
 
     def port_unbound(self, vif_id, net_uuid=None):
         """Unbind port.
@@ -695,12 +697,15 @@ class OFANeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin):
 
         :param port: a ovs_lib.VifPort object.
         """
-        self.int_br.set_db_attribute("Port", port.port_name, "tag",
-                                     DEAD_VLAN_TAG)
-        match = self.int_br.ofparser.OFPMatch(in_port=int(port.ofport))
-        msg = self.int_br.ofparser.OFPFlowMod(self.int_br.datapath,
-                                              priority=2, match=match)
-        self.ryu_send_msg(msg)
+        # Don't kill a port if it's already dead
+        cur_tag = self.int_br.db_get_val("Port", port.port_name, "tag")
+        if cur_tag != DEAD_VLAN_TAG:
+            self.int_br.set_db_attribute("Port", port.port_name, "tag",
+                                         DEAD_VLAN_TAG)
+            match = self.int_br.ofparser.OFPMatch(in_port=port.ofport)
+            msg = self.int_br.ofparser.OFPFlowMod(self.int_br.datapath,
+                                                  priority=2, match=match)
+            self.ryu_send_msg(msg)
 
     def setup_integration_br(self):
         """Setup the integration bridge.

diff --git a/neutron/tests/unit/ofagent/test_ofa_neutron_agent.py b/neutron/tests/unit/ofagent/test_ofa_neutron_agent.py
index 71afcd213d5b1bbca3ed71410613e0f26ae207f4..271ef33901a9d9d34d19e0eabafdc56339e1fcd0 100644 (file)
--- a/neutron/tests/unit/ofagent/test_ofa_neutron_agent.py
+++ b/neutron/tests/unit/ofagent/test_ofa_neutron_agent.py
@@ -243,34 +243,71 @@ class TestOFANeutronAgent(OFAAgentTestCase):
 
         self.agent.sg_agent = mock.Mock()
 
-    def _mock_port_bound(self, ofport=None):
+    def _mock_port_bound(self, ofport=None, new_local_vlan=None,
+                         old_local_vlan=None):
         port = mock.Mock()
         port.ofport = ofport
         net_uuid = 'my-net-uuid'
-        with mock.patch.object(self.mod_agent.OVSBridge,
-                               'set_db_attribute',
-                               return_value=True):
-            with mock.patch.object(self.agent,
-                                   'ryu_send_msg') as ryu_send_msg_func:
-                self.agent.port_bound(port, net_uuid, 'local', None, None)
-        self.assertEqual(ryu_send_msg_func.called, ofport != -1)
+        if old_local_vlan is not None:
+            self.agent.local_vlan_map[net_uuid] = (
+                self.mod_agent.LocalVLANMapping(
+                    old_local_vlan, None, None, None))
+        with contextlib.nested(
+            mock.patch.object(self.mod_agent.OVSBridge,
+                              'set_db_attribute', return_value=True),
+            mock.patch.object(self.mod_agent.OVSBridge,
+                              'db_get_val', return_value=str(old_local_vlan)),
+            mock.patch.object(self.agent, 'ryu_send_msg')
+        ) as (set_ovs_db_func, get_ovs_db_func, ryu_send_msg_func):
+            self.agent.port_bound(port, net_uuid, 'local', None, None)
+        get_ovs_db_func.assert_called_once_with("Port", mock.ANY, "tag")
+        if new_local_vlan != old_local_vlan:
+            set_ovs_db_func.assert_called_once_with(
+                "Port", mock.ANY, "tag", str(new_local_vlan))
+            if ofport != -1:
+                ryu_send_msg_func.assert_called_once_with(
+                    self.ofparser.OFPFlowMod.return_value)
+            else:
+                self.assertFalse(ryu_send_msg_func.called)
+        else:
+            self.assertFalse(set_ovs_db_func.called)
+            self.assertFalse(ryu_send_msg_func.called)
 
     def test_port_bound_deletes_flows_for_valid_ofport(self):
-        self._mock_port_bound(ofport=1)
+        self._mock_port_bound(ofport=1, new_local_vlan=1)
 
     def test_port_bound_ignores_flows_for_invalid_ofport(self):
-        self._mock_port_bound(ofport=-1)
+        self._mock_port_bound(ofport=-1, new_local_vlan=1)
+
+    def test_port_bound_does_not_rewire_if_already_bound(self):
+        self._mock_port_bound(ofport=-1, new_local_vlan=1, old_local_vlan=1)
+
+    def _test_port_dead(self, cur_tag=None):
+        port = mock.Mock()
+        port.ofport = 1
+        with contextlib.nested(
+            mock.patch.object(self.mod_agent.OVSBridge,
+                              'set_db_attribute', return_value=True),
+            mock.patch.object(self.mod_agent.OVSBridge,
+                              'db_get_val', return_value=cur_tag),
+            mock.patch.object(self.agent, 'ryu_send_msg')
+        ) as (set_ovs_db_func, get_ovs_db_func, ryu_send_msg_func):
+            self.agent.port_dead(port)
+        get_ovs_db_func.assert_called_once_with("Port", mock.ANY, "tag")
+        if cur_tag == self.mod_agent.DEAD_VLAN_TAG:
+            self.assertFalse(set_ovs_db_func.called)
+            self.assertFalse(ryu_send_msg_func.called)
+        else:
+            set_ovs_db_func.assert_called_once_with(
+                "Port", mock.ANY, "tag", str(self.mod_agent.DEAD_VLAN_TAG))
+            ryu_send_msg_func.assert_called_once_with(
+                self.ofparser.OFPFlowMod.return_value)
 
     def test_port_dead(self):
-        with mock.patch.object(self.mod_agent.OVSBridge,
-                               'set_db_attribute',
-                               return_value=True):
-            with mock.patch.object(self.agent,
-                                   'ryu_send_msg') as ryu_send_msg_func:
-                port = mock.Mock()
-                port.ofport = 2
-                self.agent.port_dead(port)
-        self.assertTrue(ryu_send_msg_func.called)
+        self._test_port_dead()
+
+    def test_port_dead_with_port_already_dead(self):
+        self._test_port_dead(self.mod_agent.DEAD_VLAN_TAG)
 
     def mock_update_ports(self, vif_port_set=None, registered_ports=None):
         with mock.patch.object(self.agent.int_br, 'get_vif_port_set',