From 97c98a1c6dac5788690504463a5ca33f4b181d6a Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Mon, 3 Dec 2018 18:11:26 +0000 Subject: [PATCH] [DVR] Allow multiple subnets per external network An external network can have more than one subnet. Currently only the first subnet is added to the FIP namespace routing table. Packets for FIPs with addresses in other subnets can't pass through the external port because there is no route for those FIP CIDRs. This change adds routes for those CIDRs via the external port IP and interface. These routes doesn't collide with the existing ones, added to provide a back path for the packets with a destination IP matching a FIP. E.g.: $ ip netns exec fip-e1ec0f98-b593-4514-ae08-f1c5cf1c2788 ip route (1) 169.254.106.114/31 dev fpr-3937f879-d proto kernel scope link \ src 169.254.106.115 (2) 192.168.20.250 via 169.254.106.114 dev fpr-3937f879-d (3) 192.168.30.0/24 dev fg-bee060f1-dd proto kernel scope link \ src 192.168.30.129 (4) 192.168.20.0/24 via 192.168.30.129 dev fg-bee060f1-dd scope link Rule (2) is added when a FIP is assigned. This rule permits ingress packets going into the router namespace. This FIP belongs to the second subnet of the external network (note the external port CIDR is not the same). Rule (4), added by this patch, allows egress packets to exit the FIP namespace through the external port. Rule (2), because of the prefix length (32), has more priority than rule (4). Change-Id: I4d476b47e89fa5709dca2f66ffae72a27d88340a Closes-Bug: #1805456 --- neutron/agent/l3/dvr_fip_ns.py | 32 +++++++++++++------ neutron/agent/linux/interface.py | 30 +++++++++++------ .../functional/agent/l3/test_dvr_router.py | 28 ++++++++++++++++ .../tests/unit/agent/l3/test_dvr_fip_ns.py | 16 ++++++++-- 4 files changed, 84 insertions(+), 22 deletions(-) diff --git a/neutron/agent/l3/dvr_fip_ns.py b/neutron/agent/l3/dvr_fip_ns.py index dad9fc0ae93..8b9e8b02fc8 100644 --- a/neutron/agent/l3/dvr_fip_ns.py +++ b/neutron/agent/l3/dvr_fip_ns.py @@ -193,6 +193,12 @@ class FipNamespace(namespaces.Namespace): self.driver.init_l3(interface_name, ip_cidrs, namespace=ns_name, clean_connections=True) + gw_cidrs = [sn['cidr'] for sn in ex_gw_port['subnets'] + if sn.get('cidr')] + self.driver.set_onlink_routes( + interface_name, ns_name, ex_gw_port.get('extra_subnets', []), + preserve_ips=gw_cidrs, is_ipv6=False) + self.agent_gateway_port = ex_gw_port cmd = ['sysctl', '-w', 'net.ipv4.conf.%s.proxy_arp=1' % interface_name] @@ -313,17 +319,23 @@ class FipNamespace(namespaces.Namespace): priority=rt_tbl_index) def _update_gateway_port(self, agent_gateway_port, interface_name): - if (self.agent_gateway_port and - not self._check_for_gateway_ip_change(agent_gateway_port)): - return - # Caller already holding lock - self._update_gateway_route( - agent_gateway_port, interface_name, tbl_index=None) + if (not self.agent_gateway_port or + self._check_for_gateway_ip_change(agent_gateway_port)): + # Caller already holding lock + self._update_gateway_route( + agent_gateway_port, interface_name, tbl_index=None) - # Cache the agent gateway port after successfully updating - # the gateway route, so that checking on self.agent_gateway_port - # will be a valid check - self.agent_gateway_port = agent_gateway_port + # Cache the agent gateway port after successfully updating + # the gateway route, so that checking on self.agent_gateway_port + # will be a valid check + self.agent_gateway_port = agent_gateway_port + + gw_cidrs = [sn['cidr'] for sn in agent_gateway_port['subnets'] + if sn.get('cidr')] + self.driver.set_onlink_routes( + interface_name, self.get_name(), + agent_gateway_port.get('extra_subnets', []), preserve_ips=gw_cidrs, + is_ipv6=False) def _update_gateway_route(self, agent_gateway_port, interface_name, tbl_index): diff --git a/neutron/agent/linux/interface.py b/neutron/agent/linux/interface.py index e454ca9f45a..5bcf5622982 100644 --- a/neutron/agent/linux/interface.py +++ b/neutron/agent/linux/interface.py @@ -170,22 +170,34 @@ class LinuxInterfaceDriver(object): namespace=namespace, preserve_ips=preserve_ips or [], clean_connections=clean_connections) + self.set_onlink_routes(device_name, namespace, extra_subnets, + preserve_ips) + def set_onlink_routes(self, device_name, namespace, extra_subnets, + preserve_ips=None, is_ipv6=True): + """Manage on-link routes (routes without an associate address) + + :param device_name: interface name + :param namespace: namespace name + :param extra_subnets: subnets attached to this interface without an IP + address set in this interface + :param preserve_ips: IPs or CIDRs not to be deleted from the device + on-link route list + """ device = ip_lib.IPDevice(device_name, namespace=namespace) - - # Manage on-link routes (routes without an associated address) new_onlink_cidrs = set(s['cidr'] for s in extra_subnets or []) + preserve_ips = set(preserve_ips if preserve_ips else []) - v4_onlink = device.route.list_onlink_routes(constants.IP_VERSION_4) - v6_onlink = device.route.list_onlink_routes(constants.IP_VERSION_6) - existing_onlink_cidrs = set(r['cidr'] for r in v4_onlink + v6_onlink) + onlink = device.route.list_onlink_routes(constants.IP_VERSION_4) + if is_ipv6: + onlink += device.route.list_onlink_routes(constants.IP_VERSION_6) + existing_onlink_cidrs = set(r['cidr'] for r in onlink) for route in new_onlink_cidrs - existing_onlink_cidrs: - LOG.debug("adding onlink route(%s)", route) + LOG.debug('Adding onlink route (%s)', route) device.route.add_onlink_route(route) - for route in (existing_onlink_cidrs - new_onlink_cidrs - - set(preserve_ips or [])): - LOG.debug("deleting onlink route(%s)", route) + for route in existing_onlink_cidrs - new_onlink_cidrs - preserve_ips: + LOG.debug('Deleting onlink route (%s)', route) device.route.delete_onlink_route(route) def add_ipv6_addr(self, device_name, v6addr, namespace, scope='global'): diff --git a/neutron/tests/functional/agent/l3/test_dvr_router.py b/neutron/tests/functional/agent/l3/test_dvr_router.py index f58f70d4e0b..3436b56f30c 100644 --- a/neutron/tests/functional/agent/l3/test_dvr_router.py +++ b/neutron/tests/functional/agent/l3/test_dvr_router.py @@ -601,6 +601,7 @@ class TestDvrRouter(framework.L3AgentTestFramework): {'cidr': float_subnet['cidr'], 'gateway_ip': float_subnet['gateway_ip'], 'id': fixed_ip['subnet_id']}], + 'extra_subnets': external_gw_port['extra_subnets'], 'network_id': external_gw_port['network_id'], 'device_owner': lib_constants.DEVICE_OWNER_AGENT_GW, 'mac_address': 'fa:16:3e:80:8d:89', @@ -631,6 +632,7 @@ class TestDvrRouter(framework.L3AgentTestFramework): {'cidr': float_subnet['cidr'], 'gateway_ip': float_subnet['gateway_ip'], 'id': fixed_ip['subnet_id']}], + 'extra_subnets': external_gw_port['extra_subnets'], 'network_id': external_gw_port['network_id'], 'device_owner': lib_constants.DEVICE_OWNER_AGENT_GW, 'mac_address': 'fa:16:3e:80:8d:89', @@ -2037,3 +2039,29 @@ class TestDvrRouter(framework.L3AgentTestFramework): ip_lib.IP_NONLOCAL_BIND)) raise self.assertEqual(0, ip_nonlocal_bind_value) + + def test_dvr_router_fip_namespace_routes(self): + """Test to validate the floatingip namespace subnets routes.""" + self.agent.conf.agent_mode = 'dvr' + router_info = self.generate_dvr_router_info(enable_floating_ip=False) + fip_agent_gw_port = self._get_fip_agent_gw_port_for_router( + router_info['gw_port']) + self.mock_plugin_api.get_agent_gateway_port.return_value = ( + fip_agent_gw_port) + router1 = self.manage_router(self.agent, router_info) + + fip_namespace = router1.fip_ns.get_name() + ip_wrapper = ip_lib.IPWrapper(namespace=fip_namespace) + interfaces = ip_wrapper.get_devices() + fg_interface_name = next( + interface.name for interface in interfaces + if interface.name.startswith(dvr_fip_ns.FIP_EXT_DEV_PREFIX)) + + ip_device = ip_lib.IPDevice(fg_interface_name, namespace=fip_namespace) + routes = ip_device.route.list_onlink_routes(lib_constants.IP_VERSION_4) + self.assertGreater(len(routes), 0) + self.assertEqual(len(fip_agent_gw_port['extra_subnets']), len(routes)) + extra_subnet_cidr = set(extra_subnet['cidr'] for extra_subnet + in fip_agent_gw_port['extra_subnets']) + routes_cidr = set(route['cidr'] for route in routes) + self.assertEqual(extra_subnet_cidr, routes_cidr) diff --git a/neutron/tests/unit/agent/l3/test_dvr_fip_ns.py b/neutron/tests/unit/agent/l3/test_dvr_fip_ns.py index 89e19a45a88..01790040b96 100644 --- a/neutron/tests/unit/agent/l3/test_dvr_fip_ns.py +++ b/neutron/tests/unit/agent/l3/test_dvr_fip_ns.py @@ -102,13 +102,21 @@ class TestDvrFipNs(base.BaseTestCase): agent_gw_port = self._get_agent_gw_port() device_exists.return_value = False - self.fip_ns.create_or_update_gateway_port(agent_gw_port) + with mock.patch.object(self.fip_ns.driver, 'set_onlink_routes') as \ + mock_set_onlink_routes: + self.fip_ns.create_or_update_gateway_port(agent_gw_port) self.assertTrue(fip_create.called) self.assertEqual(1, self.driver.plug.call_count) ext_net_bridge = self.conf.external_network_bridge if ext_net_bridge: self.assertEqual(1, self.driver.remove_vlan_tag.call_count) self.assertEqual(1, self.driver.init_l3.call_count) + interface_name = self.fip_ns.get_ext_device_name(agent_gw_port['id']) + gw_cidrs = [sn['cidr'] for sn in agent_gw_port['subnets'] + if sn.get('cidr')] + mock_set_onlink_routes.assert_called_once_with( + interface_name, self.fip_ns.name, [], preserve_ips=gw_cidrs, + is_ipv6=False) @mock.patch.object(ip_lib, 'IPDevice') @mock.patch.object(ip_lib, 'send_ip_addr_adv_notif') @@ -121,7 +129,8 @@ class TestDvrFipNs(base.BaseTestCase): agent_gw_port = self._get_agent_gw_port() interface_name = self.fip_ns.get_ext_device_name(agent_gw_port['id']) self.fip_ns.agent_gateway_port = agent_gw_port - self.fip_ns.create_or_update_gateway_port(agent_gw_port) + with mock.patch.object(self.fip_ns.driver, 'set_onlink_routes'): + self.fip_ns.create_or_update_gateway_port(agent_gw_port) expected = [ mock.call(self.fip_ns.get_name(), interface_name, @@ -164,7 +173,8 @@ class TestDvrFipNs(base.BaseTestCase): agent_gw_port['subnets'][0]['gateway_ip'] = '20.0.1.1' self.fip_ns._check_for_gateway_ip_change = mock.Mock(return_value=True) self.fip_ns.agent_gateway_port = agent_gw_port - self.fip_ns.create_or_update_gateway_port(agent_gw_port) + with mock.patch.object(self.fip_ns.driver, 'set_onlink_routes'): + self.fip_ns.create_or_update_gateway_port(agent_gw_port) IPDevice().route.add_route.assert_called_once_with('20.0.1.1', scope='link')