From: Rafael Sadowski Subject: Re: relayd: use imsg_compose() instead of handcrafted imsg To: tech@openbsd.org Date: Wed, 29 Jul 2026 21:21:52 +0200 On Wed Jul 29, 2026 at 08:28:30PM +0200, Claudio Jeker wrote: > 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. > I like the _notify suffix. commit 260e530376f5caf06cea0d5a7477d7e456a1a2cf Author: Rafael Sadowski Date: Wed Jul 29 20:15:32 2026 +0200 relayd: use imsg_compose() instead of handcrafted imsg 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_notify() which walks the notify-enabled control connections and calls imsg_compose(3) properly. (like control_imsg_forward). diff --git a/control.c b/control.c index 13b69ec..8b51193 100644 --- a/control.c +++ b/control.c @@ -428,6 +428,21 @@ control_dispatch_imsg(int fd, short event, void *arg) imsg_event_add(&c->iev); } +void +control_imsg_notify(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 e62ed05..0fcd547 100644 --- a/pfe.c +++ b/pfe.c @@ -750,12 +750,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); @@ -775,12 +773,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_notify(IMSG_CTL_TABLE_CHANGED, &id, + sizeof(id)); } if (rdr->conf.flags & F_DOWN) { @@ -789,21 +785,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_notify(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_notify(IMSG_CTL_PUSH_RULESET, &id, + sizeof(id)); } } diff --git a/relayd.h b/relayd.h index 4716091..6938076 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_notify(uint32_t, void *, uint16_t); struct ctl_conn *control_connbyfd(int); /* parse.y */