From 952d37e614b1bf3ac7ff6d9a8e523ffb1806e8fc Mon Sep 17 00:00:00 2001 From: Roey Chen Date: Thu, 20 Apr 2017 04:40:10 -0700 Subject: [PATCH] NSXv: Use BGP protocol to learn default gateways We'd want to use ECMP when BGP is configured, the current code implements that by adding default static routes to BGP peers. This patch remove this behaviour, instead, admin is responsible to configure the GW edge with the default-originate feature enabled and adding the appropriate redistribution rule. Change-Id: Ib3e38f07583afa583beeac86ebdb1cd2ceb1ab6f --- vmware_nsx/common/config.py | 8 ++++ .../vshield/edge_dynamic_routing_driver.py | 36 ++--------------- .../services/dynamic_routing/nsx_v/driver.py | 39 ++++++++----------- 3 files changed, 28 insertions(+), 55 deletions(-) diff --git a/vmware_nsx/common/config.py b/vmware_nsx/common/config.py index ae4ad0fb3f..207e848616 100644 --- a/vmware_nsx/common/config.py +++ b/vmware_nsx/common/config.py @@ -673,6 +673,14 @@ nsxv_opts = [ cfg.BoolOpt('exclusive_dhcp_edge', default=False, help=_("(Optional) Have exclusive DHCP edge per network.")), + cfg.IntOpt('bgp_neighbour_hold_down_timer', + default=4, + help=_("(Optional) Set the interval (Seconds) for BGP " + "neighbour hold down time.")), + cfg.IntOpt('bgp_neighbour_keep_alive_timer', + default=1, + help=_("(Optional) Set the interval (Seconds) for BGP " + "neighbour keep alive time.")), ] # define the configuration of each NSX-V availability zone. diff --git a/vmware_nsx/plugins/nsx_v/vshield/edge_dynamic_routing_driver.py b/vmware_nsx/plugins/nsx_v/vshield/edge_dynamic_routing_driver.py index 2034ff1e91..1a19e4043b 100644 --- a/vmware_nsx/plugins/nsx_v/vshield/edge_dynamic_routing_driver.py +++ b/vmware_nsx/plugins/nsx_v/vshield/edge_dynamic_routing_driver.py @@ -57,6 +57,7 @@ class EdgeDynamicRoutingDriver(object): for prx in global_config['ipPrefixes']['ipPrefixes']] global_config['ipPrefixes'] = curr_prefixes + # Don't change any static routes. static_routing = config.get('staticRouting', {}) static_routes = static_routing.get('staticRoutes', {}) current_routes = [{'route': route} @@ -128,29 +129,8 @@ class EdgeDynamicRoutingDriver(object): self.vcns.update_bgp_dynamic_routing(edge_id, bgp_config) - def _get_edge_static_routes(self, edge_id): - h, routes = self.vcns.get_routes(edge_id) - static_routes = routes if routes else {} - static_routes.setdefault('staticRoutes', {'staticRoutes': []}) - return static_routes - - def _remove_default_static_routes(self, edge_id, droutes=None): - routes = self._get_edge_static_routes(edge_id) - # if droutes is empty or None, then remove all default static routes - droutes = droutes or routes['staticRoutes']['staticRoutes'] - droutes = [r['nextHop'] for r in droutes] - routes['staticRoutes']['staticRoutes'] = [ - r for r in routes['staticRoutes']['staticRoutes'] - if r['network'] != '0.0.0.0/0' or r['nextHop'] not in droutes] - self.vcns.update_routes(edge_id, routes) - - def _add_default_static_routes(self, edge_id, default_routes): - routes = self._get_edge_static_routes(edge_id) - routes['staticRoutes']['staticRoutes'].extend(default_routes) - self.vcns.update_routes(edge_id, routes) - def add_bgp_speaker_config(self, edge_id, prot_router_id, local_as, - enabled, default_routes, bgp_neighbours, + enabled, bgp_neighbours, prefixes, redistribution_rules): with locking.LockManager.get_lock(str(edge_id)): self._update_routing_config(edge_id, @@ -161,30 +141,22 @@ class EdgeDynamicRoutingDriver(object): neighbours_to_add=bgp_neighbours, prefixes_to_add=prefixes, rules_to_add=redistribution_rules) - self._add_default_static_routes(edge_id, default_routes) def delete_bgp_speaker_config(self, edge_id): with locking.LockManager.get_lock(str(edge_id)): self.vcns.delete_bgp_routing_config(edge_id) - self._remove_default_static_routes(edge_id) self._reset_routing_global_config(edge_id) - def add_bgp_neighbours(self, edge_id, bgp_neighbours, - default_routes=None): + def add_bgp_neighbours(self, edge_id, bgp_neighbours): # Query the bgp config first and update the bgpNeighbour with locking.LockManager.get_lock(str(edge_id)): self._update_bgp_routing_config(edge_id, neighbours_to_add=bgp_neighbours) - if default_routes: - self._add_default_static_routes(edge_id, default_routes) - def remove_bgp_neighbours(self, edge_id, bgp_neighbours, - default_routes=None): + def remove_bgp_neighbours(self, edge_id, bgp_neighbours): with locking.LockManager.get_lock(str(edge_id)): self._update_bgp_routing_config( edge_id, neighbours_to_remove=bgp_neighbours) - if default_routes: - self._remove_default_static_routes(edge_id, default_routes) def update_bgp_neighbours(self, edge_id, neighbours_to_add=None, neighbours_to_remove=None): diff --git a/vmware_nsx/services/dynamic_routing/nsx_v/driver.py b/vmware_nsx/services/dynamic_routing/nsx_v/driver.py index af0232d196..f607d5d611 100644 --- a/vmware_nsx/services/dynamic_routing/nsx_v/driver.py +++ b/vmware_nsx/services/dynamic_routing/nsx_v/driver.py @@ -56,7 +56,9 @@ def bgp_neighbour(bgp_peer): 'ipAddress': bgp_peer['peer_ip'], 'remoteAS': bgp_peer['remote_as'], 'bgpFilters': bgp_filter, - 'password': bgp_peer['password'] + 'password': bgp_peer['password'], + 'holdDownTimer': cfg.CONF.nsxv.bgp_neighbour_hold_down_timer, + 'keepAliveTimer': cfg.CONF.nsxv.bgp_neighbour_keep_alive_timer } return {'bgpNeighbour': nbr} @@ -67,16 +69,13 @@ def gw_bgp_neighbour(ip_address, remote_as, password): 'ipAddress': ip_address, 'remoteAS': remote_as, 'bgpFilters': bgp_filter, - 'password': password + 'password': password, + 'holdDownTimer': cfg.CONF.nsxv.bgp_neighbour_hold_down_timer, + 'keepAliveTimer': cfg.CONF.nsxv.bgp_neighbour_keep_alive_timer } return {'bgpNeighbour': nbr} -def default_route(nexthop): - return {'network': '0.0.0.0/0', - 'nextHop': nexthop} - - class NSXvBgpDriver(object): """Class driver to address the neutron_dynamic_routing API""" @@ -274,19 +273,17 @@ class NSXvBgpDriver(object): # list of tenant edge routers to be removed as bgp-neighbours to this # peer if it's associated with specific ESG. neighbours = [] - droute = default_route(nbr['bgpNeighbour']['ipAddress']) for binding in bgp_bindings: try: - self._nsxv.add_bgp_neighbours(binding['edge_id'], [nbr], - default_routes=[droute]) + self._nsxv.add_bgp_neighbours(binding['edge_id'], [nbr]) except vcns_exc.VcnsApiException: LOG.error("Failed to add BGP neighbour on '%s'", binding['edge_id']) else: - nbr = gw_bgp_neighbour(binding['bgp_identifier'], - speaker['local_as'], - self._edge_password) - neighbours.append(nbr) + gw_nbr = gw_bgp_neighbour(binding['bgp_identifier'], + speaker['local_as'], + self._edge_password) + neighbours.append(gw_nbr) LOG.debug("Succesfully added BGP neighbor '%s' on '%s'", bgp_peer_obj['peer_ip'], binding['edge_id']) @@ -309,19 +306,17 @@ class NSXvBgpDriver(object): # list of tenant edge routers to be removed as bgp-neighbours to this # peer if it's associated with specific ESG. neighbours = [] - droute = default_route(nbr['bgpNeighbour']['ipAddress']) for binding in bgp_bindings: try: - self._nsxv.remove_bgp_neighbours(binding['edge_id'], [nbr], - default_routes=[droute]) + self._nsxv.remove_bgp_neighbours(binding['edge_id'], [nbr]) except vcns_exc.VcnsApiException: LOG.error("Failed to remove BGP neighbour on '%s'", binding['edge_id']) else: - nbr = gw_bgp_neighbour(binding['bgp_identifier'], - speaker['local_as'], - self._edge_password) - neighbours.append(nbr) + gw_nbr = gw_bgp_neighbour(binding['bgp_identifier'], + speaker['local_as'], + self._edge_password) + neighbours.append(gw_nbr) LOG.debug("Succesfully removed BGP neighbor '%s' on '%s'", bgp_peer_obj['peer_ip'], binding['edge_id']) @@ -390,11 +385,9 @@ class NSXvBgpDriver(object): subnets, advertise_static_routes) bgp_neighbours = [bgp_neighbour(bgp_peer) for bgp_peer in bgp_peers] - default_routes = [default_route(peer['peer_ip']) for peer in bgp_peers] try: self._nsxv.add_bgp_speaker_config(edge_id, bgp_identifier, local_as, enabled_state, - default_routes, bgp_neighbours, prefixes, redis_rules) except vcns_exc.VcnsApiException: