ospfd: Debug race condition in Segment Routing

Issue #7926 hilight a race condition in Segment Routing processing.

The problem occurs when Router Information Opaque LSA is received late, in
particular after SPF run and after ospf_sr_nhlfe_update() was called. This
scenario is unfrequent and takes place due to a slow DR election.
In this particular case, SR Prefix are handle but not fully fill. In fact,
SRGB for the nexthop is not yet received and thus, output label could not
be computed.

When Router Information Opaque LSA is received and processed, if the
corresponding SR node is a direct neighbor of the self node, update_out_nhlfe()
is called against all SR nodes to adjust SR prefix if the next hop is the new
SR node. The function wrongly computes output label and configure a bad MPLS
LFIB entries.

Another way to hilight the problem is to change through CLI the SRGB of a node
and look to MPLS LFIB of direct neighbor, in particular those who announce
EXPLICIT NULL Prefix SID.

This patch correct the update_out_nhlfe() function by calling the appropriate
function (sr_prefix_out_label() instead of index2label()) to compute the output
label.

Some log debugs were adjusted and unused prefix route table was removed too.

Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>

ospfd/ospf_sr.c

@@ -598,11 +598,6 @@ int ospf_sr_init(void)
 	if (OspfSR.neighbors == NULL)
 		return rc;
 
-	/* Initialize Route Table for prefix */
-	OspfSR.prefix = route_table_init();
-	if (OspfSR.prefix == NULL)
-		return rc;
-
 	/* Register Segment Routing VTY command */
 	ospf_sr_register_vty();
 
@@ -626,9 +621,6 @@ void ospf_sr_term(void)
 	if (OspfSR.neighbors)
 		hash_free(OspfSR.neighbors);
 
-	/* Clear Prefix Table */
-	if (OspfSR.prefix)
-		route_table_finish(OspfSR.prefix);
 }
 
 /*
@@ -913,8 +905,13 @@ static int compute_prefix_nhlfe(struct sr_prefix *srp)
 		 * be received before corresponding Router Information LSA
 		 */
 		if (srnext == NULL || srnext->srgb.lower_bound == 0
-		    || srnext->srgb.range_size == 0)
+		    || srnext->srgb.range_size == 0) {
+			osr_debug(
+				"    |-  SR-Node %pI4 not ready. Stop process",
+				&srnext->adv_router);
+			path->srni.label_out = MPLS_INVALID_LABEL;
 			continue;
+		}
 
 		osr_debug("    |-  Found SRGB %u/%u for next hop SR-Node %pI4",
 			  srnext->srgb.range_size, srnext->srgb.lower_bound,
@@ -1273,7 +1270,7 @@ static void update_in_nhlfe(struct hash_bucket *bucket, void *args)
 
 /*
  * When SRGB has changed, update NHLFE Output Label for all Extended Prefix
- * with SID index which use the given SR-Node as nexthop though hash_iterate()
+ * with SID index which use the given SR-Node as nexthop through hash_iterate()
  */
 static void update_out_nhlfe(struct hash_bucket *bucket, void *args)
 {
@@ -1283,21 +1280,29 @@ static void update_out_nhlfe(struct hash_bucket *bucket, void *args)
 	struct sr_prefix *srp;
 	struct ospf_path *path;
 
+	/* Skip Self SR-Node */
+	if (srn == OspfSR.self)
+		return;
+
+	osr_debug("SR (%s): Update Out NHLFE for neighbor SR-Node %pI4",
+		  __func__, &srn->adv_router);
+
 	for (ALL_LIST_ELEMENTS_RO(srn->ext_prefix, node, srp)) {
-		/* Process only SID Index with valid route */
+		/* Skip Prefix that has not yet a valid route */
 		if (srp->route == NULL)
 			continue;
 
 		for (ALL_LIST_ELEMENTS_RO(srp->route->paths, pnode, path)) {
-			/* Process only SID Index for next hop without PHP */
+			/* Skip path that has not next SR-Node as nexthop */
-			if ((path->srni.nexthop == srp->srn)
+			if (path->srni.nexthop != srnext)
-			    && (!CHECK_FLAG(srp->flags,
-					    EXT_SUBTLV_PREFIX_SID_NPFLG)))
 				continue;
-			path->srni.label_out =
+
-				index2label(srp->sid, srnext->srgb);
+			/* Compute new Output Label */
-			ospf_zebra_update_prefix_sid(srp);
+			path->srni.label_out = sr_prefix_out_label(srp, srnext);
 		}
+
+		/* Finally update MPLS table */
+		ospf_zebra_update_prefix_sid(srp);
 	}
 }
 
@@ -1438,11 +1443,6 @@ void ospf_sr_ri_lsa_update(struct ospf_lsa *lsa)
 		srn->srlb.lower_bound = GET_LABEL(ntohl(ri_srlb->lower.value));
 	}
 
