From: Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject: bgpd: sendHoldTimer cleanup
To: tech@openbsd.org
Date: Mon, 25 Mar 2024 13:37:08 +0100

While reviewing draft-ietf-idr-bgp-sendholdtimer-03 I noticed that our
implementation of SendHoldTimer could be a bit better. Especially we need
to be careful to disable the timer when HoldTime is negotiated to 0.

Now the draft itself only applies the SendHoldTimer when in state
Established while we do it all the time. This does not really matter
IMO that's why I did not add the extra check for that.

-- 
:wq Claudio

Index: session.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/session.c,v
diff -u -p -r1.466 session.c
--- session.c	22 Mar 2024 15:41:34 -0000	1.466
+++ session.c	25 Mar 2024 11:07:36 -0000
@@ -58,6 +58,7 @@ void	session_sighdlr(int);
 int	setup_listeners(u_int *);
 void	init_peer(struct peer *);
 void	start_timer_holdtime(struct peer *);
+void	start_timer_sendholdtime(struct peer *);
 void	start_timer_keepalive(struct peer *);
 void	session_close_connection(struct peer *);
 void	change_state(struct peer *, enum session_state, enum session_events);
@@ -836,6 +837,20 @@ start_timer_holdtime(struct peer *peer)
 }
 
 void
+start_timer_sendholdtime(struct peer *peer)
+{
+	uint16_t holdtime = INTERVAL_HOLD;
+
+	if (peer->holdtime > INTERVAL_HOLD)
+		holdtime = peer->holdtime;
+
+	if (peer->holdtime > 0)
+		timer_set(&peer->timers, Timer_SendHold, holdtime);
+	else
+		timer_stop(&peer->timers, Timer_SendHold);
+}
+
+void
 start_timer_keepalive(struct peer *peer)
 {
 	if (peer->holdtime > 0)
@@ -1929,10 +1944,7 @@ session_dispatch_msg(struct pollfd *pfd,
 			return (1);
 		}
 		p->stats.last_write = getmonotime();
-		if (p->holdtime > 0)
-			timer_set(&p->timers, Timer_SendHold,
-			    p->holdtime < INTERVAL_HOLD ? INTERVAL_HOLD :
-			    p->holdtime);
+		start_timer_sendholdtime(p);
 		if (p->throttled && p->wbuf.queued < SESS_MSG_LOW_MARK) {
 			if (imsg_rde(IMSG_XON, p->conf.id, NULL, 0) == -1)
 				log_peer_warn(&p->conf, "imsg_compose XON");