diff --git a/neutron/agent/l3/router_info.py b/neutron/agent/l3/router_info.py index 85c4829a03a..09bcf8f5e84 100644 --- a/neutron/agent/l3/router_info.py +++ b/neutron/agent/l3/router_info.py @@ -187,20 +187,71 @@ class RouterInfo(BaseRouterInfo): def update_routing_table(self, operation, route): self._update_routing_table(operation, route, self.ns_name) + def update_routing_table_ecmp(self, route_list): + multipath = [dict(via=route['nexthop']) + for route in route_list] + try: + ip_lib.add_ip_route(self.ns_name, route_list[0]['destination'], + via=multipath) + except (RuntimeError, OSError, pyroute2_exc.NetlinkError): + pass + + def check_and_remove_ecmp_route(self, old_routes, remove_route): + route_list = [] + for route in old_routes: + if route['destination'] == remove_route['destination']: + route_list.append(route) + # An ECMP route is composed of multiple routes with the same + # destination address, and two scenarios should be considered + # when removing a nexthop address from an ECMP route. + # a. The original ECMP route has only two nexthops, deleting + # one of them will make it a normal route. + # b. The original ECMP route has more than two nexthops, + # delete one of the nexthops, it is still an ECMP route. + if len(route_list) == 2: + for r in route_list: + if r['nexthop'] != remove_route['nexthop']: + self.update_routing_table('replace', r) + return True + + if len(route_list) > 2: + route_list.remove(remove_route) + self.update_routing_table_ecmp(route_list) + return True + + return False + + def check_and_add_ecmp_route(self, old_routes, new_route): + route_list = [] + for route in old_routes: + if route['destination'] == new_route['destination']: + route_list.append(route) + + if route_list: + route_list.append(new_route) + self.update_routing_table_ecmp(route_list) + return True + + return False + def routes_updated(self, old_routes, new_routes): adds, removes = helpers.diff_list_of_dict(old_routes, new_routes) - for route in adds: - LOG.debug("Added route entry is '%s'", route) - # remove replaced route from deleted route - for del_route in removes: - if route['destination'] == del_route['destination']: - removes.remove(del_route) - # replace success even if there is no existing route - self.update_routing_table('replace', route) for route in removes: - LOG.debug("Removed route entry is '%s'", route) - self.update_routing_table('delete', route) + # Judge if modifying an ECMP route or not, if not, + # just delete it, if it is, replace it + # update old_routes after modify + if not self.check_and_remove_ecmp_route(old_routes, route): + LOG.debug("Removed route entry is '%s'", route) + self.update_routing_table('delete', route) + old_routes.remove(route) + + for route in adds: + if not self.check_and_add_ecmp_route(old_routes, route): + LOG.debug("Added route entry is '%s'", route) + # replace success even if there is no existing route + self.update_routing_table('replace', route) + old_routes.append(route) def get_floating_ips(self): """Filter Floating IPs to be hosted on this agent.""" diff --git a/neutron/extensions/ecmp_routes.py b/neutron/extensions/ecmp_routes.py new file mode 100644 index 00000000000..d05a3270b18 --- /dev/null +++ b/neutron/extensions/ecmp_routes.py @@ -0,0 +1,20 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from neutron_lib.api.definitions import ecmp_routes as apidef +from neutron_lib.api import extensions + + +class Ecmp_routes(extensions.APIExtensionDescriptor): + """Extension class supporting configuration of ECMP routes.""" + + api_definition = apidef diff --git a/neutron/tests/functional/agent/l3/test_dvr_router.py b/neutron/tests/functional/agent/l3/test_dvr_router.py index e250f86961c..dfd18679d9c 100644 --- a/neutron/tests/functional/agent/l3/test_dvr_router.py +++ b/neutron/tests/functional/agent/l3/test_dvr_router.py @@ -2185,3 +2185,74 @@ class TestDvrRouter(DvrRouterTestFramework, framework.L3AgentTestFramework): in fip_agent_gw_port['extra_subnets']) routes_cidr = set(route['cidr'] for route in routes) self.assertEqual(extra_subnet_cidr, routes_cidr) + + def test_dvr_router_update_ecmp_routes(self): + self.agent.conf.agent_mode = 'dvr' + router_info = self.generate_dvr_router_info() + print(router_info) + router1 = self.manage_router(self.agent, router_info) + router1.router['routes'] = [{'destination': '20.0.10.10/32', + 'nexthop': '35.4.0.11'}, + {'destination': '20.0.10.10/32', + 'nexthop': '35.4.0.22'}, + {'destination': '20.0.10.10/32', + 'nexthop': '35.4.0.33'}] + self.agent._process_updated_router(router1.router) + + updated_route = ip_lib.list_ip_routes( + router1.ns_name, + ip_version=lib_constants.IP_VERSION_4,) + expected_route = [{'cidr': '20.0.10.10/32', + 'table': 'main', + 'via': [{'via': '35.4.0.11'}, + {'via': '35.4.0.22'}, + {'via': '35.4.0.33'}] + }] + actual_routes = [{key: route[key] for key in expected_route[0].keys()} + for route in updated_route] + for entry in actual_routes: + if entry['via']: + if isinstance(entry['via'], (list, tuple)): + via_list = [{'via': hop['via']} + for hop in entry['via']] + entry['via'] = sorted(via_list, key=lambda i: i['via']) + self.assertIn(expected_route[0], actual_routes) + + # delete one route + router1.router['routes'] = [{'destination': '20.0.10.10/32', + 'nexthop': '35.4.0.11'}, + {'destination': '20.0.10.10/32', + 'nexthop': '35.4.0.22'}] + self.agent._process_updated_router(router1.router) + updated_route = ip_lib.list_ip_routes( + router1.ns_name, + ip_version=lib_constants.IP_VERSION_4, ) + expected_route = [{'cidr': '20.0.10.10/32', + 'table': 'main', + 'via': [{'via': '35.4.0.11'}, + {'via': '35.4.0.22'}] + }] + actual_routes = [{key: route[key] for key in expected_route[0].keys()} + for route in updated_route] + for entry in actual_routes: + if entry['via']: + if isinstance(entry['via'], (list, tuple)): + via_list = [{'via': hop['via']} + for hop in entry['via']] + entry['via'] = sorted(via_list, key=lambda i: i['via']) + self.assertIn(expected_route[0], actual_routes) + + # delete one route again + router1.router['routes'] = [{'destination': '20.0.10.10/32', + 'nexthop': '35.4.0.11'}] + self.agent._process_updated_router(router1.router) + updated_route = ip_lib.list_ip_routes( + router1.ns_name, + ip_version=lib_constants.IP_VERSION_4, ) + expected_route = [{'cidr': '20.0.10.10/32', + 'table': 'main', + 'via': '35.4.0.11' + }] + actual_routes = [{key: route[key] for key in expected_route[0].keys()} + for route in updated_route] + self.assertIn(expected_route[0], actual_routes) diff --git a/neutron/tests/unit/agent/l3/test_router_info.py b/neutron/tests/unit/agent/l3/test_router_info.py index 34f68913e0d..58344d623f6 100644 --- a/neutron/tests/unit/agent/l3/test_router_info.py +++ b/neutron/tests/unit/agent/l3/test_router_info.py @@ -52,6 +52,74 @@ class TestRouterInfo(base.BaseTestCase): self.mock_delete_ip_route) mock_method.assert_has_calls(mock_calls, any_order=True) + def _check_ip_wrapper_method_called(self, calls): + self.mock_ip.netns.execute.assert_has_calls( + [mock.call(call, check_exit_code=False) for call in calls], + any_order=True) + + def test_update_routing_table_ecmp(self): + ri = router_info.RouterInfo(mock.Mock(), _uuid(), {}, **self.ri_kwargs) + ri.router = {} + fake_route_list = [{'destination': '135.207.111.111/32', + 'nexthop': '135.207.111.112'}, + {'destination': '135.207.111.111/32', + 'nexthop': '135.207.111.113'}] + expected_dst = '135.207.111.111/32' + expected_next_hops = [{'via': '135.207.111.112'}, + {'via': '135.207.111.113'}] + ri.update_routing_table_ecmp(fake_route_list) + self.mock_add_ip_route.assert_called_once_with( + ri.ns_name, + expected_dst, + via=expected_next_hops) + + def test_check_and_remove_ecmp_route(self): + ri = router_info.RouterInfo(mock.Mock(), _uuid(), {}, **self.ri_kwargs) + ri.router = {} + fake_old_routes1 = [{'destination': '135.207.111.111/32', + 'nexthop': '135.207.111.112'}, + {'destination': '135.207.111.111/32', + 'nexthop': '135.207.111.113'}] + fake_route1 = {'destination': '135.207.111.111/32', + 'nexthop': '135.207.111.113'} + fake_old_routes2 = [{'destination': '135.207.111.111/32', + 'nexthop': '135.207.111.112'}, + {'destination': '135.207.111.111/32', + 'nexthop': '135.207.111.113'}, + {'destination': '135.207.111.111/32', + 'nexthop': '135.207.111.114'}] + fake_route2 = [{'destination': '135.207.111.111/32', + 'nexthop': '135.207.111.113'}, + {'destination': '135.207.111.111/32', + 'nexthop': '135.207.111.114'}] + fake_remove_route = {'destination': '135.207.111.111/32', + 'nexthop': '135.207.111.112'} + ri.update_routing_table = mock.Mock() + + ri.check_and_remove_ecmp_route(fake_old_routes1, fake_remove_route) + ri.update_routing_table.assert_called_once_with('replace', + fake_route1) + + ri.update_routing_table_ecmp = mock.Mock() + ri.check_and_remove_ecmp_route(fake_old_routes2, fake_remove_route) + ri.update_routing_table_ecmp.assert_called_once_with(fake_route2) + + def test_check_and_add_ecmp_route(self): + ri = router_info.RouterInfo(mock.Mock(), _uuid(), {}, **self.ri_kwargs) + ri.router = {} + fake_old_routes = [{'destination': '135.207.111.111/32', + 'nexthop': '135.207.111.112'}] + fake_new_route = {'destination': '135.207.111.111/32', + 'nexthop': '135.207.111.113'} + ri.update_routing_table_ecmp = mock.Mock() + + ri.check_and_add_ecmp_route(fake_old_routes, fake_new_route) + expected_routes = [{'destination': '135.207.111.111/32', + 'nexthop': '135.207.111.112'}, + {'destination': '135.207.111.111/32', + 'nexthop': '135.207.111.113'}] + ri.update_routing_table_ecmp.assert_called_once_with(expected_routes) + def test_routing_table_update(self): ri = router_info.RouterInfo(mock.Mock(), _uuid(), {}, **self.ri_kwargs) ri.router = {} diff --git a/releasenotes/notes/ecmp-routes-771ff34beafee370.yaml b/releasenotes/notes/ecmp-routes-771ff34beafee370.yaml new file mode 100644 index 00000000000..146624337fd --- /dev/null +++ b/releasenotes/notes/ecmp-routes-771ff34beafee370.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + Neutron supports ECMP routes now, with this change, neutron will + consolidate multiple routes with the same destination address into + a single ECMP route. For more information see bug + `1880532 `_.