ospfd, doc, tests: combined SRGB/SRLB command

similarly to what was done for IS-IS in commit 01d43141, combine
the SRGB and SRLB commands for OSPF-SR, so that we can replace
overlapping ranges in one sweep change.

Also allow the range configuration to be stored before SR is enabled.
There is no reason why we should not - in fact that constraint meant
that we were always requesting the default label ranges regardless
of what we actually wanted to use.

Finally, update the topotests now that we do not need to refresh
the SRGB/SRLB/MSD after disabling SR. Note that the prefix-sid still
needs to be re-added.

Signed-off-by: Emanuele Di Pascale <emanuele@voltanet.io>

2 files changed, 178 insertions, 193 deletions

diff --git a/ospfd/ospf_sr.c b/ospfd/ospf_sr.c
index 3ce177618..bf2c5f3c4 100644
--- a/ospfd/ospf_sr.c
+++ b/ospfd/ospf_sr.c
@@ -535,22 +535,14 @@ static void ospf_sr_stop(void)
 	THREAD_OFF(OspfSR.t_start_lm);
 
 	/* Release SRGB & SRLB if active. */
-	if (OspfSR.srgb.reserved)
+	if (OspfSR.srgb.reserved) {
 		ospf_zebra_release_label_range(
 			OspfSR.srgb.start,
 			OspfSR.srgb.start + OspfSR.srgb.size - 1);
+		OspfSR.srgb.reserved = false;
+	}
 	sr_local_block_delete();
 
-	/* Revert SRGB, SRLB and MSD to default values */
-	OspfSR.srgb.size = DEFAULT_SRGB_SIZE;
-	OspfSR.srgb.start = DEFAULT_SRGB_LABEL;
-	OspfSR.srgb.reserved = false;
-
-	OspfSR.srlb.start = DEFAULT_SRLB_LABEL;
-	OspfSR.srlb.end = DEFAULT_SRLB_LABEL + DEFAULT_SRLB_SIZE - 1;
-	OspfSR.srlb.reserved = false;
-	OspfSR.msd = 0;
-
 	/*
 	 * Remove all SR Nodes from the Hash table. Prefix and Link SID will
 	 * be remove though list_delete() call. See sr_node_del()
@@ -2010,41 +2002,36 @@ void ospf_sr_config_write_router(struct vty *vty)
 	struct sr_prefix *srp;
 	uint32_t upper;
 
-	if (OspfSR.status == SR_UP) {
+	if (OspfSR.status == SR_UP)
 		vty_out(vty, " segment-routing on\n");
 
-		upper = OspfSR.srgb.start + OspfSR.srgb.size - 1;
-		if ((OspfSR.srgb.start != DEFAULT_SRGB_LABEL)
-		    || (OspfSR.srgb.size != DEFAULT_SRGB_SIZE))
-			vty_out(vty, " segment-routing global-block %u %u\n",
-				OspfSR.srgb.start, upper);
+	upper = OspfSR.srgb.start + OspfSR.srgb.size - 1;
+	if ((OspfSR.srgb.start != DEFAULT_SRGB_LABEL)
+	    || (OspfSR.srgb.size != DEFAULT_SRGB_SIZE))
+		vty_out(vty, " segment-routing global-block %u %u",
+			OspfSR.srgb.start, upper);
 
-		upper = DEFAULT_SRLB_LABEL + DEFAULT_SRLB_SIZE - 1;
-		if ((OspfSR.srlb.start != DEFAULT_SRLB_LABEL)
-		    || (OspfSR.srlb.end != upper))
-			vty_out(vty, " segment-routing local-block %u %u\n",
-				OspfSR.srlb.start, OspfSR.srlb.end);
+	if ((OspfSR.srlb.start != DEFAULT_SRLB_LABEL)
+	    || (OspfSR.srlb.end != DEFAULT_SRLB_END))
+		vty_out(vty, " local-block %u %u\n", OspfSR.srlb.start,
+			OspfSR.srlb.end);
+	else
+		vty_out(vty, "\n");
 
-		if (OspfSR.msd != 0)
-			vty_out(vty, " segment-routing node-msd %u\n",
-				OspfSR.msd);
+	if (OspfSR.msd != 0)
+		vty_out(vty, " segment-routing node-msd %u\n", OspfSR.msd);
 
-		if (OspfSR.self != NULL) {
-			for (ALL_LIST_ELEMENTS_RO(OspfSR.self->ext_prefix, node,
-						  srp)) {
-				vty_out(vty,
-					" segment-routing prefix %pFX index %u",
-					&srp->prefv4, srp->sid);
-				if (CHECK_FLAG(srp->flags,
-					       EXT_SUBTLV_PREFIX_SID_EFLG))
-					vty_out(vty, " explicit-null\n");
-				else if (CHECK_FLAG(
-						 srp->flags,
-						 EXT_SUBTLV_PREFIX_SID_NPFLG))
-					vty_out(vty, " no-php-flag\n");
-				else
-					vty_out(vty, "\n");
-			}
+	if (OspfSR.self != NULL) {
+		for (ALL_LIST_ELEMENTS_RO(OspfSR.self->ext_prefix, node, srp)) {
+			vty_out(vty, " segment-routing prefix %pFX index %u",
+				&srp->prefv4, srp->sid);
+			if (CHECK_FLAG(srp->flags, EXT_SUBTLV_PREFIX_SID_EFLG))
+				vty_out(vty, " explicit-null\n");
+			else if (CHECK_FLAG(srp->flags,
+					    EXT_SUBTLV_PREFIX_SID_NPFLG))
+				vty_out(vty, " no-php-flag\n");
+			else
+				vty_out(vty, "\n");
 		}
 	}
 }
@@ -2113,54 +2100,94 @@ static int ospf_sr_enabled(struct vty *vty)
 }
 
 /**
- * Update SRGB following new CLI value.
+ * Update SRGB and/or SRLB using new CLI values.
  *
- * @param lower	Lower bound of the SRGB
- * @param size	Size of the SRGB
+ * @param gb_lower  Lower bound of the SRGB
+ * @param gb_upper  Upper bound of the SRGB
+ * @param lb_lower  Lower bound of the SRLB
+ * @param lb_upper  Upper bound of the SRLB
  *
- * @return	0 on success, -1 otherwise
+ * @return          0 on success, -1 otherwise
  */
