From 7c4da6fb756189e6afc1c699e06ce74488c61d45 Mon Sep 17 00:00:00 2001 From: Swaminathan Vasudevan Date: Fri, 30 Mar 2018 16:00:40 -0700 Subject: [PATCH] DVR: Avoid address scope rules for dvr_no_external agents All FloatingIP for DVR_NO_EXTERNAL agents will be configured in the SNAT Namespace. So there is no need to configure the address scope related routes in the router namespace when the agent is configured as DVR_NO_EXTERNAL. Change-Id: I009dae9e7f485641f2f19dce8dd575da04bfb044 Related-Bug: #1753434 --- neutron/agent/l3/dvr_local_router.py | 49 ++++++++++---- .../functional/agent/l3/test_dvr_router.py | 64 +++++++++++++++++-- 2 files changed, 93 insertions(+), 20 deletions(-) diff --git a/neutron/agent/l3/dvr_local_router.py b/neutron/agent/l3/dvr_local_router.py index 031d886fb3b..44367c3509a 100644 --- a/neutron/agent/l3/dvr_local_router.py +++ b/neutron/agent/l3/dvr_local_router.py @@ -408,7 +408,12 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase): self._set_subnet_arp_info(subnet['id']) if ex_gw_port: # Check for address_scopes here if gateway exists. - if self._check_if_address_scopes_match(port, ex_gw_port): + address_scopes_match = self._check_if_address_scopes_match( + port, ex_gw_port) + if (address_scopes_match and + (self.agent_conf.agent_mode in + [lib_constants.L3_AGENT_MODE_DVR, + lib_constants.L3_AGENT_MODE_DVR_SNAT])): self._add_interface_routing_rule_to_router_ns(port) self._add_interface_route_to_fip_ns(port) self._snat_redirect_add_from_port(port) @@ -417,9 +422,12 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase): ex_gw_port = self.get_ex_gw_port() if not ex_gw_port: return - if self._check_if_address_scopes_match(port, ex_gw_port): - # If address scopes match there is no need to cleanup the - # snat redirect rules, hence return here. + address_scopes_match = self._check_if_address_scopes_match( + port, ex_gw_port) + if (address_scopes_match and + (self.agent_conf.agent_mode in + [lib_constants.L3_AGENT_MODE_DVR, + lib_constants.L3_AGENT_MODE_DVR_SNAT])): return sn_port = self.get_snat_port_for_internal_port(port) if not sn_port: @@ -438,7 +446,12 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase): # Delete DVR address_scope static route for the removed interface # Check for address_scopes here. - if self._check_if_address_scopes_match(port, self.ex_gw_port): + address_scopes_match = self._check_if_address_scopes_match( + port, self.ex_gw_port) + if (address_scopes_match and + (self.agent_conf.agent_mode in + [lib_constants.L3_AGENT_MODE_DVR, + lib_constants.L3_AGENT_MODE_DVR_SNAT])): self._delete_interface_route_in_fip_ns(port) self._delete_interface_routing_rule_in_router_ns(port) # If address scopes match there is no need to cleanup the @@ -472,20 +485,28 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase): def enable_snat_redirect_rules(self, ex_gw_port): for p in self.internal_ports: - if not self._check_if_address_scopes_match(p, ex_gw_port): - gateway = self.get_snat_port_for_internal_port(p) - if not gateway: - continue + gateway = self.get_snat_port_for_internal_port(p) + if not gateway: + continue + address_scopes_match = self._check_if_address_scopes_match( + p, ex_gw_port) + if (not address_scopes_match or + (self.agent_conf.agent_mode == + lib_constants.L3_AGENT_MODE_DVR_NO_EXTERNAL)): internal_dev = self.get_internal_device_name(p['id']) self._snat_redirect_add(gateway, p, internal_dev) def disable_snat_redirect_rules(self, ex_gw_port): for p in self.internal_ports: - if not self._check_if_address_scopes_match(p, ex_gw_port): - gateway = self.get_snat_port_for_internal_port( - p, self.snat_ports) - if not gateway: - continue + gateway = self.get_snat_port_for_internal_port( + p, self.snat_ports) + if not gateway: + continue + address_scopes_match = self._check_if_address_scopes_match( + p, ex_gw_port) + if (not address_scopes_match or + (self.agent_conf.agent_mode == + lib_constants.L3_AGENT_MODE_DVR_NO_EXTERNAL)): internal_dev = self.get_internal_device_name(p['id']) self._snat_redirect_remove(gateway, p, internal_dev) diff --git a/neutron/tests/functional/agent/l3/test_dvr_router.py b/neutron/tests/functional/agent/l3/test_dvr_router.py index 47123b36b5f..3c4bc0914b7 100644 --- a/neutron/tests/functional/agent/l3/test_dvr_router.py +++ b/neutron/tests/functional/agent/l3/test_dvr_router.py @@ -25,6 +25,7 @@ import testtools from neutron.agent.l3 import agent as neutron_l3_agent from neutron.agent.l3 import dvr_fip_ns +from neutron.agent.l3 import dvr_local_router from neutron.agent.l3 import dvr_snat_ns from neutron.agent.l3 import namespaces from neutron.agent.linux import ip_lib @@ -1515,19 +1516,20 @@ class TestDvrRouter(framework.L3AgentTestFramework): def _assert_interface_rules_on_gateway_remove( self, router, agent, address_scopes, agent_gw_port, - rfp_device, fpr_device): + rfp_device, fpr_device, no_external=False): router.router[n_const.SNAT_ROUTER_INTF_KEY] = [] router.router['gw_port'] = "" router.router['gw_port_host'] = "" self.agent._process_updated_router(router.router) router_updated = self.agent.router_info[router.router['id']] - self.assertFalse(rfp_device.exists()) - self.assertFalse(fpr_device.exists()) self.assertTrue(self._namespace_exists(router_updated.ns_name)) - self._assert_fip_namespace_deleted( - agent_gw_port, assert_ovs_interface=False) - if not address_scopes: + if not no_external: + self.assertFalse(rfp_device.exists()) + self.assertFalse(fpr_device.exists()) + self._assert_fip_namespace_deleted( + agent_gw_port, assert_ovs_interface=False) + if not address_scopes or no_external: ns_ipr = ip_lib.IPRule(namespace=router_updated.ns_name) ip4_rules_list = ns_ipr.rule.list_rules(lib_constants.IP_VERSION_4) ip6_rules_list = ns_ipr.rule.list_rules(lib_constants.IP_VERSION_6) @@ -1612,6 +1614,56 @@ class TestDvrRouter(framework.L3AgentTestFramework): self): self._setup_dvr_router_for_fast_path_exit(address_scopes=False) + @mock.patch.object(dvr_local_router.DvrLocalRouter, + '_add_interface_routing_rule_to_router_ns') + @mock.patch.object(dvr_local_router.DvrLocalRouter, + '_add_interface_route_to_fip_ns') + def test_dvr_no_external_router_namespace_rules_with_address_scopes_match( + self, mock_add_interface_route_rule, + mock_add_fip_interface_route_rule): + """Test to validate the router namespace routes. + + This test validates the router namespace routes + that are based on the address scopes. + If the address scopes of internal network and external network + matches, the traffic will be forwarded to SNAT namespace + for agents that don't have external connectivity or configured + as DVR_NO_EXTERNAL. + """ + self.agent.conf.agent_mode = ( + lib_constants.L3_AGENT_MODE_DVR_NO_EXTERNAL) + router_info = self.generate_dvr_router_info( + enable_snat=True, enable_gw=True, enable_floating_ip=True) + router_info[lib_constants.FLOATINGIP_KEY] = [] + address_scope1 = { + str(lib_constants.IP_VERSION_4): 'scope1'} + address_scope2 = { + str(lib_constants.IP_VERSION_4): 'scope1'} + router_info['gw_port']['address_scopes'] = { + str(lib_constants.IP_VERSION_4): 'scope1'} + router_info[lib_constants.INTERFACE_KEY][0]['address_scopes'] = ( + address_scope1) + router_info[lib_constants.INTERFACE_KEY][1]['address_scopes'] = ( + address_scope2) + router1 = self.manage_router(self.agent, router_info) + self.assertTrue(self._namespace_exists(router1.ns_name)) + self.assertFalse(mock_add_interface_route_rule.called) + self.assertFalse(mock_add_fip_interface_route_rule.called) + # Check if any snat redirect rules in the router namespace exist. + ns_ipr = ip_lib.IPRule(namespace=router1.ns_name) + ip4_rules_list = ns_ipr.rule.list_rules(lib_constants.IP_VERSION_4) + ip6_rules_list = ns_ipr.rule.list_rules(lib_constants.IP_VERSION_6) + # Just make sure the basic set of rules are there in the router + # namespace + self.assertEqual(5, len(ip4_rules_list)) + self.assertEqual(2, len(ip6_rules_list)) + + # Now remove the gateway and validate if the respective interface + # routes in router namespace is deleted respectively. + self. _assert_interface_rules_on_gateway_remove( + router1, self.agent, True, mock.ANY, + mock.ANY, mock.ANY, True) + def test_dvr_router_gateway_update_to_none(self): self.agent.conf.agent_mode = 'dvr_snat' router_info = self.generate_dvr_router_info(enable_snat=True)