From: Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject: Re: relayd: use imsg_compose() instead of handcrafted imsg
To: Rafael Sadowski <rafael@sizeofvoid.org>
Cc: tech@openbsd.org
Date: Wed, 29 Jul 2026 20:28:30 +0200

On Wed, Jul 29, 2026 at 08:16:36PM +0200, Rafael Sadowski wrote:
> pfe_sync() built a struct imsg by hand and pushed it through
> control_imsg_forward(). That API is meant for relaying a *received*
> imsg.
> 
> Add control_imsg_compose() which walks the notify-enabled control
> connections and calls imsg_compose(3) properly. (like
> control_imsg_forward).
> 
> If anyone has a better idea for a name for control_imsg_compose and
> control_imsg_forward, please do let me know!
> 
> OK?
 
I like this a lot better than the code before. I have no better name.
Maybe control_imsg_notify() since it notifies some listeners.

> diff --git a/control.c b/control.c
> index fb64608..56af7b6 100644
> --- a/control.c
> +++ b/control.c
> @@ -400,6 +400,21 @@ control_dispatch_imsg(int fd, short event, void *arg)
>  	imsg_event_add(&c->iev);
>  }
>  
> +void
> +control_imsg_compose(uint32_t type, void *data, uint16_t len)
> +{
> +	struct ctl_conn *c;
> +
> +	TAILQ_FOREACH(c, &ctl_conns, entry) {
> +		if (c->flags & CTL_CONN_NOTIFY) {
> +			if (imsg_compose(&c->iev.ibuf, type, 0, 0, -1, data,
> +			    len) == -1)
> +				fatal("%s: imsg_compose", __func__);
> +			imsg_event_add(&c->iev);
> +		}
> +	}
> +}
> +
>  void
>  control_imsg_forward(struct imsg *imsg)
>  {
> diff --git a/pfe.c b/pfe.c
> index c66ad06..e9d90df 100644
> --- a/pfe.c
> +++ b/pfe.c
> @@ -715,12 +715,10 @@ pfe_sync(void)
>  	struct table		*active;
>  	struct table		*table;
>  	struct ctl_id		 id;
> -	struct imsg		 imsg;
>  	struct ctl_demote	 demote;
>  	struct router		*rt;
>  
>  	bzero(&id, sizeof(id));
> -	bzero(&imsg, sizeof(imsg));
>  	TAILQ_FOREACH(rdr, env->sc_rdrs, entry) {
>  		rdr->conf.flags &= ~(F_BACKUP);
>  		rdr->conf.flags &= ~(F_DOWN);
> @@ -740,12 +738,10 @@ pfe_sync(void)
>  			active = rdr->table;
>  
>  		if (active != NULL && active->conf.flags & F_CHANGED) {
> -			id.id = active->conf.id;
> -			imsg.hdr.type = IMSG_CTL_TABLE_CHANGED;
> -			imsg.hdr.len = sizeof(id) + IMSG_HEADER_SIZE;
> -			imsg.data = &id;
>  			sync_table(env, rdr, active);
> -			control_imsg_forward(&imsg);
> +			id.id = active->conf.id;
> +			control_imsg_compose(IMSG_CTL_TABLE_CHANGED, &id,
> +			    sizeof(id));
>  		}
>  
>  		if (rdr->conf.flags & F_DOWN) {
> @@ -754,21 +750,17 @@ pfe_sync(void)
>  				log_debug("%s: disabling ruleset", __func__);
>  				rdr->conf.flags &= ~(F_ACTIVE_RULESET);
>  				id.id = rdr->conf.id;
> -				imsg.hdr.type = IMSG_CTL_PULL_RULESET;
> -				imsg.hdr.len = sizeof(id) + IMSG_HEADER_SIZE;
> -				imsg.data = &id;
>  				sync_ruleset(env, rdr, 0);
> -				control_imsg_forward(&imsg);
> +				control_imsg_compose(IMSG_CTL_PULL_RULESET,
> +				    &id, sizeof(id));
>  			}
>  		} else if (!(rdr->conf.flags & F_ACTIVE_RULESET)) {
>  			log_debug("%s: enabling ruleset", __func__);
>  			rdr->conf.flags |= F_ACTIVE_RULESET;
>  			id.id = rdr->conf.id;
> -			imsg.hdr.type = IMSG_CTL_PUSH_RULESET;
> -			imsg.hdr.len = sizeof(id) + IMSG_HEADER_SIZE;
> -			imsg.data = &id;
>  			sync_ruleset(env, rdr, 1);
> -			control_imsg_forward(&imsg);
> +			control_imsg_compose(IMSG_CTL_PUSH_RULESET, &id,
> +			    sizeof(id));
>  		}
>  	}
>  
> diff --git a/relayd.h b/relayd.h
> index 2aa6eab..07508b5 100644
> --- a/relayd.h
> +++ b/relayd.h
> @@ -1148,6 +1148,7 @@ int		 control_listen(struct control_sock *);
>  void		 control_cleanup(struct control_sock *);
>  void		 control_dispatch_imsg(int, short, void *);
>  void		 control_imsg_forward(struct imsg *);
> +void		 control_imsg_compose(uint32_t, void *, uint16_t);
>  struct ctl_conn	*control_connbyfd(int);
>  
>  /* parse.y */
> 

-- 
:wq Claudio