From 452a014f62e0893b05536fb2209a5ea08b9d2efd Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Wed, 22 May 2024 18:10:50 +0200 Subject: [PATCH] zebra: batch stream notifications messages There is a CPU issue in ZEBRA when BGP installs and removes a lot of routes at the same time. The vtysh and shell become unreachable. This is the case of BGP failover scenarios with two peers, and one of the peers becoming unreachable. The following message may appear: > 2024/05/22 16:12:42.628688 ZEBRA: [QH9AB-Y4XMZ][EC 100663314] STARVATION: > task zebra_route_process_notify_thread_loop (561f0118472f) ran for 5091ms (cpu time 40ms) This means that the task in charge of notifying the clients about a route (un) install success/failure takes too much time. Batch the stream messages as much as we can, to decrease the CPU time. Signed-off-by: Philippe Guibert --- zebra/rib.h | 1 + zebra/zapi_msg.c | 9 +++-- zebra/zapi_msg.h | 3 +- zebra/zebra_rib.c | 84 +++++++++++++++++++++++++++++++++++++++++++---- zebra/zebra_vty.c | 1 + 5 files changed, 88 insertions(+), 10 deletions(-) diff --git a/zebra/rib.h b/zebra/rib.h index 77f0eda7b6d6..4ec47bdc3aed 100644 --- a/zebra/rib.h +++ b/zebra/rib.h @@ -632,6 +632,7 @@ extern int rib_add_gr_run(afi_t afi, vrf_id_t vrf_id, uint8_t proto, extern void zebra_vty_init(void); extern void zebra_rnh_job_list_display(struct vty *vty); extern void zebra_route_notify_job_owner_list_display(struct vty *vty); +extern void zebra_process_notify_client_list_display(struct vty *vty); extern void zebra_route_notify_job_owner_list_enqueue(struct route_node *rn, const struct zebra_dplane_ctx *ctx, diff --git a/zebra/zapi_msg.c b/zebra/zapi_msg.c index 1ca1eecd9ad4..bfbcf5c96f68 100644 --- a/zebra/zapi_msg.c +++ b/zebra/zapi_msg.c @@ -727,7 +727,7 @@ int route_notify_internal_prefix(const struct prefix *p, int type, uint16_t instance, vrf_id_t vrf_id, uint32_t table_id, enum zapi_route_notify_owner note, afi_t afi, - safi_t safi) + safi_t safi, struct stream_fifo *out_fifo) { struct zserv *client; struct stream *s; @@ -771,7 +771,10 @@ int route_notify_internal_prefix(const struct prefix *p, int type, stream_putw_at(s, 0, stream_get_endp(s)); - return zserv_send_message(client, s); + if (out_fifo == NULL) + return zserv_send_message(client, s); + stream_fifo_push(out_fifo, s); + return 1; } static int route_notify_internal(const struct route_node *rn, int type, @@ -781,7 +784,7 @@ static int route_notify_internal(const struct route_node *rn, int type, safi_t safi) { return route_notify_internal_prefix(&rn->p, type, instance, vrf_id, - table_id, note, afi, safi); + table_id, note, afi, safi, false); } int zsend_route_notify_owner(const struct route_node *rn, diff --git a/zebra/zapi_msg.h b/zebra/zapi_msg.h index 51e7ddb8a062..ec2c0110fcc1 100644 --- a/zebra/zapi_msg.h +++ b/zebra/zapi_msg.h @@ -114,7 +114,8 @@ extern int route_notify_internal_prefix(const struct prefix *p, int type, uint16_t instance, vrf_id_t vrf_id, uint32_t table_id, enum zapi_route_notify_owner note, - afi_t afi, safi_t safi); + afi_t afi, safi_t safi, + struct stream_fifo *out_fifo); #ifdef __cplusplus } #endif diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c index 0c2890df4089..d5647a009656 100644 --- a/zebra/zebra_rib.c +++ b/zebra/zebra_rib.c @@ -58,6 +58,8 @@ DEFINE_MTYPE_STATIC(ZEBRA, RIB_UPDATE_CTX, "Rib update context object"); DEFINE_MTYPE_STATIC(ZEBRA, WQ_WRAPPER, "WQ wrapper"); DEFINE_MTYPE_STATIC(ZEBRA, RNH_JOB_CTX, "Rnh Job context"); DEFINE_MTYPE_STATIC(ZEBRA, ROUTE_NOTIFY_JOB_OWNER_CTX, "Route Notify Job Owner"); +DEFINE_MTYPE_STATIC(ZEBRA, PROCESS_NOTIFY_CLIENT_LIST, + "Process Notify Client List"); /* * Event, list, and mutex for delivery of dataplane results @@ -5053,11 +5055,39 @@ void zebra_route_notify_job_owner_list_display(struct vty *vty) zebra_route_notify_job_owner_list_max_batch); } +PREDECL_DLIST(zebra_process_notify_client_list); + +struct zebra_process_notify_client_ctx { + uint16_t num_msgs; + int instance; + int type; + struct zserv *zclient; + struct stream_fifo out_fifo; + /* Embedded list linkage */ + struct zebra_process_notify_client_list_item pnc_entries; +}; +DECLARE_DLIST(zebra_process_notify_client_list, + struct zebra_process_notify_client_ctx, pnc_entries); +static uint32_t zebra_process_notify_client_list_num; +static uint32_t zebra_process_notify_client_list_processed; + +void zebra_process_notify_client_list_display(struct vty *vty) +{ + vty_out(vty, "Process Notify Client list count %u, processed %u\n", + zebra_process_notify_client_list_num, + zebra_process_notify_client_list_processed); +} + static void zebra_route_process_notify_thread_loop(struct event *event) { struct zebra_route_notify_job_owner_list_head ctxlist; struct zebra_route_notify_job_owner_ctx *ctx; uint32_t count = 0; + struct zebra_process_notify_client_list_head client_list; + struct zebra_process_notify_client_ctx *client; + struct zserv *zclient; + + zebra_process_notify_client_list_init(&client_list); do { zebra_route_notify_job_owner_list_init(&ctxlist); @@ -5075,18 +5105,60 @@ static void zebra_route_process_notify_thread_loop(struct event *event) break; while (ctx) { zebra_route_notify_job_owner_list_processed++; - count++; - route_notify_internal_prefix(&ctx->prefix, ctx->type, - ctx->instance, ctx->vrf_id, - ctx->table, ctx->note, - ctx->afi, ctx->safi); + zclient = zserv_find_client(ctx->type, ctx->instance); + if (zclient && zclient->notify_owner) { + frr_each_safe (zebra_process_notify_client_list, + &client_list, client) { + if (client->type == ctx->type && + client->instance == ctx->instance) + break; + } + if (!client) { + client = XCALLOC( + MTYPE_PROCESS_NOTIFY_CLIENT_LIST, + sizeof(struct zebra_process_notify_client_ctx)); + stream_fifo_init(&client->out_fifo); + client->instance = ctx->instance; + client->type = ctx->type; + client->zclient = zclient; + zebra_process_notify_client_list_add_tail( + &client_list, client); + } + route_notify_internal_prefix(&ctx->prefix, + ctx->type, + ctx->instance, + ctx->vrf_id, + ctx->table, + ctx->note, + ctx->afi, ctx->safi, + &client->out_fifo); + client->num_msgs++; + count++; + zebra_process_notify_client_list_num++; + if (client->num_msgs == 20) { + zserv_send_batch(zclient, + &client->out_fifo); + zebra_process_notify_client_list_processed++; + client->num_msgs = 0; + } + } XFREE(MTYPE_ROUTE_NOTIFY_JOB_OWNER_CTX, ctx); ctx = zebra_route_notify_job_owner_list_pop(&ctxlist); } } while (1); - if (count > zebra_route_notify_job_owner_list_max_batch) { + if (count > zebra_route_notify_job_owner_list_max_batch) zebra_route_notify_job_owner_list_max_batch = count; + + while ((client = zebra_process_notify_client_list_pop(&client_list)) != + NULL) { + if (client->num_msgs) { + zserv_send_batch(client->zclient, &client->out_fifo); + zebra_process_notify_client_list_processed++; + client->num_msgs = 0; + } + stream_fifo_deinit(&client->out_fifo); + XFREE(MTYPE_PROCESS_NOTIFY_CLIENT_LIST, client); } } diff --git a/zebra/zebra_vty.c b/zebra/zebra_vty.c index 275ece0dbba3..bb59dd2a3343 100644 --- a/zebra/zebra_vty.c +++ b/zebra/zebra_vty.c @@ -4093,6 +4093,7 @@ DEFUN(show_rib_info, show_rib_info_cmd, "show rib info", { zebra_rnh_job_list_display(vty); zebra_route_notify_job_owner_list_display(vty); + zebra_process_notify_client_list_display(vty); return CMD_SUCCESS; }