-static int update_srgb(uint32_t lower, uint32_t size)
+static int update_sr_blocks(uint32_t gb_lower, uint32_t gb_upper,
+			    uint32_t lb_lower, uint32_t lb_upper)
 {
 
 	/* Check if values have changed */
-	if ((OspfSR.srgb.size == size) && (OspfSR.srgb.start == lower))
+	bool gb_changed, lb_changed;
+	uint32_t gb_size = gb_upper - gb_lower + 1;
+	uint32_t lb_size = lb_upper - lb_lower + 1;
+
+	gb_changed =
+		(OspfSR.srgb.size != gb_size || OspfSR.srgb.start != gb_lower);
+	lb_changed =
+		(OspfSR.srlb.end != lb_upper || OspfSR.srlb.start != lb_lower);
+	if (!gb_changed && !lb_changed)
 		return 0;
 
-	/* Release old SRGB if active. */
-	if (OspfSR.srgb.reserved) {
-		ospf_zebra_release_label_range(
-			OspfSR.srgb.start,
-			OspfSR.srgb.start + OspfSR.srgb.size - 1);
-		OspfSR.srgb.reserved = false;
+	/* Check if SR is correctly started i.e. Label Manager connected */
+	if (OspfSR.status != SR_UP) {
+		OspfSR.srgb.size = gb_size;
+		OspfSR.srgb.start = gb_lower;
+		OspfSR.srlb.end = lb_upper;
+		OspfSR.srlb.start = lb_lower;
+		return 0;
 	}
 
-	/* Set new SRGB values */
-	OspfSR.srgb.size = size;
-	OspfSR.srgb.start = lower;
-	if (OspfSR.self != NULL) {
-		OspfSR.self->srgb.range_size = size;
-		OspfSR.self->srgb.lower_bound = lower;
+	/* Release old SRGB if it has changed and is active. */
+	if (gb_changed) {
+		if (OspfSR.srgb.reserved) {
+			ospf_zebra_release_label_range(
+				OspfSR.srgb.start,
+				OspfSR.srgb.start + OspfSR.srgb.size - 1);
+			OspfSR.srgb.reserved = false;
+		}
+
+		/* Set new SRGB values - but do not reserve yet (we need to
+		 * release the SRLB too) */
+		OspfSR.srgb.size = gb_size;
+		OspfSR.srgb.start = gb_lower;
+		if (OspfSR.self != NULL) {
+			OspfSR.self->srgb.range_size = gb_size;
+			OspfSR.self->srgb.lower_bound = gb_lower;
+		}
 	}
+	/* Release old SRLB if it has changed and reserve new block as needed.
+	 */
+	if (lb_changed) {
+		if (OspfSR.srlb.reserved)
+			sr_local_block_delete();
 
-	/* Check if SR is correctly started i.e. Label Manager connected */
-	if (OspfSR.status != SR_UP)
-		return 0;
+		/* Set new SRLB values */
+		if (sr_local_block_init(lb_lower, lb_upper) < 0) {
+			ospf_sr_stop();
+			return -1;
+		}
+		if (OspfSR.self != NULL) {
+			OspfSR.self->srlb.lower_bound = lb_lower;
+			OspfSR.self->srlb.range_size = lb_size;
+		}
+	}
 
 	/*
-	 * Try to reserve the new block from the Label Manger. If the allocation
-	 * fails, disable SR until a new SRGB is successfully allocated.
+	 * Try to reserve the new SRGB from the Label Manger. If the
+	 * allocation fails, disable SR until new blocks are successfully
+	 * allocated.
 	 */
-	if (ospf_zebra_request_label_range(OspfSR.srgb.start,
-					   OspfSR.srgb.size) < 0) {
-		OspfSR.srgb.reserved = false;
-		ospf_sr_stop();
-		return -1;
-	} else
-		OspfSR.srgb.reserved = true;
+	if (gb_changed) {
+		if (ospf_zebra_request_label_range(OspfSR.srgb.start,
+						   OspfSR.srgb.size)
+		    < 0) {
+			OspfSR.srgb.reserved = false;
+			ospf_sr_stop();
+			return -1;
+		} else
+			OspfSR.srgb.reserved = true;
 
-	osr_debug("SR(%s): Got new SRGB [%u/%u]", __func__, OspfSR.srgb.start,
-		  OspfSR.srgb.start + OspfSR.srgb.size - 1);
+		osr_debug("SR(%s): Got new SRGB [%u/%u]", __func__,
+			  OspfSR.srgb.start,
+			  OspfSR.srgb.start + OspfSR.srgb.size - 1);
+	}
 
 	/* Update Self SR-Node */
 	if (OspfSR.self != NULL) {
@@ -2168,88 +2195,87 @@ static int update_srgb(uint32_t lower, uint32_t size)
 		ospf_router_info_update_sr(true, OspfSR.self);
 
 		/* and update NHLFE entries */
-		hash_iterate(
-			OspfSR.neighbors,
-			(void (*)(struct hash_bucket *, void *))update_in_nhlfe,
-			NULL);
+		if (gb_changed)
+			hash_iterate(OspfSR.neighbors,
+				     (void (*)(struct hash_bucket *,
+					       void *))update_in_nhlfe,
+				     NULL);
+
+		/* and update (LAN)-Adjacency SID */
+		if (lb_changed)
+			ospf_ext_link_srlb_update();
 	}
 
 	return 0;
 }
 
-DEFUN (sr_global_label_range,
-       sr_global_label_range_cmd,
-       "segment-routing global-block (16-1048575) (16-1048575)",
-       SR_STR
-       "Segment Routing Global Block label range\n"
-       "Lower-bound range in decimal (16-1048575)\n"
-       "Upper-bound range in decimal (16-1048575)\n")
-{
-	uint32_t upper;
-	uint32_t lower;
-	uint32_t size;
-	int idx_low = 2;
-	int idx_up = 3;
-
-	if (!ospf_sr_enabled(vty))
-		return CMD_WARNING_CONFIG_FAILED;
-
-	/* Get lower and upper bound */
-	lower = strtoul(argv[idx_low]->arg, NULL, 10);
-	upper = strtoul(argv[idx_up]->arg, NULL, 10);
-	size = upper - lower + 1;
+DEFUN(sr_global_label_range, sr_global_label_range_cmd,
+      "segment-routing global-block (16-1048575) (16-1048575) [local-block (16-1048575) (16-1048575)]",
+      SR_STR
+      "Segment Routing Global Block label range\n"
+      "Lower-bound range in decimal (16-1048575)\n"
+      "Upper-bound range in decimal (16-1048575)\n"
+      "Segment Routing Local Block label range\n"
+      "Lower-bound range in decimal (16-1048575)\n"
+      "Upper-bound range in decimal (16-1048575)\n")
+{
+	uint32_t lb_upper, lb_lower;
+	uint32_t gb_upper, gb_lower;
+	int idx_gb_low = 2, idx_gb_up = 3;
+	int idx_lb_low = 5, idx_lb_up = 6;
+
+	/* Get lower and upper bound for mandatory global-block */
+	gb_lower = strtoul(argv[idx_gb_low]->arg, NULL, 10);
+	gb_upper = strtoul(argv[idx_gb_up]->arg, NULL, 10);
+	/* SRLB values are taken from vtysh if there, else use the known ones */
+	lb_upper = argc > idx_lb_up ? strtoul(argv[idx_lb_up]->arg, NULL, 10)
+				    : OspfSR.srlb.end;
+	lb_lower = argc > idx_lb_low ? strtoul(argv[idx_lb_low]->arg, NULL, 10)
+				     : OspfSR.srlb.start;
 
 	/* Validate SRGB against SRLB */
-	if (!((upper < OspfSR.srlb.start) || (lower > OspfSR.srlb.end))) {
+	if (!((gb_upper < lb_lower) || (gb_lower > lb_upper))) {
 		vty_out(vty,
 			"New SR Global Block (%u/%u) conflict with Local Block (%u/%u)\n",
-			lower, upper, OspfSR.srlb.end, OspfSR.srlb.start);
+			gb_lower, gb_upper, lb_lower, lb_upper);
 		return CMD_WARNING_CONFIG_FAILED;
 	}
 
-	if (update_srgb(lower, size) < 0)
+	if (update_sr_blocks(gb_lower, gb_upper, lb_lower, lb_upper) < 0)
 		return CMD_WARNING_CONFIG_FAILED;
 	else
 		return CMD_SUCCESS;
 }
 
-DEFUN (no_sr_global_label_range,
-	no_sr_global_label_range_cmd,
-	"no segment-routing global-block [(16-1048575) (16-1048575)]",
-	NO_STR
-	SR_STR
-	"Segment Routing Global Block label range\n"
-	"Lower-bound range in decimal (16-1048575)\n"
-	"Upper-bound range in decimal (16-1048575)\n")
+DEFUN(no_sr_global_label_range, no_sr_global_label_range_cmd,
+      "no segment-routing global-block [(16-1048575) (16-1048575) local-block (16-1048575) (16-1048575)]",
+      NO_STR SR_STR
+      "Segment Routing Global Block label range\n"
+      "Lower-bound range in decimal (16-1048575)\n"
+      "Upper-bound range in decimal (16-1048575)\n"
+      "Segment Routing Local Block label range\n"
+      "Lower-bound range in decimal (16-1048575)\n"
+      "Upper-bound range in decimal (16-1048575)\n")
 {
-
-	if (!ospf_sr_enabled(vty))
-		return CMD_WARNING_CONFIG_FAILED;
-
-	/* Validate SRGB against SRLB */
-	uint32_t upper = DEFAULT_SRGB_LABEL + DEFAULT_SRGB_SIZE - 1;
-	if (!((upper < OspfSR.srlb.start)
-	      || (DEFAULT_SRGB_LABEL > OspfSR.srlb.end))) {
-		vty_out(vty,
-			"New SR Global Block (%u/%u) conflict with Local Block (%u/%u)\n",
-			DEFAULT_SRGB_LABEL, upper, OspfSR.srlb.end,
-			OspfSR.srlb.start);
-		return CMD_WARNING_CONFIG_FAILED;
-	}
-
-	if (update_srgb(DEFAULT_SRGB_LABEL, DEFAULT_SRGB_SIZE) < 0)
+	if (update_sr_blocks(DEFAULT_SRGB_LABEL, DEFAULT_SRGB_END,
+			     DEFAULT_SRLB_LABEL, DEFAULT_SRLB_END)
+	    < 0)
 		return CMD_WARNING_CONFIG_FAILED;
 	else
 		return CMD_SUCCESS;
 }
 
-DEFUN (sr_local_label_range,
-       sr_local_label_range_cmd,
-       "segment-routing local-block (16-1048575) (16-1048575)",
-       SR_STR
-       "Segment Routing Local Block label range\n"
-       "Lower-bound range in decimal (16-1048575)\n"
-       "Upper-bound range in decimal (16-1048575)\n")
+#if CONFDATE > 20220528
+CPP_NOTICE(
+	"Use of the segment-routing local-block command is deprecated, use the combined global-block command instead")
+#endif
+
+DEFUN_HIDDEN(sr_local_label_range, sr_local_label_range_cmd,
+	     "segment-routing local-block (16-1048575) (16-1048575)",
+	     SR_STR
+	     "Segment Routing Local Block label range\n"
+	     "Lower-bound range in decimal (16-1048575)\n"
+	     "Upper-bound range in decimal (16-1048575)\n")
 {
 	uint32_t upper;
 	uint32_t lower;
@@ -2257,9 +2283,6 @@ DEFUN (sr_local_label_range,
 	int idx_low = 2;
 	int idx_up = 3;
 
-	if (!ospf_sr_enabled(vty))
-		return CMD_WARNING_CONFIG_FAILED;
-
 	/* Get lower and upper bound */
 	lower = strtoul(argv[idx_low]->arg, NULL, 10);
 	upper = strtoul(argv[idx_up]->arg, NULL, 10);
@@ -2278,79 +2301,39 @@ DEFUN (sr_local_label_range,
 		return CMD_WARNING_CONFIG_FAILED;
 	}
 
-	/* Remove old SRLB */
-	sr_local_block_delete();
-
-	/* Try to reserve the new block from the Label Manger. If the allocation
-	 * fails, disable SR until a new SRLB is successfully allocated.
-	 */
-	if (sr_local_block_init(lower, upper) != 0) {
-		ospf_sr_stop();
+	if (update_sr_blocks(OspfSR.srgb.start, srgb_upper, lower, upper) < 0)
 		return CMD_WARNING_CONFIG_FAILED;
-	}
-
-	/* SRLB is reserved, Update Self SR-Node and Router Information LSA */
-	if (OspfSR.self != NULL) {
-		OspfSR.self->srlb.lower_bound = lower;
-		OspfSR.self->srlb.range_size = upper - lower + 1;
-		ospf_router_info_update_sr(true, OspfSR.self);
-	}
-
-	/* and update (LAN)-Adjacency SID */
-	ospf_ext_link_srlb_update();
-
-	return CMD_SUCCESS;
+	else
+		return CMD_SUCCESS;
 }
 
-DEFUN (no_sr_local_label_range,
-	no_sr_local_label_range_cmd,
-	"no segment-routing local-block [(16-1048575) (16-1048575)]",
-	NO_STR
-	SR_STR
-	"Segment Routing Local Block label range\n"
-	"Lower-bound range in decimal (16-1048575)\n"
-	"Upper-bound range in decimal (16-1048575)\n")
+DEFUN_HIDDEN(no_sr_local_label_range, no_sr_local_label_range_cmd,
+	     "no segment-routing local-block [(16-1048575) (16-1048575)]",
+	     NO_STR SR_STR
+	     "Segment Routing Local Block label range\n"
+	     "Lower-bound range in decimal (16-1048575)\n"
+	     "Upper-bound range in decimal (16-1048575)\n")
 {
 
-	uint32_t upper;
 	uint32_t srgb_end;
 
-	if (!ospf_sr_enabled(vty))
-		return CMD_WARNING_CONFIG_FAILED;
-
-	/* First, remove old SRLB */
-	sr_local_block_delete();
-
 	/* Validate SRLB against SRGB */
 	srgb_end = OspfSR.srgb.start + OspfSR.srgb.size - 1;
-	upper = DEFAULT_SRLB_LABEL + DEFAULT_SRLB_SIZE - 1;
-	if (!((upper < OspfSR.srgb.start) || (DEFAULT_SRLB_LABEL > srgb_end))) {
+	if (!((DEFAULT_SRLB_END < OspfSR.srgb.start)
+	      || (DEFAULT_SRLB_LABEL > srgb_end))) {
 		vty_out(vty,
 			"New SR Local Block (%u/%u) conflict with Global Block (%u/%u)\n",
-			DEFAULT_SRLB_LABEL, upper, OspfSR.srgb.start, srgb_end);
+			DEFAULT_SRLB_LABEL, DEFAULT_SRLB_END, OspfSR.srgb.start,
+			srgb_end);
 		return CMD_WARNING_CONFIG_FAILED;
 	}
 
-	/* Then, initialize SRLB with default value and try to reserve the new
-	 * block from the Label Manger. If the allocation fails, disable SR
-	 * until a new SRLB is successfully allocated.
-	 */
-	if (sr_local_block_init(DEFAULT_SRLB_LABEL, upper) != 0) {
-		ospf_sr_stop();
+	if (update_sr_blocks(OspfSR.srgb.start, srgb_end, DEFAULT_SRLB_LABEL,
+			     DEFAULT_SRLB_END)
+	    < 0)
 		return CMD_WARNING_CONFIG_FAILED;
-	}
-
-	/* SRLB is reserved, Update Self SR-Node and Router Information LSA */
-	if (OspfSR.self != NULL) {
-		OspfSR.self->srlb.lower_bound = DEFAULT_SRLB_LABEL;
-		OspfSR.self->srlb.range_size = DEFAULT_SRLB_SIZE;
-		ospf_router_info_update_sr(true, OspfSR.self);
-	}
-
-	/* and update (LAN)-Adjacency SID */
-	ospf_ext_link_srlb_update();
-
-	return CMD_SUCCESS;
+	else
+		return CMD_SUCCESS;
 }
 
 DEFUN (sr_node_msd,
diff --git a/ospfd/ospf_sr.h b/ospfd/ospf_sr.h
index ea54e3b31..d706d206f 100644
--- a/ospfd/ospf_sr.h
+++ b/ospfd/ospf_sr.h
@@ -186,10 +186,12 @@ struct ext_subtlv_lan_adj_sid {
 /* Default min and size of SR Global Block label range */
 #define DEFAULT_SRGB_LABEL        16000
 #define DEFAULT_SRGB_SIZE         8000
+#define DEFAULT_SRGB_END (DEFAULT_SRGB_LABEL + DEFAULT_SRGB_SIZE - 1)
 
 /* Default min and size of SR Local Block label range */
 #define DEFAULT_SRLB_LABEL        15000
 #define DEFAULT_SRLB_SIZE         1000
+#define DEFAULT_SRLB_END (DEFAULT_SRLB_LABEL + DEFAULT_SRLB_SIZE - 1)
 
 /* Structure aggregating SR Range Block info retrieved from an lsa */
 struct sr_block {