From: Claudio Jeker 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 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