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:
parent
803cc36fd8
commit
10532404b5
|
@ -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)
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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()
|
||||
|
|
|
@ -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)
|
||||
|
|
Loading…
Reference in New Issue