From 0550c0e1f66b26e7e947958fdddb60d8f309e76f Mon Sep 17 00:00:00 2001
From: RoyKing <zhangruochen@unionpay.com>
Date: Wed, 11 Sep 2019 20:38:27 +0800
Subject: [PATCH] Avoid unnecessary operation of ovsdb and flows

Type of lvm.vlan is int and other_config.get('tag') is a string,
they can never be equal. We should do type conversion before
comparing to avoid unnecessary operation of ovsdb and flows.

Change-Id: Ib84da6296ddf3c95be9e9f370eb574bf92ceec15
Closes-Bug: #1843425
---
 .../openvswitch/agent/ovs_neutron_agent.py    |  7 ++-
 .../agent/test_ovs_neutron_agent.py           | 43 ++++++++++++++++++-
 2 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py
index bde5f5df093..12517d47867 100644
--- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py
+++ b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py
@@ -1059,14 +1059,17 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin,
                 cur_info = info_by_port[port.port_name]
             except KeyError:
                 continue
+            str_vlan = str(lvm.vlan)
             other_config = cur_info['other_config']
             if (cur_info['tag'] != lvm.vlan or
-                    other_config.get('tag') != lvm.vlan):
-                other_config['tag'] = str(lvm.vlan)
+                    other_config.get('tag') != str_vlan):
+                other_config['tag'] = str_vlan
                 self.int_br.set_db_attribute(
                     "Port", port.port_name, "other_config", other_config)
                 # Uninitialized port has tag set to []
                 if cur_info['tag']:
+                    LOG.warning("Uninstall flows of ofport %s due to "
+                                "local vlan change.", port.ofport)
                     self.int_br.uninstall_flows(in_port=port.ofport)
 
     def _bind_devices(self, need_binding_ports):
diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py
index 495cc88a44e..94fbd7aca8c 100644
--- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py
+++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py
@@ -705,7 +705,7 @@ class TestOvsNeutronAgent(object):
 
     def test_add_port_tag_info(self):
         lvm = mock.Mock()
-        lvm.vlan = "1"
+        lvm.vlan = 1
         self.agent.vlan_manager.mapping["net1"] = lvm
         ovs_db_list = [{'name': 'tap1',
                         'tag': [],
@@ -738,6 +738,47 @@ class TestOvsNeutronAgent(object):
                     "other_config", {"tag": "1"})]
             int_br.assert_has_calls(set_db_attribute_calls, any_order=True)
 
+    def test_add_port_tag_info_with_tagged_ports(self):
+        lvm = mock.Mock()
+        lvm.vlan = 1
+        self.agent.vlan_manager.mapping["net1"] = lvm
+        ovs_db_list1 = [{'name': 'tap1',
+                         'tag': 1,
+                         'other_config': {'segmentation_id': '1', 'tag': '1'}}]
+        ovs_db_list2 = [{'name': 'tap2',
+                         'tag': 2,
+                         'other_config': {'segmentation_id': '1', 'tag': '1'}},
+                        {'name': 'tap3',
+                         'tag': 1,
+                         'other_config': {'segmentation_id': '2', 'tag': '2'}}]
+        vif_port1 = mock.Mock()
+        vif_port1.port_name = 'tap1'
+        vif_port2 = mock.Mock()
+        vif_port2.port_name = 'tap2'
+        vif_port2.ofport = 7
+        vif_port3 = mock.Mock()
+        vif_port3.port_name = 'tap3'
+        vif_port3.ofport = 8
+        port_details1 = [{'network_id': 'net1', 'vif_port': vif_port1}]
+        port_details2 = [{'network_id': 'net1', 'vif_port': vif_port2},
+                         {'network_id': 'net1', 'vif_port': vif_port3}]
+        with mock.patch.object(self.agent, 'int_br') as int_br:
+            int_br.get_ports_attributes.return_value = ovs_db_list1
+            self.agent._add_port_tag_info(port_details1)
+            int_br.set_db_attribute.assert_not_called()
+            # Reset mock to check port with changed tag
+            int_br.reset_mock()
+            int_br.get_ports_attributes.return_value = ovs_db_list2
+            self.agent._add_port_tag_info(port_details2)
+            expected_calls = \
+                [mock.call.set_db_attribute("Port", "tap2",
+                    "other_config", {'segmentation_id': '1', 'tag': '1'}),
+                 mock.call.uninstall_flows(in_port=7),
+                 mock.call.set_db_attribute("Port", "tap3",
+                     "other_config", {'segmentation_id': '2', 'tag': '1'}),
+                 mock.call.uninstall_flows(in_port=8)]
+            int_br.assert_has_calls(expected_calls)
+
     def test_bind_devices(self):
         devices_up = ['tap1']
         devices_down = ['tap2']