From c69407e676c483820f5c505d8d6d7923b0461819 Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Wed, 21 Oct 2015 13:54:06 +0900 Subject: [PATCH] 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 --- .../openvswitch/agent/ovs_neutron_agent.py | 31 ++++++++----------- .../agent/test_ovs_neutron_agent.py | 13 ++++---- 2 files changed, 19 insertions(+), 25 deletions(-) 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 c051e5d8357..5158c3b8c6f 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py +++ b/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 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 0e5761d254e..b841902f1c2 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 @@ -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)