openstack/neutron
Code Issues Proposed changes

Calculate MTU on every network fetch instead of on create

Browse Source 
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
This commit is contained in:
Ihar Hrachyshka 2016-07-02 17:30:21 +02:00
parent 2495885add
commit a984f9554c
12 changed files with 226 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
neutron/common/config.py
View File

@ -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 = [

1
neutron/db/db_base_plugin_v2.py
View File

@ -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)

2
neutron/db/migration/alembic_migrations/versions/CONTRACT_HEAD
View File

@ -1 +1 @@
4bcd4df1f426
b67e765a3524

30
neutron/db/migration/alembic_migrations/versions/newton/contract/b67e765a3524_remove_mtu_column_from_networks.py Normal file
View File

@ -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')

1
neutron/db/models_v2.py
View File

@ -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',

15
neutron/db/netmtu_db.py
View File

@ -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(

12
neutron/plugins/ml2/managers.py
View File

@ -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]

48
neutron/plugins/ml2/plugin.py
View File

@ -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):

2
neutron/tests/unit/db/test_db_base_plugin_v2.py
View File

@ -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}>")

23
neutron/tests/unit/extensions/test_netmtu.py
View File

@ -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)

103
neutron/tests/unit/plugins/ml2/test_plugin.py
View File

@ -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()

12
releasenotes/notes/fix-mtu-for-existing-networks-5a476cde9bc46a53.yaml Normal file
View File

@ -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.