From aa554d4b656cd01c1df69d7fb9eb5f9aead202a2 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Thu, 16 Mar 2023 19:19:04 -0400 Subject: [PATCH] bgpd: Always restart timer from scratch in OpenConfirm/Established Imagine this scenario: A peer has very large hold/keepalive timers of 600/200. This peer is using the DataCenter default time. As such the open will cause the t_holdtime to be negotiated to 600 seconds. Now also imagine that both peers are in update-delay. If we do not restart the timers and both peers are in Update Delay, we will continously reset the peer because the hold time will be hit( since the peer is not sending us any data ). Signed-off-by: Donald Sharp --- bgpd/bgp_fsm.c | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c index f90668d854..aa9a6a8602 100644 --- a/bgpd/bgp_fsm.c +++ b/bgpd/bgp_fsm.c @@ -435,12 +435,16 @@ void bgp_timer_set(struct peer *peer) THREAD_OFF(peer->t_start); THREAD_OFF(peer->t_connect); - /* If the negotiated Hold Time value is zero, then the Hold Time - timer and KeepAlive timers are not started. */ - if (peer->v_holdtime == 0) { - THREAD_OFF(peer->t_holdtime); + /* + * If the negotiated Hold Time value is zero, then the Hold Time + * timer and KeepAlive timers are not started. + * Additionally if a different hold timer has been negotiated + * than we must stop then start the timer again + */ + THREAD_OFF(peer->t_holdtime); + if (peer->v_holdtime == 0) bgp_keepalives_off(peer); - } else { + else { BGP_TIMER_ON(peer->t_holdtime, bgp_holdtime_timer, peer->v_holdtime); bgp_keepalives_on(peer); @@ -456,12 +460,16 @@ void bgp_timer_set(struct peer *peer) THREAD_OFF(peer->t_connect); THREAD_OFF(peer->t_delayopen); - /* Same as OpenConfirm, if holdtime is zero then both holdtime - and keepalive must be turned off. */ - if (peer->v_holdtime == 0) { - THREAD_OFF(peer->t_holdtime); + /* + * Same as OpenConfirm, if holdtime is zero then both holdtime + * and keepalive must be turned off. + * Additionally if a different hold timer has been negotiated + * then we must stop then start the timer again + */ + THREAD_OFF(peer->t_holdtime); + if (peer->v_holdtime == 0) bgp_keepalives_off(peer); - } else { + else { BGP_TIMER_ON(peer->t_holdtime, bgp_holdtime_timer, peer->v_holdtime); bgp_keepalives_on(peer); -- 2.39.5