From: Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject: bgpd: move the throttle code in the SE
To: tech@openbsd.org
Date: Wed, 26 Feb 2025 13:30:59 +0100

The session engine signals to the RDE when a peer gets too backlogged and
then the RDE should stop producing more messages to this peer until the SE
clears the condition. This is done with the XON and XOFF imsgs.

Until now the checks were done in session_sendmsg() and
session_dispatch_msg(). Now I want to move this to the main poll loop
where all poll decisions are made.

This should not really change anything. The queue length at start of the
poll loop is good enough to trigger the imsgs and we don't need to check
for every msg queued (session_sendmsg()) or after every write.
The XON and XOFF imsgs are asynchronous and the RDE will probably send
some extra messages before it throttles but it should be good enough for
what we want.

-- 
:wq Claudio

Index: session.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/session.c,v
diff -u -p -r1.520 session.c
--- session.c	26 Feb 2025 10:26:51 -0000	1.520
+++ session.c	26 Feb 2025 10:40:59 -0000
@@ -428,6 +428,26 @@ session_main(int debug, int verbose)
 					timeout = nextaction;
 			}
 
+			/* check if peer needs throttling or not */
+			if (!p->throttled &&
+			    msgbuf_queuelen(p->wbuf) > SESS_MSG_HIGH_MARK) {
+				if (imsg_rde(IMSG_XOFF, p->conf.id, NULL, 0) ==
+				    -1)
+					log_peer_warn(&p->conf,
+					    "imsg_compose XOFF");
+				else
+					p->throttled = 1;
+			}
+			if (p->throttled &&
+			    msgbuf_queuelen(p->wbuf) < SESS_MSG_LOW_MARK) {
+				if (imsg_rde(IMSG_XON, p->conf.id, NULL, 0) ==
+				    -1)
+					log_peer_warn(&p->conf,
+					    "imsg_compose XON");
+				else
+					p->throttled = 0;
+			}
+
 			/* are we waiting for a write? */
 			events = POLLIN;
 			if (msgbuf_queuelen(p->wbuf) > 0 ||
@@ -1454,12 +1474,6 @@ session_sendmsg(struct ibuf *msg, struct
 	session_mrt_dump_bgp_msg(p, msg, msgtype, DIR_OUT);
 
 	ibuf_close(p->wbuf, msg);
-	if (!p->throttled && msgbuf_queuelen(p->wbuf) > SESS_MSG_HIGH_MARK) {
-		if (imsg_rde(IMSG_XOFF, p->conf.id, NULL, 0) == -1)
-			log_peer_warn(&p->conf, "imsg_compose XOFF");
-		else
-			p->throttled = 1;
-	}
 }
 
 /*
@@ -2022,13 +2036,6 @@ session_dispatch_msg(struct pollfd *pfd,
 		}
 		p->stats.last_write = getmonotime();
 		start_timer_sendholdtime(p);
-		if (p->throttled &&
-		    msgbuf_queuelen(p->wbuf) < SESS_MSG_LOW_MARK) {
-			if (imsg_rde(IMSG_XON, p->conf.id, NULL, 0) == -1)
-				log_peer_warn(&p->conf, "imsg_compose XON");
-			else
-				p->throttled = 0;
-		}
 		if (!(pfd->revents & POLLIN))
 			return (1);
 	}