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 <sharpd@nvidia.com>

diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c
index f90668d854db87321417b54737d786a085de3cda..aa9a6a8602cb73736a63d77ec17be12875257fa2 100644 (file)
--- 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);