BGP: Do not apply jitter to HoldTimerky/bgp-fix-timer-jitter

Per RFC 4271 section 10:
> To minimize the likelihood that the distribution of BGP messages by a
> given BGP speaker will contain peaks, jitter SHOULD be applied to the
> timers associated with MinASOriginationIntervalTimer, KeepaliveTimer,
> MinRouteAdvertisementIntervalTimer, and ConnectRetryTimer.

3 files changed, 19 insertions, 16 deletions

diff --git a/proto/bgp/bgp.c b/proto/bgp/bgp.c
index cd57c32b..8e8f63e9 100644
--- a/proto/bgp/bgp.c
+++ b/proto/bgp/bgp.c
@@ -312,7 +312,7 @@ bgp_initiate(struct bgp_proto *p)
   {
     p->start_state = BSS_DELAY;
     BGP_TRACE(D_EVENTS, "Startup delayed by %d seconds due to errors", p->startup_delay);
-    bgp_start_timer(p->startup_timer, p->startup_delay);
+    bgp_start_timer(p->startup_timer, p->startup_delay, 1);
   }
   else
     bgp_startup(p);
@@ -335,21 +335,24 @@ err1:
  * bgp_start_timer - start a BGP timer
  * @t: timer
  * @value: time (in seconds) to fire (0 to disable the timer)
+ * @use_randomize: the randomization procedure in RFC 4271 should be applied
  *
  * This functions calls tm_start() on @t with time @value and the amount of
  * randomization suggested by the BGP standard. Please use it for all BGP
  * timers.
  */
 void
-bgp_start_timer(timer *t, uint value)
+bgp_start_timer(timer *t, uint value, int use_randomize)
 {
-  if (value)
+  if (value && use_randomize)
   {
     /* The randomization procedure is specified in RFC 4271 section 10 */
     btime time = value S;
     btime randomize = random() % ((time / 4) + 1);
     tm_start(t, time - randomize);
   }
+  else if (value)
+    tm_start(t, value S);
   else
     tm_stop(t);
 }
@@ -716,7 +719,7 @@ bgp_conn_enter_close_state(struct bgp_conn *conn)
   conn->sk->rx_hook = NULL;
 
   /* Timeout for CLOSE state, if we cannot send notification soon then we just hangup */
-  bgp_start_timer(conn->hold_timer, 10);
+  bgp_start_timer(conn->hold_timer, 10, 0);
 
   if (os == BS_ESTABLISHED)
     bgp_conn_leave_established_state(conn, p);
@@ -952,7 +955,7 @@ bgp_send_open(struct bgp_conn *conn)
   bgp_prepare_capabilities(conn);
   bgp_schedule_packet(conn, NULL, PKT_OPEN);
   bgp_conn_set_state(conn, BS_OPENSENT);
-  bgp_start_timer(conn->hold_timer, conn->bgp->cf->initial_hold_time);
+  bgp_start_timer(conn->hold_timer, conn->bgp->cf->initial_hold_time, 0);
 }
 
 static void
@@ -1029,7 +1032,7 @@ bgp_hold_timeout(timer *t)
      and perhaps just not processed BGP packets in time. */
 
   if (sk_rx_ready(conn->sk) > 0)
