Browse Source

DVR: Pings to floatingip returns with fixed-ip on same network

Pinging a floatingip of VM1 from a second VM(VM2) which has SNAT
enabled connected to a DVR router on the same network returns
with fixed-ip address rather than the floatingip address.

The NAT forwarding rules for floatingip in the router namespace
does not check for the in coming port and tries to add the rule
for all in coming ports.

This causes the packets that are originating from the router
namespace to be modified and forwarded directly to the VM2 fixed
ip instead of forwarding the traffic to the SNAT namespace.

The fix in here will make sure that for all routers, the floatingip
forwarding rules will be applied only to the 'rfp-' internal ports
and not to all ports.

Change-Id: I9453beffd94bf685afd74b0820506fb6b7c996c4
Closes-Bug: #1462154
Co-Authored-By: Hong Hui Xiao <xiaohhui@cn.ibm.com>
(cherry picked from commit a388f78c8c)
changes/74/378374/1
Hong Hui Xiao 6 years ago committed by Oleg Bondarev
parent
commit
fcb12b5c27
  1. 9
      neutron/agent/l3/dvr_fip_ns.py
  2. 53
      neutron/agent/l3/dvr_local_router.py
  3. 46
      neutron/tests/functional/agent/l3/test_dvr_router.py
  4. 13
      neutron/tests/unit/agent/l3/test_dvr_fip_ns.py
  5. 20
      neutron/tests/unit/agent/l3/test_dvr_local_router.py

9
neutron/agent/l3/dvr_fip_ns.py

@ -289,11 +289,4 @@ class FipNamespace(namespaces.Namespace):
rtr_2_fip_interface = self.get_rtr_ext_device_name(ri.router_id)
device = ip_lib.IPDevice(rtr_2_fip_interface, namespace=ri.ns_name)
if device.exists():
existing_cidrs = [addr['cidr'] for addr in device.addr.list()]
fip_cidrs = [c for c in existing_cidrs if
common_utils.is_cidr_host(c)]
for fip_cidr in fip_cidrs:
fip_ip = fip_cidr.split('/')[0]
rule_pr = self._rule_priorities.allocate(fip_ip)
ri.floating_ips_dict[fip_ip] = rule_pr
ri.dist_fip_count = len(fip_cidrs)
ri.dist_fip_count = len(ri.get_router_cidrs(device))

53
neutron/agent/l3/dvr_local_router.py

@ -55,6 +55,33 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase):
(i['host'] == self.host) or
(i.get('dest_host') == self.host))]
def floating_forward_rules(self, floating_ip, fixed_ip):
"""Override this function defined in router_info for dvr routers."""
if not self.fip_ns:
return []
rtr_2_fip_name = self.fip_ns.get_rtr_ext_device_name(self.router_id)
dnat_from_floatingip_to_fixedip = (
'PREROUTING', '-d %s/32 -i %s -j DNAT --to-destination %s' % (
floating_ip, rtr_2_fip_name, fixed_ip))
snat_from_fixedip_to_floatingip = (
'float-snat', '-s %s/32 -j SNAT --to-source %s' % (
fixed_ip, floating_ip))
return [dnat_from_floatingip_to_fixedip,
snat_from_fixedip_to_floatingip]
def floating_mangle_rules(self, floating_ip, fixed_ip, internal_mark):
if not self.fip_ns:
return []
rtr_2_fip_name = self.fip_ns.get_rtr_ext_device_name(self.router_id)
mark_traffic_to_floating_ip = (
'floatingip', '-d %s -i %s -j MARK --set-xmark %s' % (
floating_ip, rtr_2_fip_name, internal_mark))
mark_traffic_from_fixed_ip = (
'FORWARD', '-s %s -j $float-snat' % fixed_ip)
return [mark_traffic_to_floating_ip, mark_traffic_from_fixed_ip]
def floating_ip_added_dist(self, fip, fip_cidr):
"""Add floating IP to FIP namespace."""
floating_ip = fip['floating_ip_address']
@ -132,16 +159,12 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase):
self._add_floating_ip_rule(floating_ip, fip['fixed_ip_address'])
def add_floating_ip(self, fip, interface_name, device):
if not self._add_fip_addr_to_device(fip, device):
return l3_constants.FLOATINGIP_STATUS_ERROR
# Special Handling for DVR - update FIP namespace
ip_cidr = common_utils.ip_to_cidr(fip['floating_ip_address'])
self.floating_ip_added_dist(fip, ip_cidr)
return l3_constants.FLOATINGIP_STATUS_ACTIVE
def remove_floating_ip(self, device, ip_cidr):
super(DvrLocalRouter, self).remove_floating_ip(device, ip_cidr)
self.floating_ip_removed_dist(ip_cidr)
def move_floating_ip(self, fip):
@ -529,6 +552,28 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase):
# configured
self.agent.process_router_add(self)
def get_router_cidrs(self, device):
"""As no floatingip will be set on the rfp device. Get floatingip from
the route of fip namespace.
"""
if not self.fip_ns:
return set()
fip_ns_name = self.fip_ns.get_name()
fip_2_rtr_name = self.fip_ns.get_int_device_name(self.router_id)
device = ip_lib.IPDevice(fip_2_rtr_name, namespace=fip_ns_name)
if not device.exists():
return set()
if self.rtr_fip_subnet is None:
self.rtr_fip_subnet = self.fip_ns.local_subnets.allocate(
self.router_id)
rtr_2_fip, _fip_2_rtr = self.rtr_fip_subnet.get_pair()
exist_routes = device.route.list_routes(
l3_constants.IP_VERSION_4, via=str(rtr_2_fip.ip))
return {common_utils.ip_to_cidr(route['cidr'])
for route in exist_routes}
def process(self, agent):
ex_gw_port = self.get_ex_gw_port()
if ex_gw_port:

