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>
This commit is contained in:
Hong Hui Xiao 2016-05-12 05:48:15 +00:00 committed by Brian Haley
parent b934db80c6
commit a388f78c8c
5 changed files with 80 additions and 61 deletions

View File

@ -276,11 +276,4 @@ class FipNamespace(namespaces.Namespace):
rtr_2_fip_interface = self.get_rtr_ext_device_name(ri.router_id) 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) device = ip_lib.IPDevice(rtr_2_fip_interface, namespace=ri.ns_name)
if device.exists(): if device.exists():
existing_cidrs = [addr['cidr'] for addr in device.addr.list()] ri.dist_fip_count = len(ri.get_router_cidrs(device))
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)

View File

@ -56,6 +56,33 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase):
(i['host'] == self.host) or (i['host'] == self.host) or
(i.get('dest_host') == self.host))] (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): def floating_ip_added_dist(self, fip, fip_cidr):
"""Add floating IP to FIP namespace.""" """Add floating IP to FIP namespace."""
floating_ip = fip['floating_ip_address'] floating_ip = fip['floating_ip_address']
@ -121,16 +148,12 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase):
ns_ip.del_veth(fip_2_rtr_name) ns_ip.del_veth(fip_2_rtr_name)
def add_floating_ip(self, fip, interface_name, device): 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 # Special Handling for DVR - update FIP namespace
ip_cidr = common_utils.ip_to_cidr(fip['floating_ip_address']) ip_cidr = common_utils.ip_to_cidr(fip['floating_ip_address'])
self.floating_ip_added_dist(fip, ip_cidr) self.floating_ip_added_dist(fip, ip_cidr)
return l3_constants.FLOATINGIP_STATUS_ACTIVE return l3_constants.FLOATINGIP_STATUS_ACTIVE
def remove_floating_ip(self, device, ip_cidr): def remove_floating_ip(self, device, ip_cidr):
super(DvrLocalRouter, self).remove_floating_ip(device, ip_cidr)
self.floating_ip_removed_dist(ip_cidr) self.floating_ip_removed_dist(ip_cidr)
def _get_internal_port(self, subnet_id): def _get_internal_port(self, subnet_id):
@ -477,6 +500,28 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase):
(self.dist_fip_count == 0)): (self.dist_fip_count == 0)):
self.fip_ns.create_rtr_2_fip_link(self) self.fip_ns.create_rtr_2_fip_link(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): def process(self, agent):
ex_gw_port = self.get_ex_gw_port() ex_gw_port = self.get_ex_gw_port()
if ex_gw_port: if ex_gw_port:

View File

@ -413,6 +413,15 @@ class TestDvrRouter(framework.L3AgentTestFramework):
self.assertTrue(ip_lib.device_exists( self.assertTrue(ip_lib.device_exists(
device_name, namespace=router.ns_name)) 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): def test_dvr_router_rem_fips_on_restarted_agent(self):
self.agent.conf.agent_mode = 'dvr_snat' self.agent.conf.agent_mode = 'dvr_snat'
router_info = self.generate_dvr_router_info() router_info = self.generate_dvr_router_info()
@ -443,6 +452,13 @@ class TestDvrRouter(framework.L3AgentTestFramework):
router_ns, floating_ips[0]['fixed_ip_address']) router_ns, floating_ips[0]['fixed_ip_address'])
self.assertNotEqual(fip_rule_prio_1, fip_rule_prio_2) 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_prevent_snat_rule_exist_on_restarted_agent(self): def test_prevent_snat_rule_exist_on_restarted_agent(self):
self.agent.conf.agent_mode = 'dvr_snat' self.agent.conf.agent_mode = 'dvr_snat'
router_info = self.generate_dvr_router_info() router_info = self.generate_dvr_router_info()
@ -452,19 +468,15 @@ class TestDvrRouter(framework.L3AgentTestFramework):
prevent_snat_rule = router._prevent_snat_for_internal_traffic_rule( prevent_snat_rule = router._prevent_snat_for_internal_traffic_rule(
rfp_devicename) rfp_devicename)
def is_prevent_snat_rule_exist(router_iptables_manager): self._assert_iptables_rules_exist(
rules = router_iptables_manager.get_rules_for_table('nat') router.iptables_manager, 'nat', [prevent_snat_rule])
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))
restarted_agent = neutron_l3_agent.L3NATAgentWithStateReport( restarted_agent = neutron_l3_agent.L3NATAgentWithStateReport(
self.agent.host, self.agent.conf) self.agent.host, self.agent.conf)
restarted_router = self.manage_router(restarted_agent, router_info) restarted_router = self.manage_router(restarted_agent, router_info)
self.assertTrue(is_prevent_snat_rule_exist( self._assert_iptables_rules_exist(
restarted_router.iptables_manager)) restarted_router.iptables_manager, 'nat', [prevent_snat_rule])
def _get_fixed_ip_rule_priority(self, namespace, fip): def _get_fixed_ip_rule_priority(self, namespace, fip):
iprule = ip_lib.IPRule(namespace) iprule = ip_lib.IPRule(namespace)
@ -883,21 +895,3 @@ class TestDvrRouter(framework.L3AgentTestFramework):
# external networks. SNAT will be used. Direct route will not work # external networks. SNAT will be used. Direct route will not work
# here. # here.
src_machine.assert_no_ping(machine_diff_scope.ip) 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)

