From: Martijn van Duren Subject: Re: iked/snmpd: Implement new imsg API for proc.c To: tech@openbsd.org Date: Thu, 2 Jul 2026 10:32:21 +0200 On 7/2/26 10:21 AM, hshoexer wrote: > 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? > > see two white space comments inline below. Otherwise ok hshoexer@ Thnx, these came from the httpd diff. I've already committed this diff with the OK rsadowski@. I've fixed this for httpd in the sync diff I just send out, and will be fixed when syncing iked/snmpd with httpd/relayd. > > fwiw: In snmpd proc_forward_imsg() is not used at all. My goal is to have proc.c in sync as much as possible, so that fixes to one can be applied to others without issue. > >> 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__); > > spaces instead of tab. > >> + >> + 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__); > > spaces instead of tab. > >> + >> + 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 * >> >