From 06525c4f99d4dcafdf448565f7e11bd70993697d Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Wed, 5 Oct 2022 10:04:11 -0400 Subject: [PATCH] zebra: Add `zrouter.asic_notification_nexthop_control` Volta submitted notification changes for the dplane that had a special use case for their system. Volta is no more, the code is not being actively developed and from talking with ex-Volta employees there is no current plans to even maintain this code. Wrap the special handling of nexthops that their asic-dataplane did in a bit of code to isolate it and allow for future removal, as that I do not actually believe anyone else is using this code. Add a CPP_NOTICE several years into the future that will tell us to remove the code. If someone starts using it then they will have to notice this variable to set it and hopefully they will see my CPP_NOTICE to come talk to us. If this is being used then we can just remove this wrapper. Signed-off-by: Donald Sharp --- zebra/zebra_rib.c | 95 +++++++++++++++++++++++++------------------- zebra/zebra_router.c | 11 +++++ zebra/zebra_router.h | 8 ++++ zebra/zebra_vty.c | 9 +++++ 4 files changed, 83 insertions(+), 40 deletions(-) diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c index c3ab47cf4..a8caf0eee 100644 --- a/zebra/zebra_rib.c +++ b/zebra/zebra_rib.c @@ -2339,55 +2339,70 @@ static void rib_process_dplane_notify(struct zebra_dplane_ctx *ctx) /* Various fib transitions: changed nexthops; from installed to * not-installed; or not-installed to installed. */ - if (start_count > 0 && end_count > 0) { - if (debug_p) - zlog_debug( - "%s(%u:%u):%pRN applied nexthop changes from dplane notification", - VRF_LOGNAME(vrf), dplane_ctx_get_vrf(ctx), - dplane_ctx_get_table(ctx), rn); + if (zrouter.asic_notification_nexthop_control) { + if (start_count > 0 && end_count > 0) { + if (debug_p) + zlog_debug( + "%s(%u:%u):%pRN applied nexthop changes from dplane notification", + VRF_LOGNAME(vrf), + dplane_ctx_get_vrf(ctx), + dplane_ctx_get_table(ctx), rn); - /* Changed nexthops - update kernel/others */ - dplane_route_notif_update(rn, re, - DPLANE_OP_ROUTE_UPDATE, ctx); + /* Changed nexthops - update kernel/others */ + dplane_route_notif_update(rn, re, + DPLANE_OP_ROUTE_UPDATE, ctx); - } else if (start_count == 0 && end_count > 0) { - if (debug_p) - zlog_debug( - "%s(%u:%u):%pRN installed transition from dplane notification", - VRF_LOGNAME(vrf), dplane_ctx_get_vrf(ctx), - dplane_ctx_get_table(ctx), rn); + } else if (start_count == 0 && end_count > 0) { + if (debug_p) + zlog_debug( + "%s(%u:%u):%pRN installed transition from dplane notification", + VRF_LOGNAME(vrf), + dplane_ctx_get_vrf(ctx), + dplane_ctx_get_table(ctx), rn); - /* We expect this to be the selected route, so we want - * to tell others about this transition. - */ - SET_FLAG(re->status, ROUTE_ENTRY_INSTALLED); + /* We expect this to be the selected route, so we want + * to tell others about this transition. + */ + SET_FLAG(re->status, ROUTE_ENTRY_INSTALLED); - /* Changed nexthops - update kernel/others */ - dplane_route_notif_update(rn, re, DPLANE_OP_ROUTE_UPDATE, ctx); + /* Changed nexthops - update kernel/others */ + dplane_route_notif_update(rn, re, + DPLANE_OP_ROUTE_UPDATE, ctx); - /* Redistribute, lsp, and nht update */ - redistribute_update(rn, re, NULL); + /* Redistribute, lsp, and nht update */ + redistribute_update(rn, re, NULL); - } else if (start_count > 0 && end_count == 0) { - if (debug_p) - zlog_debug( - "%s(%u:%u):%pRN un-installed transition from dplane notification", - VRF_LOGNAME(vrf), dplane_ctx_get_vrf(ctx), - dplane_ctx_get_table(ctx), rn); + } else if (start_count > 0 && end_count == 0) { + if (debug_p) + zlog_debug( + "%s(%u:%u):%pRN un-installed transition from dplane notification", + VRF_LOGNAME(vrf), + dplane_ctx_get_vrf(ctx), + dplane_ctx_get_table(ctx), rn); - /* Transition from _something_ installed to _nothing_ - * installed. - */ - /* We expect this to be the selected route, so we want - * to tell others about this transistion. - */ - UNSET_FLAG(re->status, ROUTE_ENTRY_INSTALLED); + /* Transition from _something_ installed to _nothing_ + * installed. + */ + /* We expect this to be the selected route, so we want + * to tell others about this transistion. + */ + UNSET_FLAG(re->status, ROUTE_ENTRY_INSTALLED); - /* Changed nexthops - update kernel/others */ - dplane_route_notif_update(rn, re, DPLANE_OP_ROUTE_DELETE, ctx); + /* Changed nexthops - update kernel/others */ + dplane_route_notif_update(rn, re, + DPLANE_OP_ROUTE_DELETE, ctx); - /* Redistribute, lsp, and nht update */ - redistribute_delete(rn, re, NULL); + /* Redistribute, lsp, and nht update */ + redistribute_delete(rn, re, NULL); + } + } + + if (!zebra_router_notify_on_ack()) { + if (CHECK_FLAG(re->flags, ZEBRA_FLAG_OFFLOADED)) + zsend_route_notify_owner_ctx(ctx, ZAPI_ROUTE_INSTALLED); + if (CHECK_FLAG(re->flags, ZEBRA_FLAG_OFFLOAD_FAILED)) + zsend_route_notify_owner_ctx(ctx, + ZAPI_ROUTE_FAIL_INSTALL); } /* Make any changes visible for lsp and nexthop-tracking processing */ diff --git a/zebra/zebra_router.c b/zebra/zebra_router.c index b8923ef57..a9a7b66ce 100644 --- a/zebra/zebra_router.c +++ b/zebra/zebra_router.c @@ -330,6 +330,17 @@ void zebra_router_init(bool asic_offload, bool notify_on_ack) zrouter.asic_offloaded = asic_offload; zrouter.notify_on_ack = notify_on_ack; + /* + * If you start using asic_notification_nexthop_control + * come talk to the FRR community about what you are doing + * We would like to know. + */ +#if CONFDATE > 20251231 + CPP_NOTICE( + "Remove zrouter.asic_notification_nexthop_control as that it's not being maintained or used"); +#endif + zrouter.asic_notification_nexthop_control = false; + #ifdef HAVE_SCRIPTING zebra_script_init(); #endif diff --git a/zebra/zebra_router.h b/zebra/zebra_router.h index 069437ef4..e0ef86f08 100644 --- a/zebra/zebra_router.h +++ b/zebra/zebra_router.h @@ -224,6 +224,14 @@ struct zebra_router { bool asic_offloaded; bool notify_on_ack; + /* + * If the asic is notifying us about successful nexthop + * allocation/control. Some developers have made their + * asic take control of how many nexthops/ecmp they can + * have and will report what is successfull or not + */ + bool asic_notification_nexthop_control; + bool supports_nhgs; bool all_mc_forwardingv4, default_mc_forwardingv4; diff --git a/zebra/zebra_vty.c b/zebra/zebra_vty.c index 91a0c1dd3..997cd0bdc 100644 --- a/zebra/zebra_vty.c +++ b/zebra/zebra_vty.c @@ -4073,6 +4073,15 @@ DEFUN (show_zebra, ttable_add_row(table, "ASIC offload|%s", zrouter.asic_offloaded ? "Used" : "Unavailable"); + /* + * Do not display this unless someone is actually using it + * + * Why this distinction? I think this is effectively dead code + * and should not be exposed. Maybe someone proves me wrong. + */ + if (zrouter.asic_notification_nexthop_control) + ttable_add_row(table, "ASIC offload and nexthop control|Used"); + ttable_add_row(table, "RA|%s", rtadv_compiled_in() ? "Compiled in" : "Not Compiled in"); ttable_add_row(table, "RFC 5549|%s", -- 2.39.5