diff --git a/neutron/agent/l3/router_info.py b/neutron/agent/l3/router_info.py index bcf74f1d15f..bfeddbd1f0d 100644 --- a/neutron/agent/l3/router_info.py +++ b/neutron/agent/l3/router_info.py @@ -694,19 +694,12 @@ class RouterInfo(object): gw_port = self._router.get('gw_port') self._handle_router_snat_rules(gw_port, interface_name) - def external_gateway_nat_postroute_rules(self, interface_name): + def external_gateway_nat_fip_rules(self, ex_gw_ip, interface_name): dont_snat_traffic_to_internal_ports_if_not_to_floating_ip = ( 'POSTROUTING', '! -i %(interface_name)s ' '! -o %(interface_name)s -m conntrack ! ' '--ctstate DNAT -j ACCEPT' % {'interface_name': interface_name}) - return [dont_snat_traffic_to_internal_ports_if_not_to_floating_ip] - - def external_gateway_nat_snat_rules(self, ex_gw_ip, interface_name): - snat_normal_external_traffic = ( - 'snat', '-o %s -j SNAT --to-source %s' % - (interface_name, ex_gw_ip)) - # Makes replies come back through the router to reverse DNAT ext_in_mark = self.agent_conf.external_ingress_mark snat_internal_traffic_to_floating_ip = ( @@ -714,9 +707,15 @@ class RouterInfo(object): '-m conntrack --ctstate DNAT ' '-j SNAT --to-source %s' % (ext_in_mark, l3_constants.ROUTER_MARK_MASK, ex_gw_ip)) - return [snat_normal_external_traffic, + return [dont_snat_traffic_to_internal_ports_if_not_to_floating_ip, snat_internal_traffic_to_floating_ip] + def external_gateway_nat_snat_rules(self, ex_gw_ip, interface_name): + snat_normal_external_traffic = ( + 'snat', '-o %s -j SNAT --to-source %s' % + (interface_name, ex_gw_ip)) + return [snat_normal_external_traffic] + def external_gateway_mangle_rules(self, interface_name): mark = self.agent_conf.external_ingress_mark mark_packets_entering_external_gateway_port = ( @@ -740,19 +739,20 @@ class RouterInfo(object): for ip_addr in ex_gw_port['fixed_ips']: ex_gw_ip = ip_addr['ip_address'] if netaddr.IPAddress(ex_gw_ip).version == 4: - rules = self.external_gateway_nat_postroute_rules( - interface_name) - for rule in rules: - iptables_manager.ipv4['nat'].add_rule(*rule) if self._snat_enabled: rules = self.external_gateway_nat_snat_rules( ex_gw_ip, interface_name) for rule in rules: iptables_manager.ipv4['nat'].add_rule(*rule) - rules = self.external_gateway_mangle_rules( - interface_name) - for rule in rules: - iptables_manager.ipv4['mangle'].add_rule(*rule) + + rules = self.external_gateway_nat_fip_rules( + ex_gw_ip, interface_name) + for rule in rules: + iptables_manager.ipv4['nat'].add_rule(*rule) + rules = self.external_gateway_mangle_rules(interface_name) + for rule in rules: + iptables_manager.ipv4['mangle'].add_rule(*rule) + break def _handle_router_snat_rules(self, ex_gw_port, interface_name): diff --git a/neutron/tests/functional/agent/l3/framework.py b/neutron/tests/functional/agent/l3/framework.py index 395f40a1913..d3b2e434f9a 100644 --- a/neutron/tests/functional/agent/l3/framework.py +++ b/neutron/tests/functional/agent/l3/framework.py @@ -498,3 +498,14 @@ class L3AgentTestFramework(base.BaseSudoTestCase): namespace, interface, ip_address): self.assertIn( ip_address, self._get_addresses_on_device(namespace, interface)) + + def _assert_ping_reply_from_expected_address( + self, ping_result, expected_address): + ping_results = ping_result.split('\n') + self.assertGreater( + len(ping_results), 1, + "The result from ping should be multiple lines") + self.assertIn( + expected_address, ping_results[1], + ("Expect to see %s in the reply of ping, but failed" % + expected_address)) diff --git a/neutron/tests/functional/agent/l3/test_legacy_router.py b/neutron/tests/functional/agent/l3/test_legacy_router.py index f8ba0b95721..35e6272b45a 100644 --- a/neutron/tests/functional/agent/l3/test_legacy_router.py +++ b/neutron/tests/functional/agent/l3/test_legacy_router.py @@ -193,13 +193,12 @@ class L3AgentTestCase(framework.L3AgentTestFramework): routers_deleted=[], routers_deleted_during_resync=routers_deleted_during_resync) - def test_fip_connection_from_same_subnet(self): - '''Test connection to floatingip which is associated with - fixed_ip on the same subnet of the source fixed_ip. - In other words it confirms that return packets surely - go through the router. - ''' - router_info = self.generate_router_info(enable_ha=False) + def _setup_fip_with_fixed_ip_from_same_subnet(self, enable_snat): + """Setup 2 FakeMachines from same subnet, one with floatingip + associated. + """ + router_info = self.generate_router_info(enable_ha=False, + enable_snat=enable_snat) 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] @@ -218,6 +217,16 @@ class L3AgentTestCase(framework.L3AgentTestFramework): self._add_fip(router, dst_fip, fixed_address=dst_machine.ip) router.process(self.agent) + return src_machine, dst_machine, dst_fip + + def test_fip_connection_from_same_subnet(self): + '''Test connection to floatingip which is associated with + fixed_ip on the same subnet of the source fixed_ip. + In other words it confirms that return packets surely + go through the router. + ''' + src_machine, dst_machine, dst_fip = ( + self._setup_fip_with_fixed_ip_from_same_subnet(enable_snat=True)) protocol_port = net_helpers.get_free_namespace_port( l3_constants.PROTO_NAME_TCP, dst_machine.namespace) # client sends to fip @@ -228,6 +237,16 @@ class L3AgentTestCase(framework.L3AgentTestFramework): self.addCleanup(netcat.stop_processes) self.assertTrue(netcat.test_connectivity()) + def test_ping_floatingip_reply_with_floatingip(self): + src_machine, _, dst_fip = ( + self._setup_fip_with_fixed_ip_from_same_subnet(enable_snat=False)) + + # 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 _setup_address_scope(self, internal_address_scope1, internal_address_scope2, gw_address_scope=None): router_info = self.generate_router_info(enable_ha=False, diff --git a/neutron/tests/unit/agent/l3/test_agent.py b/neutron/tests/unit/agent/l3/test_agent.py index ced85c9ea03..3021d17f4dc 100644 --- a/neutron/tests/unit/agent/l3/test_agent.py +++ b/neutron/tests/unit/agent/l3/test_agent.py @@ -1148,11 +1148,11 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): # IpTablesRule instances nat_rules_delta = [r for r in orig_nat_rules if r not in ri.iptables_manager.ipv4['nat'].rules] - self.assertEqual(2, len(nat_rules_delta)) + self.assertEqual(1, len(nat_rules_delta)) mangle_rules_delta = [ r for r in orig_mangle_rules if r not in ri.iptables_manager.ipv4['mangle'].rules] - self.assertEqual(2, len(mangle_rules_delta)) + self.assertEqual(1, len(mangle_rules_delta)) self._verify_snat_mangle_rules(nat_rules_delta, mangle_rules_delta, router) self.assertEqual(1, self.send_adv_notif.call_count) @@ -1175,11 +1175,11 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): # IpTablesRule instances nat_rules_delta = [r for r in ri.iptables_manager.ipv4['nat'].rules if r not in orig_nat_rules] - self.assertEqual(2, len(nat_rules_delta)) + self.assertEqual(1, len(nat_rules_delta)) mangle_rules_delta = [ r for r in ri.iptables_manager.ipv4['mangle'].rules if r not in orig_mangle_rules] - self.assertEqual(2, len(mangle_rules_delta)) + self.assertEqual(1, len(mangle_rules_delta)) self._verify_snat_mangle_rules(nat_rules_delta, mangle_rules_delta, router) self.assertEqual(1, self.send_adv_notif.call_count) @@ -1245,15 +1245,15 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): # Get NAT rules with the gw_port router['gw_port'] = gw_port ri = l3router.RouterInfo(router['id'], router, **self.ri_kwargs) - p = ri.external_gateway_nat_postroute_rules + p = ri.external_gateway_nat_fip_rules s = ri.external_gateway_nat_snat_rules attrs_to_mock = dict( [(a, mock.DEFAULT) for a in - ['external_gateway_nat_postroute_rules', + ['external_gateway_nat_fip_rules', 'external_gateway_nat_snat_rules']] ) with mock.patch.multiple(ri, **attrs_to_mock) as mocks: - mocks['external_gateway_nat_postroute_rules'].side_effect = p + mocks['external_gateway_nat_fip_rules'].side_effect = p mocks['external_gateway_nat_snat_rules'].side_effect = s self._process_router_instance_for_agent(agent, ri, router) new_nat_rules = ri.iptables_manager.ipv4['nat'].rules[:] @@ -1261,13 +1261,13 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): # NAT rules should only change for dual_stack operation if dual_stack: self.assertTrue( - mocks['external_gateway_nat_postroute_rules'].called) + mocks['external_gateway_nat_fip_rules'].called) self.assertTrue( mocks['external_gateway_nat_snat_rules'].called) self.assertNotEqual(orig_nat_rules, new_nat_rules) else: self.assertFalse( - mocks['external_gateway_nat_postroute_rules'].called) + mocks['external_gateway_nat_fip_rules'].called) self.assertFalse( mocks['external_gateway_nat_snat_rules'].called) self.assertEqual(orig_nat_rules, new_nat_rules)