From 852aabe8defe7fc3b32f3a3522f0040b7b2d318c Mon Sep 17 00:00:00 2001 From: Slawek Kaplonski Date: Fri, 2 Jul 2021 17:55:26 +0200 Subject: [PATCH] [DVR] Fix update of the MTU in the SNAT namespace When network's MTU is changed, Neutron sends notification about it to the L3 agents. In case of DVR (and DVR HA) MTU is then changed in the qrouter- namespace but should be also changed on snat interfaces in the snat namespace. And that part was missing. This patch adds special implementation of the internal_network_updated() method in the DvrEdgeRouter class so it can configure MTU also for in the snat namespace. This patch also removed passing attributes "interface_name", "ip_cidrs" and "mtu" to the internal_network_updated() method and adds "port" dict to be passed there. It is consistent with what is already done in e.g. internal_network_added() method and "port" dict is actually necessary to configure properly snat internal interface in the snat namespace. This patch adds also functional test of update network mtu for all types of routers as there was no such test at all. There is additional issue with DVR-HA which isn't fixed with that patch and for which follow up will be proposed. Because of that this patch is marked as partial fix for the related bug. Conflicts: neutron/tests/functional/agent/l3/test_dvr_router.py Related-Bug: #1933273 Change-Id: I200acfcaaae7f056ea9a563fead9ff2de8464971 (cherry picked from commit b5dd6efdca0aa6e7405a55d66c7042a49ec72214) --- neutron/agent/l3/dvr_edge_router.py | 17 +++++ neutron/agent/l3/ha_router.py | 5 +- neutron/agent/l3/router_info.py | 13 ++-- .../functional/agent/l3/test_dvr_router.py | 66 +++++++++++++++++++ .../functional/agent/l3/test_ha_router.py | 34 ++++++++++ .../functional/agent/l3/test_legacy_router.py | 34 ++++++++++ 6 files changed, 161 insertions(+), 8 deletions(-) diff --git a/neutron/agent/l3/dvr_edge_router.py b/neutron/agent/l3/dvr_edge_router.py index 8a1dc7955ba..485fe2a3efa 100644 --- a/neutron/agent/l3/dvr_edge_router.py +++ b/neutron/agent/l3/dvr_edge_router.py @@ -129,6 +129,23 @@ class DvrEdgeRouter(dvr_local_router.DvrLocalRouter): lib_constants.SNAT_INT_DEV_PREFIX, mtu=sn_port.get('mtu')) + def internal_network_updated(self, port): + super(DvrEdgeRouter, self).internal_network_updated(port) + + if not port: + return + if not self._is_this_snat_host(): + return + + sn_port = self.get_snat_port_for_internal_port(port) + if not sn_port: + return + + ns_name = dvr_snat_ns.SnatNamespace.get_snat_ns_name(self.router['id']) + interface_name = self._get_snat_int_device_name(sn_port['id']) + self.driver.set_mtu(interface_name, port['mtu'], namespace=ns_name, + prefix=lib_constants.SNAT_INT_DEV_PREFIX) + def _dvr_internal_network_removed(self, port): super(DvrEdgeRouter, self)._dvr_internal_network_removed(port) diff --git a/neutron/agent/l3/ha_router.py b/neutron/agent/l3/ha_router.py index c66839fc98b..73f9cc71987 100644 --- a/neutron/agent/l3/ha_router.py +++ b/neutron/agent/l3/ha_router.py @@ -352,7 +352,10 @@ class HaRouter(router.RouterInfo): if device.addr.list(to=to): super(HaRouter, self).remove_floating_ip(device, ip_cidr) - def internal_network_updated(self, interface_name, ip_cidrs, mtu): + def internal_network_updated(self, port): + interface_name = self.get_internal_device_name(port['id']) + ip_cidrs = common_utils.fixed_ip_cidrs(port['fixed_ips']) + mtu = port['mtu'] self.driver.set_mtu(interface_name, mtu, namespace=self.ns_name, prefix=router.INTERNAL_DEV_PREFIX) self._clear_vips(interface_name) diff --git a/neutron/agent/l3/router_info.py b/neutron/agent/l3/router_info.py index ea2b488cd2a..33820733217 100644 --- a/neutron/agent/l3/router_info.py +++ b/neutron/agent/l3/router_info.py @@ -587,7 +587,10 @@ class RouterInfo(BaseRouterInfo): self.router_id) self.radvd.disable() - def internal_network_updated(self, interface_name, ip_cidrs, mtu): + def internal_network_updated(self, port): + interface_name = self.get_internal_device_name(port['id']) + ip_cidrs = common_utils.fixed_ip_cidrs(port['fixed_ips']) + mtu = port['mtu'] self.driver.set_mtu(interface_name, mtu, namespace=self.ns_name, prefix=INTERNAL_DEV_PREFIX) self.driver.init_router_port( @@ -646,12 +649,8 @@ class RouterInfo(BaseRouterInfo): updated_cidrs = [] for p in updated_ports: self._update_internal_ports_cache(p) - interface_name = self.get_internal_device_name(p['id']) - 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, p['mtu']) + updated_cidrs += common_utils.fixed_ip_cidrs(p['fixed_ips']) + self.internal_network_updated(p) enable_ra = enable_ra or self._port_has_ipv6_subnet(p) # Check if there is any pd prefix update diff --git a/neutron/tests/functional/agent/l3/test_dvr_router.py b/neutron/tests/functional/agent/l3/test_dvr_router.py index 64b2a37bc58..4cb2446d47a 100644 --- a/neutron/tests/functional/agent/l3/test_dvr_router.py +++ b/neutron/tests/functional/agent/l3/test_dvr_router.py @@ -2234,3 +2234,69 @@ class TestDvrRouter(DvrRouterTestFramework, framework.L3AgentTestFramework): in fip_agent_gw_port['extra_subnets']) routes_cidr = set(route['cidr'] for route in routes) self.assertEqual(extra_subnet_cidr, routes_cidr) + + def _test_router_interface_mtu_update(self, ha): + original_mtu = 1450 + router_info = self.generate_dvr_router_info( + enable_ha=ha, enable_snat=True) + router_info['_interfaces'][0]['mtu'] = original_mtu + router_info['gw_port']['mtu'] = original_mtu + router_info[lib_constants.SNAT_ROUTER_INTF_KEY][0]['mtu'] = ( + original_mtu) + + router = self.manage_router(self.agent, router_info) + if ha: + utils.wait_until_true(lambda: router.ha_state == 'primary') + # Keepalived notifies of a state transition when it starts, + # not when it ends. Thus, we have to wait until keepalived finishes + # configuring everything. We verify this by waiting until the last + # device has an IP address. + device = router.router[lib_constants.INTERFACE_KEY][-1] + device_exists = functools.partial( + self.device_exists_with_ips_and_mac, + device, + router.get_internal_device_name, + router.ns_name) + utils.wait_until_true(device_exists) + + interface_name = router.get_internal_device_name( + router_info['_interfaces'][0]['id']) + gw_interface_name = router.get_external_device_name( + router_info['gw_port']['id']) + snat_internal_port = router_info[lib_constants.SNAT_ROUTER_INTF_KEY] + snat_interface_name = router._get_snat_int_device_name( + snat_internal_port[0]['id']) + snat_namespace = dvr_snat_ns.SnatNamespace.get_snat_ns_name( + router_info['id']) + + self.assertEqual( + original_mtu, + ip_lib.IPDevice(interface_name, router.ns_name).link.mtu) + self.assertEqual( + original_mtu, + ip_lib.IPDevice(gw_interface_name, snat_namespace).link.mtu) + self.assertEqual( + original_mtu, + ip_lib.IPDevice(snat_interface_name, snat_namespace).link.mtu) + + updated_mtu = original_mtu + 1 + router_info_copy = copy.deepcopy(router_info) + router_info_copy['_interfaces'][0]['mtu'] = updated_mtu + router_info_copy['gw_port']['mtu'] = updated_mtu + router_info_copy[lib_constants.SNAT_ROUTER_INTF_KEY][0]['mtu'] = ( + updated_mtu) + + self.agent._process_updated_router(router_info_copy) + + self.assertEqual( + updated_mtu, + ip_lib.IPDevice(interface_name, router.ns_name).link.mtu) + self.assertEqual( + updated_mtu, + ip_lib.IPDevice(gw_interface_name, snat_namespace).link.mtu) + self.assertEqual( + updated_mtu, + ip_lib.IPDevice(snat_interface_name, snat_namespace).link.mtu) + + def test_dvr_router_interface_mtu_update(self): + self._test_router_interface_mtu_update(ha=False) diff --git a/neutron/tests/functional/agent/l3/test_ha_router.py b/neutron/tests/functional/agent/l3/test_ha_router.py index 0b68db3ff0d..3d152672669 100644 --- a/neutron/tests/functional/agent/l3/test_ha_router.py +++ b/neutron/tests/functional/agent/l3/test_ha_router.py @@ -388,6 +388,40 @@ class L3HATestCase(framework.L3AgentTestFramework): common_utils.wait_until_true(lambda: router.ha_state == 'master') self._wait_until_ipv6_forwarding_has_state(router.ns_name, 'all', 1) + def test_router_interface_mtu_update(self): + original_mtu = 1450 + router_info = self.generate_router_info(False) + router_info['_interfaces'][0]['mtu'] = original_mtu + router_info['gw_port']['mtu'] = original_mtu + + router = self.manage_router(self.agent, router_info) + + interface_name = router.get_internal_device_name( + router_info['_interfaces'][0]['id']) + gw_interface_name = router.get_external_device_name( + router_info['gw_port']['id']) + + self.assertEqual( + original_mtu, + ip_lib.IPDevice(interface_name, router.ns_name).link.mtu) + self.assertEqual( + original_mtu, + ip_lib.IPDevice(gw_interface_name, router.ns_name).link.mtu) + + updated_mtu = original_mtu + 1 + router_info_copy = copy.deepcopy(router_info) + router_info_copy['_interfaces'][0]['mtu'] = updated_mtu + router_info_copy['gw_port']['mtu'] = updated_mtu + + self.agent._process_updated_router(router_info_copy) + + self.assertEqual( + updated_mtu, + ip_lib.IPDevice(interface_name, router.ns_name).link.mtu) + self.assertEqual( + updated_mtu, + ip_lib.IPDevice(gw_interface_name, router.ns_name).link.mtu) + class L3HATestFailover(framework.L3AgentTestFramework): diff --git a/neutron/tests/functional/agent/l3/test_legacy_router.py b/neutron/tests/functional/agent/l3/test_legacy_router.py index 21447eb86f7..d7059d30d05 100644 --- a/neutron/tests/functional/agent/l3/test_legacy_router.py +++ b/neutron/tests/functional/agent/l3/test_legacy_router.py @@ -436,3 +436,37 @@ class L3AgentTestCase(framework.L3AgentTestFramework): # networks that are in the same address scope as external network net_helpers.assert_ping(machine_diff_scope.namespace, machine_same_scope.ip) + + def test_router_interface_mtu_update(self): + original_mtu = 1450 + router_info = self.generate_router_info(False) + router_info['_interfaces'][0]['mtu'] = original_mtu + router_info['gw_port']['mtu'] = original_mtu + + router = self.manage_router(self.agent, router_info) + + interface_name = router.get_internal_device_name( + router_info['_interfaces'][0]['id']) + gw_interface_name = router.get_external_device_name( + router_info['gw_port']['id']) + + self.assertEqual( + original_mtu, + ip_lib.IPDevice(interface_name, router.ns_name).link.mtu) + self.assertEqual( + original_mtu, + ip_lib.IPDevice(gw_interface_name, router.ns_name).link.mtu) + + updated_mtu = original_mtu + 1 + router_info_copy = copy.deepcopy(router_info) + router_info_copy['_interfaces'][0]['mtu'] = updated_mtu + router_info_copy['gw_port']['mtu'] = updated_mtu + + self.agent._process_updated_router(router_info_copy) + + self.assertEqual( + updated_mtu, + ip_lib.IPDevice(interface_name, router.ns_name).link.mtu) + self.assertEqual( + updated_mtu, + ip_lib.IPDevice(gw_interface_name, router.ns_name).link.mtu)