-	osr_debug("  |- Update SR-Node[%pI4], SRGB[%u/%u], SRLB[%u/%u], Algo[%u], MSD[%u]",
-		  &srn->adv_router, srn->srgb.lower_bound, srn->srgb.range_size,
-		  srn->srlb.lower_bound, srn->srlb.range_size, srn->algo[0],
-		  srn->msd);
-
 	/* Check if SRGB has changed */
 	if ((srn->srgb.range_size == srgb.range_size)
 	    && (srn->srgb.lower_bound == srgb.lower_bound))
@@ -1452,6 +1452,11 @@ void ospf_sr_ri_lsa_update(struct ospf_lsa *lsa)
 	srn->srgb.range_size = srgb.range_size;
 	srn->srgb.lower_bound = srgb.lower_bound;
 
+	osr_debug("  |- Update SR-Node[%pI4], SRGB[%u/%u], SRLB[%u/%u], Algo[%u], MSD[%u]",
+		  &srn->adv_router, srn->srgb.lower_bound, srn->srgb.range_size,
+		  srn->srlb.lower_bound, srn->srlb.range_size, srn->algo[0],
+		  srn->msd);
+
 	/* ... and NHLFE if it is a neighbor SR node */
 	if (srn->neighbor == OspfSR.self)
 		hash_iterate(OspfSR.neighbors, update_out_nhlfe, srn);

ospfd/ospf_sr.h

@@ -233,9 +233,6 @@ struct ospf_sr_db {
 	/* List of neighbour SR nodes */
 	struct hash *neighbors;
 
-	/* List of SR prefix */
-	struct route_table *prefix;
-
 	/* Local SR info announced in Router Info LSA */
 
 	/* Algorithms supported by the node */

ospfd/ospf_zebra.c

@@ -541,9 +541,6 @@ void ospf_zebra_update_prefix_sid(const struct sr_prefix *srp)
 	struct listnode *node;
 	struct ospf_path *path;
 
-	osr_debug("SR (%s): Update Labels %u for Prefix %pFX", __func__,
-		  srp->label_in, (struct prefix *)&srp->prefv4);
-
 	/* Prepare message. */
 	memset(&zl, 0, sizeof(zl));
 	zl.type = ZEBRA_LSP_OSPF_SR;
@@ -557,6 +554,11 @@ void ospf_zebra_update_prefix_sid(const struct sr_prefix *srp)
 		znh->ifindex = srp->nhlfe.ifindex;
 		znh->label_num = 1;
 		znh->labels[0] = srp->nhlfe.label_out;
+
+		osr_debug("SR (%s): Configure Prefix %pFX with labels %u/%u",
+			  __func__, (struct prefix *)&srp->prefv4,
+			  srp->label_in, srp->nhlfe.label_out);
+
 		break;
 
 	case PREF_SID:
@@ -572,6 +574,10 @@ void ospf_zebra_update_prefix_sid(const struct sr_prefix *srp)
 		if (srp->route == NULL) {
 			return;
 		}
+
+		osr_debug("SR (%s): Configure Prefix %pFX with",
+			  __func__, (struct prefix *)&srp->prefv4);
+
 		for (ALL_LIST_ELEMENTS_RO(srp->route->paths, node, path)) {
 			if (path->srni.label_out == MPLS_INVALID_LABEL)
 				continue;
@@ -615,6 +621,9 @@ void ospf_zebra_update_prefix_sid(const struct sr_prefix *srp)
 			znh->label_num = 1;
 			znh->labels[0] = path->srni.label_out;
 
+			osr_debug("  |- labels %u/%u", srp->label_in,
+				  srp->nhlfe.label_out);
+
 			/* Set TI-LFA backup nexthop info if present */
 			if (path->srni.backup_label_stack) {
 				SET_FLAG(zl.message, ZAPI_LABELS_HAS_BACKUPS);