From bc77c3bb8abb15ff29c5e93a97e08fe3b7a3d1e9 Mon Sep 17 00:00:00 2001 From: Mark Stapp Date: Thu, 10 Jun 2021 16:48:22 -0400 Subject: [PATCH 1/4] zebra: use const in rib_match Use const in common rib_match api. Signed-off-by: Mark Stapp --- zebra/rib.h | 2 +- zebra/zebra_rib.c | 3 ++- 2 files changed, 3 insertions(+), 2 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_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; From 6fb3580882bef9cf94aa69a4b202eb9ea5307898 Mon Sep 17 00:00:00 2001 From: Mark Stapp Date: Thu, 10 Jun 2021 16:49:56 -0400 Subject: [PATCH 2/4] zebra: add boolean to control pw reachability checking Add a boolean to control whether pseudowire reachability checking needs to be strict. Signed-off-by: Mark Stapp --- zebra/zebra_mpls.c | 2 ++ zebra/zebra_mpls.h | 1 + zebra/zebra_mpls_openbsd.c | 3 +++ 3 files changed, 6 insertions(+) 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; } From 0d145d47c89263caeb2ae21fe09ac6d5e99de789 Mon Sep 17 00:00:00 2001 From: Mark Stapp Date: Wed, 10 Mar 2021 10:36:46 -0500 Subject: [PATCH 3/4] zebra: revise pw reachability logic Modify the pseudowire reachability logic so that it returns success if there is at least one installed labelled nexthop for the route resolving the pw destination. We also check for valid backup nexthops if necessary, in case there's been a switchover event. Only OpenBSD requires that _all_ nexthops be labelled, so we have a more strict version of the logic also. Signed-off-by: Mark Stapp --- zebra/zebra_pw.c | 126 +++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 112 insertions(+), 14 deletions(-) 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) { - struct route_entry *re; - struct nexthop *nexthop; + 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 */ - /* find route to the remote end of the pseudowire */ + /* 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; + 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 */ 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; } From 072b487b8f1b07829ccc547627ef7a0aa84ab289 Mon Sep 17 00:00:00 2001 From: Mark Stapp Date: Fri, 4 Jun 2021 13:15:50 -0400 Subject: [PATCH 4/4] zebra: update pw dataplane info Include the complete set of primary and backup nexthops from the resolving route for a pseudowire. Add accessors for that info. Modify the logic that creates the fib set of pw nexthops so that only installed, labelled nexthops are included. Signed-off-by: Mark Stapp --- zebra/zebra_dplane.c | 139 ++++++++++++++++++++++++++++++++++--------- zebra/zebra_dplane.h | 4 ++ 2 files changed, 116 insertions(+), 27 deletions(-) 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.nhg.nexthop = NULL; + 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.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; - if (re) { - nhg = rib_get_fib_nhg(re); - if (nhg && nhg->nexthop) - copy_nexthops(&(ctx->u.pw.nhg.nexthop), - nhg->nexthop, NULL); + rn = route_node_match(table, &p); + if (rn == NULL) + goto done; - /* 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); - } - route_unlock_node(rn); - } + re = NULL; + RNODE_FOREACH_RE(rn, re) { + if (CHECK_FLAG(re->flags, ZEBRA_FLAG_SELECTED)) + break; } - return AOK; + 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; + } + } + + /* 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; + } + } + + /* 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); + + 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);