Index | Thread | Search

From:
Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject:
bgpd: switch mrt reopen timer to the timer API
To:
tech@openbsd.org
Date:
Mon, 3 Nov 2025 20:36:00 +0100

Download raw body.

Thread
This bothered me for a while, the main poll loop in bgpd did not use the
timer api but instead used some hand rolled stuff.

This fixes this and should also fix CID 492342 while there.
-- 
:wq Claudio

Index: bgpd.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.c,v
diff -u -p -r1.283 bgpd.c
--- bgpd.c	24 Apr 2025 20:24:12 -0000	1.283
+++ bgpd.c	3 Nov 2025 15:51:53 -0000
@@ -118,7 +118,7 @@ usage(void)
 #define PFD_SOCK_ROUTE		3
 #define PFD_SOCK_PFKEY		4
 #define PFD_CONNECT_START	5
-#define MAX_TIMEOUT		(3600 * 1000)
+#define MAX_TIMEOUT		3600
 
 int	 cmd_opts;
 
@@ -131,7 +131,7 @@ main(int argc, char *argv[])
 	struct peer		*p;
 	struct pollfd		*pfd = NULL;
 	struct connect_elm	*ce;
-	time_t			 timeout;
+	monotime_t		 timeout;
 	pid_t			 se_pid = 0, rde_pid = 0, rtr_pid = 0, pid;
 	const char		*conffile;
 	char			*saved_argv0;
