diff --git a/neutron/agent/linux/interface.py b/neutron/agent/linux/interface.py index 955b08f6b40..32554de283f 100644 --- a/neutron/agent/linux/interface.py +++ b/neutron/agent/linux/interface.py @@ -46,6 +46,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): @@ -242,6 +248,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): @@ -264,6 +275,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, @@ -349,9 +366,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) @@ -378,6 +393,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.""" @@ -458,8 +483,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) @@ -475,3 +499,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)