From 60cb0911712ad11688b4d09e5c01ac39c49f5aea Mon Sep 17 00:00:00 2001 From: Yves-Gwenael Bourhis Date: Mon, 13 Jan 2014 18:27:27 +0100 Subject: [PATCH] Fixing lost vlan ids on interfaces Sometimes a vm gets its tap interface unset and reset too fast to be caught in an agent loop, and its vlan tag was not reset. We now detect if an interface loses its vlan tag, and if it happens the interface will be reconfigured. Since the TAG ID is only available via the "Port" table (in the 'tag' column), we couldn't reuse the get_vif_port_set() method's run_vsctl call which queries the "Interface" table, and needed a specific run_vsct call to the "Port" table in the new get_port_tag_dict() method. Change-Id: I7f59e2c1e757c28dae35c44ebfad9d764ae1d3c5 Closes-Bug: 1240849 --- neutron/agent/linux/ovs_lib.py | 30 +++++++++++++++ .../openvswitch/agent/ovs_neutron_agent.py | 27 ++++++++++++++ .../tests/unit/openvswitch/test_ovs_lib.py | 37 ++++++++++++++++++- .../openvswitch/test_ovs_neutron_agent.py | 31 ++++++++++++++-- 4 files changed, 121 insertions(+), 4 deletions(-) diff --git a/neutron/agent/linux/ovs_lib.py b/neutron/agent/linux/ovs_lib.py index a6bddec3c..37d278771 100644 --- a/neutron/agent/linux/ovs_lib.py +++ b/neutron/agent/linux/ovs_lib.py @@ -393,6 +393,36 @@ class OVSBridge(BaseOVS): LOG.warn(_("Found failed openvswitch port: %s"), row) return edge_ports + def get_port_tag_dict(self): + """Get a dict of port names and associated vlan tags. + + e.g. the returned dict is of the following form:: + + {u'int-br-eth2': [], + u'patch-tun': [], + u'qr-76d9e6b6-21': 1, + u'tapce5318ff-78': 1, + u'tape1400310-e6': 1} + + The TAG ID is only available in the "Port" table and is not available + in the "Interface" table queried by the get_vif_port_set() method. + + """ + port_names = self.get_port_name_list() + args = ['--format=json', '--', '--columns=name,tag', 'list', 'Port'] + result = self.run_vsctl(args, check_error=True) + port_tag_dict = {} + if not result: + return port_tag_dict + for name, tag in jsonutils.loads(result)['data']: + if name not in port_names: + continue + # 'tag' can be [u'set', []] or an integer + if isinstance(tag, list): + tag = tag[1] + port_tag_dict[name] = tag + return port_tag_dict + def get_vif_port_by_id(self, port_id): args = ['--format=json', '--', '--columns=external_ids,name,ofport', 'find', 'Interface', diff --git a/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py index 9028b23fb..b072c2659 100644 --- a/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py @@ -818,6 +818,9 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin, cur_ports = self.int_br.get_vif_port_set() self.int_br_device_count = len(cur_ports) port_info = {'current': cur_ports} + if updated_ports is None: + updated_ports = set() + updated_ports.update(self.check_changed_vlans(registered_ports)) if updated_ports: # Some updated ports might have been removed in the # meanwhile, and therefore should not be processed. @@ -838,6 +841,30 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin, port_info['removed'] = registered_ports - cur_ports return port_info + def check_changed_vlans(self, registered_ports): + """Return ports which have lost their vlan tag. + + The returned value is a set of port ids of the ports concerned by a + vlan tag loss. + """ + port_tags = self.int_br.get_port_tag_dict() + changed_ports = set() + for lvm in self.local_vlan_map.values(): + for port in registered_ports: + if ( + port in lvm.vif_ports + and lvm.vif_ports[port].port_name in port_tags + and port_tags[lvm.vif_ports[port].port_name] != lvm.vlan + ): + LOG.info( + _("Port '%(port_name)s' has lost " + "its vlan tag '%(vlan_tag)d'!"), + {'port_name': lvm.vif_ports[port].port_name, + 'vlan_tag': lvm.vlan} + ) + changed_ports.add(port) + return changed_ports + def update_ancillary_ports(self, registered_ports): ports = set() for bridge in self.ancillary_brs: diff --git a/neutron/tests/unit/openvswitch/test_ovs_lib.py b/neutron/tests/unit/openvswitch/test_ovs_lib.py index c3334279a..90a34b752 100644 --- a/neutron/tests/unit/openvswitch/test_ovs_lib.py +++ b/neutron/tests/unit/openvswitch/test_ovs_lib.py @@ -438,8 +438,10 @@ class OVS_Lib_Test(base.BaseTestCase): ovs_row.append(cell) elif isinstance(cell, dict): ovs_row.append(["map", cell.items()]) + elif isinstance(cell, set): + ovs_row.append(["set", cell]) else: - raise TypeError('%r not int, str, list or dict' % + raise TypeError('%r not int, str, list, set or dict' % type(cell)) return jsonutils.dumps(r) @@ -536,6 +538,39 @@ class OVS_Lib_Test(base.BaseTestCase): self.assertRaises(RuntimeError, self.br.get_vif_port_set) tools.verify_mock_calls(self.execute, expected_calls_and_values) + def test_get_port_tag_dict(self): + headings = ['name', 'tag'] + data = [ + ['int-br-eth2', set()], + ['patch-tun', set()], + ['qr-76d9e6b6-21', 1], + ['tapce5318ff-78', 1], + ['tape1400310-e6', 1], + ] + + # Each element is a tuple of (expected mock call, return_value) + expected_calls_and_values = [ + (mock.call(["ovs-vsctl", self.TO, "list-ports", self.BR_NAME], + root_helper=self.root_helper), + '\n'.join((iface for iface, tag in data))), + (mock.call(["ovs-vsctl", self.TO, "--format=json", + "--", "--columns=name,tag", + "list", "Port"], + root_helper=self.root_helper), + self._encode_ovs_json(headings, data)), + ] + tools.setup_mock_calls(self.execute, expected_calls_and_values) + + port_tags = self.br.get_port_tag_dict() + self.assertEqual( + port_tags, + {u'int-br-eth2': [], + u'patch-tun': [], + u'qr-76d9e6b6-21': 1, + u'tapce5318ff-78': 1, + u'tape1400310-e6': 1} + ) + def test_clear_db_attribute(self): pname = "tap77" self.br.clear_db_attribute("Port", pname, "tag") diff --git a/neutron/tests/unit/openvswitch/test_ovs_neutron_agent.py b/neutron/tests/unit/openvswitch/test_ovs_neutron_agent.py index 0e998f9fb..a30efdaae 100644 --- a/neutron/tests/unit/openvswitch/test_ovs_neutron_agent.py +++ b/neutron/tests/unit/openvswitch/test_ovs_neutron_agent.py @@ -192,9 +192,15 @@ class TestOvsNeutronAgent(base.BaseTestCase): self._test_port_dead(ovs_neutron_agent.DEAD_VLAN_TAG) def mock_scan_ports(self, vif_port_set=None, registered_ports=None, - updated_ports=None): - with mock.patch.object(self.agent.int_br, 'get_vif_port_set', - return_value=vif_port_set): + updated_ports=None, port_tags_dict=None): + if port_tags_dict is None: # Because empty dicts evaluate as False. + port_tags_dict = {} + with contextlib.nested( + mock.patch.object(self.agent.int_br, 'get_vif_port_set', + return_value=vif_port_set), + mock.patch.object(self.agent.int_br, 'get_port_tag_dict', + return_value=port_tags_dict) + ): return self.agent.scan_ports(registered_ports, updated_ports) def test_scan_ports_returns_current_only_for_unchanged_ports(self): @@ -247,6 +253,25 @@ class TestOvsNeutronAgent(base.BaseTestCase): updated_ports) self.assertEqual(expected, actual) + def test_update_ports_returns_changed_vlan(self): + br = ovs_lib.OVSBridge('br-int', 'sudo') + mac = "ca:fe:de:ad:be:ef" + port = ovs_lib.VifPort(1, 1, 1, mac, br) + lvm = ovs_neutron_agent.LocalVLANMapping( + 1, '1', None, 1, {port.vif_id: port}) + local_vlan_map = {'1': lvm} + vif_port_set = set([1, 3]) + registered_ports = set([1, 2]) + port_tags_dict = {1: []} + expected = dict( + added=set([3]), current=vif_port_set, + removed=set([2]), updated=set([1]) + ) + with mock.patch.dict(self.agent.local_vlan_map, local_vlan_map): + actual = self.mock_scan_ports( + vif_port_set, registered_ports, port_tags_dict=port_tags_dict) + self.assertEqual(expected, actual) + def test_treat_devices_added_returns_true_for_missing_device(self): with contextlib.nested( mock.patch.object(self.agent.plugin_rpc, 'get_device_details', -- 2.45.2