openstack
/
neutron
Code Issues Proposed changes

Fix _restore_local_vlan_map race 

Local vlan mappings provisioned by _restore_local_vlan_map
might not be cleaned up properly if the corresponding ports are
removed before we enter the main loop.

This commit fixes the problem by making _restore_local_vlan_map
just record local vlan IDs and leave the rest of provisioning logic
to the main loop.

Closes-Bug: #1507776
Change-Id: I84ff6684ae3fda7d839c14b1a7382871f45ce610
(cherry picked from commit c69407e676)
This commit is contained in:
YAMAMOTO Takashi 2015-10-21 13:54:06 +09:00 committed by Claudiu Belu
parent 68e88eb28a
commit 24d0b05f83
2 changed files with 19 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py
View File

@ -334,6 +334,7 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
LOG.exception(_LE("Failed reporting state!")) LOG.exception(_LE("Failed reporting state!"))
def _restore_local_vlan_map(self): def _restore_local_vlan_map(self):
self._local_vlan_hints = {}
cur_ports = self.int_br.get_vif_ports() cur_ports = self.int_br.get_vif_ports()
port_names = [p.port_name for p in cur_ports] port_names = [p.port_name for p in cur_ports]
port_info = self.int_br.get_ports_attributes( port_info = self.int_br.get_ports_attributes(
@ -351,20 +352,15 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
if not local_vlan: if not local_vlan:
continue continue
net_uuid = local_vlan_map.get('net_uuid') net_uuid = local_vlan_map.get('net_uuid')
if (net_uuid and net_uuid not in self.local_vlan_map if (net_uuid and net_uuid not in self._local_vlan_hints
and local_vlan != DEAD_VLAN_TAG): and local_vlan != DEAD_VLAN_TAG):
segmentation_id = local_vlan_map.get('segmentation_id') self.available_local_vlans.remove(local_vlan)
if segmentation_id == 'None': self._local_vlan_hints[local_vlan_map['net_uuid']] = \
# Backward compatible check when we used to store the local_vlan
# string 'None' in OVS
segmentation_id = None def _dispose_local_vlan_hints(self):
if segmentation_id is not None: self.available_local_vlans.update(self._local_vlan_hints.values())
segmentation_id = int(segmentation_id) self._local_vlan_hints = {}
self.provision_local_vlan(local_vlan_map['net_uuid'],
local_vlan_map['network_type'],
local_vlan_map['physical_network'],
segmentation_id,
local_vlan)
def setup_rpc(self): def setup_rpc(self):
self.agent_id = 'ovs-agent-%s' % self.conf.host self.agent_id = 'ovs-agent-%s' % self.conf.host
@ -623,7 +619,7 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
segmentation_id=segmentation_id) segmentation_id=segmentation_id)
def provision_local_vlan(self, net_uuid, network_type, physical_network, def provision_local_vlan(self, net_uuid, network_type, physical_network,
segmentation_id, local_vlan=None): segmentation_id):
'''Provisions a local VLAN. '''Provisions a local VLAN.
:param net_uuid: the uuid of the network associated with this vlan. :param net_uuid: the uuid of the network associated with this vlan.
@ -640,10 +636,8 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
if lvm: if lvm:
lvid = lvm.vlan lvid = lvm.vlan
else: else:
if local_vlan in self.available_local_vlans: lvid = self._local_vlan_hints.pop(net_uuid, None)
lvid = local_vlan if lvid is None:
self.available_local_vlans.remove(local_vlan)
else:
if not self.available_local_vlans: if not self.available_local_vlans:
LOG.error(_LE("No local VLAN available for net-id=%s"), LOG.error(_LE("No local VLAN available for net-id=%s"),
net_uuid) net_uuid)
@ -1788,6 +1782,7 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
# so we can sure that no other Exception occurred. # so we can sure that no other Exception occurred.
if not sync: if not sync:
ovs_restarted = False ovs_restarted = False
self._dispose_local_vlan_hints()
except Exception: except Exception:
LOG.exception(_LE("Error while processing VIF ports")) LOG.exception(_LE("Error while processing VIF ports"))
# Put the ports back in self.updated_port # Put the ports back in self.updated_port

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

@ -242,24 +242,23 @@ class TestOvsNeutronAgent(object):
def _test_restore_local_vlan_maps(self, tag, segmentation_id='1'): def _test_restore_local_vlan_maps(self, tag, segmentation_id='1'):
port = mock.Mock() port = mock.Mock()
port.port_name = 'fake_port' port.port_name = 'fake_port'
local_vlan_map = {'net_uuid': 'fake_network_id', net_uuid = 'fake_network_id'
local_vlan_map = {'net_uuid': net_uuid,
'network_type': 'vlan', 'network_type': 'vlan',
'physical_network': 'fake_network'} 'physical_network': 'fake_network'}
if segmentation_id is not None: if segmentation_id is not None:
local_vlan_map['segmentation_id'] = segmentation_id local_vlan_map['segmentation_id'] = segmentation_id
with mock.patch.object(self.agent, 'int_br') as int_br, \ with mock.patch.object(self.agent, 'int_br') as int_br:
mock.patch.object(self.agent, 'provision_local_vlan') as \
provision_local_vlan:
int_br.get_vif_ports.return_value = [port] int_br.get_vif_ports.return_value = [port]
int_br.get_ports_attributes.return_value = [{ int_br.get_ports_attributes.return_value = [{
'name': port.port_name, 'other_config': local_vlan_map, 'name': port.port_name, 'other_config': local_vlan_map,
'tag': tag 'tag': tag
}] }]
self.agent._restore_local_vlan_map() self.agent._restore_local_vlan_map()
expected_hints = {}
if tag: if tag:
self.assertTrue(provision_local_vlan.called) expected_hints[net_uuid] = tag
else: self.assertEqual(expected_hints, self.agent._local_vlan_hints)
self.assertFalse(provision_local_vlan.called)
def test_restore_local_vlan_map_with_device_has_tag(self): def test_restore_local_vlan_map_with_device_has_tag(self):
self._test_restore_local_vlan_maps(2) self._test_restore_local_vlan_maps(2)