From: vivek <vivek@cumulusnetworks.com>
Date: Thu, 25 Jan 2018 06:41:44 +0000 (-0800)
Subject: bgpd: Handle multiple simultaneous changes for a VNI correctly
X-Git-Tag: frr-5.0-dev~191^2~8
X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=d1911c2664cf84aa674d433396bbdecf2db9d3eb;p=mirror%2Ffrr.git

bgpd: Handle multiple simultaneous changes for a VNI correctly

Ensure that if multiple parameters for a VNI change simultaneously, the
changes are processed correctly. The changes of interest are the local
tunnel IP address and the tenant VRF to which this VNI is attached. The
former is used to originate type-3 routes as well as set the next hop of
all routes, the latter helps to determine the route targets and VNIs to
include in the route.

Signed-off-by: Vivek Venkatraman <vivek@cumulusnetworks.com>
Reviewed-by:   Mitesh Kanjariya <mitesh@cumulusnetworks.com>

Ticket: CM-19099
Reviewed By: CCR-7102
Testing Done:
1. Manually reproduced problem and verified fix.
2. Additional trigger events tested with fix.
---

diff --git a/bgpd/bgp_evpn.c b/bgpd/bgp_evpn.c
index a69c3465e3..a50e3707ad 100644
--- a/bgpd/bgp_evpn.c
+++ b/bgpd/bgp_evpn.c
@@ -1725,8 +1725,10 @@ static int delete_routes_for_vni(struct bgp *bgp, struct bgpevpn *vpn)
 }
 
 /*
- * There is a tunnel endpoint IP address change for this VNI,
- * need to re-advertise routes with the new nexthop.
+ * There is a tunnel endpoint IP address change for this VNI, delete
+ * prior type-3 route (if needed) and update.
+ * Note: Route re-advertisement happens elsewhere after other processing
+ * other changes.
  */
 static int handle_tunnel_ip_change(struct bgp *bgp, struct bgpevpn *vpn,
 				   struct in_addr originator_ip)
@@ -1754,7 +1756,7 @@ static int handle_tunnel_ip_change(struct bgp *bgp, struct bgpevpn *vpn,
 
 	/* Update the tunnel IP and re-advertise all routes for this VNI. */
 	vpn->originator_ip = originator_ip;
-	return update_routes_for_vni(bgp, vpn);
+	return 0;
 }
 
 /*
@@ -4397,8 +4399,8 @@ int bgp_evpn_local_vni_del(struct bgp *bgp, vni_t vni)
 }
 
 /*
- * Handle add (or update) of a local VNI. The only VNI change we care
- * about is change to local-tunnel-ip.
+ * Handle add (or update) of a local VNI. The VNI changes we care
+ * about are for the local-tunnel-ip and the (tenant) VRF.
  */
 int bgp_evpn_local_vni_add(struct bgp *bgp, vni_t vni,
 			   struct in_addr originator_ip,
@@ -4416,24 +4418,31 @@ int bgp_evpn_local_vni_add(struct bgp *bgp, vni_t vni,
 	vpn = bgp_evpn_lookup_vni(bgp, vni);
 	if (vpn) {
 
-		/* update tenant_vrf_id if required */
+		if (is_vni_live(vpn)
+		    && IPV4_ADDR_SAME(&vpn->originator_ip, &originator_ip)
+		    && vpn->tenant_vrf_id == tenant_vrf_id)
+			/* Probably some other param has changed that we don't
+			 * care about. */
+			return 0;
+
+		/* Update tenant_vrf_id if it has changed. */
 		if (vpn->tenant_vrf_id != tenant_vrf_id) {
 			bgpevpn_unlink_from_l3vni(vpn);
 			vpn->tenant_vrf_id = tenant_vrf_id;
 			bgpevpn_link_to_l3vni(vpn);
-
-			/* update all routes with new export RT for VRFs */
-			update_routes_for_vni(bgp, vpn);
 		}
 
-		if (is_vni_live(vpn)
-		    && IPV4_ADDR_SAME(&vpn->originator_ip, &originator_ip))
-			/* Probably some other param has changed that we don't
-			 * care about. */
-			return 0;
+		/* If tunnel endpoint IP has changed, update (and delete prior
+		 * type-3 route, if needed.)
+		 */
+		if (!IPV4_ADDR_SAME(&vpn->originator_ip, &originator_ip))
+			handle_tunnel_ip_change(bgp, vpn, originator_ip);
 
-		/* Local tunnel endpoint IP address has changed */
-		handle_tunnel_ip_change(bgp, vpn, originator_ip);
+		/* Update all routes with new endpoint IP and/or export RT
+		 * for VRFs
+		 */
+		if (is_vni_live(vpn))
+			update_routes_for_vni(bgp, vpn);
 	}
 
 	/* Create or update as appropriate. */