From c511964d70324c6974740eed245044f0fe77841e Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Fri, 30 Apr 2021 10:42:31 +0000 Subject: [PATCH] Replace "get_routing_table" with "list_ip_routes" "get_routing_table" uses "pyroute2.IPDB" that has been deprecated. "list_ip_routes" has been improved to be able to read multipath routes. Closes-Bug: #1926476 Change-Id: I0299fa11a7afefbd2999f81cd4ed3beed572009c --- neutron/agent/linux/ip_lib.py | 31 +- neutron/privileged/agent/linux/ip_lib.py | 54 +-- .../tests/functional/agent/l3/framework.py | 11 +- .../functional/agent/linux/test_ip_lib.py | 92 ++--- .../privileged/agent/linux/test_ip_lib.py | 2 +- neutron/tests/unit/agent/linux/test_ip_lib.py | 336 ------------------ 6 files changed, 72 insertions(+), 454 deletions(-) diff --git a/neutron/agent/linux/ip_lib.py b/neutron/agent/linux/ip_lib.py index 3df7ec92216..b4fd21d5778 100644 --- a/neutron/agent/linux/ip_lib.py +++ b/neutron/agent/linux/ip_lib.py @@ -822,21 +822,6 @@ def flush_ip_addresses(ip_version, device, namespace=None): privileged.flush_ip_addresses(ip_version, device, namespace) -def get_routing_table(ip_version, namespace=None): - """Return a list of dictionaries, each representing a route. - - @param ip_version: the routes of version to return, for example 4 - @param namespace - @return: a list of dictionaries, each representing a route. - The dictionary format is: {'destination': cidr, - 'nexthop': ip, - 'device': device_name, - 'scope': scope} - """ - # oslo.privsep turns lists to tuples in its IPC code. Change it back - return list(privileged.get_routing_table(ip_version, namespace)) - - # NOTE(haleyb): These neighbour functions live outside the IpNeighCommand # class since not all callers require it. def add_neigh_entry(ip_address, mac_address, device, namespace=None, @@ -1544,12 +1529,24 @@ def list_ip_routes(namespace, ip_version, scope=None, via=None, table=None, 'source_prefix': get_attr(route, 'RTA_PREFSRC'), 'cidr': cidr, 'scope': IP_ADDRESS_SCOPE[int(route['scope'])], - 'device': get_device(int(get_attr(route, 'RTA_OIF')), devices), - 'via': get_attr(route, 'RTA_GATEWAY'), 'metric': metric, 'proto': proto, } + multipath = get_attr(route, 'RTA_MULTIPATH') + if multipath: + value['device'] = None + mp_via = [] + for mp in multipath: + mp_via.append({'device': get_device(int(mp['oif']), devices), + 'via': get_attr(mp, 'RTA_GATEWAY'), + 'weight': int(mp['hops']) + 1}) + value['via'] = mp_via + else: + value['device'] = get_device(int(get_attr(route, 'RTA_OIF')), + devices) + value['via'] = get_attr(route, 'RTA_GATEWAY') + ret.append(value) if scope: diff --git a/neutron/privileged/agent/linux/ip_lib.py b/neutron/privileged/agent/linux/ip_lib.py index 5db83168c56..efc0503f76c 100644 --- a/neutron/privileged/agent/linux/ip_lib.py +++ b/neutron/privileged/agent/linux/ip_lib.py @@ -39,7 +39,7 @@ NETNS_RUN_DIR = '/var/run/netns' NUD_STATES = {state[1]: state[0] for state in ndmsg.states.items()} -def _get_scope_name(scope): +def get_scope_name(scope): """Return the name of the scope (given as a number), or the scope number if the name is unknown. @@ -138,54 +138,6 @@ def _make_route_dict(destination, nexthop, device, scope): 'scope': scope} -@privileged.default.entrypoint -def get_routing_table(ip_version, namespace=None): - """Return a list of dictionaries, each representing a route. - - :param ip_version: IP version of routes to return, for example 4 - :param namespace: The name of the namespace from which to get the routes - :return: a list of dictionaries, each representing a route. - The dictionary format is: {'destination': cidr, - 'nexthop': ip, - 'device': device_name, - 'scope': scope} - """ - family = _IP_VERSION_FAMILY_MAP[ip_version] - try: - netns = pyroute2.NetNS(namespace, flags=0) if namespace else None - except OSError as e: - if e.errno == errno.ENOENT: - raise NetworkNamespaceNotFound(netns_name=namespace) - raise - routes = [] - with pyroute2.IPDB(nl=netns) as ipdb: - ipdb_routes = ipdb.routes - ipdb_interfaces = ipdb.interfaces - for route in ipdb_routes: - if route['family'] != family: - continue - dst = route['dst'] - nexthop = route.get('gateway') - oif = route.get('oif') - scope = _get_scope_name(route['scope']) - - # If there is not a valid outgoing interface id, check if - # this is a multipath route (i.e. same destination with - # multiple outgoing interfaces) - if oif: - device = ipdb_interfaces[oif]['ifname'] - rt = _make_route_dict(dst, nexthop, device, scope) - routes.append(rt) - elif route.get('multipath'): - for mpr in route['multipath']: - oif = mpr['oif'] - device = ipdb_interfaces[oif]['ifname'] - rt = _make_route_dict(dst, nexthop, device, scope) - routes.append(rt) - - return routes - - def get_iproute(namespace): # From iproute.py: # `IPRoute` -- RTNL API to the current network namespace @@ -312,7 +264,7 @@ def add_ip_address(ip_version, ip, prefixlen, device, namespace, scope, mask=prefixlen, family=family, broadcast=broadcast, - scope=_get_scope_name(scope)) + scope=get_scope_name(scope)) except NetlinkError as e: if e.code == errno.EEXIST: raise IpAddressAlreadyExists(ip=ip, device=device) @@ -714,7 +666,7 @@ def _make_pyroute2_route_args(namespace, ip_version, cidr, device, via, table, args = {'family': _IP_VERSION_FAMILY_MAP[ip_version]} if not scope: scope = 'global' if via else 'link' - scope = _get_scope_name(scope) + scope = get_scope_name(scope) if scope: args['scope'] = scope if cidr: diff --git a/neutron/tests/functional/agent/l3/framework.py b/neutron/tests/functional/agent/l3/framework.py index 10d490e8c52..f751dd848e9 100644 --- a/neutron/tests/functional/agent/l3/framework.py +++ b/neutron/tests/functional/agent/l3/framework.py @@ -563,9 +563,9 @@ class L3AgentTestFramework(base.BaseSudoTestCase): def _assert_extra_routes(self, router, namespace=None): if namespace is None: namespace = router.ns_name - routes = ip_lib.get_routing_table(4, namespace=namespace) - routes = [{'nexthop': route['nexthop'], - 'destination': route['destination']} for route in routes] + routes = ip_lib.list_ip_routes(namespace, constants.IP_VERSION_4) + routes = [{'nexthop': route['via'], + 'destination': route['cidr']} for route in routes] for extra_route in router.router['routes']: self.assertIn(extra_route, routes) @@ -575,10 +575,9 @@ class L3AgentTestFramework(base.BaseSudoTestCase): ns_name = namespace or router.ns_name routes = [] for ip_version in ip_versions: - _routes = ip_lib.get_routing_table(ip_version, - namespace=ns_name) + _routes = ip_lib.list_ip_routes(ns_name, ip_version) routes.extend(_routes) - routes = set(route['destination'] for route in routes) + routes = set(route['cidr'] for route in routes) extra_subnets = router.get_ex_gw_port()['extra_subnets'] for extra_subnet in (route['cidr'] for route in extra_subnets): self.assertIn(extra_subnet, routes) diff --git a/neutron/tests/functional/agent/linux/test_ip_lib.py b/neutron/tests/functional/agent/linux/test_ip_lib.py index 069ffe832a4..acfb8181297 100644 --- a/neutron/tests/functional/agent/linux/test_ip_lib.py +++ b/neutron/tests/functional/agent/linux/test_ip_lib.py @@ -370,49 +370,6 @@ class IpLibTestCase(IpLibTestFramework): self.assertIsNone( device.route.get_gateway(ip_version=ip_version)) - def test_get_routing_table(self): - attr = self.generate_device_details( - ip_cidrs=["%s/24" % TEST_IP, "fd00::1/64"] - ) - device = self.manage_device(attr) - device_ip = attr.ip_cidrs[0].split('/')[0] - destination = '8.8.8.0/24' - device.route.add_route(destination, device_ip) - - destination6 = 'fd01::/64' - device.route.add_route(destination6, "fd00::2") - - expected_routes = [{'nexthop': device_ip, - 'device': attr.name, - 'destination': destination, - 'scope': 'universe'}, - {'nexthop': None, - 'device': attr.name, - 'destination': str( - netaddr.IPNetwork(attr.ip_cidrs[0]).cidr), - 'scope': 'link'}] - - routes = ip_lib.get_routing_table(4, namespace=attr.namespace) - self.assertCountEqual(expected_routes, routes) - self.assertIsInstance(routes, list) - - expected_routes6 = [{'nexthop': "fd00::2", - 'device': attr.name, - 'destination': destination6, - 'scope': 'universe'}, - {'nexthop': None, - 'device': attr.name, - 'destination': str( - netaddr.IPNetwork(attr.ip_cidrs[1]).cidr), - 'scope': 'universe'}] - routes6 = ip_lib.get_routing_table(6, namespace=attr.namespace) - self.assertCountEqual(expected_routes6, routes6) - self.assertIsInstance(routes6, list) - - def test_get_routing_table_no_namespace(self): - with testtools.ExpectedException(ip_lib.NetworkNamespaceNotFound): - ip_lib.get_routing_table(4, namespace="nonexistent-netns") - def test_get_neigh_entries(self): attr = self.generate_device_details( ip_cidrs=["%s/24" % TEST_IP, "fd00::1/64"] @@ -1137,3 +1094,52 @@ class GetDevicesWithIpTestCase(functional_base.BaseSudoTestCase): ip_addresses = self._remove_loopback_interface(ip_addresses) ip_addresses = self._remove_ipv6_scope_link(ip_addresses) self.assertEqual(0, len(ip_addresses)) + + +class ListIpRoutesTestCase(functional_base.BaseSudoTestCase): + + def setUp(self): + super().setUp() + self.namespace = self.useFixture(net_helpers.NamespaceFixture()).name + self.device_names = ['test_device1', 'test_device2'] + self.device_ips = ['10.0.0.1/24', '10.0.1.1/24'] + self.device_cidrs = [netaddr.IPNetwork(ip_address).cidr for ip_address + in self.device_ips] + for idx, dev in enumerate(self.device_names): + ip_lib.IPWrapper(self.namespace).add_dummy(dev) + device = ip_lib.IPDevice(dev, namespace=self.namespace) + device.link.set_up() + device.addr.add(self.device_ips[idx]) + + def test_list_ip_routes_multipath(self): + multipath = [ + {'device': self.device_names[0], + 'via': str(self.device_cidrs[0].ip + 100), 'weight': 10}, + {'device': self.device_names[1], + 'via': str(self.device_cidrs[1].ip + 100), 'weight': 20}, + {'via': str(self.device_cidrs[1].ip + 101), 'weight': 30}, + {'via': str(self.device_cidrs[1].ip + 102)}] + ip_lib.add_ip_route(self.namespace, '1.2.3.0/24', + constants.IP_VERSION_4, via=multipath) + + routes = ip_lib.list_ip_routes(self.namespace, constants.IP_VERSION_4) + multipath[2]['device'] = self.device_names[1] + multipath[3]['device'] = self.device_names[1] + multipath[3]['weight'] = 1 + for route in (route for route in routes if + route['cidr'] == '1.2.3.0/24'): + if not isinstance(route['via'], list): + continue + + self.assertEqual(len(multipath), len(route['via'])) + for nexthop in multipath: + for mp in route['via']: + if nexthop != mp: + continue + break + else: + self.fail('Not matching route, routes: %s' % routes) + + return + + self.fail('Not matching route, routes: %s' % routes) diff --git a/neutron/tests/functional/privileged/agent/linux/test_ip_lib.py b/neutron/tests/functional/privileged/agent/linux/test_ip_lib.py index 25238217ddb..29764215502 100644 --- a/neutron/tests/functional/privileged/agent/linux/test_ip_lib.py +++ b/neutron/tests/functional/privileged/agent/linux/test_ip_lib.py @@ -509,7 +509,7 @@ class RouteTestCase(functional_base.BaseSudoTestCase): table = table or iproute_linux.DEFAULT_TABLE if not scope: scope = 'universe' if gateway else 'link' - scope = priv_ip_lib._get_scope_name(scope) + scope = priv_ip_lib.get_scope_name(scope) for cidr in cidrs: ip_version = common_utils.get_ip_version(cidr) if ip_version == n_cons.IP_VERSION_6 and not metric: diff --git a/neutron/tests/unit/agent/linux/test_ip_lib.py b/neutron/tests/unit/agent/linux/test_ip_lib.py index 415909b7f9f..2334cf4f17b 100644 --- a/neutron/tests/unit/agent/linux/test_ip_lib.py +++ b/neutron/tests/unit/agent/linux/test_ip_lib.py @@ -914,342 +914,6 @@ class TestDeviceExists(base.BaseTestCase): self.assertFalse(ip_lib.ensure_device_is_ready("lo")) -class TestGetRoutingTable(base.BaseTestCase): - ip_db_interfaces = { - 1: { - 'family': 0, - 'txqlen': 0, - 'ipdb_scope': 'system', - 'index': 1, - 'operstate': 'DOWN', - 'num_tx_queues': 1, - 'group': 0, - 'carrier_changes': 0, - 'ipaddr': [], - 'neighbours': [], - 'ifname': 'lo', - 'promiscuity': 0, - 'linkmode': 0, - 'broadcast': '00:00:00:00:00:00', - 'address': '00:00:00:00:00:00', - 'vlans': [], - 'ipdb_priority': 0, - 'qdisc': 'noop', - 'mtu': 65536, - 'num_rx_queues': 1, - 'carrier': 1, - 'flags': 8, - 'ifi_type': 772, - 'ports': [] - }, - 2: { - 'family': 0, - 'txqlen': 500, - 'ipdb_scope': 'system', - 'index': 2, - 'operstate': 'DOWN', - 'num_tx_queues': 1, - 'group': 0, - 'carrier_changes': 1, - 'ipaddr': ['1111:1111:1111:1111::3/64', '10.0.0.3/24'], - 'neighbours': [], - 'ifname': 'tap-1', - 'promiscuity': 0, - 'linkmode': 0, - 'broadcast': 'ff:ff:ff:ff:ff:ff', - 'address': 'b6:d5:f6:a8:2e:62', - 'vlans': [], - 'ipdb_priority': 0, - 'kind': 'tun', - 'qdisc': 'fq_codel', - 'mtu': 1500, - 'num_rx_queues': 1, - 'carrier': 0, - 'flags': 4099, - 'ifi_type': 1, - 'ports': [] - }, - 'tap-1': { - 'family': 0, - 'txqlen': 500, - 'ipdb_scope': 'system', - 'index': 2, - 'operstate': 'DOWN', - 'num_tx_queues': 1, - 'group': 0, - 'carrier_changes': 1, - 'ipaddr': ['1111:1111:1111:1111::3/64', '10.0.0.3/24'], - 'neighbours': [], - 'ifname': 'tap-1', - 'promiscuity': 0, - 'linkmode': 0, - 'broadcast': 'ff:ff:ff:ff:ff:ff', - 'address': 'b6:d5:f6:a8:2e:62', - 'vlans': [], - 'ipdb_priority': 0, - 'kind': 'tun', - 'qdisc': 'fq_codel', - 'mtu': 1500, - 'num_rx_queues': 1, - 'carrier': 0, - 'flags': 4099, - 'ifi_type': 1, - 'ports': [] - }, - 'lo': { - 'family': 0, - 'txqlen': 0, - 'ipdb_scope': 'system', - 'index': 1, - 'operstate': 'DOWN', - 'num_tx_queues': 1, - 'group': 0, - 'carrier_changes': 0, - 'ipaddr': [], - 'neighbours': [], - 'ifname': 'lo', - 'promiscuity': 0, - 'linkmode': 0, - 'broadcast': '00:00:00:00:00:00', - 'address': '00:00:00:00:00:00', - 'vlans': [], - 'ipdb_priority': 0, - 'qdisc': 'noop', - 'mtu': 65536, - 'num_rx_queues': 1, - 'carrier': 1, - 'flags': 8, - 'ifi_type': 772, - 'ports': [] - } - } - - ip_db_routes = [ - { - 'oif': 2, - 'dst_len': 24, - 'family': 2, - 'proto': 3, - 'tos': 0, - 'dst': '10.0.1.0/24', - 'flags': 16, - 'ipdb_priority': 0, - 'metrics': {}, - 'scope': 0, - 'encap': {}, - 'src_len': 0, - 'table': 254, - 'multipath': [], - 'type': 1, - 'gateway': '10.0.0.1', - 'ipdb_scope': 'system' - }, { - 'oif': 2, - 'type': 1, - 'dst_len': 24, - 'family': 2, - 'proto': 2, - 'tos': 0, - 'dst': '10.0.0.0/24', - 'ipdb_priority': 0, - 'metrics': {}, - 'flags': 16, - 'encap': {}, - 'src_len': 0, - 'table': 254, - 'multipath': [], - 'prefsrc': '10.0.0.3', - 'scope': 253, - 'ipdb_scope': 'system' - }, { - 'oif': 2, - 'dst_len': 0, - 'family': 2, - 'proto': 3, - 'tos': 0, - 'dst': 'default', - 'flags': 16, - 'ipdb_priority': 0, - 'metrics': {}, - 'scope': 0, - 'encap': {}, - 'src_len': 0, - 'table': 254, - 'multipath': [], - 'type': 1, - 'gateway': '10.0.0.2', - 'ipdb_scope': 'system' - }, { - 'metrics': {}, - 'oif': 2, - 'dst_len': 64, - 'family': socket.AF_INET6, - 'proto': 2, - 'tos': 0, - 'dst': '1111:1111:1111:1111::/64', - 'pref': '00', - 'ipdb_priority': 0, - 'priority': 256, - 'flags': 0, - 'encap': {}, - 'src_len': 0, - 'table': 254, - 'multipath': [], - 'type': 1, - 'scope': 0, - 'ipdb_scope': 'system' - }, { - 'metrics': {}, - 'oif': 2, - 'dst_len': 64, - 'family': socket.AF_INET6, - 'proto': 3, - 'tos': 0, - 'dst': '1111:1111:1111:1112::/64', - 'pref': '00', - 'flags': 0, - 'ipdb_priority': 0, - 'priority': 1024, - 'scope': 0, - 'encap': {}, - 'src_len': 0, - 'table': 254, - 'multipath': [], - 'type': 1, - 'gateway': '1111:1111:1111:1111::1', - 'ipdb_scope': 'system' - } - ] - - ip_db_multipath_routes = [ - { - 'dst_len': 24, - 'family': socket.AF_INET, - 'proto': 3, - 'tos': 0, - 'dst': '10.0.1.0/24', - 'flags': 16, - 'ipdb_priority': 0, - 'metrics': {}, - 'scope': 0, - 'encap': {}, - 'src_len': 0, - 'table': 254, - 'multipath': ({'oif': 1, 'family': socket.AF_INET}, - {'oif': 2, 'dst_len': 24, 'family': socket.AF_INET, - 'proto': 2, 'tos': 0, 'pref': '00', - 'priority': 256, 'flags': 0, 'encap': {}, - 'src_len': 0, 'table': 254, 'type': 1, - 'scope': 0}), - 'type': 1, - 'gateway': '10.0.0.1', - 'ipdb_scope': 'system' - }, { - 'metrics': {}, - 'dst_len': 64, - 'family': socket.AF_INET6, - 'proto': 2, - 'tos': 0, - 'dst': '1111:1111:1111:1111::/64', - 'pref': '00', - 'ipdb_priority': 0, - 'priority': 256, - 'flags': 0, - 'encap': {}, - 'src_len': 0, - 'table': 254, - 'multipath': ({'oif': 1, 'family': socket.AF_INET6}, - {'oif': 2, 'dst_len': 64, 'family': socket.AF_INET6, - 'proto': 2, 'tos': 0, 'pref': '00', - 'priority': 256, 'flags': 0, 'encap': {}, - 'src_len': 0, 'table': 254, 'type': 1, - 'scope': 0}), - 'type': 1, - 'scope': 0, - 'ipdb_scope': 'system' - } - ] - - def setUp(self): - super(TestGetRoutingTable, self).setUp() - self.addCleanup(privileged.default.set_client_mode, True) - privileged.default.set_client_mode(False) - - @mock.patch.object(pyroute2, 'IPDB') - @mock.patch.object(pyroute2, 'NetNS') - def test_get_routing_table_nonexistent_namespace(self, - mock_netns, mock_ip_db): - mock_netns.side_effect = OSError(errno.ENOENT, None) - with testtools.ExpectedException(ip_lib.NetworkNamespaceNotFound): - ip_lib.get_routing_table(4, 'ns') - - @mock.patch.object(pyroute2, 'IPDB') - @mock.patch.object(pyroute2, 'NetNS') - def test_get_routing_table_other_error(self, mock_netns, mock_ip_db): - expected_exception = OSError(errno.EACCES, None) - mock_netns.side_effect = expected_exception - with testtools.ExpectedException(expected_exception.__class__): - ip_lib.get_routing_table(4, 'ns') - - @mock.patch.object(pyroute2, 'IPDB') - @mock.patch.object(pyroute2, 'NetNS') - def _test_get_routing_table(self, version, ip_db_routes, expected, - mock_netns, mock_ip_db): - mock_ip_db_instance = mock_ip_db.return_value - mock_ip_db_enter = mock_ip_db_instance.__enter__.return_value - mock_ip_db_enter.interfaces = self.ip_db_interfaces - mock_ip_db_enter.routes = ip_db_routes - self.assertEqual(expected, ip_lib.get_routing_table(version)) - - def test_get_routing_table_4(self): - expected = [{'destination': '10.0.1.0/24', - 'nexthop': '10.0.0.1', - 'device': 'tap-1', - 'scope': 'universe'}, - {'destination': '10.0.0.0/24', - 'nexthop': None, - 'device': 'tap-1', - 'scope': 'link'}, - {'destination': 'default', - 'nexthop': '10.0.0.2', - 'device': 'tap-1', - 'scope': 'universe'}] - self._test_get_routing_table(4, self.ip_db_routes, expected) - - def test_get_routing_table_6(self): - expected = [{'destination': '1111:1111:1111:1111::/64', - 'nexthop': None, - 'device': 'tap-1', - 'scope': 'universe'}, - {'destination': '1111:1111:1111:1112::/64', - 'nexthop': '1111:1111:1111:1111::1', - 'device': 'tap-1', - 'scope': 'universe'}] - self._test_get_routing_table(6, self.ip_db_routes, expected) - - def test_get_routing_table_multipath_4(self): - expected = [{'destination': '10.0.1.0/24', - 'nexthop': '10.0.0.1', - 'device': 'lo', - 'scope': 'universe'}, - {'destination': '10.0.1.0/24', - 'nexthop': '10.0.0.1', - 'device': 'tap-1', - 'scope': 'universe'}] - self._test_get_routing_table(4, self.ip_db_multipath_routes, expected) - - def test_get_routing_table_multipath_6(self): - expected = [{'destination': '1111:1111:1111:1111::/64', - 'nexthop': None, - 'device': 'lo', - 'scope': 'universe'}, - {'destination': '1111:1111:1111:1111::/64', - 'nexthop': None, - 'device': 'tap-1', - 'scope': 'universe'}] - self._test_get_routing_table(6, self.ip_db_multipath_routes, expected) - - class TestIpNeighCommand(TestIPCmdBase): def setUp(self): super(TestIpNeighCommand, self).setUp()