Merge pull request #3882 from vivek-cumulus/refine_evpn_route_add

Refine install of EVPN-based routes to remove some special handling

bgpd/bgp_evpn.c

@@ -5396,7 +5396,8 @@ static void link_l2vni_hash_to_l3vni(struct hash_bucket *bucket,
 }
 
 int bgp_evpn_local_l3vni_add(vni_t l3vni, vrf_id_t vrf_id, struct ethaddr *rmac,
-			     struct in_addr originator_ip, int filter)
+			     struct in_addr originator_ip, int filter,
+			     ifindex_t svi_ifindex)
 {
 	struct bgp *bgp_vrf = NULL; /* bgp VRF instance */
 	struct bgp *bgp_def = NULL; /* default bgp instance */
@@ -5444,14 +5445,11 @@ int bgp_evpn_local_l3vni_add(vni_t l3vni, vrf_id_t vrf_id, struct ethaddr *rmac,
 		SET_FLAG(bgp_vrf->vrf_flags, BGP_VRF_AUTO);
 	}
 
-	/* associate with l3vni */
+	/* associate the vrf with l3vni and related parameters */
 	bgp_vrf->l3vni = l3vni;
-
-	/* set the router mac - to be used in mac-ip routes for this vrf */
 	memcpy(&bgp_vrf->rmac, rmac, sizeof(struct ethaddr));
-
-	/* set the originator ip */
 	bgp_vrf->originator_ip = originator_ip;
+	bgp_vrf->l3vni_svi_ifindex = svi_ifindex;
 
 	/* set the right filter - are we using l3vni only for prefix routes? */
 	if (filter)

bgpd/bgp_evpn.h

@@ -136,7 +136,8 @@ extern int bgp_evpn_local_macip_add(struct bgp *bgp, vni_t vni,
 				    uint8_t flags, uint32_t seq);
 extern int bgp_evpn_local_l3vni_add(vni_t vni, vrf_id_t vrf_id,
 				    struct ethaddr *rmac,
-				    struct in_addr originator_ip, int filter);
+				    struct in_addr originator_ip, int filter,
+				    ifindex_t svi_ifindex);
 extern int bgp_evpn_local_l3vni_del(vni_t vni, vrf_id_t vrf_id);
 extern int bgp_evpn_local_vni_del(struct bgp *bgp, vni_t vni);
 extern int bgp_evpn_local_vni_add(struct bgp *bgp, vni_t vni,

bgpd/bgp_zebra.c

@@ -1114,20 +1114,24 @@ int bgp_zebra_get_table_range(uint32_t chunk_size,
 }
 
 static int update_ipv4nh_for_route_install(int nh_othervrf,
+					   struct bgp *nh_bgp,
 					   struct in_addr *nexthop,
 					   struct attr *attr,
 					   bool is_evpn,
 					   struct zapi_nexthop *api_nh)
 {
 	api_nh->gate.ipv4 = *nexthop;
+	api_nh->vrf_id = nh_bgp->vrf_id;
 
 	/* Need to set fields appropriately for EVPN routes imported into
 	 * a VRF (which are programmed as onlink on l3-vni SVI) as well as
 	 * connected routes leaked into a VRF.
 	 */
-	if (is_evpn)
+	if (is_evpn) {
 		api_nh->type = NEXTHOP_TYPE_IPV4_IFINDEX;
-	else if (nh_othervrf &&
+		api_nh->onlink = true;
+		api_nh->ifindex = nh_bgp->l3vni_svi_ifindex;
+	} else if (nh_othervrf &&
 		 api_nh->gate.ipv4.s_addr == INADDR_ANY) {
 		api_nh->type = NEXTHOP_TYPE_IFINDEX;
 		api_nh->ifindex = attr->nh_ifindex;
@@ -1138,18 +1142,22 @@ static int update_ipv4nh_for_route_install(int nh_othervrf,
 }
 
 static int
-update_ipv6nh_for_route_install(int nh_othervrf, struct in6_addr *nexthop,
+update_ipv6nh_for_route_install(int nh_othervrf, struct bgp *nh_bgp,
+				struct in6_addr *nexthop,
 				ifindex_t ifindex, struct bgp_path_info *pi,
 				struct bgp_path_info *best_pi, bool is_evpn,
 				struct zapi_nexthop *api_nh)
 {
 	struct attr *attr;
 
 	attr = pi->attr;
+	api_nh->vrf_id = nh_bgp->vrf_id;
 
-	if (is_evpn)
+	if (is_evpn) {
 		api_nh->type = NEXTHOP_TYPE_IPV6_IFINDEX;
-	else if (nh_othervrf) {
+		api_nh->onlink = true;
+		api_nh->ifindex = nh_bgp->l3vni_svi_ifindex;
+	} else if (nh_othervrf) {
 		if (IN6_IS_ADDR_UNSPECIFIED(nexthop)) {
 			api_nh->type = NEXTHOP_TYPE_IFINDEX;
 			api_nh->ifindex = attr->nh_ifindex;
@@ -1300,8 +1308,6 @@ void bgp_zebra_announce(struct bgp_node *rn, struct prefix *p,
 			continue;
 
 		api_nh = &api.nexthops[valid_nh_count];
-		api_nh->vrf_id = nh_othervrf ? info->extra->bgp_orig->vrf_id
-					     : bgp->vrf_id;
 		if (nh_family == AF_INET) {
 			if (bgp_debug_zebra(&api.prefix)) {
 				if (mpinfo->extra) {
@@ -1341,6 +1347,8 @@ void bgp_zebra_announce(struct bgp_node *rn, struct prefix *p,
 
 			nh_updated = update_ipv4nh_for_route_install(
 					nh_othervrf,
+					nh_othervrf ?
+					info->extra->bgp_orig : bgp,
 					&mpinfo_cp->attr->nexthop,
 					mpinfo_cp->attr, is_evpn, api_nh);
 		} else {
@@ -1375,7 +1383,9 @@ void bgp_zebra_announce(struct bgp_node *rn, struct prefix *p,
 			nexthop = bgp_path_info_to_ipv6_nexthop(mpinfo_cp,
 								&ifindex);
 			nh_updated = update_ipv6nh_for_route_install(
-					nh_othervrf, nexthop, ifindex,
+					nh_othervrf, nh_othervrf ?
+					info->extra->bgp_orig : bgp,
+					nexthop, ifindex,
 					mpinfo, info, is_evpn, api_nh);
 		}
 
@@ -2489,6 +2499,7 @@ static int bgp_zebra_process_local_l3vni(int cmd, struct zclient *zclient,
 	struct ethaddr rmac;
 	struct in_addr originator_ip;
 	struct stream *s;
+	ifindex_t svi_ifindex;
 
 	memset(&rmac, 0, sizeof(struct ethaddr));
 	memset(&originator_ip, 0, sizeof(struct in_addr));
@@ -2498,20 +2509,24 @@ static int bgp_zebra_process_local_l3vni(int cmd, struct zclient *zclient,
 		stream_get(&rmac, s, sizeof(struct ethaddr));
 		originator_ip.s_addr = stream_get_ipv4(s);
 		stream_get(&filter, s, sizeof(int));
-	}
+		svi_ifindex = stream_getl(s);
 
-	if (BGP_DEBUG(zebra, ZEBRA))
-		zlog_debug("Rx L3-VNI %s VRF %s VNI %u RMAC %s filter %s",
-			   (cmd == ZEBRA_L3VNI_ADD) ? "add" : "del",
-			   vrf_id_to_name(vrf_id), l3vni,
-			   prefix_mac2str(&rmac, buf, sizeof(buf)),
-			   filter ? "prefix-routes-only" : "none");
+		if (BGP_DEBUG(zebra, ZEBRA))
+			zlog_debug("Rx L3-VNI ADD VRF %s VNI %u RMAC %s filter %s svi-if %u",
+				   vrf_id_to_name(vrf_id), l3vni,
+				   prefix_mac2str(&rmac, buf, sizeof(buf)),
+				   filter ? "prefix-routes-only" : "none",
+				   svi_ifindex);
 
-	if (cmd == ZEBRA_L3VNI_ADD)
 		bgp_evpn_local_l3vni_add(l3vni, vrf_id, &rmac, originator_ip,
-					 filter);
-	else
+					 filter, svi_ifindex);
+	} else {
+		if (BGP_DEBUG(zebra, ZEBRA))
+			zlog_debug("Rx L3-VNI DEL VRF %s VNI %u",
+				   vrf_id_to_name(vrf_id), l3vni);
+
 		bgp_evpn_local_l3vni_del(l3vni, vrf_id);
+	}
 
 	return 0;
 }

bgpd/bgpd.h

@@ -517,6 +517,9 @@ struct bgp {
 	/* originator ip - to be used as NH for type-5 routes */
 	struct in_addr originator_ip;
 
+	/* SVI associated with the L3-VNI corresponding to this vrf */
+	ifindex_t l3vni_svi_ifindex;
+
 	/* vrf flags */
 	uint32_t vrf_flags;
 #define BGP_VRF_AUTO                        (1 << 0)

lib/nexthop.h

@@ -83,7 +83,6 @@ struct nexthop {
 #define NEXTHOP_FLAG_MATCHED    (1 << 4) /* Already matched vs a nexthop */
 #define NEXTHOP_FLAG_FILTERED   (1 << 5) /* rmap filtered, used by static only */
 #define NEXTHOP_FLAG_DUPLICATE  (1 << 6) /* nexthop duplicates another active one */
-#define NEXTHOP_FLAG_EVPN_RVTEP (1 << 7) /* EVPN remote vtep nexthop */
 #define NEXTHOP_IS_ACTIVE(flags)                                               \
 	(CHECK_FLAG(flags, NEXTHOP_FLAG_ACTIVE)                                \
 	 && !CHECK_FLAG(flags, NEXTHOP_FLAG_DUPLICATE))

zebra/zapi_msg.c

@@ -1432,12 +1432,7 @@ static void zread_route_add(ZAPI_HANDLER_ARGS)
 		case NEXTHOP_TYPE_IPV4_IFINDEX:
 
 			memset(&vtep_ip, 0, sizeof(struct ipaddr));
-			if (CHECK_FLAG(api.flags, ZEBRA_FLAG_EVPN_ROUTE)) {
-				ifindex = get_l3vni_svi_ifindex(vrf_id);
-			} else {
-				ifindex = api_nh->ifindex;
-			}
-
+			ifindex = api_nh->ifindex;
 			if (IS_ZEBRA_DEBUG_RECV) {
 				char nhbuf[INET6_ADDRSTRLEN] = {0};
 
@@ -1452,12 +1447,10 @@ static void zread_route_add(ZAPI_HANDLER_ARGS)
 				re, &api_nh->gate.ipv4, NULL, ifindex,
 				api_nh->vrf_id);
 
-			/* if this an EVPN route entry,
-			 * program the nh as neigh
+			/* Special handling for IPv4 routes sourced from EVPN:
+			 * the nexthop and associated MAC need to be installed.
 			 */
 			if (CHECK_FLAG(api.flags, ZEBRA_FLAG_EVPN_ROUTE)) {
-				SET_FLAG(nexthop->flags,
-					 NEXTHOP_FLAG_EVPN_RVTEP);
 				vtep_ip.ipa_type = IPADDR_V4;
 				memcpy(&(vtep_ip.ipaddr_v4),
 				       &(api_nh->gate.ipv4),
@@ -1473,22 +1466,15 @@ static void zread_route_add(ZAPI_HANDLER_ARGS)
 			break;
 		case NEXTHOP_TYPE_IPV6_IFINDEX:
 			memset(&vtep_ip, 0, sizeof(struct ipaddr));
-			if (CHECK_FLAG(api.flags, ZEBRA_FLAG_EVPN_ROUTE)) {
-				ifindex = get_l3vni_svi_ifindex(vrf_id);
-			} else {
-				ifindex = api_nh->ifindex;
-			}
-
+			ifindex = api_nh->ifindex;
 			nexthop = route_entry_nexthop_ipv6_ifindex_add(
 				re, &api_nh->gate.ipv6, ifindex,
 				api_nh->vrf_id);
 
-			/* if this an EVPN route entry,
-			 * program the nh as neigh
+			/* Special handling for IPv6 routes sourced from EVPN:
+			 * the nexthop and associated MAC need to be installed.
 			 */
 			if (CHECK_FLAG(api.flags, ZEBRA_FLAG_EVPN_ROUTE)) {
-				SET_FLAG(nexthop->flags,
-					 NEXTHOP_FLAG_EVPN_RVTEP);
 				vtep_ip.ipa_type = IPADDR_V6;
 				memcpy(&vtep_ip.ipaddr_v6, &(api_nh->gate.ipv6),
 				       sizeof(struct in6_addr));

zebra/zebra_rib.c

@@ -276,10 +276,8 @@ struct nexthop *route_entry_nexthop_ipv4_ifindex_add(struct route_entry *re,
 	/*Pending: need to think if null ifp here is ok during bootup?
 	  There was a crash because ifp here was coming to be NULL */
 	if (ifp)
-		if (connected_is_unnumbered(ifp)
-		    || CHECK_FLAG(re->flags, ZEBRA_FLAG_EVPN_ROUTE)) {
+		if (connected_is_unnumbered(ifp))
 			SET_FLAG(nexthop->flags, NEXTHOP_FLAG_ONLINK);
-		}
 
 	route_entry_nexthop_add(re, nexthop);
 
@@ -314,8 +312,6 @@ struct nexthop *route_entry_nexthop_ipv6_ifindex_add(struct route_entry *re,
 	nexthop->type = NEXTHOP_TYPE_IPV6_IFINDEX;
 	nexthop->gate.ipv6 = *ipv6;
 	nexthop->ifindex = ifindex;
-	if (CHECK_FLAG(re->flags, ZEBRA_FLAG_EVPN_ROUTE))
-		SET_FLAG(nexthop->flags, NEXTHOP_FLAG_ONLINK);
 
 	route_entry_nexthop_add(re, nexthop);
 
@@ -433,10 +429,6 @@ static int nexthop_active(afi_t afi, struct route_entry *re,
 		re->nexthop_mtu = 0;
 	}
 
-	/* Next hops (remote VTEPs) for EVPN routes are fully resolved. */
-	if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_EVPN_RVTEP))
-		return 1;
-
 	/*
 	 * If the kernel has sent us a route, then
 	 * by golly gee whiz it's a good route.
@@ -459,6 +451,9 @@ static int nexthop_active(afi_t afi, struct route_entry *re,
 	 * Check to see if we should trust the passed in information
 	 * for UNNUMBERED interfaces as that we won't find the GW
 	 * address in the routing table.
+	 * This check should suffice to handle IPv4 or IPv6 routes
+	 * sourced from EVPN routes which are installed with the
+	 * next hop as the remote VTEP IP.
 	 */
 	if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_ONLINK)) {
 		ifp = if_lookup_by_index(nexthop->ifindex, nexthop->vrf_id);
@@ -2937,6 +2932,10 @@ void rib_delete(afi_t afi, safi_t safi, vrf_id_t vrf_id, int type,
 			return;
 		}
 
+		/* Special handling for IPv4 or IPv6 routes sourced from
+		 * EVPN - the nexthop (and associated MAC) need to be
+		 * uninstalled if no more refs.
+		 */
 		if (CHECK_FLAG(flags, ZEBRA_FLAG_EVPN_ROUTE)) {
 			struct nexthop *tmp_nh;
 

zebra/zebra_vxlan.c

@@ -4875,6 +4875,7 @@ static int zl3vni_send_add_to_client(zebra_l3vni_t *zl3vni)
 	stream_put(s, &rmac, sizeof(struct ethaddr));
 	stream_put_in_addr(s, &zl3vni->local_vtep_ip);
 	stream_put(s, &zl3vni->filter, sizeof(int));
+	stream_putl(s, zl3vni->svi_if->ifindex);
 
 	/* Write packet size. */
 	stream_putw_at(s, 0, stream_get_endp(s));