From: Theo Buehler <tb@theobuehler.org>
Subject: Re: bgpd: move the throttle code in the SE
To: tech@openbsd.org
Date: Wed, 26 Feb 2025 13:47:39 +0100

On Wed, Feb 26, 2025 at 01:30:59PM +0100, Claudio Jeker wrote:
> 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.

Thanks for the explanation. With this I can follow.

ok tb

> 
> -- 
> :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);
>  	}
>