Browse Source

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
changes/78/237878/6
YAMAMOTO Takashi 6 years ago
parent
commit
c69407e676
  1. 31
      neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py
  2. 13
      neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py

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

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

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

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

Loading…
Cancel
Save