Merge pull request #8731 from mjstapp/fix_pw_backups

zebra: Fix pseudowires with backup nexthops

8 files changed, 234 insertions, 40 deletions

diff --git a/zebra/rib.h b/zebra/rib.h
index 75d7ae1b67..957f38602a 100644
--- a/zebra/rib.h
+++ b/zebra/rib.h
@@ -401,7 +401,7 @@ extern void rib_delete(afi_t afi, safi_t safi, vrf_id_t vrf_id, int type,
 		       bool fromkernel);
 
 extern struct route_entry *rib_match(afi_t afi, safi_t safi, vrf_id_t vrf_id,
-				     union g_addr *addr,
+				     const union g_addr *addr,
 				     struct route_node **rn_out);
 extern struct route_entry *rib_match_ipv4_multicast(vrf_id_t vrf_id,
 						    struct in_addr addr,
diff --git a/zebra/zebra_dplane.c b/zebra/zebra_dplane.c
index a8df0c56ce..a5d672987d 100644
--- a/zebra/zebra_dplane.c
+++ b/zebra/zebra_dplane.c
@@ -157,12 +157,17 @@ struct dplane_pw_info {
 	int af;
 	int status;
 	uint32_t flags;
+	uint32_t nhg_id;
 	union g_addr dest;
 	mpls_label_t local_label;
 	mpls_label_t remote_label;
 
-	/* Nexthops */
-	struct nexthop_group nhg;
+	/* Nexthops that are valid and installed */
+	struct nexthop_group fib_nhg;
+
+	/* Primary and backup nexthop sets, copied from the resolving route. */
+	struct nexthop_group primary_nhg;
+	struct nexthop_group backup_nhg;
 
 	union pw_protocol_fields fields;
 };
@@ -664,11 +669,21 @@ static void dplane_ctx_free_internal(struct zebra_dplane_ctx *ctx)
 	case DPLANE_OP_PW_INSTALL:
 	case DPLANE_OP_PW_UNINSTALL:
 		/* Free allocated nexthops */
-		if (ctx->u.pw.nhg.nexthop) {
+		if (ctx->u.pw.fib_nhg.nexthop) {
 			/* This deals with recursive nexthops too */
-			nexthops_free(ctx->u.pw.nhg.nexthop);
+			nexthops_free(ctx->u.pw.fib_nhg.nexthop);
+
+			ctx->u.pw.fib_nhg.nexthop = NULL;
+		}
+		if (ctx->u.pw.primary_nhg.nexthop) {
+			nexthops_free(ctx->u.pw.primary_nhg.nexthop);
+
+			ctx->u.pw.primary_nhg.nexthop = NULL;
+		}
+		if (ctx->u.pw.backup_nhg.nexthop) {
+			nexthops_free(ctx->u.pw.backup_nhg.nexthop);
 
-			ctx->u.pw.nhg.nexthop = NULL;
+			ctx->u.pw.backup_nhg.nexthop = NULL;
 		}
 		break;
 
@@ -1630,7 +1645,23 @@ dplane_ctx_get_pw_nhg(const struct zebra_dplane_ctx *ctx)
 {
 	DPLANE_CTX_VALID(ctx);
 
-	return &(ctx->u.pw.nhg);
+	return &(ctx->u.pw.fib_nhg);
+}
+
+const struct nexthop_group *
+dplane_ctx_get_pw_primary_nhg(const struct zebra_dplane_ctx *ctx)
+{
+	DPLANE_CTX_VALID(ctx);
+
+	return &(ctx->u.pw.primary_nhg);
+}
+
+const struct nexthop_group *
+dplane_ctx_get_pw_backup_nhg(const struct zebra_dplane_ctx *ctx)
+{
+	DPLANE_CTX_VALID(ctx);
+
+	return &(ctx->u.pw.backup_nhg);
 }
 
 /* Accessors for interface information */
@@ -2461,12 +2492,14 @@ static int dplane_ctx_pw_init(struct zebra_dplane_ctx *ctx,
 			      enum dplane_op_e op,
 			      struct zebra_pw *pw)
 {
+	int ret = EINVAL;
 	struct prefix p;
 	afi_t afi;
 	struct route_table *table;
 	struct route_node *rn;
 	struct route_entry *re;
 	const struct nexthop_group *nhg;
+	struct nexthop *nh, *newnh, *last_nh;
 
 	if (IS_ZEBRA_DEBUG_DPLANE_DETAIL)
 		zlog_debug("init dplane ctx %s: pw '%s', loc %u, rem %u",
@@ -2509,31 +2542,83 @@ static int dplane_ctx_pw_init(struct zebra_dplane_ctx *ctx,
 
 	afi = (pw->af == AF_INET) ? AFI_IP : AFI_IP6;
 	table = zebra_vrf_table(afi, SAFI_UNICAST, pw->vrf_id);
-	if (table) {
-		rn = route_node_match(table, &p);
-		if (rn) {
-			RNODE_FOREACH_RE(rn, re) {
-				if (CHECK_FLAG(re->flags, ZEBRA_FLAG_SELECTED))
-					break;
+	if (table == NULL)
+		goto done;
+
+	rn = route_node_match(table, &p);
+	if (rn == NULL)
+		goto done;
+
+	re = NULL;
+	RNODE_FOREACH_RE(rn, re) {
+		if (CHECK_FLAG(re->flags, ZEBRA_FLAG_SELECTED))
+			break;
+	}
+
+	if (re) {
+		/* We'll capture a 'fib' list of nexthops that meet our
+		 * criteria: installed, and labelled.
+		 */
+		nhg = rib_get_fib_nhg(re);
+		last_nh = NULL;
+
+		if (nhg && nhg->nexthop) {
+			for (ALL_NEXTHOPS_PTR(nhg, nh)) {
+				if (!CHECK_FLAG(nh->flags, NEXTHOP_FLAG_ACTIVE)
+				    || CHECK_FLAG(nh->flags,
+						  NEXTHOP_FLAG_RECURSIVE)
+				    || nh->nh_label == NULL)
+					continue;
+
+				newnh = nexthop_dup(nh, NULL);
+
+				if (last_nh)
+					NEXTHOP_APPEND(last_nh, newnh);
+				else
+					ctx->u.pw.fib_nhg.nexthop = newnh;
+				last_nh = newnh;
 			}
+		}
 
-			if (re) {
-				nhg = rib_get_fib_nhg(re);
-				if (nhg && nhg->nexthop)
-					copy_nexthops(&(ctx->u.pw.nhg.nexthop),
-						      nhg->nexthop, NULL);
-
-				/* Include any installed backup nexthops */
-				nhg = rib_get_fib_backup_nhg(re);
-				if (nhg && nhg->nexthop)
-					copy_nexthops(&(ctx->u.pw.nhg.nexthop),
-						      nhg->nexthop, NULL);
+		/* Include any installed backup nexthops also. */
+		nhg = rib_get_fib_backup_nhg(re);
+		if (nhg && nhg->nexthop) {
+			for (ALL_NEXTHOPS_PTR(nhg, nh)) {
+				if (!CHECK_FLAG(nh->flags, NEXTHOP_FLAG_ACTIVE)
+				    || CHECK_FLAG(nh->flags,
+						  NEXTHOP_FLAG_RECURSIVE)
+				    || nh->nh_label == NULL)
+					continue;
+
+				newnh = nexthop_dup(nh, NULL);
+
+				if (last_nh)
+					NEXTHOP_APPEND(last_nh, newnh);
+				else
+					ctx->u.pw.fib_nhg.nexthop = newnh;
+				last_nh = newnh;
 			}
-			route_unlock_node(rn);
+		}
+
+		/* Copy primary nexthops; recursive info is included too */
+		assert(re->nhe != NULL); /* SA warning */
+		copy_nexthops(&(ctx->u.pw.primary_nhg.nexthop),
+			      re->nhe->nhg.nexthop, NULL);
+		ctx->u.pw.nhg_id = re->nhe->id;
+
+		/* Copy backup nexthop info, if present */
+		if (re->nhe->backup_info && re->nhe->backup_info->nhe) {
+			copy_nexthops(&(ctx->u.pw.backup_nhg.nexthop),
+				      re->nhe->backup_info->nhe->nhg.nexthop,
+				      NULL);
 		}
 	}
+	route_unlock_node(rn);
 
-	return AOK;
+	ret = AOK;
+
+done:
+	return ret;
 }
 
 /**
diff --git a/zebra/zebra_dplane.h b/zebra/zebra_dplane.h
index 3a8536dda5..e091655a48 100644
--- a/zebra/zebra_dplane.h
+++ b/zebra/zebra_dplane.h
@@ -437,6 +437,10 @@ const union pw_protocol_fields *dplane_ctx_get_pw_proto(
 	const struct zebra_dplane_ctx *ctx);
 const struct nexthop_group *dplane_ctx_get_pw_nhg(
 	const struct zebra_dplane_ctx *ctx);
+const struct nexthop_group *
+dplane_ctx_get_pw_primary_nhg(const struct zebra_dplane_ctx *ctx);
+const struct nexthop_group *
+dplane_ctx_get_pw_backup_nhg(const struct zebra_dplane_ctx *ctx);
 
 /* Accessors for interface information */
 uint32_t dplane_ctx_get_intf_metric(const struct zebra_dplane_ctx *ctx);
diff --git a/zebra/zebra_mpls.c b/zebra/zebra_mpls.c
index 850ca17636..a4576b310e 100644
--- a/zebra/zebra_mpls.c
+++ b/zebra/zebra_mpls.c
@@ -54,6 +54,7 @@ DEFINE_MTYPE_STATIC(ZEBRA, FEC, "MPLS FEC object");
 DEFINE_MTYPE_STATIC(ZEBRA, NHLFE, "MPLS nexthop object");
 
 int mpls_enabled;
+bool mpls_pw_reach_strict; /* Strict reachability checking */
 
 /* static function declarations */
 
@@ -3977,6 +3978,7 @@ void zebra_mpls_init_tables(struct zebra_vrf *zvrf)
 void zebra_mpls_init(void)
 {
 	mpls_enabled = 0;
+	mpls_pw_reach_strict = false;
 
 	if (mpls_kernel_init() < 0) {
 		flog_warn(EC_ZEBRA_MPLS_SUPPORT_DISABLED,
diff --git a/zebra/zebra_mpls.h b/zebra/zebra_mpls.h
index dd2d2e5658..7059d393ed 100644
--- a/zebra/zebra_mpls.h
+++ b/zebra/zebra_mpls.h
@@ -576,6 +576,7 @@ static inline int mpls_should_lsps_be_processed(struct route_node *rn)
 
 /* Global variables. */
 extern int mpls_enabled;
+extern bool mpls_pw_reach_strict; /* Strict pseudowire reachability checking */
 
 #ifdef __cplusplus
 }
diff --git a/zebra/zebra_mpls_openbsd.c b/zebra/zebra_mpls_openbsd.c
index b767929dc0..74b1e37278 100644
--- a/zebra/zebra_mpls_openbsd.c
+++ b/zebra/zebra_mpls_openbsd.c
@@ -458,6 +458,9 @@ int mpls_kernel_init(void)
 
 	kr_state.rtseq = 1;
 
+	/* Strict pseudowire reachability checking required for obsd */
+	mpls_pw_reach_strict = true;
+
 	return 0;
 }
 
diff --git a/zebra/zebra_pw.c b/zebra/zebra_pw.c
index 5afb3e5926..6b4a815151 100644
--- a/zebra/zebra_pw.c
+++ b/zebra/zebra_pw.c
@@ -48,7 +48,7 @@ static int zebra_pw_enabled(struct zebra_pw *);
 static void zebra_pw_install(struct zebra_pw *);
 static void zebra_pw_uninstall(struct zebra_pw *);
 static int zebra_pw_install_retry(struct thread *);
-static int zebra_pw_check_reachability(struct zebra_pw *);
+static int zebra_pw_check_reachability(const struct zebra_pw *);
 static void zebra_pw_update_status(struct zebra_pw *, int);
 
 static inline int zebra_pw_compare(const struct zebra_pw *a,
@@ -243,14 +243,79 @@ static void zebra_pw_update_status(struct zebra_pw *pw, int status)
 		zsend_pw_update(pw->client, pw);
 }
 
-static int zebra_pw_check_reachability(struct zebra_pw *pw)
+static int zebra_pw_check_reachability_strict(const struct zebra_pw *pw,
+					      struct route_entry *re)
+{
+	const struct nexthop *nexthop;
+	const struct nexthop_group *nhg;
+	bool found_p = false;
+	bool fail_p = false;
+
+	/* TODO: consider GRE/L2TPv3 tunnels in addition to MPLS LSPs */
+
+	/* All active nexthops must be labelled; look at
+	 * primary and backup fib lists, in case there's been
+	 * a backup nexthop activation.
+	 */
+	nhg = rib_get_fib_nhg(re);
+	if (nhg && nhg->nexthop) {
+		for (ALL_NEXTHOPS_PTR(nhg, nexthop)) {
+			if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_RECURSIVE))
+				continue;
+
+			if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_ACTIVE)) {
+				if (nexthop->nh_label != NULL)
+					found_p = true;
+				else {
+					fail_p = true;
+					break;
+				}
+			}
+		}
+	}
+
+	if (fail_p)
+		goto done;
+
+	nhg = rib_get_fib_backup_nhg(re);
+	if (nhg && nhg->nexthop) {
+		for (ALL_NEXTHOPS_PTR(nhg, nexthop)) {
+			if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_RECURSIVE))
+				continue;
+
+			if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_ACTIVE)) {
+				if (nexthop->nh_label != NULL)
+					found_p = true;
+				else {
+					fail_p = true;
+					break;
+				}
+			}
+		}
+	}
+
+done:
+
+	if (fail_p || !found_p) {
+		if (IS_ZEBRA_DEBUG_PW)
+			zlog_debug("%s: unlabeled route for %s",
+				   __func__, pw->ifname);
+		return -1;
+	}
+
+	return 0;
+}
+
+static int zebra_pw_check_reachability(const struct zebra_pw *pw)
 {
 	struct route_entry *re;
-	struct nexthop *nexthop;
+	const struct nexthop *nexthop;
+	const struct nexthop_group *nhg;
+	bool found_p = false;
 
 	/* TODO: consider GRE/L2TPv3 tunnels in addition to MPLS LSPs */
 
-	/* find route to the remote end of the pseudowire */
+	/* Find route to the remote end of the pseudowire */
 	re = rib_match(family2afi(pw->af), SAFI_UNICAST, pw->vrf_id,
 		       &pw->nexthop, NULL);
 	if (!re) {
@@ -260,19 +325,52 @@ static int zebra_pw_check_reachability(struct zebra_pw *pw)
 		return -1;
 	}
 
-	/*
-	 * Need to ensure that there's a label binding for all nexthops.
-	 * Otherwise, ECMP for this route could render the pseudowire unusable.
+	/* Stricter checking for some OSes (OBSD, e.g.) */
+	if (mpls_pw_reach_strict)
+		return zebra_pw_check_reachability_strict(pw, re);
+
+	/* There must be at least one installed labelled nexthop;
+	 * look at primary and backup fib lists, in case there's been
+	 * a backup nexthop activation.
 	 */
-	for (ALL_NEXTHOPS(re->nhe->nhg, nexthop)) {
-		if (!nexthop->nh_label) {
-			if (IS_ZEBRA_DEBUG_PW)
-				zlog_debug("%s: unlabeled route for %s",
-					   __func__, pw->ifname);
-			return -1;
+	nhg = rib_get_fib_nhg(re);
+	if (nhg && nhg->nexthop) {
+		for (ALL_NEXTHOPS_PTR(nhg, nexthop)) {
+			if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_RECURSIVE))
+				continue;
+
+			if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_ACTIVE) &&
+			    nexthop->nh_label != NULL) {
+				found_p = true;
+				break;
+			}
+		}
+	}
+
+	if (found_p)
+		return 0;
+
+	nhg = rib_get_fib_backup_nhg(re);
+	if (nhg && nhg->nexthop) {
+		for (ALL_NEXTHOPS_PTR(nhg, nexthop)) {
+			if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_RECURSIVE))
+				continue;
+
+			if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_ACTIVE) &&
+			    nexthop->nh_label != NULL) {
+				found_p = true;
+				break;
+			}
 		}
 	}
 
+	if (!found_p) {
+		if (IS_ZEBRA_DEBUG_PW)
+			zlog_debug("%s: unlabeled route for %s",
+				   __func__, pw->ifname);
+		return -1;
+	}
+
 	return 0;
 }
 
diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c
index 8afb5053c4..d0acf77936 100644
--- a/zebra/zebra_rib.c
+++ b/zebra/zebra_rib.c
@@ -332,7 +332,8 @@ void rib_handle_nhg_replace(struct nhg_hash_entry *old_entry,
 }
 
 struct route_entry *rib_match(afi_t afi, safi_t safi, vrf_id_t vrf_id,
-			      union g_addr *addr, struct route_node **rn_out)
+			      const union g_addr *addr,
+			      struct route_node **rn_out)
 {
 	struct prefix p;
 	struct route_table *table;