From a984f9554cdcbe93c840a1d8f5c04302e9331e79 Mon Sep 17 00:00:00 2001
From: Ihar Hrachyshka <ihrachys@redhat.com>
Date: Sat, 2 Jul 2016 17:30:21 +0200
Subject: [PATCH] Calculate MTU on every network fetch instead of on create
Today, existing networks may not reflect MTU configured for
neutron-server, if they were created when neutron-server was using
different MTU setup for its infrastructure, or when it was using bad
default values for network MTUs (specifically, before Mitaka, all networks
were getting MTU = 0 by default, disabling both advertisement and data
path MTU size enforcement).
This patch stops persisting MTU in the database on network create and
instead calculate it on every network resource fetch.
DocImpact Now changes to MTU configuration options immediately affect
existing network MTUs, not just new networks.
UpgradeImpact Existing networks with invalid MTU persisted in database
may change their MTU values to reflect configuration.
Change-Id: Iee4f5037bf10b73ba98464143b183aacb59c22f2
Closes-Bug: #1556182
---
neutron/common/config.py | 6 +-
neutron/db/db_base_plugin_v2.py | 1 -
.../alembic_migrations/versions/CONTRACT_HEAD | 2 +-
...65a3524_remove_mtu_column_from_networks.py | 30 +++++
neutron/db/models_v2.py | 1 -
neutron/db/netmtu_db.py | 15 ++-
neutron/plugins/ml2/managers.py | 12 +-
neutron/plugins/ml2/plugin.py | 48 +++++++-
.../tests/unit/db/test_db_base_plugin_v2.py | 2 +-
neutron/tests/unit/extensions/test_netmtu.py | 23 +++-
neutron/tests/unit/plugins/ml2/test_plugin.py | 103 +++++++++++++++++-
...or-existing-networks-5a476cde9bc46a53.yaml | 12 ++
12 files changed, 226 insertions(+), 29 deletions(-)
create mode 100644 neutron/db/migration/alembic_migrations/versions/newton/contract/b67e765a3524_remove_mtu_column_from_networks.py
create mode 100644 releasenotes/notes/fix-mtu-for-existing-networks-5a476cde9bc46a53.yaml
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.