Merge "Change duplicate OVS bridge datapath-ids" into stable/queens

neutron/agent/common/ovs_lib.py

@@ -919,6 +919,11 @@ class OVSBridge(BaseOVS):
         self.set_controller_field(
             'controller_burst_limit', controller_burst_limit)
 
+    def set_datapath_id(self, datapath_id):
+        dpid_cfg = {'datapath-id': datapath_id}
+        self.set_db_attribute('Bridge', self.br_name, 'other_config', dpid_cfg,
+                              check_error=True)
+
     def __enter__(self):
         self.create()
         return self

neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py

@@ -1078,6 +1078,25 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin,
             self.setup_physical_bridges(bridge_mappings)
         return sync
 
+    def _check_bridge_datapath_id(self, bridge, datapath_ids_set):
+        """Check for bridges with duplicate datapath-id
+
+        Bottom 48 bits auto-derived from MAC of NIC. Upper 12 bits free,
+        so we OR it with (bridge # << 48) to create a unique ID
+        It must be exactly 64 bits, else OVS will reject it - zfill
+
+        :param bridge: (OVSPhysicalBridge) bridge
+        :param datapath_ids_set: (set) used datapath ids in OVS
+        """
+        dpid = int(bridge.get_datapath_id(), 16)
+        dpid_hex = format(dpid, '0x').zfill(16)
+        if dpid_hex in datapath_ids_set:
+            dpid_hex = format(
+                dpid + (len(datapath_ids_set) << 48), '0x').zfill(16)
+            bridge.set_datapath_id(dpid_hex)
+        LOG.info('Bridge %s datapath-id = 0x%s', bridge.br_name, dpid_hex)
+        datapath_ids_set.add(dpid_hex)
+
     def setup_physical_bridges(self, bridge_mappings):
         '''Setup the physical network bridges.
 
@@ -1089,6 +1108,7 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin,
         self.phys_brs = {}
         self.int_ofports = {}
         self.phys_ofports = {}
+        datapath_ids_set = set()
         ip_wrapper = ip_lib.IPWrapper()
         ovs = ovs_lib.BaseOVS()
         ovs_bridges = ovs.get_bridges()
@@ -1106,6 +1126,8 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin,
                            'bridge': bridge})
                 sys.exit(1)
             br = self.br_phys_cls(bridge)
+            self._check_bridge_datapath_id(br, datapath_ids_set)
+
             # The bridge already exists, so create won't recreate it, but will
             # handle things like changing the datapath_type
             br.create()

neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py

@@ -1226,6 +1226,7 @@ class TestOvsNeutronAgent(object):
                 mock.patch.object(sys, "exit"),\
                 mock.patch.object(self.agent, 'br_phys_cls') as phys_br_cls,\
                 mock.patch.object(self.agent, 'int_br') as int_br,\
+                mock.patch.object(self.agent, '_check_bridge_datapath_id'),\
                 mock.patch.object(ovs_lib.BaseOVS, 'get_bridges'):
             devex_fn.return_value = True
             parent = mock.MagicMock()
@@ -1305,6 +1306,7 @@ class TestOvsNeutronAgent(object):
                 mock.patch.object(utils, "execute") as utilsexec_fn,\
                 mock.patch.object(self.agent, 'br_phys_cls') as phys_br_cls,\
                 mock.patch.object(self.agent, 'int_br') as int_br,\
+                mock.patch.object(self.agent, '_check_bridge_datapath_id'),\
                 mock.patch.object(ip_lib.IPWrapper, "add_veth") as addveth_fn,\
                 mock.patch.object(ip_lib.IpLinkCommand,
                                   "delete") as linkdel_fn,\
@@ -1343,7 +1345,8 @@ class TestOvsNeutronAgent(object):
                 mock.patch.object(self.agent, 'br_phys_cls') as phys_br_cls,\
                 mock.patch.object(self.agent, 'int_br') as int_br,\
                 mock.patch.object(self.agent.int_br, 'db_get_val',
-                                  return_value='veth'),\
+                                  return_value='veth'), \
+                mock.patch.object(self.agent, '_check_bridge_datapath_id'), \
                 mock.patch.object(ovs_lib.BaseOVS, 'get_bridges'):
             phys_br = phys_br_cls()
             parent = mock.MagicMock()
@@ -2234,6 +2237,22 @@ class TestOvsNeutronAgent(object):
             br, 'add', mock.Mock(), mock.Mock(), ip)
         self.assertFalse(br.install_arp_responder.called)
 
+    def test__check_bridge_datapath_id(self):
+        datapath_id = u'0000622486fa3f42'
+        datapath_ids_set = set()
+        for i in range(5):
+            dpid = format((i << 48) + int(datapath_id, 16), '0x').zfill(16)
+            bridge = mock.Mock()
+            bridge.br_name = 'bridge_%s' % i
+            bridge.get_datapath_id = mock.Mock(return_value=datapath_id)
+            self.agent._check_bridge_datapath_id(bridge, datapath_ids_set)
+            self.assertEqual(i + 1, len(datapath_ids_set))
+            self.assertIn(dpid, datapath_ids_set)
+            if i == 0:
+                bridge.set_datapath_id.assert_not_called()
+            else:
+                bridge.set_datapath_id.assert_called_once_with(dpid)
+
 
 class TestOvsNeutronAgentOFCtl(TestOvsNeutronAgent,
                                ovs_test_base.OVSOFCtlTestBase):

neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py

@@ -185,7 +185,9 @@ class TunnelTest(object):
             lambda bridge: bridge if bridge in self.ovs_bridges else None)
 
         self.execute = mock.patch('neutron.agent.common.utils.execute').start()
-
+        self.mock_check_bridge_datapath_id = mock.patch.object(
+            self.mod_agent.OVSNeutronAgent,
+            '_check_bridge_datapath_id').start()
         self._define_expected_calls()
 
     def _define_expected_calls(self, arp_responder=False):