@@ -337,7 +337,10 @@ BROKEN	if (pledge("stdio rpath wpath cpa
 		}
 		memset(pfd, 0, sizeof(struct pollfd) * pfd_elms);
 
-		timeout = mrt_timeout(conf->mrt);
+		timeout = monotime_add(getmonotime(),
+		    monotime_from_sec(MAX_TIMEOUT));
+
+		timeout = mrt_timeout(conf->mrt, timeout);
 
 		pfd[PFD_SOCK_ROUTE].fd = rfd;
 		pfd[PFD_SOCK_ROUTE].events = POLLIN;
@@ -357,9 +360,11 @@ BROKEN	if (pledge("stdio rpath wpath cpa
 				fatalx("polli pfd overflow");
 		}
 
-		if (timeout < 0 || timeout > MAX_TIMEOUT)
-			timeout = MAX_TIMEOUT;
-		if (poll(pfd, npfd, timeout) == -1) {
+		timeout = monotime_sub(timeout, getmonotime());
+		if (!monotime_valid(timeout))
+			timeout = monotime_clear();
+
+		if (poll(pfd, npfd, monotime_to_msec(timeout)) == -1) {
 			if (errno != EINTR) {
 				log_warn("poll error");
 				quit = 1;
Index: bgpd.h
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
diff -u -p -r1.520 bgpd.h
--- bgpd.h	30 Oct 2025 12:43:18 -0000	1.520
+++ bgpd.h	3 Nov 2025 16:26:46 -0000
@@ -248,6 +248,8 @@ struct listen_addr {
 	uint8_t				flags;
 };
 
+TAILQ_HEAD(timer_head, timer);
+
 TAILQ_HEAD(listen_addrs, listen_addr);
 TAILQ_HEAD(filter_set_head, filter_set);
 
@@ -1447,7 +1449,7 @@ struct mrt_config {
 	struct mrt		conf;
 	char			name[MRT_FILE_LEN];	/* base file name */
 	char			file[MRT_FILE_LEN];	/* actual file name */
-	time_t			ReopenTimer;
+	struct timer_head	timer;
 	int			ReopenTimerInterval;
 };
 
@@ -1529,7 +1531,7 @@ void		 log_peer_warnx(const struct peer_
 void		 mrt_write(struct mrt *);
 void		 mrt_clean(struct mrt *);
 void		 mrt_init(struct imsgbuf *, struct imsgbuf *);
-time_t		 mrt_timeout(struct mrt_head *);
+monotime_t	 mrt_timeout(struct mrt_head *, monotime_t);
 void		 mrt_reconfigure(struct mrt_head *);
 void		 mrt_handler(struct mrt_head *);
 struct mrt	*mrt_get(struct mrt_head *, struct mrt *);
@@ -1802,6 +1804,7 @@ static const char * const timernames[] =
 	"RTR RetryTimer",
 	"RTR ExpireTimer",
 	"RTR ActiveTimer",
+	"MRT ReopenTimer",
 	""
 };
 
Index: mrt.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/mrt.c,v
diff -u -p -r1.127 mrt.c
--- mrt.c	21 Aug 2025 15:15:25 -0000	1.127
+++ mrt.c	3 Nov 2025 16:26:33 -0000
@@ -46,7 +46,7 @@ static int	mrt_dump_hdr_se(struct ibuf *
 		    uint16_t, uint32_t, int);
 static int	mrt_dump_hdr_rde(struct ibuf **, uint16_t type, uint16_t,
 		    uint32_t);
-static int	mrt_open(struct mrt *, time_t);
+static int	mrt_open(struct mrt *);
 
 #define RDEIDX		0
 #define SEIDX		1
@@ -1120,11 +1120,13 @@ mrt_init(struct imsgbuf *rde, struct ims
 }
 
 int
-mrt_open(struct mrt *mrt, time_t now)
+mrt_open(struct mrt *mrt)
 {
 	enum imsg_type	type;
+	time_t		now;
 	int		fd;
 
+	now = time(NULL);
 	if (strftime(MRT2MC(mrt)->file, sizeof(MRT2MC(mrt)->file),
 	    MRT2MC(mrt)->name, localtime(&now)) == 0) {
 		log_warnx("mrt_open: strftime conversion failed");
@@ -1150,25 +1152,28 @@ mrt_open(struct mrt *mrt, time_t now)
 	return (1);
 }
 
-time_t
-mrt_timeout(struct mrt_head *mrt)
+monotime_t
+mrt_timeout(struct mrt_head *mrt, monotime_t timeout)
 {
 	struct mrt	*m;
-	time_t		 now;
-	time_t		 timeout = -1;
+	monotime_t	 now, nextaction;
 
-	now = time(NULL);
+	now = getmonotime();
 	LIST_FOREACH(m, mrt, entry) {
 		if (m->state == MRT_STATE_RUNNING &&
 		    MRT2MC(m)->ReopenTimerInterval != 0) {
-			if (MRT2MC(m)->ReopenTimer <= now) {
-				mrt_open(m, now);
-				MRT2MC(m)->ReopenTimer =
-				    now + MRT2MC(m)->ReopenTimerInterval;
+			if (timer_nextisdue(&MRT2MC(m)->timer, now) !=
+			    NULL) {
+				mrt_open(m);
+				timer_set(&MRT2MC(m)->timer,
+				    Timer_Mrt_Reopen,
+				    MRT2MC(m)->ReopenTimerInterval);
+			}
+			nextaction = timer_nextduein(&MRT2MC(m)->timer);
+			if (monotime_valid(nextaction)) {
+				if (monotime_cmp(nextaction, timeout) < 0)
+					timeout = nextaction;
 			}
-			if (timeout == -1 ||
-			    MRT2MC(m)->ReopenTimer - now < timeout)
-				timeout = MRT2MC(m)->ReopenTimer - now;
 		}
 	}
 	return (timeout);
@@ -1178,17 +1183,16 @@ void
 mrt_reconfigure(struct mrt_head *mrt)
 {
 	struct mrt	*m, *xm;
-	time_t		 now;
 
-	now = time(NULL);
 	LIST_FOREACH_SAFE(m, mrt, entry, xm) {
 		if (m->state == MRT_STATE_OPEN ||
 		    m->state == MRT_STATE_REOPEN) {
-			if (mrt_open(m, now) == -1)
+			if (mrt_open(m) == -1)
 				continue;
 			if (MRT2MC(m)->ReopenTimerInterval != 0)
-				MRT2MC(m)->ReopenTimer =
-				    now + MRT2MC(m)->ReopenTimerInterval;
+				timer_set(&MRT2MC(m)->timer,
+				    Timer_Mrt_Reopen,
+				    MRT2MC(m)->ReopenTimerInterval);
 			m->state = MRT_STATE_RUNNING;
 		}
 		if (m->state == MRT_STATE_REMOVE) {
@@ -1196,6 +1200,7 @@ mrt_reconfigure(struct mrt_head *mrt)
 			    IMSG_MRT_CLOSE, 0, 0, -1, m, sizeof(struct mrt)) ==
 			    -1)
 				log_warn("mrt_reconfigure");
+			timer_remove_all(&MRT2MC(xm)->timer);
 			LIST_REMOVE(m, entry);
 			free(m);
 			continue;
@@ -1207,19 +1212,17 @@ void
 mrt_handler(struct mrt_head *mrt)
 {
 	struct mrt	*m;
-	time_t		 now;
 
-	now = time(NULL);
 	LIST_FOREACH(m, mrt, entry) {
 		if (m->state == MRT_STATE_RUNNING &&
 		    (MRT2MC(m)->ReopenTimerInterval != 0 ||
 		     m->type == MRT_TABLE_DUMP ||
 		     m->type == MRT_TABLE_DUMP_MP ||
 		     m->type == MRT_TABLE_DUMP_V2)) {
-			if (mrt_open(m, now) == -1)
+			if (mrt_open(m) == -1)
 				continue;
-			MRT2MC(m)->ReopenTimer =
-			    now + MRT2MC(m)->ReopenTimerInterval;
+			timer_set(&MRT2MC(m)->timer, Timer_Mrt_Reopen, 
+			    MRT2MC(m)->ReopenTimerInterval);
 		}
 	}
 }
@@ -1254,6 +1257,7 @@ mrt_mergeconfig(struct mrt_head *xconf, 
 				fatal("mrt_mergeconfig");
 			memcpy(xm, m, sizeof(struct mrt_config));
 			xm->state = MRT_STATE_OPEN;
+			TAILQ_INIT(&MRT2MC(xm)->timer);
 			LIST_INSERT_HEAD(xconf, xm, entry);
 		} else {
 			/* MERGE */
@@ -1274,6 +1278,7 @@ mrt_mergeconfig(struct mrt_head *xconf, 
 
 	/* free config */
 	while ((m = LIST_FIRST(nconf)) != NULL) {
+		timer_remove_all(&MRT2MC(xm)->timer);
 		LIST_REMOVE(m, entry);
 		free(m);
 	}
Index: parse.y
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
diff -u -p -r1.483 parse.y
--- parse.y	30 Oct 2025 12:43:18 -0000	1.483
+++ parse.y	3 Nov 2025 16:27:41 -0000
@@ -4760,6 +4760,7 @@ add_mrtconfig(enum mrt_type type, char *
 		free(n);
 		return (-1);
 	}
+	TAILQ_INIT(&MRT2MC(n)->timer);
 	MRT2MC(n)->ReopenTimerInterval = timeout;
 	if (p != NULL) {
 		if (curgroup == p) {
Index: session.h
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/session.h,v
diff -u -p -r1.192 session.h
--- session.h	21 Aug 2025 15:15:25 -0000	1.192
+++ session.h	3 Nov 2025 14:24:50 -0000
@@ -183,6 +183,7 @@ enum Timer {
 	Timer_Rtr_Retry,
 	Timer_Rtr_Expire,
 	Timer_Rtr_Active,
+	Timer_Mrt_Reopen,
 	Timer_Max
 };
 
@@ -191,8 +192,6 @@ struct timer {
 	enum Timer		type;
 	monotime_t		val;
 };
-
-TAILQ_HEAD(timer_head, timer);
 
 struct peer {
 	struct peer_config	 conf;