bgpd: make sure nh is valid for MPLS vpn routes

If we are using a nexthop for a MPLS VPN route make sure the
nexthop is over a labeled path. This new check mirrors the one
in validate_paths (where routes are enabled when a nexthop
becomes reachable). The check is introduced to the code path
where routes are added and the nexthop is looked up.

Signed-off-by: Pat Ruddy <pat@voltanet.io>

5 files changed, 21 insertions, 15 deletions

diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c
index cec4a9339..29573a195 100644
--- a/bgpd/bgp_fsm.c
+++ b/bgpd/bgp_fsm.c
@@ -110,9 +110,9 @@ static int bgp_peer_reg_with_nht(struct peer *peer)
 	    && !CHECK_FLAG(peer->bgp->flags, BGP_FLAG_DISABLE_NH_CONNECTED_CHK))
 		connected = 1;
 
-	return bgp_find_or_add_nexthop(
-		peer->bgp, peer->bgp, family2afi(peer->su.sa.sa_family),
-		NULL, peer, connected);
+	return bgp_find_or_add_nexthop(peer->bgp, peer->bgp,
+				       family2afi(peer->su.sa.sa_family),
+				       SAFI_UNICAST, NULL, peer, connected);
 }
 
 static void peer_xfer_stats(struct peer *peer_dst, struct peer *peer_src)
diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c
index 1d66d7528..d9acda8bd 100644
--- a/bgpd/bgp_mplsvpn.c
+++ b/bgpd/bgp_mplsvpn.c
@@ -590,8 +590,8 @@ leak_update(struct bgp *bgp, /* destination bgp instance */
 			 * TBD do we need to do anything about the
 			 * 'connected' parameter?
 			 */
-			nh_valid = bgp_find_or_add_nexthop(bgp, bgp_nexthop,
-							   afi, bpi, NULL, 0);
+			nh_valid = bgp_find_or_add_nexthop(
+				bgp, bgp_nexthop, afi, safi, bpi, NULL, 0);
 
 		if (debug)
 			zlog_debug("%s: nexthop is %svalid (in vrf %s)",
@@ -656,8 +656,8 @@ leak_update(struct bgp *bgp, /* destination bgp instance */
 		 * TBD do we need to do anything about the
 		 * 'connected' parameter?
 		 */
-		nh_valid = bgp_find_or_add_nexthop(bgp, bgp_nexthop,
-						afi, new, NULL, 0);
+		nh_valid = bgp_find_or_add_nexthop(bgp, bgp_nexthop, afi, safi,
+						   new, NULL, 0);
 
 	if (debug)
 		zlog_debug("%s: nexthop is %svalid (in vrf %s)",
diff --git a/bgpd/bgp_nht.c b/bgpd/bgp_nht.c
index 29ab3d9c6..85a65bc2c 100644
--- a/bgpd/bgp_nht.c
+++ b/bgpd/bgp_nht.c
@@ -121,7 +121,7 @@ void bgp_unlink_nexthop_by_peer(struct peer *peer)
  * we need both the bgp_route and bgp_nexthop pointers.
  */
 int bgp_find_or_add_nexthop(struct bgp *bgp_route, struct bgp *bgp_nexthop,
-			    afi_t afi, struct bgp_path_info *pi,
+			    afi_t afi, safi_t safi, struct bgp_path_info *pi,
 			    struct peer *peer, int connected)
 {
 	struct bgp_nexthop_cache_head *tree = NULL;
@@ -257,7 +257,11 @@ int bgp_find_or_add_nexthop(struct bgp *bgp_route, struct bgp *bgp_nexthop,
 	 */
 	if (bgp_route->inst_type == BGP_INSTANCE_TYPE_VIEW)
 		return 1;
-	else
+	else if (safi == SAFI_UNICAST && pi
+		 && pi->sub_type == BGP_ROUTE_IMPORTED && pi->extra
+		 && pi->extra->num_labels) {
+		return bgp_isvalid_labeled_nexthop(bnc);
+	} else
 		return (bgp_isvalid_nexthop(bnc));
 }
 
diff --git a/bgpd/bgp_nht.h b/bgpd/bgp_nht.h
index 8451f0689..f374e8dfa 100644
--- a/bgpd/bgp_nht.h
+++ b/bgpd/bgp_nht.h
@@ -34,14 +34,15 @@ extern void bgp_parse_nexthop_update(int command, vrf_id_t vrf_id);
  *   bgp_route - BGP instance of route
  *   bgp_nexthop - BGP instance of nexthop
  *   a - afi: AFI_IP or AF_IP6
+ *   safi - safi: to check which table nhs are being imported to
  *   p - path for which the nexthop object is being looked up
  *   peer - The BGP peer associated with this NHT
  *   connected - True if NH MUST be a connected route
  */
 extern int bgp_find_or_add_nexthop(struct bgp *bgp_route,
 				   struct bgp *bgp_nexthop, afi_t a,
-				   struct bgp_path_info *p, struct peer *peer,
-				   int connected);
+				   safi_t safi, struct bgp_path_info *p,
+				   struct peer *peer, int connected);
 
 /**
  * bgp_unlink_nexthop() - Unlink the nexthop object from the path structure.
diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index e6276d060..3d73fe4d0 100644
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -4062,7 +4062,7 @@ int bgp_update(struct peer *peer, const struct prefix *p, uint32_t addpath_id,
 			nh_afi = BGP_ATTR_NH_AFI(afi, pi->attr);
 
 			if (bgp_find_or_add_nexthop(bgp, bgp_nexthop, nh_afi,
-						    pi, NULL, connected)
+						    safi, pi, NULL, connected)
 			    || CHECK_FLAG(peer->flags, PEER_FLAG_IS_RFAPI_HD))
 				bgp_path_info_set_flag(dest, pi,
 						       BGP_PATH_VALID);
@@ -4207,7 +4207,7 @@ int bgp_update(struct peer *peer, const struct prefix *p, uint32_t addpath_id,
 
 		nh_afi = BGP_ATTR_NH_AFI(afi, new->attr);
 
-		if (bgp_find_or_add_nexthop(bgp, bgp, nh_afi, new, NULL,
+		if (bgp_find_or_add_nexthop(bgp, bgp, nh_afi, safi, new, NULL,
 					    connected)
 		    || CHECK_FLAG(peer->flags, PEER_FLAG_IS_RFAPI_HD))
 			bgp_path_info_set_flag(dest, new, BGP_PATH_VALID);
@@ -5445,7 +5445,8 @@ void bgp_static_update(struct bgp *bgp, const struct prefix *p,
 					bgp_nexthop = pi->extra->bgp_orig;
 
 				if (bgp_find_or_add_nexthop(bgp, bgp_nexthop,
-							    afi, pi, NULL, 0))
+							    afi, safi, pi, NULL,
+							    0))
 					bgp_path_info_set_flag(dest, pi,
 							       BGP_PATH_VALID);
 				else {
@@ -5497,7 +5498,7 @@ void bgp_static_update(struct bgp *bgp, const struct prefix *p,
 	/* Nexthop reachability check. */
 	if (CHECK_FLAG(bgp->flags, BGP_FLAG_IMPORT_CHECK)
 	    && (safi == SAFI_UNICAST || safi == SAFI_LABELED_UNICAST)) {
-		if (bgp_find_or_add_nexthop(bgp, bgp, afi, new, NULL, 0))
+		if (bgp_find_or_add_nexthop(bgp, bgp, afi, safi, new, NULL, 0))
 			bgp_path_info_set_flag(dest, new, BGP_PATH_VALID);
 		else {
 			if (BGP_DEBUG(nht, NHT)) {