From 52f31b0ddbb978402f65850bf41ceb409c560d4f Mon Sep 17 00:00:00 2001 From: Ihar Hrachyshka Date: Tue, 29 Nov 2016 22:24:29 +0000 Subject: [PATCH] Update MTU on existing devices This patch makes OVS and Linuxbridge interface drivers to set MTU on plug() attempt if the device already exists. This helps when network MTU changes (which happens after some configuration file changes). This will allow to update MTU values on agent restart, without the need to bind all ports to new nodes, that would involve migrating agents. It will also help in case when you have no other nodes to migrate to (in single node mode). Both OVS and Linuxbridge interface drivers are updated. Other drivers (in-tree IVS as well as 3party drivers) will use the default set_mtu implementation, that only warns about the missing feature (once per process startup). DocImpact suggest to restart agents after MTU config changes instead of rewiring router/DHCP ports. Related: If438e4816b425e6c5021a55567dcaaa77d1fffff Related: If09eda334cddc74910dda7a4fb498b7987714be3 Closes-Bug: #1649845 Change-Id: I3c6d6cb55c5808facec38f87114c2ddf548f05f1 (cherry picked from commit 5c8dffa7fb6c95a04a7b50c7d6e63c9a2729231b) --- neutron/agent/linux/interface.py | 42 ++++++- .../functional/agent/linux/test_interface.py | 107 +++++++++++++----- .../tests/unit/agent/linux/test_interface.py | 15 +++ 3 files changed, 128 insertions(+), 36 deletions(-) diff --git a/neutron/agent/linux/interface.py b/neutron/agent/linux/interface.py index 99703c4dd9a..c2eb06ec74c 100644 --- a/neutron/agent/linux/interface.py +++ b/neutron/agent/linux/interface.py @@ -45,6 +45,11 @@ OPTS = [ ] +def _get_veth(name1, name2, namespace2): + return (ip_lib.IPDevice(name1), + ip_lib.IPDevice(name2, namespace=namespace2)) + + @six.add_metaclass(abc.ABCMeta) class LinuxInterfaceDriver(object): @@ -54,6 +59,7 @@ class LinuxInterfaceDriver(object): def __init__(self, conf): self.conf = conf + self._mtu_update_warn_logged = False @property def use_gateway_ips(self): @@ -240,6 +246,11 @@ class LinuxInterfaceDriver(object): bridge, namespace, prefix) else: LOG.info(_LI("Device %s already exists"), device_name) + if mtu: + self.set_mtu( + device_name, mtu, namespace=namespace, prefix=prefix) + else: + LOG.warning(_LW("No MTU configured for port %s"), port_id) @abc.abstractmethod def unplug(self, device_name, bridge=None, namespace=None, prefix=None): @@ -262,6 +273,12 @@ class LinuxInterfaceDriver(object): """ return True + def set_mtu(self, device_name, mtu, namespace=None, prefix=None): + """Set MTU on the interface.""" + if not self._mtu_update_warn_logged: + LOG.warning(_LW("Interface driver cannot update MTU for ports")) + self._mtu_update_warn_logged = True + class NullDriver(LinuxInterfaceDriver): def plug_new(self, network_id, port_id, device_name, mac_address, @@ -347,9 +364,7 @@ class OVSInterfaceDriver(LinuxInterfaceDriver): # allow to set MTU that is higher than the least of all device MTUs on # the bridge if mtu: - ns_dev.link.set_mtu(mtu) - if self.conf.ovs_use_veth: - root_dev.link.set_mtu(mtu) + self.set_mtu(device_name, mtu, namespace=namespace, prefix=prefix) else: LOG.warning(_LW("No MTU configured for port %s"), port_id) @@ -376,6 +391,16 @@ class OVSInterfaceDriver(LinuxInterfaceDriver): LOG.error(_LE("Failed unplugging interface '%s'"), device_name) + def set_mtu(self, device_name, mtu, namespace=None, prefix=None): + if self.conf.ovs_use_veth: + tap_name = self._get_tap_name(device_name, prefix) + root_dev, ns_dev = _get_veth( + tap_name, device_name, namespace2=namespace) + root_dev.link.set_mtu(mtu) + else: + ns_dev = ip_lib.IPWrapper(namespace=namespace).device(device_name) + ns_dev.link.set_mtu(mtu) + class IVSInterfaceDriver(LinuxInterfaceDriver): """Driver for creating an internal interface on an IVS bridge.""" @@ -456,8 +481,7 @@ class BridgeInterfaceDriver(LinuxInterfaceDriver): ns_veth.link.set_address(mac_address) if mtu: - root_veth.link.set_mtu(mtu) - ns_veth.link.set_mtu(mtu) + self.set_mtu(device_name, mtu, namespace=namespace, prefix=prefix) else: LOG.warning(_LW("No MTU configured for port %s"), port_id) @@ -473,3 +497,11 @@ class BridgeInterfaceDriver(LinuxInterfaceDriver): except RuntimeError: LOG.error(_LE("Failed unplugging interface '%s'"), device_name) + + def set_mtu(self, device_name, mtu, namespace=None, prefix=None): + tap_name = device_name.replace(prefix or self.DEV_NAME_PREFIX, + constants.TAP_DEVICE_PREFIX) + root_dev, ns_dev = _get_veth( + tap_name, device_name, namespace2=namespace) + root_dev.link.set_mtu(mtu) + ns_dev.link.set_mtu(mtu) diff --git a/neutron/tests/functional/agent/linux/test_interface.py b/neutron/tests/functional/agent/linux/test_interface.py index 967e5e82b48..303e6259e91 100644 --- a/neutron/tests/functional/agent/linux/test_interface.py +++ b/neutron/tests/functional/agent/linux/test_interface.py @@ -12,6 +12,8 @@ # License for the specific language governing permissions and limitations # under the License. +import functools + from oslo_config import cfg from oslo_utils import uuidutils import testtools @@ -21,15 +23,63 @@ from neutron.agent.linux import ip_lib from neutron.common import exceptions from neutron.common import utils from neutron.tests.common import net_helpers -from neutron.tests.functional.agent.linux import base +from neutron.tests.functional.agent.linux import base as linux_base +from neutron.tests.functional import base -class OVSInterfaceDriverTestCase(base.BaseOVSLinuxTestCase): +class InterfaceDriverTestCaseMixin(object): + def _test_mtu_set_after_action(self, device_name, br_name, namespace, + action=None): + mac_address = utils.get_random_mac('fa:16:3e:00:00:00'.split(':')) + + plug = functools.partial( + self.interface.plug, + network_id=uuidutils.generate_uuid(), + port_id=uuidutils.generate_uuid(), + device_name=device_name, + mac_address=mac_address, + bridge=self.bridge_name, + namespace=namespace) + plug(mtu=1500) + self.assertTrue(ip_lib.device_exists(device_name, namespace)) + + action = action or plug + for mtu in (1450, 1500, 9000, 9000, 1450): + action(mtu=mtu) + self.assertEqual( + mtu, + ip_lib.IPDevice(device_name, namespace=namespace).link.mtu) + + def test_plug_multiple_calls_update_mtu(self): + device_name = utils.get_rand_name() + namespace = self.useFixture(net_helpers.NamespaceFixture()).name + + self._test_mtu_set_after_action( + device_name, self.bridge_name, namespace) + + def test_set_mtu(self): + device_name = utils.get_rand_name() + namespace = self.useFixture(net_helpers.NamespaceFixture()).name + + self._test_mtu_set_after_action( + device_name, self.bridge_name, namespace, + functools.partial( + self.interface.set_mtu, + device_name=device_name, namespace=namespace)) + + +class OVSInterfaceDriverTestCase(linux_base.BaseOVSLinuxTestCase, + InterfaceDriverTestCaseMixin): def setUp(self): super(OVSInterfaceDriverTestCase, self).setUp() conf = cfg.ConfigOpts() conf.register_opts(interface.OPTS) self.interface = interface.OVSInterfaceDriver(conf) + self.bridge = self.useFixture(net_helpers.OVSBridgeFixture()).bridge + + @property + def bridge_name(self): + return self.bridge.br_name def test_plug_checks_if_bridge_exists(self): with testtools.ExpectedException(exceptions.BridgeDoesNotExist): @@ -44,50 +94,45 @@ class OVSInterfaceDriverTestCase(base.BaseOVSLinuxTestCase): device_name = utils.get_rand_name() mac_address = utils.get_random_mac('fa:16:3e:00:00:00'.split(':')) namespace = self.useFixture(net_helpers.NamespaceFixture()).name - bridge = self.useFixture(net_helpers.OVSBridgeFixture()).bridge - self.assertFalse(bridge.get_port_name_list()) + self.assertFalse(self.bridge.get_port_name_list()) self.interface.plug(network_id=uuidutils.generate_uuid(), port_id=uuidutils.generate_uuid(), device_name=device_name, mac_address=mac_address, - bridge=bridge.br_name, + bridge=self.bridge.br_name, namespace=namespace) - self.assertIn(device_name, bridge.get_port_name_list()) + self.assertIn(device_name, self.bridge.get_port_name_list()) self.assertTrue(ip_lib.device_exists(device_name, namespace)) def test_plug_with_namespace_sets_mtu_higher_than_bridge(self): - device_mtu = 1450 - - # Create a new OVS bridge - ovs_bridge = self.useFixture(net_helpers.OVSBridgeFixture()).bridge - self.assertFalse(ovs_bridge.get_port_name_list()) - - # Add a new linuxbridge port with reduced MTU to OVS bridge + # First, add a new linuxbridge port with reduced MTU to OVS bridge lb_bridge = self.useFixture( net_helpers.LinuxBridgeFixture()).bridge lb_bridge_port = self.useFixture( net_helpers.LinuxBridgePortFixture(lb_bridge)) - lb_bridge_port.port.link.set_mtu(device_mtu - 1) - ovs_bridge.add_port(lb_bridge_port.port.name) + lb_bridge_port.port.link.set_mtu(1400) + self.bridge.add_port(lb_bridge_port.port.name) + + device_name = utils.get_rand_name() + namespace = self.useFixture(net_helpers.NamespaceFixture()).name # Now plug a device with intended MTU that is higher than for the port # above and validate that its MTU is not reduced to the least MTU on # the bridge - device_name = utils.get_rand_name() - mac_address = utils.get_random_mac('fa:16:3e:00:00:00'.split(':')) - namespace = self.useFixture(net_helpers.NamespaceFixture()).name - self.interface.plug(network_id=uuidutils.generate_uuid(), - port_id=uuidutils.generate_uuid(), - device_name=device_name, - mac_address=mac_address, - bridge=ovs_bridge.br_name, - namespace=namespace, - mtu=device_mtu) + self._test_mtu_set_after_action( + device_name, self.bridge_name, namespace) - self.assertIn(device_name, ovs_bridge.get_port_name_list()) - self.assertTrue(ip_lib.device_exists(device_name, namespace)) - self.assertEqual( - device_mtu, - ip_lib.IPDevice(device_name, namespace=namespace).link.mtu - ) + +class BridgeInterfaceDriverTestCase(base.BaseSudoTestCase, + InterfaceDriverTestCaseMixin): + def setUp(self): + super(BridgeInterfaceDriverTestCase, self).setUp() + conf = cfg.ConfigOpts() + conf.register_opts(interface.OPTS) + self.interface = interface.BridgeInterfaceDriver(conf) + self.bridge = self.useFixture(net_helpers.LinuxBridgeFixture()).bridge + + @property + def bridge_name(self): + return self.bridge.name diff --git a/neutron/tests/unit/agent/linux/test_interface.py b/neutron/tests/unit/agent/linux/test_interface.py index 09772c9b49e..01cfc691c78 100644 --- a/neutron/tests/unit/agent/linux/test_interface.py +++ b/neutron/tests/unit/agent/linux/test_interface.py @@ -348,6 +348,12 @@ class TestABCDriver(TestBase): [mock.call(device_name, namespace=ns), mock.call().addr.list(scope='link', ip_version=6)]) + def test_set_mtu_logs_once(self): + bc = BaseChild(self.conf) + with mock.patch('neutron.agent.linux.interface.LOG.warning') as log: + bc.set_mtu('dev', 9999) + log.assert_called_once_with(mock.ANY) + class TestOVSInterfaceDriver(TestBase): @@ -431,6 +437,8 @@ class TestOVSInterfaceDriver(TestBase): mock.call().ensure_namespace().add_device_to_namespace( mock.ANY)]) expected.extend([ + mock.call(namespace=namespace), + mock.call().device('tap0'), mock.call().device().link.set_mtu(9000), mock.call().device().link.set_up(), ]) @@ -480,6 +488,10 @@ class TestOVSInterfaceDriverWithVeth(TestOVSInterfaceDriver): root_dev = mock.Mock() ns_dev = mock.Mock() self.ip().add_veth = mock.Mock(return_value=(root_dev, ns_dev)) + mock.patch.object( + interface, '_get_veth', + return_value=(root_dev, ns_dev)).start() + expected = [mock.call(), mock.call().add_veth('tap0', devname, namespace2=namespace)] @@ -542,6 +554,9 @@ class TestBridgeInterfaceDriver(TestBase): ns_veth = mock.Mock() self.ip().add_veth = mock.Mock(return_value=(root_veth, ns_veth)) + mock.patch.object( + interface, '_get_veth', + return_value=(root_veth, ns_veth)).start() self.device_exists.side_effect = device_exists br = interface.BridgeInterfaceDriver(self.conf)