46
neutron/tests/functional/agent/l3/test_dvr_router.py

@ -548,6 +548,15 @@ class TestDvrRouter(framework.L3AgentTestFramework):
self.assertTrue(ip_lib.device_exists(
device_name, namespace=router.ns_name))
# In the router namespace, check the iptables rules are set correctly
for fip in floating_ips:
floatingip = fip['floating_ip_address']
fixedip = fip['fixed_ip_address']
expected_rules = router.floating_forward_rules(floatingip,
fixedip)
self._assert_iptables_rules_exist(
router.iptables_manager, 'nat', expected_rules)
def test_dvr_router_rem_fips_on_restarted_agent(self):
self.agent.conf.agent_mode = 'dvr_snat'
router_info = self.generate_dvr_router_info()
@ -578,6 +587,13 @@ class TestDvrRouter(framework.L3AgentTestFramework):
router_ns, floating_ips[0]['fixed_ip_address'])
self.assertNotEqual(fip_rule_prio_1, fip_rule_prio_2)
def _assert_iptables_rules_exist(
self, router_iptables_manager, table_name, expected_rules):
rules = router_iptables_manager.get_rules_for_table(table_name)
for rule in expected_rules:
self.assertIn(
str(iptables_manager.IptablesRule(rule[0], rule[1])), rules)
def test_dvr_router_floating_ip_moved(self):
self.agent.conf.agent_mode = 'dvr'
router_info = self.generate_dvr_router_info()
@ -603,19 +619,15 @@ class TestDvrRouter(framework.L3AgentTestFramework):
prevent_snat_rule = router._prevent_snat_for_internal_traffic_rule(
rfp_devicename)
def is_prevent_snat_rule_exist(router_iptables_manager):
rules = router_iptables_manager.get_rules_for_table('nat')
return (str(iptables_manager.IptablesRule(
prevent_snat_rule[0], prevent_snat_rule[1])) in rules)
self.assertTrue(is_prevent_snat_rule_exist(router.iptables_manager))
self._assert_iptables_rules_exist(
router.iptables_manager, 'nat', [prevent_snat_rule])
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))
self._assert_iptables_rules_exist(
restarted_router.iptables_manager, 'nat', [prevent_snat_rule])
def _get_fixed_ip_rule_priority(self, namespace, fip):
iprule = ip_lib.IPRule(namespace)
@ -1045,21 +1057,3 @@ class TestDvrRouter(framework.L3AgentTestFramework):
# external networks. SNAT will be used. Direct route will not work
# here.
src_machine.assert_no_ping(machine_diff_scope.ip)
def test_connection_from_diff_address_scope_with_fip(self):
(machine_same_scope, machine_diff_scope,
router) = self._setup_address_scope('scope1', 'scope2', 'scope1')
fip = '19.4.4.11'
self._add_fip(router, fip,
fixed_address=machine_diff_scope.ip,
host=self.agent.conf.host,
fixed_ip_address_scope='scope2')
router.process(self.agent)
# For the internal networks that are in the same address scope as
# external network, they should be able to reach the floating ip
net_helpers.assert_ping(machine_same_scope.namespace, fip, 5)
# For the port with fip, it should be able to reach the internal
# networks that are in the same address scope as external network
net_helpers.assert_ping(machine_diff_scope.namespace,
machine_same_scope.ip, 5)

