From a01120fc9a9922b5cc9ae1bdbb7a8c0272f514ed Mon Sep 17 00:00:00 2001 From: Katerina Kubecova Date: Tue, 3 Dec 2024 15:37:56 +0100 Subject: [PATCH] bgp/bgp.c, bgp/packets.c, io-loop.c: solving slow shutdown when bird is busy --- proto/bgp/bgp.c | 2 +- proto/bgp/packets.c | 16 ++++++++++------ sysdep/unix/io-loop.c | 38 ++++++++++++++++++++++++++++++++------ 3 files changed, 43 insertions(+), 13 deletions(-) diff --git a/proto/bgp/bgp.c b/proto/bgp/bgp.c index 408053d5..bb1ff099 100644 --- a/proto/bgp/bgp.c +++ b/proto/bgp/bgp.c @@ -862,7 +862,7 @@ bgp_conn_enter_close_state(struct bgp_conn *conn) bgp_conn_set_state(conn, BS_CLOSE); tm_stop(conn->keepalive_timer); - conn->sk->rx_hook = NULL; + //conn->sk->rx_hook = NULL; // deleting rx_hook here may cause problems with pool listening /* Timeout for CLOSE state, if we cannot send notification soon then we just hangup */ bgp_start_timer(p, conn->hold_timer, 10); diff --git a/proto/bgp/packets.c b/proto/bgp/packets.c index 8a61a1c5..5069bdf2 100644 --- a/proto/bgp/packets.c +++ b/proto/bgp/packets.c @@ -3094,6 +3094,8 @@ bgp_send(struct bgp_conn *conn, uint type, uint len) int success = sk_send(sk, len); if (success && ((conn->state == BS_ESTABLISHED) || (conn->state == BS_OPENCONFIRM))) bgp_start_timer(conn->bgp, conn->send_hold_timer, conn->send_hold_time); + struct bgp_proto *p = conn->bgp; + BGP_TRACE(D_PACKETS, "Scheduled packet type %d %s", type, success ? "succesfuly" : "unsuccesfully"); return success; } @@ -3234,12 +3236,11 @@ bgp_schedule_packet(struct bgp_conn *conn, struct bgp_channel *c, int type) ASSERT(conn->sk); struct bgp_proto *p = conn->bgp; + bool wont_ping = conn->channels_to_send || conn->packets_to_send || (conn->sk->tpos != conn->sk->tbuf); if (c) - BGP_TRACE(D_PACKETS, "Scheduling packet type %d for channel %s", type, c->c.name); + BGP_TRACE(D_PACKETS, "Scheduling packet type %d for channel %s%s", type, c->c.name, wont_ping ? " (TX already scheduled)" : ""); else - BGP_TRACE(D_PACKETS, "Scheduling packet type %d", type); - - bool was_active = conn->channels_to_send || conn->packets_to_send; + BGP_TRACE(D_PACKETS, "Scheduling packet type %d %s", type, wont_ping ? " (TX already scheduled)" : ""); if (c) { @@ -3255,7 +3256,7 @@ bgp_schedule_packet(struct bgp_conn *conn, struct bgp_channel *c, int type) else conn->packets_to_send |= 1 << type; - if (was_active || (conn->sk->tpos != conn->sk->tbuf)) + if (wont_ping && (type != PKT_SCHEDULE_CLOSE)) return; else if ((type == PKT_KEEPALIVE) || (this_birdloop != p->p.loop)) while (bgp_fire_tx(conn) > 0) @@ -3572,7 +3573,10 @@ bgp_rx(sock *sk, uint size) while (end >= pkt_start + BGP_HEADER_LENGTH) { if ((conn->state == BS_CLOSE) || (conn->sk != sk)) - return 0; + { + BGP_TRACE(D_PACKETS,"Packet arrived after closing"); + return 0; + } if ((conn->state == BS_ESTABLISHED) && rt_cork_check(&conn->bgp->uncork)) { sk_pause_rx(p->p.loop, sk); diff --git a/sysdep/unix/io-loop.c b/sysdep/unix/io-loop.c index 6dca5f3c..9210efba 100644 --- a/sysdep/unix/io-loop.c +++ b/sysdep/unix/io-loop.c @@ -535,8 +535,16 @@ sockets_fire(struct birdloop *loop, bool read, bool write) if (write && (rev & POLLOUT)) { /* Write until task limit is up */ - while ((s == loop->sock_active) && (e = sk_write(s)) && task_still_in_limit()) - ; + while (s == loop->sock_active) + { + int fd = s->fd; /* The socket may disappear after sk_write! */ + e = sk_write(s); + bool allowed = task_still_in_limit(); + LOOP_TRACE(loop, DL_SOCKETS, "Writing to fd %d: %s%s", fd, e ? "again" : "done", (e && !allowed) ? " (out of time)" : ""); + + if (!e || !allowed) + break; + } if (s != loop->sock_active) continue; @@ -547,11 +555,21 @@ sockets_fire(struct birdloop *loop, bool read, bool write) /* Read until task limit is up */ if (read && (rev & POLLIN)) - while ((s == loop->sock_active) && s->rx_hook && sk_read(s, rev) && (s->fast_rx || task_still_in_limit())) - ; + { - if (s != loop->sock_active) - continue; + while (s == loop->sock_active && s->rx_hook ) + { + int fd = s->fd; /* The socket may disappear after sk_write! */ + e = sk_read(s, rev); + bool allowed = task_still_in_limit(); + LOOP_TRACE(loop, DL_SOCKETS, "Read from fd %d: %s%s", fd, e ? "again" : "done", (e && !allowed) ? " (out of time)" : ""); + + if (!e || !allowed) + break; + } + if (s != loop->sock_active) + continue; + } if (!(rev & (POLLOUT | POLLIN)) && (rev & POLLERR)) sk_err(s, rev); @@ -914,7 +932,15 @@ bird_thread_main(void *arg) account_to(&this_thread->idle); birdloop_leave(thr->meta); poll_retry:; + for (uint i=0; i