From aff8eaa4a2dec64a66ea7349dfffac1b0dac2820 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Fri, 25 Oct 2024 21:21:32 -0400 Subject: [PATCH 1/3] sharpd: Allow sharpd to watch nexthops in the mrib Nothing special here, just allow sharpd to ask to watch nexthops in the mrib. Signed-off-by: Donald Sharp --- sharpd/sharp_vty.c | 16 ++++++++-------- sharpd/sharp_zebra.c | 11 ++++++----- sharpd/sharp_zebra.h | 4 ++-- 3 files changed, 16 insertions(+), 15 deletions(-) diff --git a/sharpd/sharp_vty.c b/sharpd/sharp_vty.c index 21c596bf7124..c9211a152ffb 100644 --- a/sharpd/sharp_vty.c +++ b/sharpd/sharp_vty.c @@ -82,7 +82,7 @@ DEFPY(watch_redistribute, watch_redistribute_cmd, } DEFPY(watch_nexthop_v6, watch_nexthop_v6_cmd, - "sharp watch [vrf NAME$vrf_name] [connected$connected]", + "sharp watch [vrf NAME$vrf_name] [connected$connected] [mrib$mrib]", "Sharp routing Protocol\n" "Watch for changes\n" "The vrf we would like to watch if non-default\n" @@ -91,7 +91,8 @@ DEFPY(watch_nexthop_v6, watch_nexthop_v6_cmd, "The v6 nexthop to signal for watching\n" "Watch for import check changes\n" "The v6 prefix to signal for watching\n" - "Should the route be connected\n") + "Should the route be connected\n" + "In the Multicast rib\n") { struct vrf *vrf; struct prefix p; @@ -119,14 +120,13 @@ DEFPY(watch_nexthop_v6, watch_nexthop_v6_cmd, } sharp_nh_tracker_get(&p); - sharp_zebra_nexthop_watch(&p, vrf->vrf_id, type_import, - true, !!connected); + sharp_zebra_nexthop_watch(&p, vrf->vrf_id, type_import, true, !!connected, !!mrib); return CMD_SUCCESS; } DEFPY(watch_nexthop_v4, watch_nexthop_v4_cmd, - "sharp watch [vrf NAME$vrf_name] [connected$connected]", + "sharp watch [vrf NAME$vrf_name] [connected$connected] [mrib$mrib]", "Sharp routing Protocol\n" "Watch for changes\n" "The vrf we would like to watch if non-default\n" @@ -135,7 +135,8 @@ DEFPY(watch_nexthop_v4, watch_nexthop_v4_cmd, "The v4 address to signal for watching\n" "Watch for import check changes\n" "The v4 prefix for import check to watch\n" - "Should the route be connected\n") + "Should the route be connected\n" + "In the Multicast rib\n") { struct vrf *vrf; struct prefix p; @@ -164,8 +165,7 @@ DEFPY(watch_nexthop_v4, watch_nexthop_v4_cmd, } sharp_nh_tracker_get(&p); - sharp_zebra_nexthop_watch(&p, vrf->vrf_id, type_import, - true, !!connected); + sharp_zebra_nexthop_watch(&p, vrf->vrf_id, type_import, true, !!connected, !!mrib); return CMD_SUCCESS; } diff --git a/sharpd/sharp_zebra.c b/sharpd/sharp_zebra.c index 1048436b4313..4447b69bf619 100644 --- a/sharpd/sharp_zebra.c +++ b/sharpd/sharp_zebra.c @@ -618,18 +618,19 @@ void nhg_del(uint32_t id) zclient_nhg_send(zclient, ZEBRA_NHG_DEL, &api_nhg); } -void sharp_zebra_nexthop_watch(struct prefix *p, vrf_id_t vrf_id, bool import, - bool watch, bool connected) +void sharp_zebra_nexthop_watch(struct prefix *p, vrf_id_t vrf_id, bool import, bool watch, + bool connected, bool mrib) { - int command; + int command = ZEBRA_NEXTHOP_REGISTER; + safi_t safi = mrib ? SAFI_MULTICAST : SAFI_UNICAST; command = ZEBRA_NEXTHOP_REGISTER; if (!watch) command = ZEBRA_NEXTHOP_UNREGISTER; - if (zclient_send_rnh(zclient, command, p, SAFI_UNICAST, connected, - false, vrf_id) == ZCLIENT_SEND_FAILURE) + if (zclient_send_rnh(zclient, command, p, safi, connected, false, vrf_id) == + ZCLIENT_SEND_FAILURE) zlog_warn("%s: Failure to send nexthop to zebra", __func__); } diff --git a/sharpd/sharp_zebra.h b/sharpd/sharp_zebra.h index 5cbcc146654c..7a86897beb54 100644 --- a/sharpd/sharp_zebra.h +++ b/sharpd/sharp_zebra.h @@ -18,8 +18,8 @@ extern void vrf_label_add(vrf_id_t vrf_id, afi_t afi, mpls_label_t label); extern void nhg_add(uint32_t id, const struct nexthop_group *nhg, const struct nexthop_group *backup_nhg); extern void nhg_del(uint32_t id); -extern void sharp_zebra_nexthop_watch(struct prefix *p, vrf_id_t vrf_id, - bool import, bool watch, bool connected); +extern void sharp_zebra_nexthop_watch(struct prefix *p, vrf_id_t vrf_id, bool import, bool watch, + bool connected, bool mrib); extern void sharp_install_routes_helper(struct prefix *p, vrf_id_t vrf_id, uint8_t instance, uint32_t nhgid, From 811168ecc3850a159fc22beaf30e42be63111413 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Fri, 25 Oct 2024 15:44:34 -0400 Subject: [PATCH 2/3] zebra: Add safi to some debugs Trying to figure out what safi we are talking about is fun when it is not put into the debugs. Signed-off-by: Donald Sharp --- zebra/zebra_rib.c | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c index 4c1401124bc2..8acb07374d6b 100644 --- a/zebra/zebra_rib.c +++ b/zebra/zebra_rib.c @@ -1264,8 +1264,15 @@ static void rib_process(struct route_node *rn) struct zebra_vrf *zvrf = NULL; struct vrf *vrf; struct route_entry *proto_re_changed = NULL; - vrf_id_t vrf_id = VRF_UNKNOWN; + safi_t safi = SAFI_UNICAST; + + if (IS_ZEBRA_DEBUG_RIB || IS_ZEBRA_DEBUG_RIB_DETAILED) { + struct rib_table_info *info = srcdest_rnode_table_info(rn); + + assert(info); + safi = info->safi; + } assert(rn); @@ -1291,9 +1298,8 @@ static void rib_process(struct route_node *rn) if (IS_ZEBRA_DEBUG_RIB_DETAILED) { struct route_entry *re = re_list_first(&dest->routes); - zlog_debug("%s(%u:%u):%pRN: Processing rn %p", - VRF_LOGNAME(vrf), vrf_id, re->table, rn, - rn); + zlog_debug("%s(%u:%u:%u):%pRN: Processing rn %p", VRF_LOGNAME(vrf), vrf_id, + re->table, safi, rn, rn); } old_fib = dest->selected_fib; @@ -1303,15 +1309,12 @@ static void rib_process(struct route_node *rn) char flags_buf[128]; char status_buf[128]; - zlog_debug( - "%s(%u:%u):%pRN: Examine re %p (%s) status: %sflags: %sdist %d metric %d", - VRF_LOGNAME(vrf), vrf_id, re->table, rn, re, - zebra_route_string(re->type), - _dump_re_status(re, status_buf, - sizeof(status_buf)), - zclient_dump_route_flags(re->flags, flags_buf, - sizeof(flags_buf)), - re->distance, re->metric); + zlog_debug("%s(%u:%u:%u):%pRN: Examine re %p (%s) status: %sflags: %sdist %d metric %d", + VRF_LOGNAME(vrf), vrf_id, re->table, safi, rn, re, + zebra_route_string(re->type), + _dump_re_status(re, status_buf, sizeof(status_buf)), + zclient_dump_route_flags(re->flags, flags_buf, sizeof(flags_buf)), + re->distance, re->metric); } /* Currently selected re. */ @@ -1435,11 +1438,10 @@ static void rib_process(struct route_node *rn) : old_fib ? old_fib : new_fib ? new_fib : NULL; - zlog_debug( - "%s(%u:%u):%pRN: After processing: old_selected %p new_selected %p old_fib %p new_fib %p", - VRF_LOGNAME(vrf), vrf_id, entry ? entry->table : 0, rn, - (void *)old_selected, (void *)new_selected, - (void *)old_fib, (void *)new_fib); + zlog_debug("%s(%u:%u:%u):%pRN: After processing: old_selected %p new_selected %p old_fib %p new_fib %p", + VRF_LOGNAME(vrf), vrf_id, entry ? entry->table : 0, safi, rn, + (void *)old_selected, (void *)new_selected, (void *)old_fib, + (void *)new_fib); } /* Buffer ROUTE_ENTRY_CHANGED here, because it will get cleared if From 3bff65abc75e952a15b98666e7271189ac28a2bc Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Fri, 25 Oct 2024 21:56:14 -0400 Subject: [PATCH 3/3] zebra: When installing a mroute, allow it to flow Currently the mroute code was not allowing the mroute to be sent to the dataplane. This leaves us with a situation where the routes being installed where never being set as installed and additionally nht against the mrib would not work if the route came into existence after the nexthop tracking was asked for. Turns out all the pieces where there to let this work. Modify the code to pass it to the dplane and to send it back up as having worked. Signed-off-by: Donald Sharp --- zebra/dplane_fpm_nl.c | 12 +++++++++++- zebra/rt_netlink.c | 3 +++ zebra/rt_socket.c | 8 ++++---- zebra/zebra_rib.c | 16 ---------------- 4 files changed, 18 insertions(+), 21 deletions(-) diff --git a/zebra/dplane_fpm_nl.c b/zebra/dplane_fpm_nl.c index 8a967978cb31..e6b4af367429 100644 --- a/zebra/dplane_fpm_nl.c +++ b/zebra/dplane_fpm_nl.c @@ -1712,6 +1712,16 @@ static int fpm_nl_process(struct zebra_dplane_provider *prov) * anyway. */ if (fnc->socket != -1 && fnc->connecting == false) { + enum dplane_op_e op = dplane_ctx_get_op(ctx); + + /* + * Just skip multicast routes and let them flow through + */ + if ((op == DPLANE_OP_ROUTE_DELETE || op == DPLANE_OP_ROUTE_INSTALL || + op == DPLANE_OP_ROUTE_UPDATE) && + dplane_ctx_get_safi(ctx) == SAFI_MULTICAST) + goto skip; + frr_with_mutex (&fnc->ctxqueue_mutex) { dplane_ctx_enqueue_tail(&fnc->ctxqueue, ctx); cur_queue = @@ -1722,7 +1732,7 @@ static int fpm_nl_process(struct zebra_dplane_provider *prov) peak_queue = cur_queue; continue; } - +skip: dplane_ctx_set_status(ctx, ZEBRA_DPLANE_REQUEST_SUCCESS); dplane_provider_enqueue_out_ctx(prov, ctx); } diff --git a/zebra/rt_netlink.c b/zebra/rt_netlink.c index dc679ed4954d..ab07ef8d2124 100644 --- a/zebra/rt_netlink.c +++ b/zebra/rt_netlink.c @@ -3178,6 +3178,9 @@ netlink_put_route_update_msg(struct nl_batch *bth, struct zebra_dplane_ctx *ctx) } else return FRR_NETLINK_ERROR; + if (dplane_ctx_get_safi(ctx) == SAFI_MULTICAST) + return FRR_NETLINK_SUCCESS; + if (RSYSTEM_ROUTE(dplane_ctx_get_type(ctx))) return FRR_NETLINK_SUCCESS; diff --git a/zebra/rt_socket.c b/zebra/rt_socket.c index 0bfcd518cade..4444eda94ba3 100644 --- a/zebra/rt_socket.c +++ b/zebra/rt_socket.c @@ -317,12 +317,12 @@ enum zebra_dplane_result kernel_route_update(struct zebra_dplane_ctx *ctx) frr_with_privs(&zserv_privs) { if (dplane_ctx_get_op(ctx) == DPLANE_OP_ROUTE_DELETE) { - if (!RSYSTEM_ROUTE(type)) + if (!RSYSTEM_ROUTE(type) && dplane_ctx_get_safi(ctx) != SAFI_MULTICAST) kernel_rtm(RTM_DELETE, dplane_ctx_get_dest(ctx), dplane_ctx_get_ng(ctx), dplane_ctx_get_metric(ctx)); } else if (dplane_ctx_get_op(ctx) == DPLANE_OP_ROUTE_INSTALL) { - if (!RSYSTEM_ROUTE(type)) + if (!RSYSTEM_ROUTE(type) && dplane_ctx_get_safi(ctx) != SAFI_MULTICAST) kernel_rtm(RTM_ADD, dplane_ctx_get_dest(ctx), dplane_ctx_get_ng(ctx), dplane_ctx_get_metric(ctx)); @@ -330,12 +330,12 @@ enum zebra_dplane_result kernel_route_update(struct zebra_dplane_ctx *ctx) /* Must do delete and add separately - * no update available */ - if (!RSYSTEM_ROUTE(old_type)) + if (!RSYSTEM_ROUTE(old_type) && dplane_ctx_get_safi(ctx) != SAFI_MULTICAST) kernel_rtm(RTM_DELETE, dplane_ctx_get_dest(ctx), dplane_ctx_get_old_ng(ctx), dplane_ctx_get_old_metric(ctx)); - if (!RSYSTEM_ROUTE(type)) + if (!RSYSTEM_ROUTE(type) && dplane_ctx_get_safi(ctx) != SAFI_MULTICAST) kernel_rtm(RTM_ADD, dplane_ctx_get_dest(ctx), dplane_ctx_get_ng(ctx), dplane_ctx_get_metric(ctx)); diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c index 8acb07374d6b..aea39b8ecf18 100644 --- a/zebra/zebra_rib.c +++ b/zebra/zebra_rib.c @@ -637,19 +637,12 @@ int zebra_rib_labeled_unicast(struct route_entry *re) void rib_install_kernel(struct route_node *rn, struct route_entry *re, struct route_entry *old) { - struct nexthop *nexthop; struct rib_table_info *info = srcdest_rnode_table_info(rn); struct zebra_vrf *zvrf = zebra_vrf_lookup_by_id(re->vrf_id); enum zebra_dplane_result ret; rib_dest_t *dest = rib_dest_from_rnode(rn); - if (info->safi != SAFI_UNICAST) { - for (ALL_NEXTHOPS(re->nhe->nhg, nexthop)) - SET_FLAG(nexthop->flags, NEXTHOP_FLAG_FIB); - return; - } - /* * Install the resolved nexthop object first. */ @@ -716,17 +709,8 @@ void rib_install_kernel(struct route_node *rn, struct route_entry *re, /* Uninstall the route from kernel. */ void rib_uninstall_kernel(struct route_node *rn, struct route_entry *re) { - struct nexthop *nexthop; - struct rib_table_info *info = srcdest_rnode_table_info(rn); struct zebra_vrf *zvrf = zebra_vrf_lookup_by_id(re->vrf_id); - if (info->safi != SAFI_UNICAST) { - UNSET_FLAG(re->status, ROUTE_ENTRY_INSTALLED); - for (ALL_NEXTHOPS(re->nhe->nhg, nexthop)) - UNSET_FLAG(nexthop->flags, NEXTHOP_FLAG_FIB); - return; - } - /* * Make sure we update the FPM any time we send new information to * the dataplane.