From 67c9a305df31b9abc55c364e669e6b4cefb83a72 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 29dd05fd737..03cdfda096e 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 d026b1cf2e8..97d4ff65208 100644 --- a/neutron/agent/l3/ha_router.py +++ b/neutron/agent/l3/ha_router.py @@ -359,7 +359,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 dccff6a395b..fb7bf7c5c69 100644 --- a/neutron/agent/l3/router_info.py +++ b/neutron/agent/l3/router_info.py @@ -589,7 +589,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( @@ -648,12 +651,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 bfa4abb89f4..baac8552283 100644 --- a/neutron/tests/functional/agent/l3/test_dvr_router.py +++ b/neutron/tests/functional/agent/l3/test_dvr_router.py @@ -2245,3 +2245,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 e20f4bed76d..4fa166900eb 100644 --- a/neutron/tests/functional/agent/l3/test_ha_router.py +++ b/neutron/tests/functional/agent/l3/test_ha_router.py @@ -384,6 +384,40 @@ class L3HATestCase(framework.L3AgentTestFramework): common_utils.wait_until_true(lambda: router.ha_state == 'primary') 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 32dd28e8210..a958432a80f 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)