From c7031e2cd303fca0c418e040e89d3428fce5dffe Mon Sep 17 00:00:00 2001
From: Arjun Baindur <xagent@gmail.com>
Date: Mon, 30 Jul 2018 15:31:50 -0700
Subject: [PATCH] Change duplicate OVS bridge datapath-ids
The native OVS/ofctl controllers talk to the bridges using a
datapath-id, instead of the bridge name. The datapath ID is
auto-generated based on the MAC address of the bridge's NIC.
In the case where bridges are on VLAN interfaces, they would
have the same MACs, therefore the same datapath-id, causing
flows for one physical bridge to be programmed on each other.
The datapath-id is a 64-bit field, with lower 48 bits being
the MAC. We set the upper 12 unused bits to identify each
unique physical bridge
This could also be fixed manually using ovs-vsctl set, but
it might be beneficial to automate this in the code.
ovs-vsctl set bridge <mybr> other-config:datapath-id=<datapathid>
You can change this yourself using above command.
You can view/verify current datapath-id via
ovs-vsctl get Bridge br-vlan datapath-id
"00006ea5a4b38a4a"
(please note that other-config is needed in the set, but not get)
Closes-Bug: #1697243
Co-Authored-By: Rodolfo Alonso Hernandez <ralonsoh@redhat.com>
Change-Id: I575ddf0a66e2cfe745af3874728809cf54e37745
(cherry picked from commit 379a9faf6206039903555ce7e3fc4221e5f06a7a)
(cherry picked from commit c02b1148db6c5183a9de0f032aec90e0bd5d8b9e)
---
neutron/agent/common/ovs_lib.py | 5 +++++
.../openvswitch/agent/ovs_neutron_agent.py | 22 +++++++++++++++++++
.../agent/test_ovs_neutron_agent.py | 21 +++++++++++++++++-
.../openvswitch/agent/test_ovs_tunnel.py | 4 +++-
4 files changed, 50 insertions(+), 2 deletions(-)
diff --git a/neutron/agent/common/ovs_lib.py b/neutron/agent/common/ovs_lib.py
index 7ad0daacf3d..9e7afd2b24f 100644
--- a/neutron/agent/common/ovs_lib.py
+++ b/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
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 a0a1ed0373d..fa4570103f2 100644
--- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py
+++ b/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()
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 e4fa72e5ef9..fb2f0a206f1 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
@@ -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):
diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py
index 32c607cae5b..23976b5c778 100644
--- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py
+++ b/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):