From: Theo Buehler <tb@theobuehler.org>
Subject: Re: bgpd: cleanup listerner handling in session.c
To: tech@openbsd.org
Date: Thu, 13 Nov 2025 11:48:12 +0100

On Thu, Nov 13, 2025 at 11:09:41AM +0100, Claudio Jeker wrote:
> I dislike how the listener handling in session.c RECONF_DONE is spread
> over multiple functions. Just move everything into one place
> (setup_listeners) and call it a day.
> 
> I think the result is much simpler to read.

Yes, it is much more pleasant. It is plausible but not entirely obvious
to me that none of the nconf->listen_addrs has ->reconf == RECONF_NONE.

If you are sure about that,

ok

> -- 
> :wq Claudio
> 
> Index: session.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/session.c,v
> diff -u -p -r1.527 session.c
> --- session.c	30 Oct 2025 12:43:18 -0000	1.527
> +++ session.c	13 Nov 2025 09:40:03 -0000
> @@ -108,10 +108,24 @@ int
>  setup_listeners(u_int *la_cnt)
>  {
>  	int			 ttl = 255;
> -	struct listen_addr	*la;
> +	struct listen_addr	*la, *next;
>  	u_int			 cnt = 0;
>  
> -	TAILQ_FOREACH(la, conf->listen_addrs, entry) {
> +	/* add new listeners */
> +	TAILQ_CONCAT(conf->listen_addrs, nconf->listen_addrs, entry);
> +
> +	TAILQ_FOREACH_SAFE(la, conf->listen_addrs, entry, next) {
> +		/* check for and delete no longer used listeners */
> +		if (la->reconf == RECONF_NONE) {
> +			log_info("not listening on %s any more",
> +			    log_sockaddr((struct sockaddr *)
> +			    &la->sa, la->sa_len));
> +			TAILQ_REMOVE(conf->listen_addrs, la, entry);
> +			close(la->fd);
> +			free(la);
> +			continue;
> +		}
> +
>  		la->reconf = RECONF_NONE;
>  		cnt++;
>  
> @@ -1173,7 +1187,7 @@ session_dispatch_imsg(struct imsgbuf *im
>  	struct mrt		*mrt;
>  	struct imsgbuf		*i;
>  	struct peer		*p;
> -	struct listen_addr	*la, *next, nla;
> +	struct listen_addr	*la, nla;
>  	struct session_dependon	 sdon;
>  	struct bgpd_config	 tconf;
>  	uint32_t		 peerid;
> @@ -1342,25 +1356,8 @@ session_dispatch_imsg(struct imsgbuf *im
>  			copy_config(conf, nconf);
>  			merge_peers(conf, nconf);
>  
> -			/* delete old listeners */
> -			TAILQ_FOREACH_SAFE(la, conf->listen_addrs, entry,
> -			    next) {
> -				if (la->reconf == RECONF_NONE) {
> -					log_info("not listening on %s any more",
> -					    log_sockaddr((struct sockaddr *)
> -					    &la->sa, la->sa_len));
> -					TAILQ_REMOVE(conf->listen_addrs, la,
> -					    entry);
> -					close(la->fd);
> -					free(la);
> -				}
> -			}
> -
> -			/* add new listeners */
> -			TAILQ_CONCAT(conf->listen_addrs, nconf->listen_addrs,
> -			    entry);
> -
>  			setup_listeners(listener_cnt);
> +
>  			free_config(nconf);
>  			nconf = NULL;
>  			pending_reconf = 0;
>