DVR: Fix issue of SNAT rule for DVR with floating ip

With current code, there are 2 issues.

1) The prevent snat rule that is added for floating ip will be
cleaned, when restarting the l3 agent. Without this rule, the fixed
ip will be SNATed to floating ip, even if the network request is to
an internal IP.

2) The prevent snat rule will not be cleaned, even if the external
device(rfp device) is deleted. So, when the floating ips are removed
from DVR router, there are still dump rules in iptables. Restarting
the l3 agent can clean these dump rules.

The fix in this patch will handle DVR floating ip nat rules at the
same step to handle nat rules for other routers(legacy router, dvr
edge router)

After the change in [1], the fip nat rules for external port have
been extracted together into a method. Add all rules in that method
in the same step can fix the issue of ping floating ip, but reply
with fixed ip.

[1] https://review.openstack.org/#/c/286392/

Conflicts:
    neutron/agent/l3/dvr_fip_ns.py
    neutron/tests/functional/agent/l3/test_dvr_router.py

Change-Id: I018232c03f5df2237a11b48ac877793d1cb5c1bf
Closes-Bug: #1549311
Related-Bug: #1462154
(cherry picked from commit 1cea77b0aa)
This commit is contained in:
Hong Hui Xiao 2016-02-29 11:07:15 +00:00 committed by Elena Ezhova
parent 803cc36fd8
commit 10532404b5
5 changed files with 96 additions and 26 deletions

View File

@ -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)

View File

@ -274,8 +274,6 @@ class FipNamespace(namespaces.Namespace):
# add default route for the link local interface
rtr_2_fip_dev.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

View File

@ -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']
@ -429,7 +412,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

View File

@ -24,6 +24,7 @@ from neutron.agent.l3 import dvr_fip_ns
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
@ -521,6 +522,59 @@ class TestDvrRouter(framework.L3AgentTestFramework):
self.assertFalse(self._fixed_ip_rule_exists(router_ns, fixed_ip))
self.assertTrue(self._fixed_ip_rule_exists(router_ns, new_fixed_ip))
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()

View File

@ -1700,15 +1700,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)