View File

@ -272,24 +272,21 @@ class TestDvrFipNs(base.BaseTestCase):
@mock.patch.object(ip_lib, 'IPDevice') @mock.patch.object(ip_lib, 'IPDevice')
def _test_scan_fip_ports(self, ri, ip_list, IPDevice): def _test_scan_fip_ports(self, ri, ip_list, IPDevice):
IPDevice.return_value = device = mock.Mock() 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( self.fip_ns.get_rtr_ext_device_name = mock.Mock(
return_value=mock.sentinel.rtr_ext_device_name) return_value=mock.sentinel.rtr_ext_device_name)
self.fip_ns.scan_fip_ports(ri) self.fip_ns.scan_fip_ports(ri)
@mock.patch.object(ip_lib, 'device_exists') def test_scan_fip_ports_restart_fips(self):
def test_scan_fip_ports_restart_fips(self, device_exists):
device_exists.return_value = True
ri = mock.Mock() ri = mock.Mock()
ri.dist_fip_count = None ri.dist_fip_count = None
ri.floating_ips_dict = {} 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._test_scan_fip_ports(ri, ip_list)
self.assertEqual(2, ri.dist_fip_count) self.assertEqual(2, ri.dist_fip_count)
@mock.patch.object(ip_lib, 'device_exists') def test_scan_fip_ports_restart_none(self):
def test_scan_fip_ports_restart_none(self, device_exists):
device_exists.return_value = True
ri = mock.Mock() ri = mock.Mock()
ri.dist_fip_count = None ri.dist_fip_count = None
ri.floating_ips_dict = {} ri.floating_ips_dict = {}

View File

@ -283,14 +283,13 @@ class TestDvrRouterOperations(base.BaseTestCase):
ri.fip_ns.local_subnets.allocate.assert_called_once_with(ri.router_id) ri.fip_ns.local_subnets.allocate.assert_called_once_with(ri.router_id)
def _test_add_floating_ip(self, ri, fip, is_failure): 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() ri.floating_ip_added_dist = mock.Mock()
result = ri.add_floating_ip(fip, result = ri.add_floating_ip(fip,
mock.sentinel.interface_name, mock.sentinel.interface_name,
mock.sentinel.device) mock.sentinel.device)
ri._add_fip_addr_to_device.assert_called_once_with( ri.floating_ip_added_dist.assert_called_once_with(
fip, mock.sentinel.device) fip, mock.ANY)
return result return result
def test_add_floating_ip(self): def test_add_floating_ip(self):
@ -301,13 +300,6 @@ class TestDvrRouterOperations(base.BaseTestCase):
ri.floating_ip_added_dist.assert_called_once_with(fip, ip + '/32') ri.floating_ip_added_dist.assert_called_once_with(fip, ip + '/32')
self.assertEqual(l3_constants.FLOATINGIP_STATUS_ACTIVE, result) 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') @mock.patch.object(router_info.RouterInfo, 'remove_floating_ip')
def test_remove_floating_ip(self, super_remove_floating_ip): def test_remove_floating_ip(self, super_remove_floating_ip):
ri = self._create_router(mock.MagicMock()) ri = self._create_router(mock.MagicMock())
@ -315,8 +307,7 @@ class TestDvrRouterOperations(base.BaseTestCase):
ri.remove_floating_ip(mock.sentinel.device, mock.sentinel.ip_cidr) ri.remove_floating_ip(mock.sentinel.device, mock.sentinel.ip_cidr)
super_remove_floating_ip.assert_called_once_with( self.assertFalse(super_remove_floating_ip.called)
mock.sentinel.device, mock.sentinel.ip_cidr)
ri.floating_ip_removed_dist.assert_called_once_with( ri.floating_ip_removed_dist.assert_called_once_with(
mock.sentinel.ip_cidr) mock.sentinel.ip_cidr)
@ -630,9 +621,8 @@ class TestDvrRouterOperations(base.BaseTestCase):
_, fip_to_rtr = ri.rtr_fip_subnet.get_pair() _, fip_to_rtr = ri.rtr_fip_subnet.get_pair()
self.mock_ip.get_devices.return_value = [ self.mock_ip.get_devices.return_value = [
l3_test_common.FakeDev(ri.fip_ns.get_ext_device_name(_uuid()))] l3_test_common.FakeDev(ri.fip_ns.get_ext_device_name(_uuid()))]
self.mock_ip_dev.addr.list.return_value = [ ri.get_router_cidrs = mock.Mock(
{'cidr': vm_floating_ip + '/32'}, return_value={vm_floating_ip + '/32', '19.4.4.1/24'})
{'cidr': '19.4.4.1/24'}]
self.device_exists.return_value = True self.device_exists.return_value = True
ri.external_gateway_removed( ri.external_gateway_removed(