From bf4b614b5d8d551cece82e8c6aeb37737bce118f Mon Sep 17 00:00:00 2001 From: Kevin Benton Date: Tue, 21 Feb 2017 08:27:56 -0800 Subject: [PATCH] LB Trunk: Stop matching MAC of subport to port model Matching the MAC of the tap interface on the hypervisor to the MAC that the VM will actually use to send traffic through the tap causes the following error messages: brq9e974900-cd: received packet on tapa8a6ce4a-e8 with own address as source address This is triggering a warning because there are now two forwarding entries on the bridge, a static one from the tap device, and a learned one from the packets received.[1] Due to the presence of a static MAC entry, this will break things like allowed_address_pair use cases that allow a MAC to be used by multiple ports. This patch removes all of the logic to set the MAC address on the TAP device because it didn't serve any other purpose than easy to correlate output from ip link show commands and neutron port MAC addresses. TAP devices will now just get whatever MAC address the kernel selects for them. 1. https://lists.linuxfoundation.org/pipermail/bridge/2004-January/003740.html Closes-Bug: #1668209 (cherry picked from commit 1fae7ad1085c4548b4019652256bcae49a36f192) Change-Id: I0ff46f9550a79f486063a8e2810ed3b1140a4769 --- .../trunk/drivers/linuxbridge/agent/driver.py | 6 ------ .../linuxbridge/agent/trunk_plumber.py | 19 ------------------- .../functional/agent/test_l2_lb_agent.py | 14 -------------- .../drivers/linuxbridge/agent/test_driver.py | 4 ---- .../linuxbridge/agent/test_trunk_plumber.py | 10 ---------- 5 files changed, 53 deletions(-) diff --git a/neutron/services/trunk/drivers/linuxbridge/agent/driver.py b/neutron/services/trunk/drivers/linuxbridge/agent/driver.py index d2a2bfb93fa..81138145a74 100644 --- a/neutron/services/trunk/drivers/linuxbridge/agent/driver.py +++ b/neutron/services/trunk/drivers/linuxbridge/agent/driver.py @@ -102,12 +102,6 @@ class LinuxBridgeTrunkDriver(trunk_rpc.TrunkSkeleton): # clear any VLANs in case this was a trunk that changed status while # agent was offline. self._plumber.delete_subports_by_port_id(device_details['port_id']) - if self._tapi.get_trunk_for_subport(context, - device_details['port_id']): - # This is a subport. We need to ensure the correct mac address is - # set now that we have the port data to see the data model MAC. - self._plumber.set_port_mac(device_details['port_id'], - device_details['mac_address']) def wire_trunk(self, context, trunk): """Wire up subports while keeping the server trunk status apprised.""" diff --git a/neutron/services/trunk/drivers/linuxbridge/agent/trunk_plumber.py b/neutron/services/trunk/drivers/linuxbridge/agent/trunk_plumber.py index 97fedac24a0..75ace4a2cbf 100644 --- a/neutron/services/trunk/drivers/linuxbridge/agent/trunk_plumber.py +++ b/neutron/services/trunk/drivers/linuxbridge/agent/trunk_plumber.py @@ -76,25 +76,6 @@ class Plumber(object): dict(name=subname, tag=vlan_id)) self._safe_delete_device(subname) - def set_port_mac(self, port_id, mac_address): - """Sets mac address of physical device for port_id to mac_address.""" - dev_name = self._get_tap_device_name(port_id) - ipd = ip_lib.IPDevice(dev_name, namespace=self.namespace) - try: - if mac_address == ipd.link.address: - return False - LOG.debug("Changing MAC from %(old)s to %(new)s for device " - "%(dev)s", dict(old=ipd.link.address, new=mac_address, - dev=dev_name)) - ipd.link.set_down() - ipd.link.set_address(mac_address) - ipd.link.set_up() - except Exception: - with excutils.save_and_reraise_exception() as ectx: - ectx.reraise = ip_lib.IPDevice( - dev_name, namespace=self.namespace).exists() - return True - def _trunk_lock(self, trunk_dev): lock_name = 'trunk-%s' % trunk_dev return lockutils.lock(lock_name, utils.SYNCHRONIZED_PREFIX) diff --git a/neutron/tests/functional/agent/test_l2_lb_agent.py b/neutron/tests/functional/agent/test_l2_lb_agent.py index bde56bad8d3..27b3404504a 100644 --- a/neutron/tests/functional/agent/test_l2_lb_agent.py +++ b/neutron/tests/functional/agent/test_l2_lb_agent.py @@ -18,7 +18,6 @@ from oslo_utils import uuidutils import testtools from neutron.agent.linux import ip_lib -from neutron.common import utils from neutron.objects import trunk from neutron.plugins.ml2.drivers.linuxbridge.agent import \ linuxbridge_neutron_agent @@ -60,19 +59,6 @@ class LinuxBridgeAgentTests(test_ip_lib.IpLibTestFramework): name='br-eth1')) lba.LinuxBridgeManager(mappings, {}) - def test_set_port_mac(self): - attr = self.generate_device_details() - self.manage_device(attr) - plumber = trunk_plumber.Plumber(namespace=attr.namespace) - # force it to return name of above - plumber._get_tap_device_name = lambda x: attr.name - new_mac = utils.get_random_mac('fa:16:3e:00:00:00'.split(':')) - self.assertTrue(plumber.set_port_mac('port_id', new_mac)) - self.assertFalse(plumber.set_port_mac('port_id', new_mac)) - new_mac = utils.get_random_mac('fa:16:3e:00:00:00'.split(':')) - self.assertTrue(plumber.set_port_mac('port_id', new_mac)) - self.assertFalse(plumber.set_port_mac('port_id', new_mac)) - def test_vlan_subinterfaces(self): attr = self.generate_device_details() device = self.manage_device(attr) diff --git a/neutron/tests/unit/services/trunk/drivers/linuxbridge/agent/test_driver.py b/neutron/tests/unit/services/trunk/drivers/linuxbridge/agent/test_driver.py index 9e3dce19b75..e723239134a 100644 --- a/neutron/tests/unit/services/trunk/drivers/linuxbridge/agent/test_driver.py +++ b/neutron/tests/unit/services/trunk/drivers/linuxbridge/agent/test_driver.py @@ -103,10 +103,6 @@ class LinuxBridgeTrunkDriverTestCase(base.BaseTestCase): 'mac_address': 'mac_addr'}) self.plumber.delete_subports_by_port_id.assert_called_once_with( self.trunk.sub_ports[0].port_id) - self.tapi.get_trunk_for_subport.assert_called_once_with( - mock.ANY, self.trunk.sub_ports[0].port_id) - self.plumber.set_port_mac.assert_called_once_with( - self.trunk.sub_ports[0].port_id, 'mac_addr') def test_wire_trunk_happy_path(self): self.lbd.wire_trunk('ctx', self.trunk) diff --git a/neutron/tests/unit/services/trunk/drivers/linuxbridge/agent/test_trunk_plumber.py b/neutron/tests/unit/services/trunk/drivers/linuxbridge/agent/test_trunk_plumber.py index b9bdc5bc260..a10b40d5bf1 100644 --- a/neutron/tests/unit/services/trunk/drivers/linuxbridge/agent/test_trunk_plumber.py +++ b/neutron/tests/unit/services/trunk/drivers/linuxbridge/agent/test_trunk_plumber.py @@ -68,16 +68,6 @@ class PlumberTestCase(base.BaseTestCase): mock.call('dev1')], any_order=True) - def test_set_port_mac(self): - ipd = mock.patch.object(trunk_plumber.ip_lib, 'IPDevice').start() - ipdi = ipd.return_value - self.plumber.set_port_mac('port_id', mac_address='44') - ipdi.link.set_address.assert_called_once_with('44') - ipdi.exists.return_value = False - ipdi.link.set_address.side_effect = ValueError() - # exception suppressed since it no longer 'exists' - self.plumber.set_port_mac('port_id', mac_address='44') - def test__get_vlan_children(self): expected = [('tap47198374-5a', 777), ('tap47198374-5b', 2),