Merge "L3 router support ECMP"

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)
+            # Judge if modifying an ECMP route or not, if not,
-            self.update_routing_table('delete', route)
+            # 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."""

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

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)

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 = {}

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 <https://bugs.launchpad.net/neutron/+bug/1880532>`_.