-    bgp_start_timer(conn->hold_timer, 10);
+    bgp_start_timer(conn->hold_timer, 10, 0);
   else if ((conn->state == BS_ESTABLISHED) && p->llgr_ready)
   {
     BGP_TRACE(D_EVENTS, "Hold timer expired");
@@ -1111,7 +1114,7 @@ bgp_active(struct bgp_proto *p)
   BGP_TRACE(D_EVENTS, "Connect delayed by %d seconds", delay);
   bgp_setup_conn(p, conn);
   bgp_conn_set_state(conn, BS_ACTIVE);
-  bgp_start_timer(conn->connect_timer, delay);
+  bgp_start_timer(conn->connect_timer, delay, 1);
 }
 
 /**
@@ -1159,7 +1162,7 @@ bgp_connect(struct bgp_proto *p)	/* Enter Connect state and start establishing c
       goto err;
 
   DBG("BGP: Waiting for connect success\n");
-  bgp_start_timer(conn->connect_timer, p->cf->connect_retry_time);
+  bgp_start_timer(conn->connect_timer, p->cf->connect_retry_time, 1);
   return;
 
 err:
diff --git a/proto/bgp/bgp.h b/proto/bgp/bgp.h
index 7127bc88..c29b8f6d 100644
--- a/proto/bgp/bgp.h
+++ b/proto/bgp/bgp.h
@@ -552,7 +552,7 @@ bgp_parse_error(struct bgp_parse_state *s, uint subcode)
 }
 
 
-void bgp_start_timer(timer *t, uint value);
+void bgp_start_timer(timer *t, uint value, int use_randomize);
 void bgp_check_config(struct bgp_config *c);
 void bgp_error(struct bgp_conn *c, unsigned code, unsigned subcode, byte *data, int len);
 void bgp_close_conn(struct bgp_conn *c);
diff --git a/proto/bgp/packets.c b/proto/bgp/packets.c
index e8cc4718..62fcd015 100644
--- a/proto/bgp/packets.c
+++ b/proto/bgp/packets.c
@@ -1047,8 +1047,8 @@ bgp_rx_open(struct bgp_conn *conn, byte *pkt, uint len)
       conn->hold_time, conn->keepalive_time, p->remote_as, p->remote_id, conn->as4_session);
 
   bgp_schedule_packet(conn, NULL, PKT_KEEPALIVE);
-  bgp_start_timer(conn->hold_timer, conn->hold_time);
-  bgp_start_timer(conn->send_hold_timer, conn->send_hold_time);
+  bgp_start_timer(conn->hold_timer, conn->hold_time, 0);
+  bgp_start_timer(conn->send_hold_timer, conn->send_hold_time, 0);
   bgp_conn_enter_openconfirm_state(conn);
 }
 
@@ -2776,7 +2776,7 @@ bgp_rx_update(struct bgp_conn *conn, byte *pkt, uint len)
   if (conn->state != BS_ESTABLISHED)
   { bgp_error(conn, 5, fsm_err_subcode[conn->state], NULL, 0); return; }
 
-  bgp_start_timer(conn->hold_timer, conn->hold_time);
+  bgp_start_timer(conn->hold_timer, conn->hold_time, 0);
 
   struct lp_state tmpp;
   lp_save(tmp_linpool, &tmpp);
@@ -3068,7 +3068,7 @@ 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->send_hold_timer, conn->send_hold_time);
+    bgp_start_timer(conn->send_hold_timer, conn->send_hold_time, 0);
 
   return success;
 }
@@ -3124,7 +3124,7 @@ bgp_fire_tx(struct bgp_conn *conn)
   {
     conn->packets_to_send &= ~(1 << PKT_KEEPALIVE);
     BGP_TRACE(D_PACKETS, "Sending KEEPALIVE");
-    bgp_start_timer(conn->keepalive_timer, conn->keepalive_time);
+    bgp_start_timer(conn->keepalive_timer, conn->keepalive_time, 1);
     return bgp_send(conn, PKT_KEEPALIVE, BGP_HEADER_LENGTH);
   }
   else while (conn->channels_to_send)
@@ -3232,7 +3232,7 @@ bgp_tx(sock *sk)
 
   /* Pending message was passed to kernel */
   if ((conn->state == BS_ESTABLISHED) || (conn->state == BS_OPENCONFIRM))
-    bgp_start_timer(conn->send_hold_timer, conn->send_hold_time);
+    bgp_start_timer(conn->send_hold_timer, conn->send_hold_time, 0);
 
   DBG("BGP: TX hook\n");
   uint max = 1024;
@@ -3435,7 +3435,7 @@ bgp_rx_keepalive(struct bgp_conn *conn)
   struct bgp_proto *p = conn->bgp;
 
   BGP_TRACE(D_PACKETS, "Got KEEPALIVE");
-  bgp_start_timer(conn->hold_timer, conn->hold_time);
+  bgp_start_timer(conn->hold_timer, conn->hold_time, 0);
 
   if (conn->state == BS_OPENCONFIRM)
   { bgp_conn_enter_established_state(conn); return; }