From cc69828ff09f5dbc37488f3f56bb045772e35634 Mon Sep 17 00:00:00 2001 From: Ihar Hrachyshka Date: Mon, 7 Aug 2017 10:19:57 -0700 Subject: [PATCH] Apply network MTU changes to l3 ports This patch makes L3 agent to update its ports' MTU when it's changed on core plugin side. Related-Bug: #1671634 Change-Id: I4444da6358e8b8420a3a365e1107b02f5bb1161d --- doc/source/admin/config-mtu.rst | 3 --- neutron/agent/l3/agent.py | 21 +++++++++++++++- neutron/agent/l3/ha_router.py | 4 ++- neutron/agent/l3/router_info.py | 16 ++++++++---- neutron/plugins/ml2/plugin.py | 5 +++- neutron/tests/common/l3_test_common.py | 5 ++++ neutron/tests/fullstack/resources/client.py | 10 ++++++++ neutron/tests/fullstack/test_l3_agent.py | 28 +++++++++++++++++++++ neutron/tests/unit/agent/l3/test_agent.py | 15 ++++++++++- 9 files changed, 95 insertions(+), 12 deletions(-) diff --git a/doc/source/admin/config-mtu.rst b/doc/source/admin/config-mtu.rst index 0059eaa7661..73955d56389 100644 --- a/doc/source/admin/config-mtu.rst +++ b/doc/source/admin/config-mtu.rst @@ -34,9 +34,6 @@ architectures should avoid cases 2 and 3. networks that need a new MTU. (Network MTU update is available for all core plugins that implement the ``net-mtu-writable`` API extension.) - When using the Open vSwitch or Linux bridge drivers, new MTU calculations - will be propogated automatically after restarting the ``l3-agent`` service. - Case 1 ------ diff --git a/neutron/agent/l3/agent.py b/neutron/agent/l3/agent.py index 0f9964e9a61..59ed801fa85 100644 --- a/neutron/agent/l3/agent.py +++ b/neutron/agent/l3/agent.py @@ -13,6 +13,8 @@ # under the License. # +import itertools + import eventlet import netaddr from neutron_lib.callbacks import events @@ -29,6 +31,7 @@ from oslo_service import loopingcall from oslo_service import periodic_task from oslo_utils import excutils from oslo_utils import timeutils +from osprofiler import profiler from neutron._i18n import _, _LE, _LI, _LW from neutron.agent.common import utils as common_utils @@ -167,6 +170,7 @@ class L3PluginApi(object): host=self.host, network_id=fip_net) +@profiler.trace_cls("l3-agent") class L3NATAgent(ha.AgentMixin, dvr.AgentMixin, manager.Manager): @@ -185,8 +189,9 @@ class L3NATAgent(ha.AgentMixin, 1.3 - fipnamespace_delete_on_ext_net - to delete fipnamespace after the external network is removed Needed by the L3 service when dealing with DVR + 1.4 - support network_update to get MTU updates """ - target = oslo_messaging.Target(version='1.3') + target = oslo_messaging.Target(version='1.4') def __init__(self, host, conf=None): if conf: @@ -256,6 +261,10 @@ class L3NATAgent(ha.AgentMixin, self.create_pd_router_update, self.conf) + # Consume network updates to trigger router resync + consumers = [[topics.NETWORK, topics.UPDATE]] + agent_rpc.create_consumers([self], topics.AGENT, consumers) + def _check_config_params(self): """Check items in configuration files. @@ -429,6 +438,16 @@ class L3NATAgent(ha.AgentMixin, LOG.debug('Got router added to agent :%r', payload) self.routers_updated(context, payload) + def network_update(self, context, **kwargs): + network_id = kwargs['network']['id'] + for ri in self.router_info.values(): + ports = itertools.chain(ri.internal_ports, [ri.ex_gw_port]) + port_belongs = lambda p: p['network_id'] == network_id + if any(port_belongs(p) for p in ports): + update = queue.RouterUpdate( + ri.router_id, queue.PRIORITY_SYNC_ROUTERS_TASK) + self._resync_router(update) + def _process_router_if_compatible(self, router): if (self.conf.external_network_bridge and not ip_lib.device_exists(self.conf.external_network_bridge)): diff --git a/neutron/agent/l3/ha_router.py b/neutron/agent/l3/ha_router.py index 0ac12a946b5..e98cca1cef3 100644 --- a/neutron/agent/l3/ha_router.py +++ b/neutron/agent/l3/ha_router.py @@ -297,7 +297,9 @@ class HaRouter(router.RouterInfo): if device.addr.list(to=ip_cidr): super(HaRouter, self).remove_floating_ip(device, ip_cidr) - def internal_network_updated(self, interface_name, ip_cidrs): + def internal_network_updated(self, interface_name, ip_cidrs, mtu): + self.driver.set_mtu(interface_name, mtu, namespace=self.ns_name, + prefix=router.INTERNAL_DEV_PREFIX) self._clear_vips(interface_name) self._disable_ipv6_addressing_on_interface(interface_name) for ip_cidr in ip_cidrs: diff --git a/neutron/agent/l3/router_info.py b/neutron/agent/l3/router_info.py index 14727dfb012..cf9ff63fda7 100644 --- a/neutron/agent/l3/router_info.py +++ b/neutron/agent/l3/router_info.py @@ -476,10 +476,13 @@ class RouterInfo(object): for existing_port in existing_ports: current_port = current_ports_dict.get(existing_port['id']) if current_port: - if (sorted(existing_port['fixed_ips'], + fixed_ips_changed = ( + sorted(existing_port['fixed_ips'], key=helpers.safe_sort_key) != - sorted(current_port['fixed_ips'], - key=helpers.safe_sort_key)): + sorted(current_port['fixed_ips'], + key=helpers.safe_sort_key)) + mtu_changed = existing_port['mtu'] != current_port['mtu'] + if fixed_ips_changed or mtu_changed: updated_ports[current_port['id']] = current_port return updated_ports @@ -502,7 +505,9 @@ class RouterInfo(object): self.router_id) self.radvd.disable() - def internal_network_updated(self, interface_name, ip_cidrs): + def internal_network_updated(self, interface_name, ip_cidrs, mtu): + self.driver.set_mtu(interface_name, mtu, namespace=self.ns_name, + prefix=INTERNAL_DEV_PREFIX) self.driver.init_router_port( interface_name, ip_cidrs=ip_cidrs, @@ -562,7 +567,8 @@ class RouterInfo(object): ip_cidrs = common_utils.fixed_ip_cidrs(p['fixed_ips']) LOG.debug("updating internal network for port %s", p) updated_cidrs += ip_cidrs - self.internal_network_updated(interface_name, ip_cidrs) + self.internal_network_updated( + interface_name, ip_cidrs, p['mtu']) enable_ra = enable_ra or self._port_has_ipv6_subnet(p) # Check if there is any pd prefix update diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py index 29a40c781d0..21e84d0075d 100644 --- a/neutron/plugins/ml2/plugin.py +++ b/neutron/plugins/ml2/plugin.py @@ -858,6 +858,7 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, def update_network(self, context, id, network): net_data = network[net_def.RESOURCE_NAME] provider._raise_if_updates_provider_attributes(net_data) + need_network_update_notify = False with db_api.context_manager.writer.using(context): original_network = super(Ml2Plugin, self).get_network(context, id) @@ -883,6 +884,8 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, db_network.mtu is None): db_network.mtu = self._get_network_mtu(db_network, validate=False) + # agents should now update all ports to reflect new MTU + need_network_update_notify = True updated_network = self._make_network_dict( db_network, context=context) @@ -896,7 +899,7 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, resources.NETWORK, events.PRECOMMIT_UPDATE, self, **kwargs) # TODO(QoS): Move out to the extension framework somehow. - need_network_update_notify = ( + need_network_update_notify |= ( qos_consts.QOS_POLICY_ID in net_data and original_network[qos_consts.QOS_POLICY_ID] != updated_network[qos_consts.QOS_POLICY_ID]) diff --git a/neutron/tests/common/l3_test_common.py b/neutron/tests/common/l3_test_common.py index 748c0f464aa..4b17f1a5496 100644 --- a/neutron/tests/common/l3_test_common.py +++ b/neutron/tests/common/l3_test_common.py @@ -41,6 +41,7 @@ def get_ha_interface(ip='169.254.192.1', mac='12:34:56:78:2b:5d'): 'id': _uuid(), 'mac_address': mac, 'name': u'L3 HA Admin port 0', + 'mtu': 1500, 'network_id': _uuid(), 'status': u'ACTIVE', 'subnets': [{'cidr': '169.254.192.0/18', @@ -92,6 +93,7 @@ def prepare_router_data(ip_version=4, enable_snat=None, num_internal_ports=1, if enable_gw: ex_gw_port = {'id': _uuid(), 'mac_address': gateway_mac, + 'mtu': 1500, 'network_id': _uuid(), 'fixed_ips': fixed_ips, 'subnets': subnets, @@ -182,6 +184,7 @@ def router_append_interface(router, count=1, ip_version=4, ra_mode=None, interfaces.append( {'id': _uuid(), + 'mtu': 1500, 'network_id': _uuid(), 'admin_state_up': True, 'fixed_ips': fixed_ips, @@ -274,6 +277,7 @@ def router_append_pd_enabled_subnet(router, count=1): for i in range(current, current + count): subnet_id = _uuid() intf = {'id': _uuid(), + 'mtu': 1500, 'network_id': _uuid(), 'admin_state_up': True, 'fixed_ips': [{'ip_address': '::1', @@ -331,6 +335,7 @@ def prepare_ext_gw_test(context, ri, dual_stack=False): 'subnets': subnets, 'extra_subnets': [{'cidr': '172.16.0.0/24'}], 'id': _uuid(), + 'mtu': 1500, 'network_id': _uuid(), 'mac_address': 'ca:fe:de:ad:be:ef'} interface_name = ri.get_external_device_name(ex_gw_port['id']) diff --git a/neutron/tests/fullstack/resources/client.py b/neutron/tests/fullstack/resources/client.py index 25bd35dfbc4..cf4a258ba3e 100644 --- a/neutron/tests/fullstack/resources/client.py +++ b/neutron/tests/fullstack/resources/client.py @@ -50,6 +50,13 @@ class ClientFixture(fixtures.Fixture): self.addCleanup(_safe_method(delete), data['id']) return data + def _update_resource(self, resource_type, id, spec): + update = getattr(self.client, 'update_%s' % resource_type) + + body = {resource_type: spec} + resp = update(id, body=body) + return resp[resource_type] + def create_router(self, tenant_id, name=None, ha=False, external_network=None): resource_type = 'router' @@ -79,6 +86,9 @@ class ClientFixture(fixtures.Fixture): return self._create_resource(resource_type, spec) + def update_network(self, id, **kwargs): + return self._update_resource('network', id, kwargs) + def create_subnet(self, tenant_id, network_id, cidr, gateway_ip=None, name=None, enable_dhcp=True, ipv6_address_mode='slaac', ipv6_ra_mode='slaac'): diff --git a/neutron/tests/fullstack/test_l3_agent.py b/neutron/tests/fullstack/test_l3_agent.py index ba2b8d09dbd..fcb5a62505f 100644 --- a/neutron/tests/fullstack/test_l3_agent.py +++ b/neutron/tests/fullstack/test_l3_agent.py @@ -110,6 +110,34 @@ class TestLegacyL3Agent(TestL3Agent): self.environment.hosts[0].l3_agent.get_namespace_suffix(), ) self._assert_namespace_exists(namespace) + def test_mtu_update(self): + tenant_id = uuidutils.generate_uuid() + + router = self.safe_client.create_router(tenant_id) + network = self.safe_client.create_network(tenant_id) + subnet = self.safe_client.create_subnet( + tenant_id, network['id'], '20.0.0.0/24', gateway_ip='20.0.0.1') + self.safe_client.add_router_interface(router['id'], subnet['id']) + + namespace = "%s@%s" % ( + self._get_namespace(router['id']), + self.environment.hosts[0].l3_agent.get_namespace_suffix(), ) + self._assert_namespace_exists(namespace) + + ip = ip_lib.IPWrapper(namespace) + common_utils.wait_until_true(lambda: ip.get_devices()) + + devices = ip.get_devices() + self.assertEqual(1, len(devices)) + + ri_dev = devices[0] + mtu = ri_dev.link.mtu + self.assertEqual(1500, mtu) + + mtu -= 1 + network = self.safe_client.update_network(network['id'], mtu=mtu) + common_utils.wait_until_true(lambda: ri_dev.link.mtu == mtu) + def test_east_west_traffic(self): tenant_id = uuidutils.generate_uuid() router = self.safe_client.create_router(tenant_id) diff --git a/neutron/tests/unit/agent/l3/test_agent.py b/neutron/tests/unit/agent/l3/test_agent.py index 2bdaed6148e..03042de4de6 100644 --- a/neutron/tests/unit/agent/l3/test_agent.py +++ b/neutron/tests/unit/agent/l3/test_agent.py @@ -149,6 +149,7 @@ class BasicRouterOperationsFramework(base.BaseTestCase): self.snat_ports = [{'subnets': [{'cidr': '152.2.0.0/16', 'gateway_ip': '152.2.0.1', 'id': subnet_id_1}], + 'mtu': 1500, 'network_id': _uuid(), 'device_owner': lib_constants.DEVICE_OWNER_ROUTER_SNAT, @@ -160,6 +161,7 @@ class BasicRouterOperationsFramework(base.BaseTestCase): {'subnets': [{'cidr': '152.10.0.0/16', 'gateway_ip': '152.10.0.1', 'id': subnet_id_2}], + 'mtu': 1450, 'network_id': _uuid(), 'device_owner': lib_constants.DEVICE_OWNER_ROUTER_SNAT, @@ -425,6 +427,7 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): port = {'network_id': _uuid(), 'id': _uuid(), 'mac_address': 'ca:fe:de:ad:be:ef', + 'mtu': 1500, 'fixed_ips': [{'subnet_id': _uuid(), 'ip_address': '99.0.1.9', 'prefixlen': 24}]} @@ -459,6 +462,7 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): port = {'network_id': _uuid(), 'id': _uuid(), 'mac_address': 'ca:fe:de:ad:be:ef', + 'mtu': 1500, 'fixed_ips': [{'subnet_id': subnet_id, 'ip_address': '99.0.1.9', 'prefixlen': 24}], @@ -473,6 +477,7 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): 'extra_subnets': [{'cidr': '172.16.0.0/24'}], 'id': _uuid(), 'network_id': _uuid(), + 'mtu': 1500, 'mac_address': 'ca:fe:de:ad:be:ef'} ex_gw_port = {'fixed_ips': [{'ip_address': '20.0.0.30', 'prefixlen': 24, @@ -482,6 +487,7 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): 'id': _uuid(), portbindings.HOST_ID: HOSTNAME, 'network_id': _uuid(), + 'mtu': 1500, 'mac_address': 'ca:fe:de:ad:be:ef'} ri.snat_ports = sn_port ri.ex_gw_port = ex_gw_port @@ -513,7 +519,7 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): sn_port['mac_address'], ri._get_snat_int_device_name(sn_port['id']), lib_constants.SNAT_INT_DEV_PREFIX, - mtu=None) + mtu=1500) self.assertTrue(ri._check_if_address_scopes_match.called) if scope_match: self.assertTrue( @@ -667,10 +673,12 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): 'extra_subnets': [{'cidr': '172.16.0.0/24'}], 'id': _uuid(), 'network_id': ex_net_id, + 'mtu': 1500, 'mac_address': 'ca:fe:de:ad:be:ef'} ex_gw_port_no_sub = {'fixed_ips': [], 'id': _uuid(), 'network_id': ex_net_id, + 'mtu': 1500, 'mac_address': 'ca:fe:de:ad:be:ef'} interface_name = ri.get_external_device_name(ex_gw_port['id']) @@ -1159,6 +1167,7 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): 'gateway_ip': '20.0.0.1'}], 'id': _uuid(), 'network_id': fake_network_id, + 'mtu': 1500, 'mac_address': 'ca:fe:de:ad:be:ef'}] ) @@ -1290,6 +1299,7 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): 'gateway_ip': '20.0.0.1'}], 'id': _uuid(), 'network_id': fake_network_id, + 'mtu': 1500, 'mac_address': 'ca:fe:de:ad:be:ef'} ) @@ -1336,6 +1346,7 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): 'gateway_ip': '20.0.0.1'}], 'id': _uuid(), 'network_id': fake_network_id, + 'mtu': 1500, 'mac_address': 'ca:fe:de:ad:be:ef'}] ) @@ -1388,6 +1399,7 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): 'gateway_ip': '20.0.0.1'}], 'id': _uuid(), 'network_id': 'fake_network_id', + 'mtu': 1500, 'mac_address': 'ca:fe:de:ad:be:ef'}] ) @@ -2588,6 +2600,7 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): 'gateway_ip': '20.0.0.1'}], 'id': port_id, 'network_id': _uuid(), + 'mtu': 1500, 'mac_address': 'ca:fe:de:ad:be:ef'} interface_name = ri._get_snat_int_device_name(port_id)