From: Rafael Sadowski <rafael@sizeofvoid.org>
Subject: Re: iked/snmpd: Implement new imsg API for proc.c
To: Martijn van Duren <openbsd+tech@list.imperialat.at>
Cc: tech@openbsd.org
Date: Thu, 2 Jul 2026 06:23:01 +0200

On Wed Jul 01, 2026 at 08:20:55PM +0200, Martijn van Duren wrote:
> Hello tech@,
> 
> Similar to a diff relayd's proc.c r1.54, and a pending diff for httpd by
> rsadowski@, here's a diff to implement the new imsg API inside iked, and
> snmpd, bringing the implementations a bit closer once again.
> 
> OK?

OK rsadowski

> 
> martijn@
> 
> diff d010102fe4efde3ee95164581695909c7c6b672a d99e3d210e154f2f1322afa2aa01cecfa7172ebf
> commit - d010102fe4efde3ee95164581695909c7c6b672a
> commit + d99e3d210e154f2f1322afa2aa01cecfa7172ebf
> blob - 7bba4cba8ec28d87db99f7ca6a1b47495f86396b
> blob + 35a34480c04d6125e64ec3ba2d623b392ab71146
> --- sbin/iked/control.c
> +++ sbin/iked/control.c
> @@ -310,7 +310,7 @@ control_dispatch_imsg(int fd, short event, void *arg)
>  			memcpy(&v, imsg.data, sizeof(v));
>  			log_setverbose(v);
>  
> -			proc_forward_imsg(&env->sc_ps, &imsg, PROC_PARENT, -1);
> +			proc_forward_imsg(&env->sc_ps, &imsg, PROC_PARENT);
>  			break;
>  		case IMSG_CTL_RELOAD:
>  		case IMSG_CTL_RESET:
> @@ -318,17 +318,17 @@ control_dispatch_imsg(int fd, short event, void *arg)
>  		case IMSG_CTL_DECOUPLE:
>  		case IMSG_CTL_ACTIVE:
>  		case IMSG_CTL_PASSIVE:
> -			proc_forward_imsg(&env->sc_ps, &imsg, PROC_PARENT, -1);
> +			proc_forward_imsg(&env->sc_ps, &imsg, PROC_PARENT);
>  			break;
>  		case IMSG_CTL_RESET_ID:
> -			proc_forward_imsg(&env->sc_ps, &imsg, PROC_IKEV2, -1);
> +			proc_forward_imsg(&env->sc_ps, &imsg, PROC_IKEV2);
>  			break;
>  		case IMSG_CTL_SHOW_SA:
>  		case IMSG_CTL_SHOW_STATS:
> -			proc_forward_imsg(&env->sc_ps, &imsg, PROC_IKEV2, -1);
> +			proc_forward_imsg(&env->sc_ps, &imsg, PROC_IKEV2);
>  			break;
>  		case IMSG_CTL_SHOW_CERTSTORE:
> -			proc_forward_imsg(&env->sc_ps, &imsg, PROC_CERT, -1);
> +			proc_forward_imsg(&env->sc_ps, &imsg, PROC_CERT);
>  			break;
>  		default:
>  			log_debug("%s: error handling imsg %d",
> blob - 8feaadef5073b9f8311e5f9226a320f3bbca624f
> blob + d361d8f5abcbcdbdaa683330ef2702ab050c5be0
> --- sbin/iked/iked.c
> +++ sbin/iked/iked.c
> @@ -434,7 +434,7 @@ parent_dispatch_ca(int fd, struct privsep_proc *p, str
>  	switch (imsg->hdr.type) {
>  	case IMSG_CTL_ACTIVE:
>  	case IMSG_CTL_PASSIVE:
> -		proc_forward_imsg(&env->sc_ps, imsg, PROC_IKEV2, -1);
> +		proc_forward_imsg(&env->sc_ps, imsg, PROC_IKEV2);
>  		break;
>  	case IMSG_OCSP_FD:
>  		ocsp_connect(env, imsg);
> @@ -473,8 +473,8 @@ parent_dispatch_control(int fd, struct privsep_proc *p
>  		free(str);
>  		break;
>  	case IMSG_CTL_VERBOSE:
> -		proc_forward_imsg(&env->sc_ps, imsg, PROC_IKEV2, -1);
> -		proc_forward_imsg(&env->sc_ps, imsg, PROC_CERT, -1);
> +		proc_forward_imsg(&env->sc_ps, imsg, PROC_IKEV2);
> +		proc_forward_imsg(&env->sc_ps, imsg, PROC_CERT);
>  
>  		/* return 1 to let proc.c handle it locally */
>  		return (1);
> blob - 367ea76047ae3d7ec009bd36cab0e3bbeb9ec36a
> blob + 9d0aa6cbd06ad919737bf59c91a38dd91f956b9e
> --- sbin/iked/iked.h
> +++ sbin/iked/iked.h
> @@ -1333,8 +1333,8 @@ int	 proc_composev_imsg(struct privsep *, enum privsep
>  	    uint16_t, uint32_t, int, const struct iovec *, int);
>  int	 proc_composev(struct privsep *, enum privsep_procid,
>  	    uint16_t, const struct iovec *, int);
> -int	 proc_forward_imsg(struct privsep *, struct imsg *,
> -	    enum privsep_procid, int);
> +void	 proc_forward_imsg(struct privsep *, struct imsg *,
> +	    enum privsep_procid);
>  struct imsgbuf *
>  	 proc_ibuf(struct privsep *, enum privsep_procid, int);
>  struct imsgev *
> blob - 7db459f43bd5a9cdd3f1200f78543470ef2d57c7
> blob + 56737effa0689ab1dd4221337bd2233974b49625
> --- sbin/iked/proc.c
> +++ sbin/iked/proc.c
> @@ -615,7 +615,7 @@ proc_dispatch(int fd, short event, void *arg)
>  	struct imsgbuf		*ibuf;
>  	struct imsg		 imsg;
>  	ssize_t			 n;
> -	int			 verbose;
> +	int			 ver;
>  	const char		*title;
>  	struct privsep_fd	 pf;
>  
> @@ -623,9 +623,11 @@ proc_dispatch(int fd, short event, void *arg)
>  	ibuf = &iev->ibuf;
>  
>  	if (event & EV_READ) {
> -		if ((n = imsgbuf_read(ibuf)) == -1)
> +		switch (imsgbuf_read(ibuf)) {
> +		case -1:
>  			fatal("%s: imsgbuf_read", __func__);
> -		if (n == 0) {
> +			break;
> +		case 0:
>  			/* this pipe is dead, so remove the event handler */
>  			event_del(&iev->ev);
>  			event_loopexit(NULL);
> @@ -645,15 +647,16 @@ proc_dispatch(int fd, short event, void *arg)
>  	}
>  
>  	for (;;) {
> -		if ((n = imsg_get(ibuf, &imsg)) == -1)
> -			fatal("%s: imsg_get", __func__);
> +		if ((n = imsgbuf_get(ibuf, &imsg)) == -1)
> +			fatal("%s: imsgbuf_get", __func__);
>  		if (n == 0)
>  			break;
>  
>  #if DEBUG > 1
>  		log_debug("%s: %s %d got imsg %d peerid %d from %s %d",
>  		    __func__, title, ps->ps_instance + 1,
> -		    imsg.hdr.type, imsg.hdr.peerid, p->p_title, imsg.hdr.pid);
> +		    imsg_get_type(&imsg), imsg_get_id(&imsg),
> +		    p->p_title, imsg_get_pid(&imsg));
>  #endif
>  
>  		/*
> @@ -668,11 +671,12 @@ proc_dispatch(int fd, short event, void *arg)
>  		/*
>  		 * Generic message handling
>  		 */
> -		switch (imsg.hdr.type) {
> +		switch (imsg_get_type(&imsg)) {
>  		case IMSG_CTL_VERBOSE:
> -			IMSG_SIZE_CHECK(&imsg, &verbose);
> -			memcpy(&verbose, imsg.data, sizeof(verbose));
> -			log_setverbose(verbose);
> +			if (imsg_get_data(&imsg, &ver, sizeof(ver)) == -1)
> +			       fatalx("%s: imsg_get_data", __func__);
> +
> +			log_setverbose(ver);
>  			break;
>  		case IMSG_CTL_PROCFD:
>  			if (p->p_id != PROC_PARENT) {
> @@ -680,8 +684,10 @@ proc_dispatch(int fd, short event, void *arg)
>  				    "IMSG_CTL_PROCFD from %s",
>  				    __func__, p->p_title);
>  			}
> -			IMSG_SIZE_CHECK(&imsg, &pf);
> -			memcpy(&pf, imsg.data, sizeof(pf));
> +
> +			if (imsg_get_data(&imsg, &pf, sizeof(pf)) == -1)
> +				fatalx("%s: imsg_get_data", __func__);
> +
>  			proc_accept(ps, imsg_get_fd(&imsg), pf.pf_procid,
>  			    pf.pf_instance);
>  			break;
> @@ -835,12 +841,18 @@ proc_composev(struct privsep *ps, enum privsep_procid 
>  	return (proc_composev_imsg(ps, id, -1, type, -1, -1, iov, iovcnt));
>  }
>  
> -int
> +void
>  proc_forward_imsg(struct privsep *ps, struct imsg *imsg,
> -    enum privsep_procid id, int n)
> +    enum privsep_procid id)
>  {
> -	return (proc_compose_imsg(ps, id, n, imsg->hdr.type,
> -	    imsg->hdr.peerid, -1, imsg->data, IMSG_DATA_SIZE(imsg)));
> +	int	 m, n = -1;
> +
> +	proc_range(ps, id, &n, &m);
> +	for (; n < m; n++) {
> +		if (imsg_forward(&ps->ps_ievs[id][n].ibuf, imsg) == -1)
> +			fatal("%s: imsg_forward", __func__);
> +		imsg_event_add(&ps->ps_ievs[id][n]);
> +	}
>  }
>  
>  struct imsgbuf *
> blob - e0ae3f6adb3a22c7b8b272311d994be93efed4fe
> blob + fbe3a7c2f3487d829e32c1d539732e895bc5844b
> --- usr.sbin/snmpd/proc.c
> +++ usr.sbin/snmpd/proc.c
> @@ -606,7 +606,7 @@ proc_dispatch(int fd, short event, void *arg)
>  	struct imsgbuf		*ibuf;
>  	struct imsg		 imsg;
>  	ssize_t			 n;
> -	int			 verbose;
> +	int			 ver;
>  	const char		*title;
>  	struct privsep_fd	 pf;
>  
> @@ -614,9 +614,11 @@ proc_dispatch(int fd, short event, void *arg)
>  	ibuf = &iev->ibuf;
>  
>  	if (event & EV_READ) {
> -		if ((n = imsgbuf_read(ibuf)) == -1)
> +		switch (imsgbuf_read(ibuf)) {
> +		case -1:
>  			fatal("%s: imsgbuf_read", __func__);
> -		if (n == 0) {
> +			break;
> +		case 0:
>  			/* this pipe is dead, so remove the event handler */
>  			event_del(&iev->ev);
>  			event_loopexit(NULL);
> @@ -636,15 +638,16 @@ proc_dispatch(int fd, short event, void *arg)
>  	}
>  
>  	for (;;) {
> -		if ((n = imsg_get(ibuf, &imsg)) == -1)
> -			fatal("%s: imsg_get", __func__);
> +		if ((n = imsgbuf_get(ibuf, &imsg)) == -1)
> +			fatal("%s: imsgbuf_get", __func__);
>  		if (n == 0)
>  			break;
>  
>  #if DEBUG > 1
>  		log_debug("%s: %s %d got imsg %d peerid %d from %s %d",
>  		    __func__, title, ps->ps_instance + 1,
> -		    imsg.hdr.type, imsg.hdr.peerid, p->p_title, imsg.hdr.pid);
> +		    imsg_get_type(&imsg), imsg_get_id(&imsg),
> +		    p->p_title, imsg_get_pid(&imsg));
>  #endif
>  
>  		/*
> @@ -659,11 +662,12 @@ proc_dispatch(int fd, short event, void *arg)
>  		/*
>  		 * Generic message handling
>  		 */
> -		switch (imsg.hdr.type) {
> +		switch (imsg_get_type(&imsg)) {
>  		case IMSG_CTL_VERBOSE:
> -			IMSG_SIZE_CHECK(&imsg, &verbose);
> -			memcpy(&verbose, imsg.data, sizeof(verbose));
> -			log_setverbose(verbose);
> +			if (imsg_get_data(&imsg, &ver, sizeof(ver)) == -1)
> +			       fatalx("%s: imsg_get_data", __func__);
> +
> +			log_setverbose(ver);
>  			break;
>  		case IMSG_CTL_PROCFD:
>  			if (p->p_id != PROC_PARENT || ps->ps_run == NULL) {
> @@ -671,8 +675,10 @@ proc_dispatch(int fd, short event, void *arg)
>  				    "IMSG_CTL_PROCFD from %s",
>  				    __func__, p->p_title);
>  			}
> -			IMSG_SIZE_CHECK(&imsg, &pf);
> -			memcpy(&pf, imsg.data, sizeof(pf));
> +
> +			if (imsg_get_data(&imsg, &pf, sizeof(pf)) == -1)
> +				fatalx("%s: imsg_get_data", __func__);
> +
>  			proc_accept(ps, imsg_get_fd(&imsg), pf.pf_procid,
>  			    pf.pf_instance);
>  			break;
> @@ -829,12 +835,18 @@ proc_composev(struct privsep *ps, enum privsep_procid 
>  	return (proc_composev_imsg(ps, id, -1, type, -1, -1, iov, iovcnt));
>  }
>  
> -int
> +void
>  proc_forward_imsg(struct privsep *ps, struct imsg *imsg,
> -    enum privsep_procid id, int n)
> +    enum privsep_procid id)
>  {
> -	return (proc_compose_imsg(ps, id, n, imsg->hdr.type,
> -	    imsg->hdr.peerid, -1, imsg->data, IMSG_DATA_SIZE(imsg)));
> +	int	 m, n = -1;
> +
> +	proc_range(ps, id, &n, &m);
> +	for (; n < m; n++) {
> +		if (imsg_forward(&ps->ps_ievs[id][n].ibuf, imsg) == -1)
> +			fatal("%s: imsg_forward", __func__);
> +		imsg_event_add(&ps->ps_ievs[id][n]);
> +	}
>  }
>  
>  struct imsgbuf *
> blob - 1db290b765c0beff7e5b7680e6a13e29a648c4cd
> blob + 1db6a3d3ad412fc3bca80571a6ad465430ea4cc6
> --- usr.sbin/snmpd/snmpd.h
> +++ usr.sbin/snmpd/snmpd.h
> @@ -519,8 +519,8 @@ int	 proc_composev_imsg(struct privsep *, enum privsep
>  	    u_int16_t, u_int32_t, int, const struct iovec *, int);
>  int	 proc_composev(struct privsep *, enum privsep_procid,
>  	    uint16_t, const struct iovec *, int);
> -int	 proc_forward_imsg(struct privsep *, struct imsg *,
> -	    enum privsep_procid, int);
> +void	 proc_forward_imsg(struct privsep *, struct imsg *,
> +	    enum privsep_procid);
>  struct imsgbuf *
>  	 proc_ibuf(struct privsep *, enum privsep_procid, int);
>  struct imsgev *
>