13
neutron/tests/unit/agent/l3/test_dvr_fip_ns.py

@ -284,24 +284,21 @@ class TestDvrFipNs(base.BaseTestCase):
@mock.patch.object(ip_lib, 'IPDevice')
def _test_scan_fip_ports(self, ri, ip_list, IPDevice):
IPDevice.return_value = device = mock.Mock()
device.addr.list.return_value = ip_list
device.exists.return_value = True
ri.get_router_cidrs.return_value = ip_list
self.fip_ns.get_rtr_ext_device_name = mock.Mock(
return_value=mock.sentinel.rtr_ext_device_name)
self.fip_ns.scan_fip_ports(ri)
@mock.patch.object(ip_lib, 'device_exists')
def test_scan_fip_ports_restart_fips(self, device_exists):
device_exists.return_value = True
def test_scan_fip_ports_restart_fips(self):
ri = mock.Mock()
ri.dist_fip_count = None
ri.floating_ips_dict = {}
ip_list = [{'cidr': '111.2.3.4/32'}, {'cidr': '111.2.3.5/32'}]
ip_list = [{'cidr': '111.2.3.4'}, {'cidr': '111.2.3.5'}]
self._test_scan_fip_ports(ri, ip_list)
self.assertEqual(2, ri.dist_fip_count)
@mock.patch.object(ip_lib, 'device_exists')
def test_scan_fip_ports_restart_none(self, device_exists):
device_exists.return_value = True
def test_scan_fip_ports_restart_none(self):
ri = mock.Mock()
ri.dist_fip_count = None
ri.floating_ips_dict = {}

20
neutron/tests/unit/agent/l3/test_dvr_local_router.py

@ -304,14 +304,13 @@ class TestDvrRouterOperations(base.BaseTestCase):
priority=FIP_PRI)
def _test_add_floating_ip(self, ri, fip, is_failure):
ri._add_fip_addr_to_device = mock.Mock(return_value=is_failure)
ri.floating_ip_added_dist = mock.Mock()
result = ri.add_floating_ip(fip,
mock.sentinel.interface_name,
mock.sentinel.device)
ri._add_fip_addr_to_device.assert_called_once_with(
fip, mock.sentinel.device)
ri.floating_ip_added_dist.assert_called_once_with(
fip, mock.ANY)
return result
def test_add_floating_ip(self):
@ -322,13 +321,6 @@ class TestDvrRouterOperations(base.BaseTestCase):
ri.floating_ip_added_dist.assert_called_once_with(fip, ip + '/32')
self.assertEqual(l3_constants.FLOATINGIP_STATUS_ACTIVE, result)
def test_add_floating_ip_error(self):
ri = self._create_router(mock.MagicMock())
result = self._test_add_floating_ip(
ri, {'floating_ip_address': '15.1.2.3'}, False)
self.assertFalse(ri.floating_ip_added_dist.called)
self.assertEqual(l3_constants.FLOATINGIP_STATUS_ERROR, result)
@mock.patch.object(router_info.RouterInfo, 'remove_floating_ip')
def test_remove_floating_ip(self, super_remove_floating_ip):
ri = self._create_router(mock.MagicMock())
@ -336,8 +328,7 @@ class TestDvrRouterOperations(base.BaseTestCase):
ri.remove_floating_ip(mock.sentinel.device, mock.sentinel.ip_cidr)
super_remove_floating_ip.assert_called_once_with(
mock.sentinel.device, mock.sentinel.ip_cidr)
self.assertFalse(super_remove_floating_ip.called)
ri.floating_ip_removed_dist.assert_called_once_with(
mock.sentinel.ip_cidr)
@ -651,9 +642,8 @@ class TestDvrRouterOperations(base.BaseTestCase):
_, fip_to_rtr = ri.rtr_fip_subnet.get_pair()
self.mock_ip.get_devices.return_value = [
l3_test_common.FakeDev(ri.fip_ns.get_ext_device_name(_uuid()))]
self.mock_ip_dev.addr.list.return_value = [
{'cidr': vm_floating_ip + '/32'},
{'cidr': '19.4.4.1/24'}]
ri.get_router_cidrs = mock.Mock(
return_value={vm_floating_ip + '/32', '19.4.4.1/24'})
self.device_exists.return_value = True
ri.external_gateway_removed(

Loading…
Cancel
Save