diff --git a/neutron/common/config.py b/neutron/common/config.py index e9eca9b27ba..048b88cf636 100644 --- a/neutron/common/config.py +++ b/neutron/common/config.py @@ -184,7 +184,11 @@ core_opts = [ 'this value without modification. For overlay networks ' 'such as VXLAN, neutron automatically subtracts the ' 'overlay protocol overhead from this value. Defaults ' - 'to 1500, the standard value for Ethernet.')) + 'to 1500, the standard value for Ethernet. If you want ' + 'to propagate a change to infrastructure MTU into the ' + 'backend, you may need to resync agents that manage ' + 'ports, as well as re-attach VIFs to affected ' + 'instances.')) ] core_cli_opts = [ diff --git a/neutron/db/db_base_plugin_v2.py b/neutron/db/db_base_plugin_v2.py index f89fa012c80..d63460763ef 100644 --- a/neutron/db/db_base_plugin_v2.py +++ b/neutron/db/db_base_plugin_v2.py @@ -353,7 +353,6 @@ class NeutronDbPluginV2(db_base_plugin_common.DbBasePluginCommon, 'id': n.get('id') or uuidutils.generate_uuid(), 'name': n['name'], 'admin_state_up': n['admin_state_up'], - 'mtu': n.get('mtu', n_const.DEFAULT_NETWORK_MTU), 'status': n.get('status', constants.NET_STATUS_ACTIVE), 'description': n.get('description')} network = models_v2.Network(**args) diff --git a/neutron/db/migration/alembic_migrations/versions/CONTRACT_HEAD b/neutron/db/migration/alembic_migrations/versions/CONTRACT_HEAD index 028a06a4a0c..145207b5017 100644 --- a/neutron/db/migration/alembic_migrations/versions/CONTRACT_HEAD +++ b/neutron/db/migration/alembic_migrations/versions/CONTRACT_HEAD @@ -1 +1 @@ -4bcd4df1f426 +b67e765a3524 diff --git a/neutron/db/migration/alembic_migrations/versions/newton/contract/b67e765a3524_remove_mtu_column_from_networks.py b/neutron/db/migration/alembic_migrations/versions/newton/contract/b67e765a3524_remove_mtu_column_from_networks.py new file mode 100644 index 00000000000..9948f4a3529 --- /dev/null +++ b/neutron/db/migration/alembic_migrations/versions/newton/contract/b67e765a3524_remove_mtu_column_from_networks.py @@ -0,0 +1,30 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +# + +"""Remove mtu column from networks. + +Revision ID: b67e765a3524 +Revises: 4bcd4df1f426 +Create Date: 2016-07-17 02:07:36.625196 + +""" + +# revision identifiers, used by Alembic. +revision = 'b67e765a3524' +down_revision = '4bcd4df1f426' + +from alembic import op + + +def upgrade(): + op.drop_column('networks', 'mtu') diff --git a/neutron/db/models_v2.py b/neutron/db/models_v2.py index 4d004bf496c..4098f946934 100644 --- a/neutron/db/models_v2.py +++ b/neutron/db/models_v2.py @@ -275,7 +275,6 @@ class Network(model_base.HasStandardAttributes, model_base.BASEV2, lazy="joined") status = sa.Column(sa.String(16)) admin_state_up = sa.Column(sa.Boolean) - mtu = sa.Column(sa.Integer, nullable=True) vlan_transparent = sa.Column(sa.Boolean, nullable=True) rbac_entries = orm.relationship(rbac_db_models.NetworkRBAC, backref='network', lazy='joined', diff --git a/neutron/db/netmtu_db.py b/neutron/db/netmtu_db.py index 7d6acf79f69..101477a4966 100644 --- a/neutron/db/netmtu_db.py +++ b/neutron/db/netmtu_db.py @@ -13,16 +13,27 @@ # License for the specific language governing permissions and limitations # under the License. +from oslo_config import cfg + from neutron.api.v2 import attributes from neutron.db import db_base_plugin_v2 from neutron.extensions import netmtu +from neutron.plugins.common import utils +CONF = cfg.CONF + + +# TODO(ihrachys): the class is not used in the tree; mixins are generally +# discouraged these days, so maybe it's worth considering deprecation for the +# class. Interested plugins would be able to ship it on their own, if they want +# to stick to mixins, or implement the behaviour in another way. class Netmtu_db_mixin(object): - """Mixin class to add network MTU methods to db_base_plugin_v2.""" + """Mixin class to add network MTU support to db_base_plugin_v2.""" def _extend_network_dict_mtu(self, network_res, network_db): - network_res[netmtu.MTU] = network_db.mtu + # don't use network_db argument since MTU is not persisted in database + network_res[netmtu.MTU] = utils.get_deployment_physnet_mtu() return network_res db_base_plugin_v2.NeutronDbPluginV2.register_dict_extend_funcs( diff --git a/neutron/plugins/ml2/managers.py b/neutron/plugins/ml2/managers.py index cf88160f98f..12fcaee93cd 100644 --- a/neutron/plugins/ml2/managers.py +++ b/neutron/plugins/ml2/managers.py @@ -182,18 +182,15 @@ class TypeManager(stevedore.named.NamedExtensionManager): LOG.info(_LI("Initializing driver for type '%s'"), network_type) driver.obj.initialize() - def _add_network_segment(self, context, network_id, segment, mtu, + def _add_network_segment(self, context, network_id, segment, segment_index=0): segments_db.add_network_segment( context, network_id, segment, segment_index) - if segment.get(api.MTU, 0) > 0: - mtu.append(segment[api.MTU]) def create_network_segments(self, context, network, tenant_id): """Call type drivers to create network segments.""" segments = self._process_provider_create(network) session = context.session - mtu = [] with session.begin(subtransactions=True): network_id = network['id'] if segments: @@ -201,15 +198,14 @@ class TypeManager(stevedore.named.NamedExtensionManager): segment = self.reserve_provider_segment( session, segment) self._add_network_segment(context, network_id, segment, - mtu, segment_index) + segment_index) elif (cfg.CONF.ml2.external_network_type and self._get_attribute(network, external_net.EXTERNAL)): segment = self._allocate_ext_net_segment(session) - self._add_network_segment(context, network_id, segment, mtu) + self._add_network_segment(context, network_id, segment) else: segment = self._allocate_tenant_net_segment(session) - self._add_network_segment(context, network_id, segment, mtu) - network[api.MTU] = min(mtu) if mtu else 0 + self._add_network_segment(context, network_id, segment) def is_partial_segment(self, segment): network_type = segment[api.NETWORK_TYPE] diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py index 4be8b26ac59..e68eb14a3c3 100644 --- a/neutron/plugins/ml2/plugin.py +++ b/neutron/plugins/ml2/plugin.py @@ -57,7 +57,6 @@ from neutron.db import dvr_mac_db from neutron.db import external_net_db from neutron.db import extradhcpopt_db from neutron.db import models_v2 -from neutron.db import netmtu_db from neutron.db import provisioning_blocks from neutron.db.quota import driver # noqa from neutron.db import securitygroups_db @@ -67,6 +66,7 @@ from neutron.db import vlantransparent_db from neutron.extensions import allowedaddresspairs as addr_pair from neutron.extensions import availability_zone as az_ext from neutron.extensions import extra_dhcp_opt as edo_ext +from neutron.extensions import multiprovidernet as mpnet from neutron.extensions import portbindings from neutron.extensions import portsecurity as psec from neutron.extensions import providernet as provider @@ -103,7 +103,6 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, addr_pair_db.AllowedAddressPairsMixin, vlantransparent_db.Vlantransparent_db_mixin, extradhcpopt_db.ExtraDhcpOptMixin, - netmtu_db.Netmtu_db_mixin, address_scope_db.AddressScopeDbMixin): """Implement the Neutron L2 abstractions using modules. @@ -685,6 +684,41 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, 'resource_ids': ', '.join(resource_ids)}) self._delete_objects(context, resource, objects) + def _get_network_mtu(self, network): + mtus = [] + try: + segments = network[mpnet.SEGMENTS] + except KeyError: + segments = [network] + for s in segments: + segment_type = s[provider.NETWORK_TYPE] + try: + type_driver = self.type_manager.drivers[segment_type].obj + except KeyError: + # NOTE(ihrachys) This can happen when type driver is not loaded + # for an existing segment. While it's probably an indication of + # a bad setup, it's better to be safe than sorry here. Also, + # several unit tests use non-existent driver types that may + # trigger the exception here. + LOG.warning( + _LW("Failed to determine MTU for segment " + "%(segment_type)s:%(segment_id)s; network " + "%(network_id)s MTU calculation may be not accurate"), + { + 'segment_type': segment_type, + 'segment_id': s[provider.SEGMENTATION_ID], + 'network_id': network['id'], + } + ) + else: + mtu = type_driver.get_mtu(provider.PHYSICAL_NETWORK) + # Some drivers, like 'local', may return None; the assumption + # then is that for the segment type, MTU has no meaning or + # unlimited, and so we should then ignore those values. + if mtu: + mtus.append(mtu) + return min(mtus) if mtus else 0 + def _create_network_db(self, context, network): net_data = network[attributes.NETWORK] tenant_id = net_data['tenant_id'] @@ -705,9 +739,7 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, result) self.mechanism_manager.create_network_precommit(mech_context) - if net_data.get(api.MTU, 0) > 0: - net_db[api.MTU] = net_data[api.MTU] - result[api.MTU] = net_data[api.MTU] + result[api.MTU] = self._get_network_mtu(result) if az_ext.AZ_HINTS in net_data: self.validate_availability_zones(context, 'network', @@ -758,6 +790,8 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, self.type_manager.extend_network_dict_provider(context, updated_network) + updated_network[api.MTU] = self._get_network_mtu(updated_network) + # TODO(QoS): Move out to the extension framework somehow. need_network_update_notify = ( qos_consts.QOS_POLICY_ID in net_data and @@ -783,6 +817,7 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, with session.begin(subtransactions=True): result = super(Ml2Plugin, self).get_network(context, id, None) self.type_manager.extend_network_dict_provider(context, result) + result[api.MTU] = self._get_network_mtu(result) return self._fields(result, fields) @@ -797,6 +832,9 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, nets = self._filter_nets_provider(context, nets, filters) + for net in nets: + net[api.MTU] = self._get_network_mtu(net) + return [self._fields(net, fields) for net in nets] def _delete_ports(self, context, port_ids): diff --git a/neutron/tests/unit/db/test_db_base_plugin_v2.py b/neutron/tests/unit/db/test_db_base_plugin_v2.py index a9f88da566d..bfed743c4f3 100644 --- a/neutron/tests/unit/db/test_db_base_plugin_v2.py +++ b/neutron/tests/unit/db/test_db_base_plugin_v2.py @@ -6018,7 +6018,7 @@ class DbModelTestCase(testlib_api.SqlTestCase): exp_middle = "[object at %x]" % id(network) exp_end_with = (" {tenant_id=None, id=None, " "name='net_net', status='OK', " - "admin_state_up=True, mtu=None, " + "admin_state_up=True, " "vlan_transparent=None, " "availability_zone_hints=None, " "standard_attr_id=None}>") diff --git a/neutron/tests/unit/extensions/test_netmtu.py b/neutron/tests/unit/extensions/test_netmtu.py index aae2554978e..8cbbfb4eb53 100644 --- a/neutron/tests/unit/extensions/test_netmtu.py +++ b/neutron/tests/unit/extensions/test_netmtu.py @@ -12,6 +12,8 @@ # License for the specific language governing permissions and limitations # under the License. +from oslo_config import cfg + from neutron.common import constants from neutron.db import db_base_plugin_v2 from neutron.db import netmtu_db @@ -64,11 +66,20 @@ class NetmtuExtensionTestCase(test_db_base_plugin_v2.TestNetworksV2): self.assertEqual(constants.DEFAULT_NETWORK_MTU, res['networks'][0].get('mtu')) + def _assert_network_mtu(self, net_id, expected_mtu): + req = self.new_show_request('networks', net_id) + res = self.deserialize(self.fmt, req.get_response(self.api)) + self.assertEqual(expected_mtu, res['network']['mtu']) + def test_show_network_mtu(self): with self.network(name='net1') as net: - req = self.new_show_request('networks', net['network']['id']) - res = self.deserialize(self.fmt, req.get_response(self.api)) - self.assertEqual(res['network']['name'], - net['network']['name']) - self.assertEqual(constants.DEFAULT_NETWORK_MTU, - res['network']['mtu']) + self._assert_network_mtu( + net['network']['id'], constants.DEFAULT_NETWORK_MTU) + + def test_network_mtu_immediately_reflects_config_option(self): + with self.network(name='net1') as net: + self._assert_network_mtu( + net['network']['id'], cfg.CONF.global_physnet_mtu) + cfg.CONF.set_override('global_physnet_mtu', 1400) + self._assert_network_mtu( + net['network']['id'], cfg.CONF.global_physnet_mtu) diff --git a/neutron/tests/unit/plugins/ml2/test_plugin.py b/neutron/tests/unit/plugins/ml2/test_plugin.py index 15c89f255cc..6a9aa22a1d6 100644 --- a/neutron/tests/unit/plugins/ml2/test_plugin.py +++ b/neutron/tests/unit/plugins/ml2/test_plugin.py @@ -1027,6 +1027,97 @@ class TestMl2PluginOnly(Ml2PluginV2TestCase): self.context, port_id)) +class Test_GetNetworkMtu(Ml2PluginV2TestCase): + + def _register_type_driver_with_mtu(self, driver, mtu): + plugin = manager.NeutronManager.get_plugin() + + class FakeDriver(object): + def get_mtu(self, physical_network=None): + return mtu + + driver_mock = mock.Mock() + driver_mock.obj = FakeDriver() + plugin.type_manager.drivers[driver] = driver_mock + + def test_single_segment(self): + plugin = manager.NeutronManager.get_plugin() + self._register_type_driver_with_mtu('driver1', 1400) + + net = { + 'name': 'net1', + mpnet.SEGMENTS: [ + { + pnet.NETWORK_TYPE: 'driver1', + pnet.PHYSICAL_NETWORK: 'physnet1' + }, + ] + } + self.assertEqual(1400, plugin._get_network_mtu(net)) + + def test_multiple_segments_returns_minimal_mtu(self): + plugin = manager.NeutronManager.get_plugin() + self._register_type_driver_with_mtu('driver1', 1400) + self._register_type_driver_with_mtu('driver2', 1300) + + net = { + 'name': 'net1', + mpnet.SEGMENTS: [ + { + pnet.NETWORK_TYPE: 'driver1', + pnet.PHYSICAL_NETWORK: 'physnet1' + }, + { + pnet.NETWORK_TYPE: 'driver2', + pnet.PHYSICAL_NETWORK: 'physnet2' + }, + ] + } + self.assertEqual(1300, plugin._get_network_mtu(net)) + + def test_no_segments(self): + plugin = manager.NeutronManager.get_plugin() + self._register_type_driver_with_mtu('driver1', 1400) + + net = { + 'name': 'net1', + pnet.NETWORK_TYPE: 'driver1', + pnet.PHYSICAL_NETWORK: 'physnet1', + } + self.assertEqual(1400, plugin._get_network_mtu(net)) + + def test_get_mtu_None_returns_0(self): + plugin = manager.NeutronManager.get_plugin() + self._register_type_driver_with_mtu('driver1', None) + + net = { + 'name': 'net1', + pnet.NETWORK_TYPE: 'driver1', + pnet.PHYSICAL_NETWORK: 'physnet1', + } + self.assertEqual(0, plugin._get_network_mtu(net)) + + def test_unknown_segment_type_ignored(self): + plugin = manager.NeutronManager.get_plugin() + self._register_type_driver_with_mtu('driver1', None) + self._register_type_driver_with_mtu('driver2', 1300) + + net = { + 'name': 'net1', + mpnet.SEGMENTS: [ + { + pnet.NETWORK_TYPE: 'driver1', + pnet.PHYSICAL_NETWORK: 'physnet1' + }, + { + pnet.NETWORK_TYPE: 'driver2', + pnet.PHYSICAL_NETWORK: 'physnet2' + }, + ] + } + self.assertEqual(1300, plugin._get_network_mtu(net)) + + class TestMl2DvrPortsV2(TestMl2PortsV2): def setUp(self): super(TestMl2DvrPortsV2, self).setUp() @@ -2098,7 +2189,9 @@ class TestMl2PluginCreateUpdateDeletePort(base.BaseTestCase): mock.patch.object(base_plugin.NeutronDbPluginV2, 'update_port'),\ mock.patch.object(base_plugin.NeutronDbPluginV2, - 'create_port_db'): + 'create_port_db'),\ + mock.patch.object(ml2_plugin.Ml2Plugin, + '_get_network_mtu'): init.return_value = None new_port = mock.MagicMock() @@ -2136,7 +2229,9 @@ class TestMl2PluginCreateUpdateDeletePort(base.BaseTestCase): mock.patch.object(ml2_db, 'get_locked_port_and_binding', return_value=(original_port_db, binding)),\ mock.patch.object(base_plugin.NeutronDbPluginV2, - 'update_port') as db_update_port: + 'update_port') as db_update_port,\ + mock.patch.object(ml2_plugin.Ml2Plugin, + '_get_network_mtu'): init.return_value = None updated_port = mock.MagicMock() db_update_port.return_value = updated_port @@ -2167,7 +2262,9 @@ class TestMl2PluginCreateUpdateDeletePort(base.BaseTestCase): return_value=None),\ mock.patch.object(manager.NeutronManager, 'get_service_plugins', - return_value={'L3_ROUTER_NAT': l3plugin}): + return_value={'L3_ROUTER_NAT': l3plugin}),\ + mock.patch.object(ml2_plugin.Ml2Plugin, + '_get_network_mtu'): plugin = self._create_plugin_for_create_update_port() # Set backend manually here since __init__ was mocked plugin.set_ipam_backend() diff --git a/releasenotes/notes/fix-mtu-for-existing-networks-5a476cde9bc46a53.yaml b/releasenotes/notes/fix-mtu-for-existing-networks-5a476cde9bc46a53.yaml new file mode 100644 index 00000000000..e65a3b9a150 --- /dev/null +++ b/releasenotes/notes/fix-mtu-for-existing-networks-5a476cde9bc46a53.yaml @@ -0,0 +1,12 @@ +--- +features: + - net-mtu extension now recalculates network MTU on each network access, not + just on creation. It now allows operators to tweak MTU related + configuration options and see them applied to all network resources right + after controller restart, both old and new. +upgrade: + - Existing networks with MTU values that don't reflect configuration + will receive new MTU values after controller upgrade. Note that to + propagate new correct MTU values to your backend, you may need to resync + all agents that set up ports, as well as re-attach VIFs to affected + instances.