Re: [3/7] relayd: convert control imsg forwarding to imsg_forward()

Claudio Jeker <[email protected]>
Newsgroups gmane.os.openbsd.tech
Message-ID <[email protected]> 
On Sun, Jun 07, 2026 at 09:01:24AM +0200, Rafael Sadowski wrote:
> This is a series of commits for an imsg APIv2 rework in relayd. These
> were committed and tested individually. They can be reviewed in their
> entirety here:
> 
> gothub: https://rsadowski.gothub.org/?action=summary&headref=imsg_v2&path=relayd.git
> PR: https://codeberg.org/rsadowski/relayd/pulls/1/files
> 
> commit cdac3e43ca7856c92948a16de7bd0e4ec3e12f26
> Author: Rafael Sadowski <[email protected]>
> Date:   Sat Jun 6 16:22:49 2026 +0200
> 
>     relayd: convert control imsg forwarding to imsg_forward()
>     
>     Rework control_imsg_forward() to forward the message unaltered via
>     imsg_forward() instead of rebuilding it with imsg_compose_event().
>     
>     read the type via imsg_get_type(), dropping the manual header-length
>     Switch to use read the payload with imsg_get_data() and checks and the
>     memcpy() that wrote the data back into the imsg before forwarding.
> 
> diff --git a/control.c b/control.c
> index 6c29f2b..fe829ce 100644
> --- a/control.c
> +++ b/control.c
> @@ -228,7 +228,7 @@ control_dispatch_imsg(int fd, short event, void *arg)
>  	struct imsg		 imsg;
>  	struct ctl_id		 id;
>  	int			 n;
> -	int			 verbose;
> +	int			 verb;
>  	struct relayd		*env = cs->cs_env;
>  	struct privsep		*ps = env->sc_ps;
>  
> @@ -238,7 +238,11 @@ control_dispatch_imsg(int fd, short event, void *arg)
>  	}
>  
>  	if (event & EV_READ) {
> -		if (imsgbuf_read(&c->iev.ibuf) != 1) {
> +		switch (imsgbuf_read(&c->iev.ibuf)) {
> +		case -1:
> +			fatal("%s: imsgbuf_read", __func__);
> +			break;
> +		case 0:
>  			control_close(fd, cs);
>  			return;
>  		}
> @@ -262,13 +266,13 @@ control_dispatch_imsg(int fd, short event, void *arg)
>  
>  		if (c->waiting) {
>  			log_debug("%s: unexpected imsg %d",
> -			    __func__, imsg.hdr.type);
> +			    __func__, imsg_get_type(&imsg));
>  			imsg_free(&imsg);
>  			control_close(fd, cs);
>  			return;
>  		}
>  
> -		switch (imsg.hdr.type) {
> +		switch (imsg_get_type(&imsg)) {
>  		case IMSG_CTL_SHOW_SUM:
>  			show(c);
>  			break;
> @@ -276,85 +280,79 @@ control_dispatch_imsg(int fd, short event, void *arg)
>  			show_sessions(c);
>  			break;
>  		case IMSG_CTL_RDR_DISABLE:
> -			if (imsg.hdr.len != IMSG_HEADER_SIZE + sizeof(id))
> -				fatalx("invalid imsg header len");
> -			memcpy(&id, imsg.data, sizeof(id));
> +			if (imsg_get_data(&imsg, &id, sizeof(id)) == -1)
> +				fatalx("%s: imsg_get_data", __func__);
> +
>  			if (disable_rdr(c, &id))
>  				imsg_compose_event(&c->iev, IMSG_CTL_FAIL,
>  				    0, ps->ps_instance + 1, -1, NULL, 0);
>  			else {
> -				memcpy(imsg.data, &id, sizeof(id));
> -				control_imsg_forward(ps, &imsg);
> +				control_imsg_forward(&imsg);
>  				imsg_compose_event(&c->iev, IMSG_CTL_OK,
>  				    0, ps->ps_instance + 1, -1, NULL, 0);
>  			}
>  			break;
>  		case IMSG_CTL_RDR_ENABLE:
> -			if (imsg.hdr.len != IMSG_HEADER_SIZE + sizeof(id))
> -				fatalx("invalid imsg header len");
> -			memcpy(&id, imsg.data, sizeof(id));
> +			if (imsg_get_data(&imsg, &id, sizeof(id)) == -1)
> +				fatalx("%s: imsg_get_data", __func__);
> +
>  			if (enable_rdr(c, &id))
>  				imsg_compose_event(&c->iev, IMSG_CTL_FAIL,
>  				    0, ps->ps_instance + 1, -1, NULL, 0);
>  			else {
> -				memcpy(imsg.data, &id, sizeof(id));
> -				control_imsg_forward(ps, &imsg);
> +				control_imsg_forward(&imsg);
>  				imsg_compose_event(&c->iev, IMSG_CTL_OK,
>  				    0, ps->ps_instance + 1, -1, NULL, 0);
>  			}
>  			break;
>  		case IMSG_CTL_TABLE_DISABLE:
> -			if (imsg.hdr.len != IMSG_HEADER_SIZE + sizeof(id))
> -				fatalx("invalid imsg header len");
> -			memcpy(&id, imsg.data, sizeof(id));
> +			if (imsg_get_data(&imsg, &id, sizeof(id)) == -1)
> +				fatalx("%s: imsg_get_data", __func__);
> +
>  			if (disable_table(c, &id))
>  				imsg_compose_event(&c->iev, IMSG_CTL_FAIL,
>  				    0, ps->ps_instance + 1, -1, NULL, 0);
>  			else {
> -				memcpy(imsg.data, &id, sizeof(id));
> -				control_imsg_forward(ps, &imsg);
> +				control_imsg_forward(&imsg);
>  				imsg_compose_event(&c->iev, IMSG_CTL_OK,
>  				    0, ps->ps_instance + 1, -1, NULL, 0);
>  			}
>  			break;
>  		case IMSG_CTL_TABLE_ENABLE:
> -			if (imsg.hdr.len != IMSG_HEADER_SIZE + sizeof(id))
> -				fatalx("invalid imsg header len");
> -			memcpy(&id, imsg.data, sizeof(id));
> +			if (imsg_get_data(&imsg, &id, sizeof(id)) == -1)
> +				fatalx("%s: imsg_get_data", __func__);
> +
>  			if (enable_table(c, &id))
>  				imsg_compose_event(&c->iev, IMSG_CTL_FAIL,
>  				    0, ps->ps_instance + 1, -1, NULL, 0);
>  			else {
> -				memcpy(imsg.data, &id, sizeof(id));
> -				control_imsg_forward(ps, &imsg);
> +				control_imsg_forward(&imsg);
>  				imsg_compose_event(&c->iev, IMSG_CTL_OK,
>  				    0, ps->ps_instance + 1, -1, NULL, 0);
>  			}
>  			break;
>  		case IMSG_CTL_HOST_DISABLE:
> -			if (imsg.hdr.len != IMSG_HEADER_SIZE + sizeof(id))
> -				fatalx("invalid imsg header len");
> -			memcpy(&id, imsg.data, sizeof(id));
> +			if (imsg_get_data(&imsg, &id, sizeof(id)) == -1)
> +				fatalx("%s: imsg_get_data", __func__);
> +
>  			if (disable_host(c, &id, NULL))
>  				imsg_compose_event(&c->iev, IMSG_CTL_FAIL,
>  				    0, ps->ps_instance + 1, -1, NULL, 0);
>  			else {
> -				memcpy(imsg.data, &id, sizeof(id));
> -				control_imsg_forward(ps, &imsg);
> +				control_imsg_forward(&imsg);
>  				imsg_compose_event(&c->iev, IMSG_CTL_OK,
>  				    0, ps->ps_instance + 1, -1, NULL, 0);
>  			}
>  			break;
>  		case IMSG_CTL_HOST_ENABLE:
> -			if (imsg.hdr.len != IMSG_HEADER_SIZE + sizeof(id))
> -				fatalx("invalid imsg header len");
> -			memcpy(&id, imsg.data, sizeof(id));
> +			if (imsg_get_data(&imsg, &id, sizeof(id)) == -1)
> +				fatalx("%s: imsg_get_data", __func__);
> +
>  			if (enable_host(c, &id, NULL))
>  				imsg_compose_event(&c->iev, IMSG_CTL_FAIL,
>  				    0, ps->ps_instance + 1, -1, NULL, 0);
>  			else {
> -				memcpy(imsg.data, &id, sizeof(id));
> -				control_imsg_forward(ps, &imsg);
> +				control_imsg_forward(&imsg);
>  				imsg_compose_event(&c->iev, IMSG_CTL_OK,
>  				    0, ps->ps_instance + 1, -1, NULL, 0);
>  			}
> @@ -381,21 +379,19 @@ control_dispatch_imsg(int fd, short event, void *arg)
>  			c->flags |= CTL_CONN_NOTIFY;
>  			break;
>  		case IMSG_CTL_VERBOSE:
> -			IMSG_SIZE_CHECK(&imsg, &verbose);
> -
> -			memcpy(&verbose, imsg.data, sizeof(verbose));
> +			if (imsg_get_data(&imsg, &verb, sizeof(verb)) == -1)
> +				fatalx("%s: imsg_get_data", __func__);
>  
>  			proc_forward_imsg(env->sc_ps, &imsg, PROC_PARENT);
>  			proc_forward_imsg(env->sc_ps, &imsg, PROC_HCE);
>  			proc_forward_imsg(env->sc_ps, &imsg, PROC_RELAY);
>  
> -			memcpy(imsg.data, &verbose, sizeof(verbose));
> -			control_imsg_forward(ps, &imsg);
> -			log_setverbose(verbose);
> +			control_imsg_forward(&imsg);
> +			log_setverbose(verb);
>  			break;
>  		default:
>  			log_debug("%s: error handling imsg %d",
> -			    __func__, imsg.hdr.type);
> +			    __func__, imsg_get_type(&imsg));
>  			break;
>  		}
>  		imsg_free(&imsg);
> @@ -405,13 +401,15 @@ control_dispatch_imsg(int fd, short event, void *arg)
>  }
>  
>  void
> -control_imsg_forward(struct privsep *ps, struct imsg *imsg)
> +control_imsg_forward(struct imsg *imsg)
>  {
> -	struct ctl_conn *c;
> +        struct ctl_conn *c;
>  
> -	TAILQ_FOREACH(c, &ctl_conns, entry)
> -		if (c->flags & CTL_CONN_NOTIFY)
> -			imsg_compose_event(&c->iev, imsg->hdr.type,
> -			    0, ps->ps_instance + 1, -1, imsg->data,
> -			    imsg->hdr.len - IMSG_HEADER_SIZE);
> +	TAILQ_FOREACH(c, &ctl_conns, entry) {
> +		if (c->flags & CTL_CONN_NOTIFY) {
> +			if (imsg_forward(&c->iev.ibuf, imsg) == -1)
> +				fatal("%s: imsg_forward", __func__);
> +			imsg_event_add(&c->iev);
> +		}
> +	}
>  }
> diff --git a/pfe.c b/pfe.c
> index 0f865b8..0856ae9 100644
> --- a/pfe.c
> +++ b/pfe.c
> @@ -124,7 +124,7 @@ pfe_dispatch_hce(int fd, struct privsep_proc *p, struct imsg *imsg)
>  	struct table		*table;
>  	struct ctl_status	 st;
>  
> -	control_imsg_forward(p->p_ps, imsg);
> +	control_imsg_forward(imsg);
>  
>  	switch (imsg_get_type(imsg)) {
>  	case IMSG_HOST_STATUS:
> @@ -737,7 +737,7 @@ pfe_sync(void)
>  			imsg.hdr.len = sizeof(id) + IMSG_HEADER_SIZE;
>  			imsg.data = &id;
>  			sync_table(env, rdr, active);
> -			control_imsg_forward(env->sc_ps, &imsg);
> +			control_imsg_forward(&imsg);
>  		}
>  
>  		if (rdr->conf.flags & F_DOWN) {
> @@ -750,7 +750,7 @@ pfe_sync(void)
>  				imsg.hdr.len = sizeof(id) + IMSG_HEADER_SIZE;
>  				imsg.data = &id;
>  				sync_ruleset(env, rdr, 0);
> -				control_imsg_forward(env->sc_ps, &imsg);
> +				control_imsg_forward(&imsg);
>  			}
>  		} else if (!(rdr->conf.flags & F_ACTIVE_RULESET)) {
>  			log_debug("%s: enabling ruleset", __func__);
> @@ -760,7 +760,7 @@ pfe_sync(void)
>  			imsg.hdr.len = sizeof(id) + IMSG_HEADER_SIZE;
>  			imsg.data = &id;
>  			sync_ruleset(env, rdr, 1);
> -			control_imsg_forward(env->sc_ps, &imsg);
> +			control_imsg_forward(&imsg);
>  		}
>  	}
>  
> diff --git a/relayd.h b/relayd.h
> index 16fe2bf..373255e 100644
> --- a/relayd.h
> +++ b/relayd.h
> @@ -1154,7 +1154,7 @@ int	 control_init(struct privsep *, struct control_sock *);
>  int	 control_listen(struct control_sock *);
>  void	 control_cleanup(struct control_sock *);
>  void	 control_dispatch_imsg(int, short, void *);
> -void	 control_imsg_forward(struct privsep *ps, struct imsg *);
> +void	 control_imsg_forward(struct imsg *);
>  struct ctl_conn	*
>  	 control_connbyfd(int);
>  
> 

OK claudio@
-- 
:wq Claudio
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.