From 6f609c4980e2ae057476589374ee9bb66624fde0 Mon Sep 17 00:00:00 2001 From: Fredi Raspall Date: Thu, 27 May 2021 20:15:15 +0200 Subject: [PATCH 01/15] ospfd: fix display of debug flags * Some of the debug flags were not shown in show debugging. * The check for TI-LFA debug was made against the wrong variable. * Some of the debugs were not cleared with 'no debug ospf' Signed-off-by: Fredi Raspall --- ospfd/ospf_dump.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/ospfd/ospf_dump.c b/ospfd/ospf_dump.c index f11c84b092..29fda822bf 100644 --- a/ospfd/ospf_dump.c +++ b/ospfd/ospf_dump.c @@ -1616,11 +1616,15 @@ DEFUN (no_debug_ospf, DEBUG_OFF(nsm, NSM_EVENTS); DEBUG_OFF(nsm, NSM_STATUS); DEBUG_OFF(nsm, NSM_TIMERS); + DEBUG_OFF(event, EVENT); DEBUG_OFF(zebra, ZEBRA); DEBUG_OFF(zebra, ZEBRA_INTERFACE); DEBUG_OFF(zebra, ZEBRA_REDISTRIBUTE); DEBUG_OFF(defaultinfo, DEFAULTINFO); DEBUG_OFF(ldp_sync, LDP_SYNC); + DEBUG_OFF(te, TE); + DEBUG_OFF(sr, SR); + DEBUG_OFF(ti_lfa, TI_LFA); /* BFD debugging is two parts: OSPF and library. */ DEBUG_OFF(bfd, BFD_LIB); @@ -1653,6 +1657,9 @@ DEFUN (no_debug_ospf, TERM_DEBUG_OFF(zebra, ZEBRA_REDISTRIBUTE); TERM_DEBUG_OFF(defaultinfo, DEFAULTINFO); TERM_DEBUG_OFF(ldp_sync, LDP_SYNC); + TERM_DEBUG_OFF(te, TE); + TERM_DEBUG_OFF(sr, SR); + TERM_DEBUG_OFF(ti_lfa, TI_LFA); TERM_DEBUG_OFF(bfd, BFD_LIB); return CMD_SUCCESS; @@ -1763,6 +1770,18 @@ static int show_debugging_ospf_common(struct vty *vty) if (IS_DEBUG_OSPF(gr, GR) == OSPF_DEBUG_GR) vty_out(vty, " OSPF Graceful Restart debugging is on\n"); + /* Show debug status for TE */ + if (IS_DEBUG_OSPF(te, TE) == OSPF_DEBUG_TE) + vty_out(vty, " OSPF TE debugging is on\n"); + + /* Show debug status for SR */ + if (IS_DEBUG_OSPF(sr, SR) == OSPF_DEBUG_SR) + vty_out(vty, " OSPF SR debugging is on\n"); + + /* Show debug status for TI-LFA */ + if (IS_DEBUG_OSPF(ti_lfa, TI_LFA) == OSPF_DEBUG_TI_LFA) + vty_out(vty, " OSPF TI-LFA debugging is on\n"); + if (IS_DEBUG_OSPF(bfd, BFD_LIB) == OSPF_DEBUG_BFD_LIB) vty_out(vty, " OSPF BFD integration library debugging is on\n"); @@ -1937,7 +1956,7 @@ static int config_write_debug(struct vty *vty) } /* debug ospf sr ti-lfa */ - if (IS_CONF_DEBUG_OSPF(sr, TI_LFA) == OSPF_DEBUG_TI_LFA) { + if (IS_CONF_DEBUG_OSPF(ti_lfa, TI_LFA) == OSPF_DEBUG_TI_LFA) { vty_out(vty, "debug ospf%s ti-lfa\n", str); write = 1; } From 2110d03e061be4285b212dfde9de6726b96b2c99 Mon Sep 17 00:00:00 2001 From: Fredi Raspall Date: Thu, 27 May 2021 21:39:06 +0200 Subject: [PATCH 02/15] ospfd: remove unused enum case for state of SR .. In addition, make the relationship among several macros more explicit. Signed-off-by: Fredi Raspall --- ospfd/ospf_sr.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ospfd/ospf_sr.h b/ospfd/ospf_sr.h index d706d206fb..a5a6d38330 100644 --- a/ospfd/ospf_sr.h +++ b/ospfd/ospf_sr.h @@ -221,7 +221,7 @@ struct sr_local_block { enum sid_type { PREF_SID, LOCAL_SID, ADJ_SID, LAN_ADJ_SID }; /* Status of Segment Routing: Off (Disable), On (Enable), (Up) Started */ -enum sr_status { SR_OFF, SR_ON, SR_UP, SR_DOWN }; +enum sr_status { SR_OFF, SR_ON, SR_UP }; /* Structure aggregating all OSPF Segment Routing information for the node */ struct ospf_sr_db { From 5bd62f051de4f5f8fcd1632eb45ec6971cfb91c9 Mon Sep 17 00:00:00 2001 From: Fredi Raspall Date: Fri, 28 May 2021 11:35:09 +0200 Subject: [PATCH 03/15] ospfd: use existing macro on initialization of SR Signed-off-by: Fredi Raspall --- ospfd/ospf_sr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ospfd/ospf_sr.c b/ospfd/ospf_sr.c index 9a9e64cc23..38fe022293 100644 --- a/ospfd/ospf_sr.c +++ b/ospfd/ospf_sr.c @@ -581,7 +581,7 @@ int ospf_sr_init(void) OspfSR.srgb.reserved = false; OspfSR.srlb.start = DEFAULT_SRLB_LABEL; - OspfSR.srlb.end = DEFAULT_SRLB_LABEL + DEFAULT_SRLB_SIZE - 1; + OspfSR.srlb.end = DEFAULT_SRLB_END; OspfSR.srlb.reserved = false; OspfSR.msd = 0; From 5403ff15dbb519a64dc6ccabf4af4d0035605588 Mon Sep 17 00:00:00 2001 From: Fredi Raspall Date: Fri, 28 May 2021 11:32:24 +0200 Subject: [PATCH 04/15] ospfd: remove unused macro Signed-off-by: Fredi Raspall --- ospfd/ospf_sr.h | 1 - 1 file changed, 1 deletion(-) diff --git a/ospfd/ospf_sr.h b/ospfd/ospf_sr.h index a5a6d38330..1669562c33 100644 --- a/ospfd/ospf_sr.h +++ b/ospfd/ospf_sr.h @@ -38,7 +38,6 @@ #define SET_LABEL(label) ((label << 8) & SET_LABEL_MASK) #define GET_LABEL(label) ((label >> 8) & GET_LABEL_MASK) -#define OSPF_SR_DEFAULT_METRIC 1 /* Segment Routing TLVs as per RFC 8665 */ From 143661c17f083a479c793132adb793fcce3f0105 Mon Sep 17 00:00:00 2001 From: Fredi Raspall Date: Fri, 28 May 2021 13:56:49 +0200 Subject: [PATCH 05/15] ospfd: remove unnecessary assignment Signed-off-by: Fredi Raspall --- ospfd/ospf_sr.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/ospfd/ospf_sr.c b/ospfd/ospf_sr.c index 38fe022293..27ffdd4cf5 100644 --- a/ospfd/ospf_sr.c +++ b/ospfd/ospf_sr.c @@ -278,10 +278,8 @@ static int sr_local_block_init(uint32_t lower_bound, uint32_t upper_bound) * an error to disable SR until a new SRLB is successfully allocated. */ size = upper_bound - lower_bound + 1; - if (ospf_zebra_request_label_range(lower_bound, size)) { - srlb->reserved = false; + if (ospf_zebra_request_label_range(lower_bound, size)) return -1; - } osr_debug("SR (%s): Got new SRLB [%u/%u]", __func__, lower_bound, upper_bound); From c181efbe14cfa71efb3c263f8021c87a5fbfe7e1 Mon Sep 17 00:00:00 2001 From: Fredi Raspall Date: Fri, 28 May 2021 18:39:52 +0200 Subject: [PATCH 06/15] ospfd: homogenize deletion of SRGB & SRLB Homogenize the code dealing with SRGBs and SRLBs by defining the same set of utility functions for the deletion of SR blocks. Signed-off-by: Fredi Raspall --- ospfd/ospf_sr.c | 45 ++++++++++++++++++++++++++++++--------------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/ospfd/ospf_sr.c b/ospfd/ospf_sr.c index 27ffdd4cf5..cf95b8b778 100644 --- a/ospfd/ospf_sr.c +++ b/ospfd/ospf_sr.c @@ -320,9 +320,30 @@ static void sr_local_block_delete(void) /* Then reset SRLB structure */ if (srlb->used_mark != NULL) XFREE(MTYPE_OSPF_SR_PARAMS, srlb->used_mark); + srlb->reserved = false; } +/** + * Remove Segment Routing Global block + */ +static void sr_global_block_delete(void) +{ + struct sr_global_block *srgb = &OspfSR.srgb; + + if (!srgb->reserved) + return; + + osr_debug("SR (%s): Remove SRGB [%u/%u]", __func__, srgb->start, + srgb->start + srgb->size - 1); + + ospf_zebra_release_label_range(srgb->start, + srgb->start + srgb->size - 1); + + srgb->reserved = false; +} + + /** * Request a label from the Segment Routing Local Block. * @@ -532,13 +553,10 @@ static void ospf_sr_stop(void) /* Disable any re-attempt to connect to Label Manager */ THREAD_OFF(OspfSR.t_start_lm); - /* Release SRGB & SRLB 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; - } + /* Release SRGB if active */ + sr_global_block_delete(); + + /* Release SRLB if active */ sr_local_block_delete(); /* @@ -2134,12 +2152,8 @@ static int update_sr_blocks(uint32_t gb_lower, uint32_t gb_upper, /* 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; - } + + sr_global_block_delete(); /* Set new SRGB values - but do not reserve yet (we need to * release the SRLB too) */ @@ -2153,8 +2167,8 @@ static int update_sr_blocks(uint32_t gb_lower, uint32_t gb_upper, /* Release old SRLB if it has changed and reserve new block as needed. */ if (lb_changed) { - if (OspfSR.srlb.reserved) - sr_local_block_delete(); + + sr_local_block_delete(); /* Set new SRLB values */ if (sr_local_block_init(lb_lower, lb_upper) < 0) { @@ -2463,6 +2477,7 @@ DEFUN (sr_prefix_sid, if (srp == new && CHECK_FLAG(srp->flags, EXT_SUBTLV_PREFIX_SID_NPFLG) && !CHECK_FLAG(srp->flags, EXT_SUBTLV_PREFIX_SID_EFLG)) ospf_zebra_delete_prefix_sid(srp); + /* Then, reset Flag & labels to handle flag update */ new->flags = 0; new->label_in = 0; From daaebd394c6844da354f9897c91144f816392244 Mon Sep 17 00:00:00 2001 From: Fredi Raspall Date: Fri, 28 May 2021 19:22:40 +0200 Subject: [PATCH 07/15] ospfd: homogenize reservation of SRGB & SRLB Homogenize the code dealing with SRGBs and SRLBs by defining the same set of utility functions for their reservation. Unify also the logs and don't display function names since the operations are only performed from the same functions. Signed-off-by: Fredi Raspall --- ospfd/ospf_sr.c | 61 +++++++++++++++++++++++++++++++------------------ 1 file changed, 39 insertions(+), 22 deletions(-) diff --git a/ospfd/ospf_sr.c b/ospfd/ospf_sr.c index cf95b8b778..9faf9708cf 100644 --- a/ospfd/ospf_sr.c +++ b/ospfd/ospf_sr.c @@ -278,16 +278,20 @@ static int sr_local_block_init(uint32_t lower_bound, uint32_t upper_bound) * an error to disable SR until a new SRLB is successfully allocated. */ size = upper_bound - lower_bound + 1; - if (ospf_zebra_request_label_range(lower_bound, size)) + if (ospf_zebra_request_label_range(lower_bound, size)) { + zlog_err("SR: Error reserving SRLB [%u/%u] %u labels", + lower_bound, upper_bound, size); return -1; + } - osr_debug("SR (%s): Got new SRLB [%u/%u]", __func__, lower_bound, - upper_bound); + osr_debug("SR: Got new SRLB [%u/%u], %u labels", lower_bound, + upper_bound, size); /* Initialize the SRLB */ srlb->start = lower_bound; srlb->end = upper_bound; srlb->current = 0; + /* Compute the needed Used Mark number and allocate them */ srlb->max_block = size / SRLB_BLOCK_SIZE; if ((size % SRLB_BLOCK_SIZE) != 0) @@ -299,6 +303,31 @@ static int sr_local_block_init(uint32_t lower_bound, uint32_t upper_bound) return 0; } +static int sr_global_block_init(uint32_t start, uint32_t size) +{ + struct sr_global_block *srgb = &OspfSR.srgb; + + /* Check if already configured */ + if (srgb->reserved) + return 0; + + /* request chunk */ + uint32_t end = start + size - 1; + if (ospf_zebra_request_label_range(start, size) < 0) { + zlog_err("SR: Error reserving SRGB [%u/%u], %u labels", start, + end, size); + return -1; + } + + osr_debug("SR: Got new SRGB [%u/%u], %u labels", start, end, size); + + /* success */ + srgb->start = start; + srgb->size = size; + srgb->reserved = true; + return 0; +} + /** * Remove Segment Routing Local Block. * @@ -488,16 +517,11 @@ static int ospf_sr_start(struct ospf *ospf) * If the allocation fails, return an error to disable SR until a new * SRLB and/or SRGB are successfully allocated. */ - sr_local_block_init(OspfSR.srlb.start, OspfSR.srlb.end); - if (!OspfSR.srgb.reserved) { - if (ospf_zebra_request_label_range(OspfSR.srgb.start, - OspfSR.srgb.size) - < 0) { - OspfSR.srgb.reserved = false; - return -1; - } else - OspfSR.srgb.reserved = true; - } + if (sr_local_block_init(OspfSR.srlb.start, OspfSR.srlb.end) < 0) + return -1; + + if (sr_global_block_init(OspfSR.srgb.start, OspfSR.srgb.size) < 0) + return -1; /* SR is UP and ready to flood LSA */ OspfSR.status = SR_UP; @@ -2187,18 +2211,11 @@ static int update_sr_blocks(uint32_t gb_lower, uint32_t gb_upper, * allocated. */ if (gb_changed) { - if (ospf_zebra_request_label_range(OspfSR.srgb.start, - OspfSR.srgb.size) + if (sr_global_block_init(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); + } } /* Update Self SR-Node */ From 4e10b4dfba74c0984e1f3f289e8a6c0265cb6914 Mon Sep 17 00:00:00 2001 From: Fredi Raspall Date: Fri, 28 May 2021 22:51:01 +0200 Subject: [PATCH 08/15] ospfd: add util func to detect SR range overlap Replaces several complex if conditions by a lookup to a utility to determine if two ranges of numbers overlap. Signed-off-by: Fredi Raspall --- ospfd/ospf_sr.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/ospfd/ospf_sr.c b/ospfd/ospf_sr.c index 9faf9708cf..e19dbc7d3b 100644 --- a/ospfd/ospf_sr.c +++ b/ospfd/ospf_sr.c @@ -2139,6 +2139,13 @@ static int ospf_sr_enabled(struct vty *vty) return 0; } +/* tell if two ranges [r1_lower, r1_upper] and [r2_lower,r2_upper] overlap */ +static bool ranges_overlap(uint32_t r1_lower, uint32_t r1_upper, + uint32_t r2_lower, uint32_t r2_upper) +{ + return !((r1_upper < r2_lower) || (r1_lower > r2_upper)); +} + /** * Update SRGB and/or SRLB using new CLI values. * @@ -2263,9 +2270,9 @@ DEFUN(sr_global_label_range, sr_global_label_range_cmd, : OspfSR.srlb.start; /* Validate SRGB against SRLB */ - if (!((gb_upper < lb_lower) || (gb_lower > lb_upper))) { + if (ranges_overlap(gb_lower, gb_upper, lb_lower, lb_upper)) { vty_out(vty, - "New SR Global Block (%u/%u) conflict with Local Block (%u/%u)\n", + "New SR Global Block (%u/%u) conflicts with Local Block (%u/%u)\n", gb_lower, gb_upper, lb_lower, lb_upper); return CMD_WARNING_CONFIG_FAILED; } @@ -2323,9 +2330,10 @@ DEFUN_HIDDEN(sr_local_label_range, sr_local_label_range_cmd, /* Validate SRLB against SRGB */ srgb_upper = OspfSR.srgb.start + OspfSR.srgb.size - 1; - if (!((upper < OspfSR.srgb.start) || (lower > srgb_upper))) { + + if (ranges_overlap(OspfSR.srgb.start, srgb_upper, lower, upper)) { vty_out(vty, - "New SR Local Block (%u/%u) conflict with Global Block (%u/%u)\n", + "New SR Local Block (%u/%u) conflicts with Global Block (%u/%u)\n", lower, upper, OspfSR.srgb.start, srgb_upper); return CMD_WARNING_CONFIG_FAILED; } @@ -2348,10 +2356,10 @@ DEFUN_HIDDEN(no_sr_local_label_range, no_sr_local_label_range_cmd, /* Validate SRLB against SRGB */ srgb_end = OspfSR.srgb.start + OspfSR.srgb.size - 1; - if (!((DEFAULT_SRLB_END < OspfSR.srgb.start) - || (DEFAULT_SRLB_LABEL > srgb_end))) { + if (ranges_overlap(OspfSR.srgb.start, srgb_end, DEFAULT_SRLB_LABEL, + DEFAULT_SRLB_END)) { vty_out(vty, - "New SR Local Block (%u/%u) conflict with Global Block (%u/%u)\n", + "New SR Local Block (%u/%u) conflicts with Global Block (%u/%u)\n", DEFAULT_SRLB_LABEL, DEFAULT_SRLB_END, OspfSR.srgb.start, srgb_end); return CMD_WARNING_CONFIG_FAILED; From 37a331f8c2ba2ba72bed447fb80ee91a72dae8ec Mon Sep 17 00:00:00 2001 From: Fredi Raspall Date: Sat, 29 May 2021 03:22:04 +0200 Subject: [PATCH 09/15] ospfd: fix condition to get label from SRLB The prior condition was wrong since it ended up allowing for labels past the end of the SRLB. Variable 'current' should be in range [0, size-1] for labels not to exceed the SRLB upper boundary. In addition, emit a warning log when all labels in the SRLB have been used. Signed-off-by: Fredi Raspall --- ospfd/ospf_sr.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/ospfd/ospf_sr.c b/ospfd/ospf_sr.c index e19dbc7d3b..60485a770f 100644 --- a/ospfd/ospf_sr.c +++ b/ospfd/ospf_sr.c @@ -385,9 +385,10 @@ mpls_label_t ospf_sr_local_block_request_label(void) mpls_label_t label; uint32_t index; uint32_t pos; + uint32_t size = srlb->end - srlb->start + 1; /* Check if we ran out of available labels */ - if (srlb->current >= srlb->end) + if (srlb->current >= size) return MPLS_INVALID_LABEL; /* Get first available label and mark it used */ @@ -399,7 +400,7 @@ mpls_label_t ospf_sr_local_block_request_label(void) /* Jump to the next free position */ srlb->current++; pos = srlb->current % SRLB_BLOCK_SIZE; - while (srlb->current < srlb->end) { + while (srlb->current < size) { if (pos == 0) index++; if (!((1ULL << pos) & srlb->used_mark[index])) @@ -410,6 +411,10 @@ mpls_label_t ospf_sr_local_block_request_label(void) } } + if (srlb->current == size) + zlog_warn( + "SR: Warning, SRLB is depleted and next label request will fail"); + return label; } From e90c0383244bd9dfe6748c8921320f676697bfdc Mon Sep 17 00:00:00 2001 From: Fredi Raspall Date: Sun, 30 May 2021 17:13:54 +0200 Subject: [PATCH 10/15] isisd: fix condition to get label from SRLB The fix is the same as for OSPF SR. Signed-off-by: Fredi Raspall --- isisd/isis_sr.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/isisd/isis_sr.c b/isisd/isis_sr.c index f7cef43d0d..54e31f0040 100644 --- a/isisd/isis_sr.c +++ b/isisd/isis_sr.c @@ -550,13 +550,13 @@ static void sr_local_block_delete(struct isis_area *area) */ static mpls_label_t sr_local_block_request_label(struct sr_local_block *srlb) { - mpls_label_t label; uint32_t index; uint32_t pos; + uint32_t size = srlb->end - srlb->start + 1; /* Check if we ran out of available labels */ - if (srlb->current >= srlb->end) + if (srlb->current >= size) return MPLS_INVALID_LABEL; /* Get first available label and mark it used */ @@ -568,7 +568,7 @@ static mpls_label_t sr_local_block_request_label(struct sr_local_block *srlb) /* Jump to the next free position */ srlb->current++; pos = srlb->current % SRLB_BLOCK_SIZE; - while (srlb->current < srlb->end) { + while (srlb->current < size) { if (pos == 0) index++; if (!((1ULL << pos) & srlb->used_mark[index])) @@ -579,6 +579,10 @@ static mpls_label_t sr_local_block_request_label(struct sr_local_block *srlb) } } + if (srlb->current == size) + zlog_warn( + "SR: Warning, SRLB is depleted and next label request will fail"); + return label; } From 6c57d010739c1b4f5ce4905ecd7aaeaf6371a48b Mon Sep 17 00:00:00 2001 From: Fredi Raspall Date: Fri, 28 May 2021 23:06:50 +0200 Subject: [PATCH 11/15] ospfd: validate input SRGB and SRLB The code was not checking if in these label ranges [a,b], a is smaller than b, which is assumed in several places, including when determining the size of the block as b-a+1. As a consequence, the results of a bad configuration can be unpredictable. Some effects observed were: 1) segfault 2) de-activation of SR due to label reservation failure. The added validation function also checks if the SR blocks are larger than some minimal size. RFC 8665 mandates that the blocks be srictly larger than zero. In this patch, the minimum sized is arbitrarily defined to be 16. Checking if ranges would fall outside [16,1048575] is omitted since the vty filtering takes care of that. Signed-off-by: Fredi Raspall --- ospfd/ospf_sr.c | 26 ++++++++++++++++++++++++++ ospfd/ospf_sr.h | 3 +++ 2 files changed, 29 insertions(+) diff --git a/ospfd/ospf_sr.c b/ospfd/ospf_sr.c index 60485a770f..7d96dc1ba8 100644 --- a/ospfd/ospf_sr.c +++ b/ospfd/ospf_sr.c @@ -2151,6 +2151,13 @@ static bool ranges_overlap(uint32_t r1_lower, uint32_t r1_upper, return !((r1_upper < r2_lower) || (r1_lower > r2_upper)); } + +/* tell if a range is valid */ +static bool sr_range_is_valid(uint32_t lower, uint32_t upper, uint32_t min_size) +{ + return (upper >= lower + min_size); +} + /** * Update SRGB and/or SRLB using new CLI values. * @@ -2268,12 +2275,25 @@ DEFUN(sr_global_label_range, sr_global_label_range_cmd, /* 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; + /* check correctness of input SRGB */ + if (!sr_range_is_valid(gb_lower, gb_upper, MIN_SRGB_SIZE)) { + vty_out(vty, "Invalid SRGB range\n"); + return CMD_WARNING_CONFIG_FAILED; + } + + /* check correctness of SRLB */ + if (!sr_range_is_valid(lb_lower, lb_upper, MIN_SRLB_SIZE)) { + vty_out(vty, "Invalid SRLB range\n"); + return CMD_WARNING_CONFIG_FAILED; + } + /* Validate SRGB against SRLB */ if (ranges_overlap(gb_lower, gb_upper, lb_lower, lb_upper)) { vty_out(vty, @@ -2328,6 +2348,12 @@ DEFUN_HIDDEN(sr_local_label_range, sr_local_label_range_cmd, lower = strtoul(argv[idx_low]->arg, NULL, 10); upper = strtoul(argv[idx_up]->arg, NULL, 10); + /* check correctness of SRLB */ + if (!sr_range_is_valid(lower, upper, MIN_SRLB_SIZE)) { + vty_out(vty, "Invalid SRLB range\n"); + return CMD_WARNING_CONFIG_FAILED; + } + /* Check if values have changed */ if ((OspfSR.srlb.start == lower) && (OspfSR.srlb.end == upper)) diff --git a/ospfd/ospf_sr.h b/ospfd/ospf_sr.h index 1669562c33..b153a220f5 100644 --- a/ospfd/ospf_sr.h +++ b/ospfd/ospf_sr.h @@ -38,6 +38,9 @@ #define SET_LABEL(label) ((label << 8) & SET_LABEL_MASK) #define GET_LABEL(label) ((label >> 8) & GET_LABEL_MASK) +/* smallest configurable SRGB / SRLB sizes */ +#define MIN_SRLB_SIZE 16 +#define MIN_SRGB_SIZE 16 /* Segment Routing TLVs as per RFC 8665 */ From d1db7359ad40d36ad0be583b619fa60032a93597 Mon Sep 17 00:00:00 2001 From: Fredi Raspall Date: Mon, 31 May 2021 20:06:51 +0200 Subject: [PATCH 12/15] ospfd: fix logic on SR prefix configuration The logic was broken in some corner cases. Signed-off-by: Fredi Raspall --- ospfd/ospf_sr.c | 113 ++++++++++++++++++++++++++++-------------------- 1 file changed, 65 insertions(+), 48 deletions(-) diff --git a/ospfd/ospf_sr.c b/ospfd/ospf_sr.c index 7d96dc1ba8..60e02e6523 100644 --- a/ospfd/ospf_sr.c +++ b/ospfd/ospf_sr.c @@ -2480,11 +2480,15 @@ DEFUN (sr_prefix_sid, "Upstream neighbor must replace prefix-sid with explicit null label\n") { int idx = 0; - struct prefix p; + struct prefix p, pexist; uint32_t index; struct listnode *node; - struct sr_prefix *srp, *new = NULL; + struct sr_prefix *srp, *exist = NULL; struct interface *ifp; + bool no_php_flag = false; + bool exp_null = false; + bool index_in_use = false; + uint8_t desired_flags = 0; if (!ospf_sr_enabled(vty)) return CMD_WARNING_CONFIG_FAILED; @@ -2505,54 +2509,67 @@ DEFUN (sr_prefix_sid, return CMD_WARNING_CONFIG_FAILED; } + /* Get options */ + no_php_flag = argv_find(argv, argc, "no-php-flag", &idx); + exp_null = argv_find(argv, argc, "explicit-null", &idx); + + desired_flags |= no_php_flag ? EXT_SUBTLV_PREFIX_SID_NPFLG : 0; + desired_flags |= exp_null ? EXT_SUBTLV_PREFIX_SID_NPFLG : 0; + desired_flags |= exp_null ? EXT_SUBTLV_PREFIX_SID_EFLG : 0; + /* Search for an existing Prefix-SID */ for (ALL_LIST_ELEMENTS_RO(OspfSR.self->ext_prefix, node, srp)) { + if (prefix_same((struct prefix *)&srp->prefv4, &p)) + exist = srp; if (srp->sid == index) { - if (prefix_same((struct prefix *)&srp->prefv4, &p)) { - new = srp; - break; - } else { - vty_out(vty, "Index %u is already used\n", - index); - return CMD_WARNING_CONFIG_FAILED; - } + index_in_use = true; + pexist = p; } } - /* Create new Extended Prefix to SRDB if not found */ - if (new == NULL) { - new = XCALLOC(MTYPE_OSPF_SR_PARAMS, sizeof(struct sr_prefix)); - IPV4_ADDR_COPY(&new->prefv4.prefix, &p.u.prefix4); - new->prefv4.prefixlen = p.prefixlen; - new->prefv4.family = p.family; - new->sid = index; - new->type = LOCAL_SID; + /* done if prefix segment already there with same index and flags */ + if (exist && exist->sid == index && exist->flags == desired_flags) + return CMD_SUCCESS; + + /* deny if index is already in use by a distinct prefix */ + if (!exist && index_in_use) { + vty_out(vty, "Index %u is already used by %pFX\n", index, + &pexist); + return CMD_WARNING_CONFIG_FAILED; } /* First, remove old NHLFE if installed */ - if (srp == new && CHECK_FLAG(srp->flags, EXT_SUBTLV_PREFIX_SID_NPFLG) - && !CHECK_FLAG(srp->flags, EXT_SUBTLV_PREFIX_SID_EFLG)) - ospf_zebra_delete_prefix_sid(srp); + if (exist && CHECK_FLAG(exist->flags, EXT_SUBTLV_PREFIX_SID_NPFLG) + && !CHECK_FLAG(exist->flags, EXT_SUBTLV_PREFIX_SID_EFLG)) + ospf_zebra_delete_prefix_sid(exist); - /* Then, reset Flag & labels to handle flag update */ - new->flags = 0; - new->label_in = 0; - new->nhlfe.label_out = 0; - - /* Set NO PHP flag if present and compute NHLFE */ - if (argv_find(argv, argc, "no-php-flag", &idx)) { - SET_FLAG(new->flags, EXT_SUBTLV_PREFIX_SID_NPFLG); - new->label_in = index2label(new->sid, OspfSR.self->srgb); - new->nhlfe.label_out = MPLS_LABEL_IMPLICIT_NULL; + /* Create new Extended Prefix to SRDB if not found */ + if (exist == NULL) { + srp = XCALLOC(MTYPE_OSPF_SR_PARAMS, sizeof(struct sr_prefix)); + IPV4_ADDR_COPY(&srp->prefv4.prefix, &p.u.prefix4); + srp->prefv4.prefixlen = p.prefixlen; + srp->prefv4.family = p.family; + srp->sid = index; + srp->type = LOCAL_SID; + } else { + /* we work on the existing SR prefix */ + srp = exist; } - /* Set EXPLICIT NULL flag is present */ - if (argv_find(argv, argc, "explicit-null", &idx)) { - SET_FLAG(new->flags, EXT_SUBTLV_PREFIX_SID_NPFLG); - SET_FLAG(new->flags, EXT_SUBTLV_PREFIX_SID_EFLG); + + /* Reset labels to handle flag update */ + srp->label_in = 0; + srp->nhlfe.label_out = 0; + srp->sid = index; + srp->flags = desired_flags; + + /* If NO PHP flag is present, compute NHLFE and set label */ + if (no_php_flag) { + srp->label_in = index2label(srp->sid, OspfSR.self->srgb); + srp->nhlfe.label_out = MPLS_LABEL_IMPLICIT_NULL; } osr_debug("SR (%s): Add new index %u to Prefix %pFX", __func__, index, - (struct prefix *)&new->prefv4); + (struct prefix *)&srp->prefv4); /* Get Interface and check if it is a Loopback */ ifp = if_lookup_prefix(&p, VRF_DEFAULT); @@ -2563,7 +2580,8 @@ DEFUN (sr_prefix_sid, * ready. In this case, store the prefix SID for latter * update of this Extended Prefix */ - listnode_add(OspfSR.self->ext_prefix, new); + if (exist == NULL) + listnode_add(OspfSR.self->ext_prefix, srp); zlog_info( "Interface for prefix %pFX not found. Deferred LSA flooding", &p); @@ -2572,27 +2590,26 @@ DEFUN (sr_prefix_sid, if (!if_is_loopback(ifp)) { vty_out(vty, "interface %s is not a Loopback\n", ifp->name); - XFREE(MTYPE_OSPF_SR_PARAMS, new); + XFREE(MTYPE_OSPF_SR_PARAMS, srp); return CMD_WARNING_CONFIG_FAILED; } - new->nhlfe.ifindex = ifp->ifindex; + srp->nhlfe.ifindex = ifp->ifindex; - /* Add this new SR Prefix if not already found */ - if (srp != new) - listnode_add(OspfSR.self->ext_prefix, new); + /* Add SR Prefix if new */ + if (!exist) + listnode_add(OspfSR.self->ext_prefix, srp); /* Update Prefix SID if SR is UP */ if (OspfSR.status == SR_UP) { - if (CHECK_FLAG(new->flags, EXT_SUBTLV_PREFIX_SID_NPFLG) - && !CHECK_FLAG(new->flags, EXT_SUBTLV_PREFIX_SID_EFLG)) - ospf_zebra_update_prefix_sid(new); + if (no_php_flag && !exp_null) + ospf_zebra_update_prefix_sid(srp); } else return CMD_SUCCESS; /* Finally, update Extended Prefix LSA id SR is UP */ - new->instance = ospf_ext_schedule_prefix_index( - ifp, new->sid, &new->prefv4, new->flags); - if (new->instance == 0) { + srp->instance = ospf_ext_schedule_prefix_index( + ifp, srp->sid, &srp->prefv4, srp->flags); + if (srp->instance == 0) { vty_out(vty, "Unable to set index %u for prefix %pFX\n", index, &p); return CMD_WARNING; From 152656d8ef57dda11f8b84241de28bca410e891b Mon Sep 17 00:00:00 2001 From: Fredi Raspall Date: Mon, 31 May 2021 22:17:16 +0200 Subject: [PATCH 13/15] ospfd: replace iterator by RO version .. ..since it's used for read-only lookups. Signed-off-by: Fredi Raspall --- ospfd/ospf_ext.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ospfd/ospf_ext.c b/ospfd/ospf_ext.c index 2d08eeece2..6c0d0a19d4 100644 --- a/ospfd/ospf_ext.c +++ b/ospfd/ospf_ext.c @@ -254,10 +254,10 @@ static uint32_t get_ext_link_instance_value(void) /* Lookup Extended Prefix/Links by ifp from OspfEXT struct iflist */ static struct ext_itf *lookup_ext_by_ifp(struct interface *ifp) { - struct listnode *node, *nnode; + struct listnode *node; struct ext_itf *exti; - for (ALL_LIST_ELEMENTS(OspfEXT.iflist, node, nnode, exti)) + for (ALL_LIST_ELEMENTS_RO(OspfEXT.iflist, node, exti)) if (exti->ifp == ifp) return exti; From 1a41378eccde679efc2ca5c04e3f0e4ea1eef53b Mon Sep 17 00:00:00 2001 From: Fredi Raspall Date: Mon, 31 May 2021 22:38:07 +0200 Subject: [PATCH 14/15] ospfd: remove duplicate field update In update_ext_prefix_sid(), the sr_prefix is associated to the SR node and inherits the adv router ID regardless. Signed-off-by: Fredi Raspall --- ospfd/ospf_sr.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/ospfd/ospf_sr.c b/ospfd/ospf_sr.c index 60e02e6523..fb2d078823 100644 --- a/ospfd/ospf_sr.c +++ b/ospfd/ospf_sr.c @@ -1276,9 +1276,6 @@ static void update_ext_prefix_sid(struct sr_node *srn, struct sr_prefix *srp) /* Replace Segment Prefix */ listnode_delete(srn->ext_prefix, pref); XFREE(MTYPE_OSPF_SR_PARAMS, pref); - srp->srn = srn; - IPV4_ADDR_COPY(&srp->adv_router, - &srn->adv_router); listnode_add(srn->ext_prefix, srp); ospf_zebra_update_prefix_sid(srp); } else { From d97e415dd5f2ffccf6e500f678b0efe25c909af3 Mon Sep 17 00:00:00 2001 From: Fredi Raspall Date: Tue, 1 Jun 2021 10:59:36 +0200 Subject: [PATCH 15/15] ospfd: fix processing of ext prefix in SR The existing logic was not comparing the prefix of the extended prefix TLV. As a result, the code was removing all of the prefix SIDs except the one received on every LSA update. Signed-off-by: Fredi Raspall --- ospfd/ospf_sr.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ospfd/ospf_sr.c b/ospfd/ospf_sr.c index fb2d078823..67e6630c83 100644 --- a/ospfd/ospf_sr.c +++ b/ospfd/ospf_sr.c @@ -1245,7 +1245,9 @@ static void update_ext_prefix_sid(struct sr_node *srn, struct sr_prefix *srp) /* Search for existing Segment Prefix */ for (ALL_LIST_ELEMENTS_RO(srn->ext_prefix, node, pref)) - if (pref->instance == srp->instance) { + if (pref->instance == srp->instance + && prefix_same((struct prefix *)&srp->prefv4, + &pref->prefv4)) { found = true; break; }