Index | Thread | Search

From:
Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject:
bgpd: use new imsg API in bgpd.c and kroute.c
To:
tech@openbsd.org
Date:
Mon, 8 Jan 2024 17:52:51 +0100

Download raw body.

Thread
This is another bit of changes to introduce the new ibuf API in bgpd.

The conversion of bgpd.c and kroute.c (the code used in the parent) is
simple and so the conversion is simple. The dispatch_imsg() becomes less
error prone since imsg.data is not passed around anymore.

-- 
:wq Claudio

Index: bgpd.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.c,v
diff -u -p -r1.261 bgpd.c
--- bgpd.c	4 Jan 2024 10:26:14 -0000	1.261
+++ bgpd.c	4 Jan 2024 16:48:57 -0000
@@ -834,6 +834,11 @@ dispatch_imsg(struct imsgbuf *imsgbuf, i
 	struct imsg		 imsg;
 	struct peer		*p;
 	struct rtr_config	*r;
+	struct kroute_full	 kf;
+	struct bgpd_addr	 addr;
+	struct pftable_msg	 pfmsg;
+	struct demote_msg	 demote;
+	char			 reason[REASON_LEN], ifname[IFNAMSIZ];
 	ssize_t			 n;
 	u_int			 rtableid;
 	int			 rv, verbose;
@@ -846,81 +851,73 @@ dispatch_imsg(struct imsgbuf *imsgbuf, i
 		if (n == 0)
 			break;
 
-		switch (imsg.hdr.type) {
+		switch (imsg_get_type(&imsg)) {
 		case IMSG_KROUTE_CHANGE:
 			if (idx != PFD_PIPE_RDE)
 				log_warnx("route request not from RDE");
-			else if (imsg.hdr.len != IMSG_HEADER_SIZE +
-			    sizeof(struct kroute_full))
-				log_warnx("wrong imsg len");
-			else if (kr_change(imsg.hdr.peerid, imsg.data))
+			else if (imsg_get_data(&imsg, &kf, sizeof(kf)) == -1)
+				log_warn("wrong imsg len");
+			else if (kr_change(imsg_get_id(&imsg), &kf))
 				rv = -1;
 			break;
 		case IMSG_KROUTE_DELETE:
 			if (idx != PFD_PIPE_RDE)
 				log_warnx("route request not from RDE");
-			else if (imsg.hdr.len != IMSG_HEADER_SIZE +
-			    sizeof(struct kroute_full))
-				log_warnx("wrong imsg len");
-			else if (kr_delete(imsg.hdr.peerid, imsg.data))
+			else if (imsg_get_data(&imsg, &kf, sizeof(kf)) == -1)
+				log_warn("wrong imsg len");
+			else if (kr_delete(imsg_get_id(&imsg), &kf))
 				rv = -1;
 			break;
 		case IMSG_KROUTE_FLUSH:
 			if (idx != PFD_PIPE_RDE)
 				log_warnx("route request not from RDE");
-			else if (imsg.hdr.len != IMSG_HEADER_SIZE)
-				log_warnx("wrong imsg len");
-			else if (kr_flush(imsg.hdr.peerid))
+			else if (kr_flush(imsg_get_id(&imsg)))
 				rv = -1;
 			break;
 		case IMSG_NEXTHOP_ADD:
 			if (idx != PFD_PIPE_RDE)
 				log_warnx("nexthop request not from RDE");
-			else if (imsg.hdr.len != IMSG_HEADER_SIZE +
-			    sizeof(struct bgpd_addr))
-				log_warnx("wrong imsg len");
+			else if (imsg_get_data(&imsg, &addr, sizeof(addr)) ==
+			    -1)
+				log_warn("wrong imsg len");
 			else {
 				rtableid = conf->default_tableid;
-				if (kr_nexthop_add(rtableid, imsg.data) == -1)
+				if (kr_nexthop_add(rtableid, &addr) == -1)
 					rv = -1;
 			}
 			break;
 		case IMSG_NEXTHOP_REMOVE:
 			if (idx != PFD_PIPE_RDE)
 				log_warnx("nexthop request not from RDE");
-			else if (imsg.hdr.len != IMSG_HEADER_SIZE +
-			    sizeof(struct bgpd_addr))
-				log_warnx("wrong imsg len");
+			else if (imsg_get_data(&imsg, &addr, sizeof(addr)) ==
+			    -1)
+				log_warn("wrong imsg len");
 			else {
 				rtableid = conf->default_tableid;
-				kr_nexthop_delete(rtableid, imsg.data);
+				kr_nexthop_delete(rtableid, &addr);
 			}
 			break;
 		case IMSG_PFTABLE_ADD:
 			if (idx != PFD_PIPE_RDE)
 				log_warnx("pftable request not from RDE");
-			else
-				if (imsg.hdr.len != IMSG_HEADER_SIZE +
-				    sizeof(struct pftable_msg))
-					log_warnx("wrong imsg len");
-				else if (pftable_addr_add(imsg.data) != 0)
-					rv = -1;
+			else if (imsg_get_data(&imsg, &pfmsg, sizeof(pfmsg)) ==
+			    -1)
+				log_warn("wrong imsg len");
+			else if (pftable_addr_add(&pfmsg) != 0)
+				rv = -1;
 			break;
 		case IMSG_PFTABLE_REMOVE:
 			if (idx != PFD_PIPE_RDE)
 				log_warnx("pftable request not from RDE");
-			else
-				if (imsg.hdr.len != IMSG_HEADER_SIZE +
-				    sizeof(struct pftable_msg))
-					log_warnx("wrong imsg len");
-				else if (pftable_addr_remove(imsg.data) != 0)
-					rv = -1;
+			else if (imsg_get_data(&imsg, &pfmsg, sizeof(pfmsg)) ==
+			    -1)
+				log_warn("wrong imsg len");
+			else if (pftable_addr_remove(&pfmsg) != 0)
+				rv = -1;
 			break;
 		case IMSG_PFTABLE_COMMIT:
 			if (idx != PFD_PIPE_RDE)
 				log_warnx("pftable request not from RDE");
-			else if (imsg.hdr.len != IMSG_HEADER_SIZE)
-				log_warnx("wrong imsg len");
 			else if (pftable_commit() != 0)
 				rv = -1;
 			break;
@@ -929,7 +926,7 @@ dispatch_imsg(struct imsgbuf *imsgbuf, i
 				log_warnx("pfkey reload request not from SE");
 				break;
 			}
-			p = getpeerbyid(conf, imsg.hdr.peerid);
+			p = getpeerbyid(conf, imsg_get_id(&imsg));
 			if (p != NULL) {
 				if (pfkey_establish(p) == -1)
 					log_peer_warnx(&p->conf,
@@ -941,24 +938,24 @@ dispatch_imsg(struct imsgbuf *imsgbuf, i
 				log_warnx("reload request not from SE");
 			else {
 				reconfig = 1;
-				reconfpid = imsg.hdr.pid;
-				if (imsg.hdr.len == IMSG_HEADER_SIZE +
-				    REASON_LEN && ((char *)imsg.data)[0])
+				reconfpid = imsg_get_pid(&imsg);
+				if (imsg_get_data(&imsg, reason,
+				    sizeof(reason)) == 0 && reason[0] != '\0')
 					log_info("reload due to: %s",
-					    log_reason(imsg.data));
+					    log_reason(reason));
 			}
 			break;
 		case IMSG_CTL_FIB_COUPLE:
 			if (idx != PFD_PIPE_SESSION)
 				log_warnx("couple request not from SE");
 			else
-				kr_fib_couple(imsg.hdr.peerid);
+				kr_fib_couple(imsg_get_id(&imsg));
 			break;
 		case IMSG_CTL_FIB_DECOUPLE:
 			if (idx != PFD_PIPE_SESSION)
 				log_warnx("decouple request not from SE");
 			else
-				kr_fib_decouple(imsg.hdr.peerid);
+				kr_fib_decouple(imsg_get_id(&imsg));
 			break;
 		case IMSG_CTL_KROUTE:
 		case IMSG_CTL_KROUTE_ADDR:
@@ -973,28 +970,29 @@ dispatch_imsg(struct imsgbuf *imsgbuf, i
 		case IMSG_SESSION_DEPENDON:
 			if (idx != PFD_PIPE_SESSION)
 				log_warnx("DEPENDON request not from SE");
-			else if (imsg.hdr.len != IMSG_HEADER_SIZE + IFNAMSIZ)
-				log_warnx("DEPENDON request with wrong len");
+			else if (imsg_get_data(&imsg, ifname, sizeof(ifname)) ==
+			    -1)
+				log_warn("wrong imsg len");
 			else
-				kr_ifinfo(imsg.data);
+				kr_ifinfo(ifname);
 			break;
 		case IMSG_DEMOTE:
 			if (idx != PFD_PIPE_SESSION)
 				log_warnx("demote request not from SE");
-			else if (imsg.hdr.len != IMSG_HEADER_SIZE +
-			    sizeof(struct demote_msg))
-				log_warnx("DEMOTE request with wrong len");
-			else {
-				struct demote_msg	*msg;
-
-				msg = imsg.data;
-				carp_demote_set(msg->demote_group, msg->level);
-			}
+			else if (imsg_get_data(&imsg, &demote, sizeof(demote))
+			    == -1)
+				log_warn("wrong imsg len");
+			else
+				carp_demote_set(demote.demote_group,
+				    demote.level);
 			break;
 		case IMSG_CTL_LOG_VERBOSE:
 			/* already checked by SE */
-			memcpy(&verbose, imsg.data, sizeof(verbose));
-			log_setverbose(verbose);
+			if (imsg_get_data(&imsg, &verbose, sizeof(verbose)) ==
+			    -1)
+				log_warn("wrong imsg len");
+			else
+				log_setverbose(verbose);
 			break;
 		case IMSG_RECONF_DONE:
 			if (reconfpending == 0) {
@@ -1037,12 +1035,12 @@ dispatch_imsg(struct imsgbuf *imsgbuf, i
 				log_warnx("connect request not from RTR");
 			} else {
 				SIMPLEQ_FOREACH(r, &conf->rtrs, entry) {
-					if (imsg.hdr.peerid == r->id)
+					if (imsg_get_id(&imsg) == r->id)
 						break;
 				}
 				if (r == NULL)
 					log_warnx("unknown rtr id %d",
-					    imsg.hdr.peerid);
+					    imsg_get_id(&imsg));
 				else
 					bgpd_rtr_connect(r);
 			}
@@ -1050,32 +1048,35 @@ dispatch_imsg(struct imsgbuf *imsgbuf, i
 		case IMSG_CTL_SHOW_RTR:
 			if (idx == PFD_PIPE_SESSION) {
 				SIMPLEQ_FOREACH(r, &conf->rtrs, entry) {
-					imsg_compose(ibuf_rtr, imsg.hdr.type,
-					    r->id, imsg.hdr.pid, -1, NULL, 0);
+					imsg_compose(ibuf_rtr,
+					    IMSG_CTL_SHOW_RTR, r->id,
+					    imsg_get_pid(&imsg), -1, NULL, 0);
 				}
 				imsg_compose(ibuf_rtr, IMSG_CTL_END,
-				    0, imsg.hdr.pid, -1, NULL, 0);
-			} else if (imsg.hdr.len != IMSG_HEADER_SIZE +
-			    sizeof(struct ctl_show_rtr)) {
-				log_warnx("IMSG_CTL_SHOW_RTR with wrong len");
+				    0, imsg_get_pid(&imsg), -1, NULL, 0);
 			} else if (idx == PFD_PIPE_RTR) {
+				struct ctl_show_rtr rtr;
+				if (imsg_get_data(&imsg, &rtr, sizeof(rtr)) ==
+				    -1) {
+					log_warn("wrong imsg len");
+					break;
+				}
+
 				SIMPLEQ_FOREACH(r, &conf->rtrs, entry) {
-					if (imsg.hdr.peerid == r->id)
+					if (imsg_get_id(&imsg) == r->id)
 						break;
 				}
 				if (r != NULL) {
-					struct ctl_show_rtr *msg;
-					msg = imsg.data;
-					strlcpy(msg->descr, r->descr,
-					    sizeof(msg->descr));
-					msg->local_addr = r->local_addr;
-					msg->remote_addr = r->remote_addr;
-					msg->remote_port = r->remote_port;
-
-					imsg_compose(ibuf_se, imsg.hdr.type,
-					    imsg.hdr.peerid, imsg.hdr.pid,
-					    -1, imsg.data,
-					    imsg.hdr.len - IMSG_HEADER_SIZE);
+					strlcpy(rtr.descr, r->descr,
+					    sizeof(rtr.descr));
+					rtr.local_addr = r->local_addr;
+					rtr.remote_addr = r->remote_addr;
+					rtr.remote_port = r->remote_port;
+
+					imsg_compose(ibuf_se, IMSG_CTL_SHOW_RTR,
+					    imsg_get_id(&imsg),
+					    imsg_get_pid(&imsg), -1,
+					    &rtr, sizeof(rtr));
 				}
 			}
 			break;
@@ -1085,9 +1086,7 @@ dispatch_imsg(struct imsgbuf *imsgbuf, i
 				log_warnx("connect request not from RTR");
 				break;
 			}
-			imsg_compose(ibuf_se, imsg.hdr.type, imsg.hdr.peerid,
-			    imsg.hdr.pid, -1, imsg.data,
-			    imsg.hdr.len - IMSG_HEADER_SIZE);
+			imsg_forward(ibuf_se, &imsg);
 			break;
 		default:
 			break;
Index: kroute.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/kroute.c,v
diff -u -p -r1.308 kroute.c
--- kroute.c	8 Jan 2024 15:08:34 -0000	1.308
+++ kroute.c	8 Jan 2024 16:16:48 -0000
@@ -883,27 +883,30 @@ kr_show_route(struct imsg *imsg)
 	struct kroute		*kr, *kn;
 	struct kroute6		*kr6, *kn6;
 	struct kroute_full	*kf;
-	struct bgpd_addr	*addr;
+	struct bgpd_addr	 addr;
 	struct ctl_kroute_req	 req;
 	struct ctl_show_nexthop	 snh;
 	struct knexthop		*h;
 	struct kif		*kif;
+	uint32_t		 tableid;
+	pid_t			 pid;
 	u_int			 i;
 	u_short			 ifindex = 0;
 
-	switch (imsg->hdr.type) {
+	tableid = imsg_get_id(imsg);
+	pid = imsg_get_pid(imsg);
+	switch (imsg_get_type(imsg)) {
 	case IMSG_CTL_KROUTE:
-		if (imsg->hdr.len != IMSG_HEADER_SIZE + sizeof(req)) {
+		if (imsg_get_data(imsg, &req, sizeof(req)) == -1) {
 			log_warnx("%s: wrong imsg len", __func__);
 			break;
 		}
-		kt = ktable_get(imsg->hdr.peerid);
+		kt = ktable_get(tableid);
 		if (kt == NULL) {
 			log_warnx("%s: table %u does not exist", __func__,
-			    imsg->hdr.peerid);
+			    tableid);
 			break;
 		}
-		memcpy(&req, imsg->data, sizeof(req));
 		if (!req.af || req.af == AF_INET)
 			RB_FOREACH(kr, kroute_tree, &kt->krt) {
 				if (req.flags && (kr->flags & req.flags) == 0)
@@ -913,7 +916,7 @@ kr_show_route(struct imsg *imsg)
 					kf = kr_tofull(kn);
 					kf->priority = kr_priority(kf);
 					send_imsg_session(IMSG_CTL_KROUTE,
-					    imsg->hdr.pid, kf, sizeof(*kf));
+					    pid, kf, sizeof(*kf));
 				} while ((kn = kn->next) != NULL);
 			}
 		if (!req.af || req.af == AF_INET6)
@@ -925,50 +928,48 @@ kr_show_route(struct imsg *imsg)
 					kf = kr6_tofull(kn6);
 					kf->priority = kr_priority(kf);
 					send_imsg_session(IMSG_CTL_KROUTE,
-					    imsg->hdr.pid, kf, sizeof(*kf));
+					    pid, kf, sizeof(*kf));
 				} while ((kn6 = kn6->next) != NULL);
 			}
 		break;
 	case IMSG_CTL_KROUTE_ADDR:
-		if (imsg->hdr.len != IMSG_HEADER_SIZE +
-		    sizeof(struct bgpd_addr)) {
+		if (imsg_get_data(imsg, &addr, sizeof(addr)) == -1) {
 			log_warnx("%s: wrong imsg len", __func__);
 			break;
 		}
-		kt = ktable_get(imsg->hdr.peerid);
+		kt = ktable_get(tableid);
 		if (kt == NULL) {
 			log_warnx("%s: table %u does not exist", __func__,
-			    imsg->hdr.peerid);
+			    tableid);
 			break;
 		}
-		addr = imsg->data;
 		kr = NULL;
-		switch (addr->aid) {
+		switch (addr.aid) {
 		case AID_INET:
-			kr = kroute_match(kt, addr, 1);
+			kr = kroute_match(kt, &addr, 1);
 			if (kr != NULL) {
 				kf = kr_tofull(kr);
 				kf->priority = kr_priority(kf);
 				send_imsg_session(IMSG_CTL_KROUTE,
-				    imsg->hdr.pid, kf, sizeof(*kf));
+				    pid, kf, sizeof(*kf));
 			}
 			break;
 		case AID_INET6:
-			kr6 = kroute6_match(kt, addr, 1);
+			kr6 = kroute6_match(kt, &addr, 1);
 			if (kr6 != NULL) {
 				kf = kr6_tofull(kr6);
 				kf->priority = kr_priority(kf);
 				send_imsg_session(IMSG_CTL_KROUTE,
-				    imsg->hdr.pid, kf, sizeof(*kf));
+				    pid, kf, sizeof(*kf));
 			}
 			break;
 		}
 		break;
 	case IMSG_CTL_SHOW_NEXTHOP:
-		kt = ktable_get(imsg->hdr.peerid);
+		kt = ktable_get(tableid);
 		if (kt == NULL) {
 			log_warnx("%s: table %u does not exist", __func__,
-			    imsg->hdr.peerid);
+			    tableid);
 			break;
 		}
 		RB_FOREACH(h, knexthop_tree, KT2KNT(kt)) {
@@ -997,14 +998,14 @@ kr_show_route(struct imsg *imsg)
 					    kr_show_interface(kif),
 					    sizeof(snh.iface));
 			}
-			send_imsg_session(IMSG_CTL_SHOW_NEXTHOP, imsg->hdr.pid,
+			send_imsg_session(IMSG_CTL_SHOW_NEXTHOP, pid,
 			    &snh, sizeof(snh));
 		}
 		break;
 	case IMSG_CTL_SHOW_INTERFACE:
 		RB_FOREACH(kif, kif_tree, &kit)
 			send_imsg_session(IMSG_CTL_SHOW_INTERFACE,
-			    imsg->hdr.pid, kr_show_interface(kif),
+			    pid, kr_show_interface(kif),
 			    sizeof(struct ctl_show_interface));
 		break;
 	case IMSG_CTL_SHOW_FIB_TABLES:
@@ -1022,14 +1023,14 @@ kr_show_route(struct imsg *imsg)
 			TAILQ_INIT(&ktab.krn);
 
 			send_imsg_session(IMSG_CTL_SHOW_FIB_TABLES,
-			    imsg->hdr.pid, &ktab, sizeof(ktab));
+			    pid, &ktab, sizeof(ktab));
 		}
 		break;
 	default:	/* nada */
 		break;
 	}
 
-	send_imsg_session(IMSG_CTL_END, imsg->hdr.pid, NULL, 0);
+	send_imsg_session(IMSG_CTL_END, pid, NULL, 0);
 }
 
 static void