From: YAMAMOTO Takashi Date: Thu, 10 Jul 2014 08:36:43 +0000 (+0900) Subject: ofagent: Handle device name prefixes other than "tap" X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=f676d9215958b1af1105d4ff4671ce6cefd83eb7;p=openstack-build%2Fneutron-build.git ofagent: Handle device name prefixes other than "tap" This fixes regressions in commit 9d13ea884bff749b3975acb5eb5630e5aca4a665. Handle device name prefixes other than "tap". For example, nova hybrid interface driver uses "qvo" prefix. Also, ignore non neutron ports correctly. For example, veth pairs used to connect physical bridges. Closes-Bug: #1341465 Change-Id: I1d71c8a2cf8c2f71f0dbcfb54c9b347e24c03562 --- diff --git a/neutron/plugins/ofagent/agent/ofa_neutron_agent.py b/neutron/plugins/ofagent/agent/ofa_neutron_agent.py index 8083c6ce0..f5b497a75 100644 --- a/neutron/plugins/ofagent/agent/ofa_neutron_agent.py +++ b/neutron/plugins/ofagent/agent/ofa_neutron_agent.py @@ -302,20 +302,6 @@ class OFANeutronAgent(n_rpc.RpcCallback, self._report_state) heartbeat.start(interval=report_interval) - @staticmethod - def _get_ofport_name(interface_id): - """Convert from neutron device id (uuid) to OpenFlow port name. - - This needs to be synced with ML2 plugin's _device_to_port_id(). - - An assumption: The switch uses an OS's interface name as the - corresponding OpenFlow port name. - NOTE(yamamoto): While it's true for Open vSwitch, it isn't - necessarily true everywhere. For example, LINC uses something - like "LogicalSwitch0-Port2". - """ - return "tap" + interface_id[0:11] - def _get_ports(self, br): """Generate ports.Port instances for the given bridge.""" datapath = br.datapath @@ -330,7 +316,8 @@ class OFANeutronAgent(n_rpc.RpcCallback, def _get_ofport_names(self, br): """Return a set of OpenFlow port names for the given bridge.""" - return set(p.port_name for p in self._get_ports(br)) + return set(p.normalized_port_name() for p in + self._get_ports(br) if p.is_neutron_port()) def get_net_uuid(self, vif_id): for network_id, vlan_mapping in self.local_vlan_map.iteritems(): @@ -353,7 +340,7 @@ class OFANeutronAgent(n_rpc.RpcCallback, # Even if full port details might be provided to this call, # they are not used since there is no guarantee the notifications # are processed in the same order as the relevant API requests - self.updated_ports.add(self._get_ofport_name(port['id'])) + self.updated_ports.add(ports.get_normalized_port_name(port['id'])) LOG.debug(_("port_update received port %s"), port['id']) def tunnel_update(self, context, **kwargs): @@ -1089,8 +1076,8 @@ class OFANeutronAgent(n_rpc.RpcCallback, def treat_devices_added_or_updated(self, devices): resync = False - all_ports = dict((p.port_name, p) for p in - self._get_ports(self.int_br)) + all_ports = dict((p.normalized_port_name(), p) for p in + self._get_ports(self.int_br) if p.is_neutron_port()) for device in devices: LOG.debug(_("Processing port %s"), device) if device not in all_ports: diff --git a/neutron/plugins/ofagent/agent/ports.py b/neutron/plugins/ofagent/agent/ports.py index c78c5cd6c..995118fbf 100644 --- a/neutron/plugins/ofagent/agent/ports.py +++ b/neutron/plugins/ofagent/agent/ports.py @@ -16,7 +16,7 @@ # @author: YAMAMOTO Takashi, VA Linux Systems Japan K.K. -class Port(object): +class OFPort(object): def __init__(self, port_name, ofport): self.port_name = port_name self.ofport = ofport @@ -25,3 +25,61 @@ class Port(object): def from_ofp_port(cls, ofp_port): """Convert from ryu OFPPort.""" return cls(port_name=ofp_port.name, ofport=ofp_port.port_no) + + +PORT_NAME_LEN = 14 +PORT_NAME_PREFIXES = [ + "tap", # common cases, including ovs_use_veth=True + "qvo", # nova hybrid interface driver + "qr-", # l3-agent INTERNAL_DEV_PREFIX (ovs_use_veth=False) + "qg-", # l3-agent EXTERNAL_DEV_PREFIX (ovs_use_veth=False) +] + + +def _is_neutron_port(name): + """Return True if the port name looks like a neutron port.""" + if len(name) != PORT_NAME_LEN: + return False + for pref in PORT_NAME_PREFIXES: + if name.startswith(pref): + return True + return False + + +def get_normalized_port_name(interface_id): + """Convert from neutron device id (uuid) to "normalized" port name. + + This needs to be synced with ML2 plugin's _device_to_port_id(). + + An assumption: The switch uses an OS's interface name as the + corresponding OpenFlow port name. + NOTE(yamamoto): While it's true for Open vSwitch, it isn't + necessarily true everywhere. For example, LINC uses something + like "LogicalSwitch0-Port2". + + NOTE(yamamoto): The actual prefix might be different. For example, + with the hybrid interface driver, it's "qvo". However, we always + use "tap" prefix throughout the agent and plugin for simplicity. + Some care should be taken when talking to the switch. + """ + return ("tap" + interface_id)[0:PORT_NAME_LEN] + + +def _normalize_port_name(name): + """Normalize port name. + + See comments in _get_ofport_name. + """ + for pref in PORT_NAME_PREFIXES: + if name.startswith(pref): + return "tap" + name[len(pref):] + return name + + +class Port(OFPort): + def is_neutron_port(self): + """Return True if the port looks like a neutron port.""" + return _is_neutron_port(self.port_name) + + def normalized_port_name(self): + return _normalize_port_name(self.port_name) diff --git a/neutron/tests/unit/ofagent/test_ofa_neutron_agent.py b/neutron/tests/unit/ofagent/test_ofa_neutron_agent.py index 4ac50390b..5e0daf289 100644 --- a/neutron/tests/unit/ofagent/test_ofa_neutron_agent.py +++ b/neutron/tests/unit/ofagent/test_ofa_neutron_agent.py @@ -38,6 +38,14 @@ NOTIFIER = ('neutron.plugins.ml2.rpc.AgentNotifierApi') OVS_LINUX_KERN_VERS_WITHOUT_VXLAN = "3.12.0" +def _mock_port(is_neutron=True, normalized_name=None): + p = mock.Mock() + p.is_neutron_port.return_value = is_neutron + if normalized_name: + p.normalized_port_name.return_value = normalized_name + return p + + class OFAAgentTestCase(base.BaseTestCase): _AGENT_NAME = 'neutron.plugins.ofagent.agent.ofa_neutron_agent' @@ -419,14 +427,16 @@ class TestOFANeutronAgent(OFAAgentTestCase): mock.patch.object(self.agent.plugin_rpc, 'get_device_details', side_effect=Exception()), mock.patch.object(self.agent, '_get_ports', - return_value=[mock.Mock(port_name='xxx')])): + return_value=[_mock_port(True, 'xxx')])): self.assertTrue(self.agent.treat_devices_added_or_updated(['xxx'])) - def _mock_treat_devices_added_updated(self, details, port, func_name): + def _mock_treat_devices_added_updated(self, details, port, all_ports, + func_name): """Mock treat devices added or updated. :param details: the details to return for the device - :param port: the port that get_vif_port_by_id should return + :param port: port name to process + :param all_ports: the port that _get_ports return :param func_name: the function that should be called :returns: whether the named function was called """ @@ -434,27 +444,28 @@ class TestOFANeutronAgent(OFAAgentTestCase): mock.patch.object(self.agent.plugin_rpc, 'get_device_details', return_value=details), mock.patch.object(self.agent, '_get_ports', - return_value=[port]), + return_value=all_ports), mock.patch.object(self.agent.plugin_rpc, 'update_device_up'), mock.patch.object(self.agent.plugin_rpc, 'update_device_down'), mock.patch.object(self.agent, func_name) ) as (get_dev_fn, _get_ports, upd_dev_up, upd_dev_down, func): - self.assertFalse(self.agent.treat_devices_added_or_updated( - [port.port_name])) + self.assertFalse(self.agent.treat_devices_added_or_updated([port])) _get_ports.assert_called_once_with(self.agent.int_br) return func.called def test_treat_devices_added_updated_ignores_invalid_ofport(self): - port = mock.Mock() - port.ofport = -1 + port_name = 'hoge' + p1 = _mock_port(True, port_name) + p1.ofport = -1 self.assertFalse(self._mock_treat_devices_added_updated( - mock.MagicMock(), port, 'port_dead')) + mock.MagicMock(), port_name, [p1], 'port_dead')) def test_treat_devices_added_updated_marks_unknown_port_as_dead(self): - port = mock.Mock() - port.ofport = 1 + port_name = 'hoge' + p1 = _mock_port(True, port_name) + p1.ofport = 1 self.assertTrue(self._mock_treat_devices_added_updated( - mock.MagicMock(), port, 'port_dead')) + mock.MagicMock(), port_name, [p1], 'port_dead')) def test_treat_devices_added_does_not_process_missing_port(self): with contextlib.nested( @@ -465,10 +476,14 @@ class TestOFANeutronAgent(OFAAgentTestCase): self.assertFalse(get_dev_fn.called) def test_treat_devices_added_updated_updates_known_port(self): + port_name = 'tapd3315981-0b' + p1 = _mock_port(False) + p2 = _mock_port(True, port_name) + ports = [p1, p2] details = mock.MagicMock() details.__contains__.side_effect = lambda x: True self.assertTrue(self._mock_treat_devices_added_updated( - details, mock.Mock(), 'treat_vif_port')) + details, port_name, ports, 'treat_vif_port')) def test_treat_devices_added_updated_put_port_down(self): fake_details_dict = {'admin_state_up': False, @@ -482,16 +497,17 @@ class TestOFANeutronAgent(OFAAgentTestCase): mock.patch.object(self.agent.plugin_rpc, 'get_device_details', return_value=fake_details_dict), mock.patch.object(self.agent, '_get_ports', - return_value=[mock.Mock(port_name='xxx')]), + return_value=[_mock_port(True, 'xxx')]), mock.patch.object(self.agent.plugin_rpc, 'update_device_up'), mock.patch.object(self.agent.plugin_rpc, 'update_device_down'), mock.patch.object(self.agent, 'treat_vif_port') - ) as (get_dev_fn, get_vif_func, upd_dev_up, + ) as (get_dev_fn, _get_ports, upd_dev_up, upd_dev_down, treat_vif_port): self.assertFalse(self.agent.treat_devices_added_or_updated( ['xxx'])) self.assertTrue(treat_vif_port.called) self.assertTrue(upd_dev_down.called) + _get_ports.assert_called_once_with(self.agent.int_br) def test_treat_devices_removed_returns_true_for_missing_device(self): with mock.patch.object(self.agent.plugin_rpc, 'update_device_down', @@ -989,7 +1005,7 @@ class TestOFANeutronAgent(OFAAgentTestCase): def test__get_ofport_names(self): names = ['p111', 'p222', 'p333'] - ps = [mock.Mock(port_name=x, ofport=names.index(x)) for x in names] + ps = [_mock_port(True, x) for x in names] with mock.patch.object(self.agent, '_get_ports', return_value=ps) as _get_ports: result = self.agent._get_ofport_names('hoge') diff --git a/neutron/tests/unit/ofagent/test_ofa_ports.py b/neutron/tests/unit/ofagent/test_ofa_ports.py index 7cd2bc60f..6e36da749 100644 --- a/neutron/tests/unit/ofagent/test_ofa_ports.py +++ b/neutron/tests/unit/ofagent/test_ofa_ports.py @@ -24,9 +24,31 @@ from neutron.tests import base class TestOFAgentPorts(base.BaseTestCase): def test_port(self): - p1 = ports.Port(port_name='foo', ofport=999) + name = 'foo03b9a237-0b' + p1 = ports.Port(port_name=name, ofport=999) ryu_ofp_port = mock.Mock(port_no=999) - ryu_ofp_port.name = 'foo' + ryu_ofp_port.name = name p2 = ports.Port.from_ofp_port(ofp_port=ryu_ofp_port) self.assertEqual(p1.port_name, p2.port_name) self.assertEqual(p1.ofport, p2.ofport) + self.assertFalse(p1.is_neutron_port()) + self.assertFalse(p2.is_neutron_port()) + + def test_neutron_port(self): + for pref in ['qvo', 'qr-', 'qg-', 'tap']: + name = pref + '03b9a237-0b' + p1 = ports.Port(port_name=name, ofport=999) + ryu_ofp_port = mock.Mock(port_no=999) + ryu_ofp_port.name = name + p2 = ports.Port.from_ofp_port(ofp_port=ryu_ofp_port) + self.assertEqual(p1.port_name, p2.port_name) + self.assertEqual(p1.ofport, p2.ofport) + self.assertTrue(p1.is_neutron_port()) + self.assertTrue(p2.is_neutron_port()) + self.assertTrue('tap03b9a237-0b', p1.normalized_port_name()) + self.assertTrue('tap03b9a237-0b', p2.normalized_port_name()) + + def test_get_normalized_port_name(self): + self.assertEqual('tap03b9a237-0b', + ports.get_normalized_port_name( + '03b9a237-0b1b-11e4-b537-08606e7f74e7'))