From: Theo Buehler <tb@theobuehler.org>
Subject: Re: bgpd: simplify timer handling
To: tech@openbsd.org
Date: Mon, 14 Apr 2025 15:55:20 +0200

On Mon, Apr 14, 2025 at 02:03:45PM +0200, Claudio Jeker wrote:
> Instead of checking the timeout for poll against a relative timeout 
> just use the abolute monotime that the timer API uses as well.
> This way only right before poll we need to convert to a relative timeout.

I like it.

ok tb

> 
> -- 
> :wq Claudio
> 
> Index: rtr.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/rtr.c,v
> diff -u -p -r1.30 rtr.c
> --- rtr.c	20 Feb 2025 19:47:31 -0000	1.30
> +++ rtr.c	8 Apr 2025 14:48:24 -0000
> @@ -245,7 +245,6 @@ rtr_main(int debug, int verbose)
>  		timeout = timer_nextduein(&expire_timer);
>  		if (!monotime_valid(timeout))
>  			fatalx("roa-set expire timer no longer running");
> -		timeout = monotime_sub(timeout, getmonotime());
>  
>  		memset(pfd, 0, sizeof(struct pollfd) * pfd_elms);
>  
> @@ -254,6 +253,10 @@ rtr_main(int debug, int verbose)
>  
>  		i = PFD_PIPE_COUNT;
>  		i += rtr_poll_events(pfd + i, pfd_elms - i, &timeout);
> +
> +		timeout = monotime_sub(timeout, getmonotime());
> +		if (!monotime_valid(timeout))
> +			timeout = monotime_clear();
>  
>  		if (poll(pfd, i, monotime_to_msec(timeout)) == -1) {
>  			if (errno == EINTR)
> Index: rtr_proto.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/rtr_proto.c,v
> diff -u -p -r1.51 rtr_proto.c
> --- rtr_proto.c	20 Feb 2025 19:47:31 -0000	1.51
> +++ rtr_proto.c	8 Apr 2025 14:40:14 -0000
> @@ -1365,7 +1365,6 @@ size_t
>  rtr_poll_events(struct pollfd *pfds, size_t npfds, monotime_t *timeout)
>  {
>  	struct rtr_session *rs;
> -	monotime_t now = getmonotime();
>  	size_t i = 0;
>  
>  	TAILQ_FOREACH(rs, &rtrs, entry) {
> @@ -1377,7 +1376,6 @@ rtr_poll_events(struct pollfd *pfds, siz
>  
>  		nextaction = timer_nextduein(&rs->timers);
>  		if (monotime_valid(nextaction)) {
> -			monotime_sub(nextaction, now);
>  			if (monotime_cmp(nextaction, *timeout) < 0)
>  				*timeout = nextaction;
>  		}
> Index: session.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/session.c,v
> diff -u -p -r1.524 session.c
> --- session.c	26 Feb 2025 19:31:31 -0000	1.524
> +++ session.c	8 Apr 2025 14:52:38 -0000
> @@ -323,9 +323,9 @@ session_main(int debug, int verbose)
>  			i++;
>  		}
>  		idx_listeners = i;
> -		timeout = monotime_from_sec(MAX_TIMEOUT);
> -
>  		now = getmonotime();
> +		timeout = monotime_add(now, monotime_from_sec(MAX_TIMEOUT));
> +
>  		RB_FOREACH(p, peer_head, &conf->peers) {
>  			monotime_t nextaction;
>  			struct timer *pt;
> @@ -386,7 +386,6 @@ session_main(int debug, int verbose)
>  			}
>  			nextaction = timer_nextduein(&p->timers);
>  			if (monotime_valid(nextaction)) {
> -				nextaction = monotime_sub(nextaction, now);
>  				if (monotime_cmp(nextaction, timeout) < 0)
>  					timeout = nextaction;
>  			}
> @@ -438,11 +437,14 @@ session_main(int debug, int verbose)
>  		if (i > pfd_elms)
>  			fatalx("poll pfd overflow");
>  
> -		if (monotime_valid(pauseaccept) && monotime_cmp(timeout,
> -		    monotime_from_sec(PAUSEACCEPT_TIMEOUT)) > 0)
> -			timeout = monotime_from_sec(PAUSEACCEPT_TIMEOUT);
> +		if (monotime_valid(pauseaccept) &&
> +		    monotime_cmp(timeout, pauseaccept) > 0)
> +			timeout = pauseaccept;
> +
> +		timeout = monotime_sub(timeout, getmonotime());
>  		if (!monotime_valid(timeout))
>  			timeout = monotime_clear();
> +
>  		if (poll(pfd, i, monotime_to_msec(timeout)) == -1) {
>  			if (errno == EINTR)
>  				continue;
> @@ -454,8 +456,7 @@ session_main(int debug, int verbose)
>  		 * for 1 second to throttle the accept() loop.
>  		 */
>  		if (monotime_valid(pauseaccept) &&
> -		    monotime_cmp(getmonotime(), monotime_add(pauseaccept,
> -		    monotime_from_sec(PAUSEACCEPT_TIMEOUT))) > 0)
> +		    monotime_cmp(getmonotime(), pauseaccept) > 0)
>  			pauseaccept = monotime_clear();
>  
>  		if (handle_pollfd(&pfd[PFD_PIPE_MAIN], ibuf_main) == -1) {
> @@ -691,7 +692,8 @@ session_accept(int listenfd)
>  	    (struct sockaddr *)&cliaddr, &len,
>  	    SOCK_CLOEXEC | SOCK_NONBLOCK)) == -1) {
>  		if (errno == ENFILE || errno == EMFILE)
> -			pauseaccept = getmonotime();
> +			pauseaccept = monotime_add(getmonotime(),
> +			    monotime_from_sec(PAUSEACCEPT_TIMEOUT));
>  		else if (errno != EWOULDBLOCK && errno != EINTR &&
>  		    errno != ECONNABORTED)
>  			log_warn("accept");
>