]> git.puffer.fish Git - mirror/frr.git/commitdiff
bgpd: Handle possible non-selection of local route 5432/head
authorChirag Shah <chirag@cumulusnetworks.com>
Mon, 25 Nov 2019 22:34:29 +0000 (14:34 -0800)
committerChirag Shah <chirag@cumulusnetworks.com>
Tue, 26 Nov 2019 05:41:14 +0000 (21:41 -0800)
In rare situations, the local route in a VNI may not get selected as the
best route. One situation is during a race between bgp and zebra which
was addressed in a prior commit. This change addresses another situation
where due to a change of tunnel IP, it is possible that a received route
may be selected as the best route if the path selection needs to take
next hop IPs into consideration. This is a pretty convoluted scenario,
but the code should handle it and delete and withdraw the local route
as well as (re)install the received route.

Ticket: CM-24114
Reviewed By: CCR-9487
Testing Done:
1. Manual tests - note, problem is not readily reproducible
2. evpn-smoke - results documented in the ticket

Signed-off-by: Vivek Venkatraman <vivek@cumulusnetworks.com>
Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>
bgpd/bgp_evpn.c

index f3c514fb15f659f0cfce470eccc5cff41058d051..1b356af6d8b0b7a1b37e4af94becfc337fbda917 100644 (file)
@@ -1825,9 +1825,15 @@ static int update_evpn_route(struct bgp *bgp, struct bgpevpn *vpn,
 
        /* lock ri to prevent freeing in evpn_route_select_install */
        bgp_path_info_lock(pi);
-       /* Perform route selection; this is just to set the flags correctly
-        * as local route in the VNI always wins.
-        */
+
+       /* Perform route selection. Normally, the local route in the
+        * VNI is expected to win and be the best route. However, if
+        * there is a race condition where a host moved from local to
+        * remote and the remote route was received in BGP just prior
+        * to the local MACIP notification from zebra, the remote
+        * route would win, and we should evict the defunct local route
+        * and (re)install the remote route into zebra.
+       */
        evpn_route_select_install(bgp, vpn, rn);
        /*
         * If the new local route was not selected evict it and tell zebra
@@ -2100,24 +2106,40 @@ static int update_all_type2_routes(struct bgp *bgp, struct bgpevpn *vpn)
                update_evpn_route_entry(bgp, vpn, afi, safi, rn, &attr, 0, &pi,
                                        0, seq);
 
-               /* Perform route selection; this is just to set the flags
-                * correctly as local route in the VNI always wins.
+               /* lock ri to prevent freeing in evpn_route_select_install */
+               bgp_path_info_lock(pi);
+
+               /* Perform route selection. Normally, the local route in the
+                * VNI is expected to win and be the best route. However,
+                * under peculiar situations (e.g., tunnel (next hop) IP change
+                * that causes best selection to be based on next hop), a
+                * remote route could win. If the local route is the best,
+                * ensure it is updated in the global EVPN route table and
+                * advertised to peers; otherwise, ensure it is evicted and
+                * (re)install the remote route into zebra.
                 */
                evpn_route_select_install(bgp, vpn, rn);
-
-               attr_new = pi->attr;
-
-               /* Update route in global routing table. */
-               rd_rn = bgp_afi_node_get(bgp->rib[afi][safi], afi, safi,
-                                        (struct prefix *)evp, &vpn->prd);
-               assert(rd_rn);
-               update_evpn_route_entry(bgp, vpn, afi, safi, rd_rn, attr_new, 0,
-                                       &global_pi, 0,
-                                       mac_mobility_seqnum(attr_new));
-
-               /* Schedule for processing and unlock node. */
-               bgp_process(bgp, rd_rn, afi, safi);
-               bgp_unlock_node(rd_rn);
+               if (!CHECK_FLAG(pi->flags, BGP_PATH_SELECTED)) {
+                       evpn_cleanup_local_non_best_route(bgp, vpn, rn, pi);
+                       /* unlock pi */
+                       bgp_path_info_unlock(pi);
+               } else {
+                       attr_new = pi->attr;
+                       /* unlock pi */
+                       bgp_path_info_unlock(pi);
+
+                       /* Update route in global routing table. */
+                       rd_rn = bgp_afi_node_get(bgp->rib[afi][safi], afi, safi,
+                                                (struct prefix *)evp, &vpn->prd);
+                       assert(rd_rn);
+                       update_evpn_route_entry(bgp, vpn, afi, safi, rd_rn,
+                                               attr_new, 0, &global_pi, 0,
+                                               mac_mobility_seqnum(attr_new));
+
+                       /* Schedule for processing and unlock node. */
+                       bgp_process(bgp, rd_rn, afi, safi);
+                       bgp_unlock_node(rd_rn);
+               }
 
                /* Unintern temporary. */
                aspath_unintern(&attr.aspath);