From: Itsuro Oda Date: Wed, 29 Oct 2014 23:00:07 +0000 (+0900) Subject: Enable default SNAT from networks connected to a router indirectly X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=f852a89f1ea5eb6a6deae9cf8c4f63c40e420742;p=openstack-build%2Fneutron-build.git Enable default SNAT from networks connected to a router indirectly Make outgoing packets to an external interface SNATed regardless of source address of the packets. As a result of deep review, any problem was not found with this change. Change-Id: I71a1288633bb6af2951d571540bbb9ec5e1270e2 Closes-bug: #1386041 --- diff --git a/neutron/agent/l3_agent.py b/neutron/agent/l3_agent.py index 4c36873c3..cf2b38042 100644 --- a/neutron/agent/l3_agent.py +++ b/neutron/agent/l3_agent.py @@ -973,11 +973,8 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, # Process SNAT rules for external gateway if (not ri.router['distributed'] or ex_gw_port and ri.router['gw_port_host'] == self.host): - # Get IPv4 only internal CIDRs - internal_cidrs = [p['ip_cidr'] for p in ri.internal_ports - if netaddr.IPNetwork(p['ip_cidr']).version == 4] ri.perform_snat_action(self._handle_router_snat_rules, - internal_cidrs, interface_name) + interface_name) # Process SNAT/DNAT rules for floating IPs fip_statuses = {} @@ -1016,7 +1013,7 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, else: ri.disable_keepalived() - def _handle_router_snat_rules(self, ri, ex_gw_port, internal_cidrs, + def _handle_router_snat_rules(self, ri, ex_gw_port, interface_name, action): # Remove all the rules # This is safe because if use_namespaces is set as False @@ -1045,7 +1042,6 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, ex_gw_ip = ip_addr['ip_address'] if netaddr.IPAddress(ex_gw_ip).version == 4: rules = self.external_gateway_nat_rules(ex_gw_ip, - internal_cidrs, interface_name) for rule in rules: iptables_manager.ipv4['nat'].add_rule(*rule) @@ -1461,14 +1457,13 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, '--to-port %s' % self.conf.metadata_port)) return rules - def external_gateway_nat_rules(self, ex_gw_ip, internal_cidrs, - interface_name): + def external_gateway_nat_rules(self, ex_gw_ip, interface_name): rules = [('POSTROUTING', '! -i %(interface_name)s ' '! -o %(interface_name)s -m conntrack ! ' '--ctstate DNAT -j ACCEPT' % - {'interface_name': interface_name})] - for cidr in internal_cidrs: - rules.extend(self.internal_network_nat_rules(ex_gw_ip, cidr)) + {'interface_name': interface_name}), + ('snat', '-o %s -j SNAT --to-source %s' % + (interface_name, ex_gw_ip))] return rules def _snat_redirect_add(self, ri, gateway, sn_port, sn_int): @@ -1581,11 +1576,6 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, self.driver.unplug(interface_name, namespace=ri.ns_name, prefix=INTERNAL_DEV_PREFIX) - def internal_network_nat_rules(self, ex_gw_ip, internal_cidr): - rules = [('snat', '-s %s -j SNAT --to-source %s' % - (internal_cidr, ex_gw_ip))] - return rules - def _create_agent_gateway_port(self, ri, network_id): """Create Floating IP gateway port. diff --git a/neutron/services/firewall/agents/varmour/varmour_router.py b/neutron/services/firewall/agents/varmour/varmour_router.py index c24d8e7b2..2381c262b 100755 --- a/neutron/services/firewall/agents/varmour/varmour_router.py +++ b/neutron/services/firewall/agents/varmour/varmour_router.py @@ -294,7 +294,7 @@ class vArmourL3NATAgent(l3_agent.L3NATAgent, self._va_config_router_snat_rules(ri, plist) self._va_config_floating_ips(ri) - def _handle_router_snat_rules(self, ri, ex_gw_port, internal_cidrs, + def _handle_router_snat_rules(self, ri, ex_gw_port, interface_name, action): return diff --git a/neutron/tests/unit/test_l3_agent.py b/neutron/tests/unit/test_l3_agent.py index b4e63d604..3a5424cbb 100644 --- a/neutron/tests/unit/test_l3_agent.py +++ b/neutron/tests/unit/test_l3_agent.py @@ -796,15 +796,8 @@ class TestBasicRouterOperations(base.BaseTestCase): interface_name = ('qg-%s' % router['gw_port']['id'])[:14] expected_rules = [ '! -i %s ! -o %s -m conntrack ! --ctstate DNAT -j ACCEPT' % - (interface_name, interface_name)] - for source_cidr in source_cidrs: - # Create SNAT rules for IPv4 only - if (netaddr.IPNetwork(source_cidr).version == 4 and - netaddr.IPNetwork(source_nat_ip).version == 4): - value_dict = {'source_cidr': source_cidr, - 'source_nat_ip': source_nat_ip} - expected_rules.append('-s %(source_cidr)s -j SNAT --to-source ' - '%(source_nat_ip)s' % value_dict) + (interface_name, interface_name), + '-o %s -j SNAT --to-source %s' % (interface_name, source_nat_ip)] for r in rules: if negate: self.assertNotIn(r.rule, expected_rules) @@ -1333,18 +1326,11 @@ vrrp_instance VR_1 { agent.external_gateway_added = mock.Mock() # Process with NAT agent.process_router(ri) - orig_nat_rules = ri.iptables_manager.ipv4['nat'].rules[:] # Add an interface and reprocess router_append_interface(router) # Reassign the router object to RouterInfo ri.router = router agent.process_router(ri) - # For some reason set logic does not work well with - # IpTablesRule instances - nat_rules_delta = [r for r in ri.iptables_manager.ipv4['nat'].rules - if r not in orig_nat_rules] - self.assertEqual(len(nat_rules_delta), 1) - self._verify_snat_rules(nat_rules_delta, router) # send_arp is called both times process_router is called self.assertEqual(self.send_arp.call_count, 2) @@ -1440,7 +1426,6 @@ vrrp_instance VR_1 { agent.external_gateway_added = mock.Mock() # Process with NAT agent.process_router(ri) - orig_nat_rules = ri.iptables_manager.ipv4['nat'].rules[:] # Add an IPv4 and IPv6 interface and reprocess router_append_interface(router, count=1, ip_version=4) router_append_interface(router, count=1, ip_version=6) @@ -1448,12 +1433,6 @@ vrrp_instance VR_1 { ri.router = router agent.process_router(ri) self._assert_ri_process_enabled(ri, 'radvd') - # For some reason set logic does not work well with - # IpTablesRule instances - nat_rules_delta = [r for r in ri.iptables_manager.ipv4['nat'].rules - if r not in orig_nat_rules] - self.assertEqual(1, len(nat_rules_delta)) - self._verify_snat_rules(nat_rules_delta, router) def test_process_router_interface_removed(self): agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) @@ -1463,18 +1442,11 @@ vrrp_instance VR_1 { agent.external_gateway_added = mock.Mock() # Process with NAT agent.process_router(ri) - orig_nat_rules = ri.iptables_manager.ipv4['nat'].rules[:] # Add an interface and reprocess del router[l3_constants.INTERFACE_KEY][1] # Reassign the router object to RouterInfo ri.router = router agent.process_router(ri) - # For some reason set logic does not work well with - # IpTablesRule instances - nat_rules_delta = [r for r in orig_nat_rules - if r not in ri.iptables_manager.ipv4['nat'].rules] - self.assertEqual(len(nat_rules_delta), 1) - self._verify_snat_rules(nat_rules_delta, router, negate=True) # send_arp is called both times process_router is called self.assertEqual(self.send_arp.call_count, 2) @@ -1618,7 +1590,7 @@ vrrp_instance VR_1 { agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) with mock.patch.object(l3_agent.LOG, 'debug') as log_debug: agent._handle_router_snat_rules( - ri, mock.ANY, mock.ANY, mock.ANY, mock.ANY) + ri, mock.ANY, mock.ANY, mock.ANY) self.assertIsNone(ri.snat_iptables_manager) self.assertFalse(ri.iptables_manager.called) self.assertTrue(log_debug.called) @@ -1629,7 +1601,7 @@ vrrp_instance VR_1 { port = {'fixed_ips': [{'ip_address': '192.168.1.4'}]} ri.router = {'distributed': False} - agent._handle_router_snat_rules(ri, port, [], "iface", "add_rules") + agent._handle_router_snat_rules(ri, port, "iface", "add_rules") nat = ri.iptables_manager.ipv4['nat'] nat.empty_chain.assert_any_call('snat') @@ -1645,9 +1617,8 @@ vrrp_instance VR_1 { agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) ri = l3_agent.RouterInfo(_uuid(), self.conf.root_helper, {}) ex_gw_port = {'fixed_ips': [{'ip_address': '192.168.1.4'}]} - internal_cidrs = ['10.0.0.0/24'] ri.router = {'distributed': False} - agent._handle_router_snat_rules(ri, ex_gw_port, internal_cidrs, + agent._handle_router_snat_rules(ri, ex_gw_port, "iface", "add_rules") nat_rules = map(str, ri.iptables_manager.ipv4['nat'].rules) @@ -1655,15 +1626,14 @@ vrrp_instance VR_1 { jump_float_rule = "-A %s-snat -j %s-float-snat" % (wrap_name, wrap_name) - internal_net_rule = ("-A %s-snat -s %s -j SNAT --to-source %s") % ( - wrap_name, internal_cidrs[0], - ex_gw_port['fixed_ips'][0]['ip_address']) + snat_rule = ("-A %s-snat -o iface -j SNAT --to-source %s") % ( + wrap_name, ex_gw_port['fixed_ips'][0]['ip_address']) self.assertIn(jump_float_rule, nat_rules) - self.assertIn(internal_net_rule, nat_rules) + self.assertIn(snat_rule, nat_rules) self.assertThat(nat_rules.index(jump_float_rule), - matchers.LessThan(nat_rules.index(internal_net_rule))) + matchers.LessThan(nat_rules.index(snat_rule))) def test_process_router_delete_stale_internal_devices(self): class FakeDev(object):