diff --git a/neutron/agent/l3/dvr_edge_router.py b/neutron/agent/l3/dvr_edge_router.py index 48c75213677..8afe688ea96 100644 --- a/neutron/agent/l3/dvr_edge_router.py +++ b/neutron/agent/l3/dvr_edge_router.py @@ -174,6 +174,9 @@ class DvrEdgeRouter(dvr_local_router.DvrLocalRouter): return host == self.host def _handle_router_snat_rules(self, ex_gw_port, interface_name): + super(DvrEdgeRouter, self)._handle_router_snat_rules( + ex_gw_port, interface_name) + if not self._is_this_snat_host(): return if not self.get_ex_gw_port(): @@ -186,7 +189,8 @@ class DvrEdgeRouter(dvr_local_router.DvrLocalRouter): with self.snat_iptables_manager.defer_apply(): self._empty_snat_chains(self.snat_iptables_manager) - # NOTE DVR doesn't add the jump to float snat like the super class. + # NOTE: DVR adds the jump to float snat via super class, + # but that is in the router namespace and not snat. self._add_snat_rules(ex_gw_port, self.snat_iptables_manager, interface_name) diff --git a/neutron/agent/l3/dvr_fip_ns.py b/neutron/agent/l3/dvr_fip_ns.py index e1b674108a9..00241e48cc6 100644 --- a/neutron/agent/l3/dvr_fip_ns.py +++ b/neutron/agent/l3/dvr_fip_ns.py @@ -269,8 +269,6 @@ class FipNamespace(namespaces.Namespace): # add default route for the link local interface device = ip_lib.IPDevice(rtr_2_fip_name, namespace=ri.ns_name) device.route.add_gateway(str(fip_2_rtr.ip), table=FIP_RT_TBL) - #setup the NAT rules and chains - ri._handle_fip_nat_rules(rtr_2_fip_name) def scan_fip_ports(self, ri): # don't scan if not dvr or count is not None diff --git a/neutron/agent/l3/dvr_local_router.py b/neutron/agent/l3/dvr_local_router.py index be894b80f95..93a9967a664 100644 --- a/neutron/agent/l3/dvr_local_router.py +++ b/neutron/agent/l3/dvr_local_router.py @@ -55,23 +55,6 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase): (i['host'] == self.host) or (i.get('dest_host') == self.host))] - def _handle_fip_nat_rules(self, interface_name): - """Configures NAT rules for Floating IPs for DVR.""" - self.iptables_manager.ipv4['nat'].empty_chain('POSTROUTING') - self.iptables_manager.ipv4['nat'].empty_chain('snat') - - # Add back the jump to float-snat - self.iptables_manager.ipv4['nat'].add_rule('snat', '-j $float-snat') - - # And add the NAT rule back - rule = ('POSTROUTING', '! -i %(interface_name)s ' - '! -o %(interface_name)s -m conntrack ! ' - '--ctstate DNAT -j ACCEPT' % - {'interface_name': interface_name}) - self.iptables_manager.ipv4['nat'].add_rule(*rule) - - self.iptables_manager.apply() - def floating_ip_added_dist(self, fip, fip_cidr): """Add floating IP to FIP namespace.""" floating_ip = fip['floating_ip_address'] @@ -411,7 +394,40 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase): self._snat_redirect_remove(gateway, p, internal_interface) def _handle_router_snat_rules(self, ex_gw_port, interface_name): - pass + """Configures NAT rules for Floating IPs for DVR.""" + + self.iptables_manager.ipv4['nat'].empty_chain('POSTROUTING') + self.iptables_manager.ipv4['nat'].empty_chain('snat') + self.iptables_manager.ipv4['mangle'].empty_chain('mark') + + ex_gw_port = self.get_ex_gw_port() + if not ex_gw_port: + return + + ext_device_name = self.get_external_device_interface_name(ex_gw_port) + floatingips = self.get_floating_ips() + if not ext_device_name or not floatingips: + # Without router to fip device, or without any floating ip, + # the snat rules should not be added + return + + # We can surely get ipv4 external gateway address, as we have + # ex_gw_port and floating ip here + ipv4_fixed_ips = [ip for ip in ex_gw_port['fixed_ips'] + if netaddr.IPAddress(ip['ip_address']).version == 4] + if not ipv4_fixed_ips: + return + ex_gw_ip = ipv4_fixed_ips[0]['ip_address'] + + # Add back the jump to float-snat + self.iptables_manager.ipv4['nat'].add_rule('snat', '-j $float-snat') + + rules = self.external_gateway_nat_fip_rules(ex_gw_ip, ext_device_name) + for rule in rules: + self.iptables_manager.ipv4['nat'].add_rule(*rule) + rules = self.external_gateway_mangle_rules(ext_device_name) + for rule in rules: + self.iptables_manager.ipv4['mangle'].add_rule(*rule) def _get_address_scope_mark(self): # Prepare address scope iptables rule for internal ports diff --git a/neutron/tests/functional/agent/l3/test_dvr_router.py b/neutron/tests/functional/agent/l3/test_dvr_router.py index 2e9eef9f793..71659cc4047 100644 --- a/neutron/tests/functional/agent/l3/test_dvr_router.py +++ b/neutron/tests/functional/agent/l3/test_dvr_router.py @@ -23,6 +23,7 @@ from neutron.agent.l3 import agent as neutron_l3_agent from neutron.agent.l3 import dvr_snat_ns from neutron.agent.l3 import namespaces from neutron.agent.linux import ip_lib +from neutron.agent.linux import iptables_manager from neutron.agent.linux import utils from neutron.common import constants as l3_constants from neutron.extensions import portbindings @@ -435,6 +436,59 @@ class TestDvrRouter(framework.L3AgentTestFramework): router_ns, floating_ips[0]['fixed_ip_address']) self.assertNotEqual(fip_rule_prio_1, fip_rule_prio_2) + def test_prevent_snat_rule_exist_on_restarted_agent(self): + self.agent.conf.agent_mode = 'dvr_snat' + router_info = self.generate_dvr_router_info() + router = self.manage_router(self.agent, router_info) + ext_port = router.get_ex_gw_port() + gw_ip = self._port_first_ip_cidr(ext_port).partition('/')[0] + rfp_devicename = router.get_external_device_interface_name(ext_port) + prevent_snat_rule = router.external_gateway_nat_fip_rules( + gw_ip, rfp_devicename) + + def is_prevent_snat_rule_exist(router_iptables_manager): + rules = router_iptables_manager.get_rules_for_table('nat') + return all(str(iptables_manager.IptablesRule( + nat_rule[0], nat_rule[1])) in rules + for nat_rule in prevent_snat_rule) + + self.assertTrue(is_prevent_snat_rule_exist(router.iptables_manager)) + + restarted_agent = neutron_l3_agent.L3NATAgentWithStateReport( + self.agent.host, self.agent.conf) + restarted_router = self.manage_router(restarted_agent, router_info) + + self.assertTrue(is_prevent_snat_rule_exist( + restarted_router.iptables_manager)) + + def test_ping_floatingip_reply_with_floatingip(self): + router_info = self.generate_dvr_router_info() + router = self.manage_router(self.agent, router_info) + router_ip_cidr = self._port_first_ip_cidr(router.internal_ports[0]) + router_ip = router_ip_cidr.partition('/')[0] + + br_int = framework.get_ovs_bridge( + self.agent.conf.ovs_integration_bridge) + + src_machine, dst_machine = self.useFixture( + machine_fixtures.PeerMachines( + br_int, + net_helpers.increment_ip_cidr(router_ip_cidr), + router_ip)).machines + + dst_fip = '19.4.4.10' + router.router[l3_constants.FLOATINGIP_KEY] = [] + self._add_fip(router, dst_fip, + fixed_address=dst_machine.ip, + host=self.agent.conf.host) + router.process(self.agent) + + # Verify that the ping replys with fip + ns_ip_wrapper = ip_lib.IPWrapper(src_machine.namespace) + result = ns_ip_wrapper.netns.execute( + ['ping', '-c', 1, '-W', 5, dst_fip]) + self._assert_ping_reply_from_expected_address(result, dst_fip) + def _get_fixed_ip_rule_priority(self, namespace, fip): iprule = ip_lib.IPRule(namespace) lines = iprule.rule._as_root([4], ['show']).splitlines() diff --git a/neutron/tests/unit/agent/l3/test_agent.py b/neutron/tests/unit/agent/l3/test_agent.py index 3021d17f4dc..b1b31af5d31 100644 --- a/neutron/tests/unit/agent/l3/test_agent.py +++ b/neutron/tests/unit/agent/l3/test_agent.py @@ -1699,15 +1699,13 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): 'foo_router_id', {}, **self.ri_kwargs) - ri.iptables_manager = mock.Mock() + ri.iptables_manager = mock.MagicMock() ri._is_this_snat_host = mock.Mock(return_value=True) - ri.get_ex_gw_port = mock.Mock(return_value=mock.ANY) + ri.get_ex_gw_port = mock.Mock(return_value=None) - with mock.patch.object(dvr_router.LOG, 'debug') as log_debug: - ri._handle_router_snat_rules(mock.ANY, mock.ANY) + ri._handle_router_snat_rules(None, mock.ANY) self.assertIsNone(ri.snat_iptables_manager) self.assertFalse(ri.iptables_manager.called) - self.assertTrue(log_debug.called) def test_handle_router_snat_rules_add_back_jump(self): ri = l3router.RouterInfo(_uuid(), {}, **self.ri_kwargs)