Index | Thread | Search

From:
Rafael Sadowski <rafael@sizeofvoid.org>
Subject:
Re: relayd: use imsg_compose() instead of handcrafted imsg
To:
tech@openbsd.org
Date:
Wed, 29 Jul 2026 21:21:52 +0200

Download raw body.

Thread
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 <rafael@sizeofvoid.org>
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 */