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
This commit is contained in:
parent
367288e99a
commit
379a9faf62
|
@ -921,6 +921,11 @@ class OVSBridge(BaseOVS):
|
||||||
self.set_controller_field(
|
self.set_controller_field(
|
||||||
'controller_burst_limit', controller_burst_limit)
|
'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):
|
def __enter__(self):
|
||||||
self.create()
|
self.create()
|
||||||
return self
|
return self
|
||||||
|
|
|
@ -1143,6 +1143,25 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin,
|
||||||
self.setup_physical_bridges(bridge_mappings)
|
self.setup_physical_bridges(bridge_mappings)
|
||||||
return sync
|
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):
|
def setup_physical_bridges(self, bridge_mappings):
|
||||||
'''Setup the physical network bridges.
|
'''Setup the physical network bridges.
|
||||||
|
|
||||||
|
@ -1154,6 +1173,7 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin,
|
||||||
self.phys_brs = {}
|
self.phys_brs = {}
|
||||||
self.int_ofports = {}
|
self.int_ofports = {}
|
||||||
self.phys_ofports = {}
|
self.phys_ofports = {}
|
||||||
|
datapath_ids_set = set()
|
||||||
ip_wrapper = ip_lib.IPWrapper()
|
ip_wrapper = ip_lib.IPWrapper()
|
||||||
ovs = ovs_lib.BaseOVS()
|
ovs = ovs_lib.BaseOVS()
|
||||||
ovs_bridges = ovs.get_bridges()
|
ovs_bridges = ovs.get_bridges()
|
||||||
|
@ -1171,6 +1191,8 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin,
|
||||||
'bridge': bridge})
|
'bridge': bridge})
|
||||||
sys.exit(1)
|
sys.exit(1)
|
||||||
br = self.br_phys_cls(bridge)
|
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
|
# The bridge already exists, so create won't recreate it, but will
|
||||||
# handle things like changing the datapath_type
|
# handle things like changing the datapath_type
|
||||||
br.create()
|
br.create()
|
||||||
|
|
|
@ -1319,6 +1319,7 @@ class TestOvsNeutronAgent(object):
|
||||||
mock.patch.object(sys, "exit"),\
|
mock.patch.object(sys, "exit"),\
|
||||||
mock.patch.object(self.agent, 'br_phys_cls') as phys_br_cls,\
|
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') as int_br,\
|
||||||
|
mock.patch.object(self.agent, '_check_bridge_datapath_id'),\
|
||||||
mock.patch.object(ovs_lib.BaseOVS, 'get_bridges'):
|
mock.patch.object(ovs_lib.BaseOVS, 'get_bridges'):
|
||||||
devex_fn.return_value = True
|
devex_fn.return_value = True
|
||||||
parent = mock.MagicMock()
|
parent = mock.MagicMock()
|
||||||
|
@ -1398,6 +1399,7 @@ class TestOvsNeutronAgent(object):
|
||||||
mock.patch.object(utils, "execute") as utilsexec_fn,\
|
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, 'br_phys_cls') as phys_br_cls,\
|
||||||
mock.patch.object(self.agent, 'int_br') as int_br,\
|
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.IPWrapper, "add_veth") as addveth_fn,\
|
||||||
mock.patch.object(ip_lib.IpLinkCommand,
|
mock.patch.object(ip_lib.IpLinkCommand,
|
||||||
"delete") as linkdel_fn,\
|
"delete") as linkdel_fn,\
|
||||||
|
@ -1436,7 +1438,8 @@ class TestOvsNeutronAgent(object):
|
||||||
mock.patch.object(self.agent, 'br_phys_cls') as phys_br_cls,\
|
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') as int_br,\
|
||||||
mock.patch.object(self.agent.int_br, 'db_get_val',
|
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'):
|
mock.patch.object(ovs_lib.BaseOVS, 'get_bridges'):
|
||||||
phys_br = phys_br_cls()
|
phys_br = phys_br_cls()
|
||||||
parent = mock.MagicMock()
|
parent = mock.MagicMock()
|
||||||
|
@ -2342,6 +2345,22 @@ class TestOvsNeutronAgent(object):
|
||||||
'OVS')
|
'OVS')
|
||||||
self.assertRaises(ValueError, self._make_agent)
|
self.assertRaises(ValueError, self._make_agent)
|
||||||
|
|
||||||
|
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,
|
class TestOvsNeutronAgentOFCtl(TestOvsNeutronAgent,
|
||||||
ovs_test_base.OVSOFCtlTestBase):
|
ovs_test_base.OVSOFCtlTestBase):
|
||||||
|
|
|
@ -189,7 +189,9 @@ class TunnelTest(object):
|
||||||
lambda bridge: bridge if bridge in self.ovs_bridges else None)
|
lambda bridge: bridge if bridge in self.ovs_bridges else None)
|
||||||
|
|
||||||
self.execute = mock.patch('neutron.agent.common.utils.execute').start()
|
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()
|
self._define_expected_calls()
|
||||||
|
|
||||||
def _define_expected_calls(self, arp_responder=False):
|
def _define_expected_calls(self, arp_responder=False):
|
||||||
|
|
Loading…
Reference in New Issue