From a9133b7255355942a386756ef98df0bee6f0c33b Mon Sep 17 00:00:00 2001 From: Ihar Hrachyshka Date: Fri, 15 Jul 2016 18:14:12 +0200 Subject: [PATCH] Remove deprecated network_device_mtu option The right way to configure Neutron to work with infrastructure MTU is by using plugin agnostic global_physnet_mtu and ml2 specific path_mtu/physical_network_mtus options. The deprecated option is error prone and does not allow to use different MTUs per network. Closes-Bug: #1603493 Related-Bug: #1549470 Related-Bug: #1542108 Related-Bug: #1542475 DocImpact Remove all references to network_device_mtu option from Neutron documentation. Note that Nova has a deprecated option with the same name that will need a separate patch to be removed. Depends-On: I8e6cc99fe70d0c41a705431fb3160e8fccacff10 Depends-On: I337b284076a794027fbd63796119d56bd1923cf2 Change-Id: I7287db9df25a78a59b2dfa28acfde7fe69d17f40 --- neutron/agent/l3/dvr_fip_ns.py | 3 +- neutron/agent/linux/interface.py | 22 ------ .../functional/agent/l3/test_dvr_router.py | 22 ++---- .../tests/unit/agent/l3/test_dvr_fip_ns.py | 2 +- .../tests/unit/agent/linux/test_interface.py | 74 ++++++------------- ...rk_device_mtu-option-a1a96e99dc7f0a02.yaml | 8 ++ 6 files changed, 39 insertions(+), 92 deletions(-) create mode 100644 releasenotes/notes/remove-network_device_mtu-option-a1a96e99dc7f0a02.yaml diff --git a/neutron/agent/l3/dvr_fip_ns.py b/neutron/agent/l3/dvr_fip_ns.py index 9ce41b3516d..c931743d0a3 100644 --- a/neutron/agent/l3/dvr_fip_ns.py +++ b/neutron/agent/l3/dvr_fip_ns.py @@ -266,8 +266,7 @@ class FipNamespace(namespaces.Namespace): rtr_2_fip_dev, fip_2_rtr_dev = ip_wrapper.add_veth(rtr_2_fip_name, fip_2_rtr_name, fip_ns_name) - mtu = (self.agent_conf.network_device_mtu or - ri.get_ex_gw_port().get('mtu')) + mtu = ri.get_ex_gw_port().get('mtu') if mtu: rtr_2_fip_dev.link.set_mtu(mtu) fip_2_rtr_dev.link.set_mtu(mtu) diff --git a/neutron/agent/linux/interface.py b/neutron/agent/linux/interface.py index 4e4b74cb052..18c9ae15e2e 100644 --- a/neutron/agent/linux/interface.py +++ b/neutron/agent/linux/interface.py @@ -26,9 +26,7 @@ from neutron._i18n import _, _LE, _LI, _LW from neutron.agent.common import ovs_lib from neutron.agent.linux import ip_lib from neutron.agent.linux import utils -from neutron.common import constants as n_const from neutron.common import exceptions -from neutron.common import ipv6_utils LOG = logging.getLogger(__name__) @@ -43,12 +41,6 @@ OPTS = [ 'Support kernels with limited namespace support ' '(e.g. RHEL 6.5) so long as ovs_use_veth is set to ' 'True.')), - cfg.IntOpt('network_device_mtu', - deprecated_for_removal=True, - help=_('MTU setting for device. This option will be removed in ' - 'Newton. Please use the system-wide segment_mtu setting ' - 'which the agents will take into account when wiring ' - 'VIFs.')), ] @@ -61,17 +53,6 @@ class LinuxInterfaceDriver(object): def __init__(self, conf): self.conf = conf - if self.conf.network_device_mtu: - self._validate_network_device_mtu() - - def _validate_network_device_mtu(self): - if (ipv6_utils.is_enabled() and - self.conf.network_device_mtu < n_const.IPV6_MIN_MTU): - LOG.error(_LE("IPv6 protocol requires a minimum MTU of " - "%(min_mtu)s, while the configured value is " - "%(current_mtu)s"), {'min_mtu': n_const.IPV6_MIN_MTU, - 'current_mtu': self.conf.network_device_mtu}) - raise SystemExit(1) @property def use_gateway_ips(self): @@ -352,7 +333,6 @@ class OVSInterfaceDriver(LinuxInterfaceDriver): # the device is moved into a namespace, otherwise OVS bridge does not # allow to set MTU that is higher than the least of all device MTUs on # the bridge - mtu = self.conf.network_device_mtu or mtu if mtu: ns_dev.link.set_mtu(mtu) if self.conf.ovs_use_veth: @@ -416,7 +396,6 @@ class IVSInterfaceDriver(LinuxInterfaceDriver): ns_dev = ip.device(device_name) ns_dev.link.set_address(mac_address) - mtu = self.conf.network_device_mtu or mtu if mtu: ns_dev.link.set_mtu(mtu) root_dev.link.set_mtu(mtu) @@ -463,7 +442,6 @@ class BridgeInterfaceDriver(LinuxInterfaceDriver): root_veth.disable_ipv6() ns_veth.link.set_address(mac_address) - mtu = self.conf.network_device_mtu or mtu if mtu: root_veth.link.set_mtu(mtu) ns_veth.link.set_mtu(mtu) diff --git a/neutron/tests/functional/agent/l3/test_dvr_router.py b/neutron/tests/functional/agent/l3/test_dvr_router.py index e4896869b25..9f1ca4d7035 100644 --- a/neutron/tests/functional/agent/l3/test_dvr_router.py +++ b/neutron/tests/functional/agent/l3/test_dvr_router.py @@ -57,10 +57,6 @@ class TestDvrRouter(framework.L3AgentTestFramework): self.agent.conf.agent_mode = 'dvr' self._test_update_floatingip_statuses(self.generate_dvr_router_info()) - def test_dvr_router_lifecycle_ha_with_snat_with_fips_nmtu(self): - self._dvr_router_lifecycle(enable_ha=True, enable_snat=True, - use_port_mtu=True) - def test_dvr_router_lifecycle_without_ha_without_snat_with_fips(self): self._dvr_router_lifecycle(enable_ha=False, enable_snat=False) @@ -174,9 +170,7 @@ class TestDvrRouter(framework.L3AgentTestFramework): self._validate_fips_for_external_network(router2, fip2_ns) def _dvr_router_lifecycle(self, enable_ha=False, enable_snat=False, - custom_mtu=2000, use_port_mtu=False, - ip_version=4, - dual_stack=False): + custom_mtu=2000, ip_version=4, dual_stack=False): '''Test dvr router lifecycle :param enable_ha: sets the ha value for the router. @@ -192,15 +186,13 @@ class TestDvrRouter(framework.L3AgentTestFramework): # We get the router info particular to a dvr router router_info = self.generate_dvr_router_info( enable_ha, enable_snat, extra_routes=True) - if use_port_mtu: - for key in ('_interfaces', '_snat_router_interfaces', - '_floatingip_agent_interfaces'): - for port in router_info[key]: - port['mtu'] = custom_mtu - router_info['gw_port']['mtu'] = custom_mtu + for key in ('_interfaces', '_snat_router_interfaces', + '_floatingip_agent_interfaces'): + for port in router_info[key]: + port['mtu'] = custom_mtu + router_info['gw_port']['mtu'] = custom_mtu + if enable_ha: router_info['_ha_interface']['mtu'] = custom_mtu - else: - self.agent.conf.network_device_mtu = custom_mtu # We need to mock the get_agent_gateway_port return value # because the whole L3PluginApi is mocked and we need the port diff --git a/neutron/tests/unit/agent/l3/test_dvr_fip_ns.py b/neutron/tests/unit/agent/l3/test_dvr_fip_ns.py index 089b3c93e6f..96b5cfb1fbc 100644 --- a/neutron/tests/unit/agent/l3/test_dvr_fip_ns.py +++ b/neutron/tests/unit/agent/l3/test_dvr_fip_ns.py @@ -224,6 +224,7 @@ class TestDvrFipNs(base.BaseTestCase): ri.router_id = _uuid() ri.rtr_fip_subnet = None ri.ns_name = mock.sentinel.router_ns + ri.get_ex_gw_port.return_value = {'mtu': 2000} rtr_2_fip_name = self.fip_ns.get_rtr_ext_device_name(ri.router_id) fip_2_rtr_name = self.fip_ns.get_int_device_name(ri.router_id) @@ -234,7 +235,6 @@ class TestDvrFipNs(base.BaseTestCase): allocator.allocate.return_value = pair addr_pair = pair.get_pair() ip_wrapper = IPWrapper() - self.conf.network_device_mtu = 2000 ip_wrapper.add_veth.return_value = (IPDevice(), IPDevice()) device = IPDevice() device.exists.return_value = dev_exists diff --git a/neutron/tests/unit/agent/linux/test_interface.py b/neutron/tests/unit/agent/linux/test_interface.py index ccda7a8b4a3..2a5cf2935d6 100644 --- a/neutron/tests/unit/agent/linux/test_interface.py +++ b/neutron/tests/unit/agent/linux/test_interface.py @@ -16,7 +16,6 @@ import mock from neutron_lib import constants from oslo_log import versionutils -import testtools from neutron.agent.common import config from neutron.agent.common import ovs_lib @@ -391,11 +390,9 @@ class TestOVSInterfaceDriver(TestBase): 'aa:bb:cc:dd:ee:ff', internal=True) - def _test_plug(self, additional_expectation=None, bridge=None, - namespace=None): + def _test_plug(self, bridge=None, namespace=None): with mock.patch('neutron.agent.ovsdb.native.connection.' 'Connection.start'): - additional_expectation = additional_expectation or [] if not bridge: bridge = 'br-int' @@ -411,7 +408,8 @@ class TestOVSInterfaceDriver(TestBase): 'tap0', 'aa:bb:cc:dd:ee:ff', bridge=bridge, - namespace=namespace) + namespace=namespace, + mtu=9000) replace.assert_called_once_with( 'tap0', ('type', 'internal'), @@ -424,32 +422,18 @@ class TestOVSInterfaceDriver(TestBase): mock.call(), mock.call().device('tap0'), mock.call().device().link.set_address('aa:bb:cc:dd:ee:ff')] - expected.extend(additional_expectation) if namespace: expected.extend( [mock.call().ensure_namespace(namespace), mock.call().ensure_namespace().add_device_to_namespace( mock.ANY)]) - expected.extend([mock.call().device().link.set_up()]) + expected.extend([ + mock.call().device().link.set_mtu(9000), + mock.call().device().link.set_up(), + ]) self.ip.assert_has_calls(expected) - def test_mtu_int(self): - self.assertIsNone(self.conf.network_device_mtu) - self.conf.set_override('network_device_mtu', 9000) - self.assertEqual(self.conf.network_device_mtu, 9000) - - def test_validate_min_ipv6_mtu(self): - self.conf.set_override('network_device_mtu', 1200) - with mock.patch('neutron.common.ipv6_utils.is_enabled') as ipv6_status: - with testtools.ExpectedException(SystemExit): - ipv6_status.return_value = True - BaseChild(self.conf) - - def test_plug_mtu(self): - self.conf.set_override('network_device_mtu', 9000) - self._test_plug([mock.call().device().link.set_mtu(9000)]) - def test_unplug(self, bridge=None): if not bridge: bridge = 'br-int' @@ -475,7 +459,7 @@ class TestOVSInterfaceDriverWithVeth(TestOVSInterfaceDriver): self._test_plug(devname='qr-0', prefix='qr-') def _test_plug(self, devname=None, bridge=None, namespace=None, - prefix=None, mtu=None): + prefix=None): with mock.patch('neutron.agent.ovsdb.native.connection.' 'Connection.start'): @@ -505,7 +489,8 @@ class TestOVSInterfaceDriverWithVeth(TestOVSInterfaceDriver): 'aa:bb:cc:dd:ee:ff', bridge=bridge, namespace=namespace, - prefix=prefix) + prefix=prefix, + mtu=9000) replace.assert_called_once_with( 'tap0', ('external_ids', { @@ -515,18 +500,13 @@ class TestOVSInterfaceDriverWithVeth(TestOVSInterfaceDriver): ns_dev.assert_has_calls( [mock.call.link.set_address('aa:bb:cc:dd:ee:ff')]) - if mtu: - ns_dev.assert_has_calls([mock.call.link.set_mtu(mtu)]) - root_dev.assert_has_calls([mock.call.link.set_mtu(mtu)]) + ns_dev.assert_has_calls([mock.call.link.set_mtu(9000)]) + root_dev.assert_has_calls([mock.call.link.set_mtu(9000)]) self.ip.assert_has_calls(expected) root_dev.assert_has_calls([mock.call.link.set_up()]) ns_dev.assert_has_calls([mock.call.link.set_up()]) - def test_plug_mtu(self): - self.conf.set_override('network_device_mtu', 9000) - self._test_plug(mtu=9000) - def test_unplug(self, bridge=None): if not bridge: bridge = 'br-int' @@ -551,7 +531,7 @@ class TestBridgeInterfaceDriver(TestBase): def test_plug_with_ns(self): self._test_plug(namespace='01234567-1234-1234-99') - def _test_plug(self, namespace=None, mtu=None): + def _test_plug(self, namespace=None): def device_exists(device, namespace=None): return device.startswith('brq') @@ -567,14 +547,14 @@ class TestBridgeInterfaceDriver(TestBase): 'port-1234', 'ns-0', mac_address, - namespace=namespace) + namespace=namespace, + mtu=9000) ip_calls = [mock.call(), mock.call().add_veth('tap0', 'ns-0', namespace2=namespace)] ns_veth.assert_has_calls([mock.call.link.set_address(mac_address)]) - if mtu: - ns_veth.assert_has_calls([mock.call.link.set_mtu(mtu)]) - root_veth.assert_has_calls([mock.call.link.set_mtu(mtu)]) + ns_veth.assert_has_calls([mock.call.link.set_mtu(9000)]) + root_veth.assert_has_calls([mock.call.link.set_mtu(9000)]) self.ip.assert_has_calls(ip_calls) @@ -592,11 +572,6 @@ class TestBridgeInterfaceDriver(TestBase): self.assertFalse(self.ip_dev.called) self.assertEqual(log.call_count, 1) - def test_plug_mtu(self): - self.device_exists.return_value = False - self.conf.set_override('network_device_mtu', 9000) - self._test_plug(mtu=9000) - def test_unplug_no_device(self): self.device_exists.return_value = False self.ip_dev().link.delete.side_effect = RuntimeError @@ -630,8 +605,7 @@ class TestIVSInterfaceDriver(TestBase): def test_plug_with_prefix(self): self._test_plug(devname='qr-0', prefix='qr-') - def _test_plug(self, devname=None, namespace=None, - prefix=None, mtu=None): + def _test_plug(self, devname=None, namespace=None, prefix=None): if not devname: devname = 'ns-0' @@ -658,14 +632,14 @@ class TestIVSInterfaceDriver(TestBase): devname, 'aa:bb:cc:dd:ee:ff', namespace=namespace, - prefix=prefix) + prefix=prefix, + mtu=9000) execute.assert_called_once_with(ivsctl_cmd, run_as_root=True) ns_dev.assert_has_calls( [mock.call.link.set_address('aa:bb:cc:dd:ee:ff')]) - if mtu: - ns_dev.assert_has_calls([mock.call.link.set_mtu(mtu)]) - root_dev.assert_has_calls([mock.call.link.set_mtu(mtu)]) + ns_dev.assert_has_calls([mock.call.link.set_mtu(9000)]) + root_dev.assert_has_calls([mock.call.link.set_mtu(9000)]) if namespace: expected.extend( [mock.call().ensure_namespace(namespace), @@ -676,10 +650,6 @@ class TestIVSInterfaceDriver(TestBase): root_dev.assert_has_calls([mock.call.link.set_up()]) ns_dev.assert_has_calls([mock.call.link.set_up()]) - def test_plug_mtu(self): - self.conf.set_override('network_device_mtu', 9000) - self._test_plug(mtu=9000) - def test_plug_namespace(self): self._test_plug(namespace='mynamespace') diff --git a/releasenotes/notes/remove-network_device_mtu-option-a1a96e99dc7f0a02.yaml b/releasenotes/notes/remove-network_device_mtu-option-a1a96e99dc7f0a02.yaml new file mode 100644 index 00000000000..e4d9827d0a7 --- /dev/null +++ b/releasenotes/notes/remove-network_device_mtu-option-a1a96e99dc7f0a02.yaml @@ -0,0 +1,8 @@ +--- +upgrade: + - The network_device_mtu option is removed. Existing + users of the option are advised to adopt new + configuration options to accommodate for their + underlying physical infrastructure. The relevant + options are global_physnet_mtu for all plugins, + and also path_mtu and physical_network_mtus for ML2.