From: Claudio Jeker Subject: ospfd: change imsg handling for kroute messages To: tech@openbsd.org Date: Tue, 8 Sep 2026 10:57:03 +0200 kroute change and delete messages fumble directly with imsg.data. I like to fix this and instead pass the imsg to these functions. kr_delete is simple, just extract the kroute and off you go. In kr_change this is more complex since multiple struct kroute are sent for multipath routes. I split the kr_change_fib code into two versions. kr_change_one() does an RTM_CHANGE for a single kroute. By doing that kr_change_fib() becomes a bit simpler since that code now handles only RTM_ADD. This reduces the complexity of that function. Internally this still uses ibuf_data() to access the array of kroutes. I did this since the code really needs to access this as an array and it uses the right bounds. Using diff -w will show that kr_change_fib() was actually only minimally changed. -- :wq Claudio Index: kroute.c =================================================================== RCS file: /cvs/src/usr.sbin/ospfd/kroute.c,v diff -u -p -r1.121 kroute.c --- kroute.c 31 Aug 2026 08:57:41 -0000 1.121 +++ kroute.c 8 Sep 2026 08:21:31 -0000 @@ -74,7 +74,8 @@ int kr_redist_eval(struct kroute *, stru void kr_redistribute(struct kroute_node *); int kroute_compare(struct kroute_node *, struct kroute_node *); int kif_compare(struct kif_node *, struct kif_node *); -int kr_change_fib(struct kroute_node *, struct kroute *, int, int); +int kr_change_one(struct kroute_node *, struct kroute *); +int kr_change_fib(struct kroute_node *, struct ibuf *); int kr_delete_fib(struct kroute_node *); struct kroute_node *kroute_find(in_addr_t, u_int8_t, u_int8_t); @@ -200,37 +201,59 @@ kr_init(int fs, u_int rdomain, int redis } int -kr_change_fib(struct kroute_node *kr, struct kroute *kroute, int krcount, - int action) +kr_change_one(struct kroute_node *kr, struct kroute *kroute) { - int i; + /* nexthop within 127/8 -> ignore silently */ + if ((kroute->nexthop.s_addr & htonl(IN_CLASSA_NET)) == + htonl(INADDR_LOOPBACK & IN_CLASSA_NET)) + return (0); + + /* send update */ + if (send_rtmsg(kr_state.fd, RTM_CHANGE, kroute) == -1) + return (-1); + + /* modify first and only entry */ + kr->r.nexthop.s_addr = kroute->nexthop.s_addr; + kr->r.flags = kroute->flags | F_OSPFD_INSERTED; + kr->r.ext_tag = kroute->ext_tag; + rtlabel_unref(kr->r.rtlabel); + kr->r.rtlabel = rtlabel_tag2id(kroute->ext_tag); + rtlabel_ref(kr->r.rtlabel); + + return (0); +} + +int +kr_change_fib(struct kroute_node *kr, struct ibuf *ibuf) +{ + unsigned int i, krcount; struct kroute_node *kn, *nkn; + struct kroute *kroute; - if (action == RTM_ADD) { - /* - * First remove all stale multipath routes. - * This step must be skipped when the action is RTM_CHANGE - * because it is already a single path route that will be - * changed. - */ - for (kn = kr; kn != NULL; kn = nkn) { - for (i = 0; i < krcount; i++) { - if (kn->r.nexthop.s_addr == - kroute[i].nexthop.s_addr) - break; - } - nkn = kn->next; - if (i == krcount) { - /* stale route */ - if (kr_delete_fib(kn) == -1) - log_warnx("kr_delete_fib failed"); - /* - * if head element was removed we need to adjust - * the head - */ - if (kr == kn) - kr = nkn; - } + + krcount = ibuf_size(ibuf) / sizeof(*kroute); + kroute = ibuf_data(ibuf); + + /* + * First remove all stale multipath routes. + */ + for (kn = kr; kn != NULL; kn = nkn) { + for (i = 0; i < krcount; i++) { + if (kn->r.nexthop.s_addr == + kroute[i].nexthop.s_addr) + break; + } + nkn = kn->next; + if (i == krcount) { + /* stale route */ + if (kr_delete_fib(kn) == -1) + log_warnx("kr_delete_fib failed"); + /* + * if head element was removed we need to adjust + * the head + */ + if (kr == kn) + kr = nkn; } } @@ -243,41 +266,36 @@ kr_change_fib(struct kroute_node *kr, st htonl(INADDR_LOOPBACK & IN_CLASSA_NET)) continue; - if (action == RTM_ADD && kr) { - for (kn = kr; kn != NULL; kn = kn->next) { - if (kn->r.nexthop.s_addr == - kroute[i].nexthop.s_addr) - break; - } + for (kn = kr; kn != NULL; kn = kn->next) { + if (kn->r.nexthop.s_addr == + kroute[i].nexthop.s_addr) + break; + } - if (kn != NULL) { - uint16_t label; + if (kn != NULL) { + uint16_t label; - /* nexthop already present, just change it */ - kn->r.flags = kroute[i].flags | - F_OSPFD_INSERTED; - kn->r.ext_tag = kroute[i].ext_tag; - label = kn->r.rtlabel; - rtlabel_unref(kn->r.rtlabel); - kn->r.rtlabel = rtlabel_tag2id(kn->r.ext_tag); - rtlabel_ref(kn->r.rtlabel); - - /* update if label changed */ - if (kn->r.rtlabel != label) { - if (send_rtmsg(kr_state.fd, RTM_CHANGE, - &kn->r) == -1) - return (-1); - } - continue; + /* nexthop already present, just change it */ + kn->r.flags = kroute[i].flags | + F_OSPFD_INSERTED; + kn->r.ext_tag = kroute[i].ext_tag; + label = kn->r.rtlabel; + rtlabel_unref(kn->r.rtlabel); + kn->r.rtlabel = rtlabel_tag2id(kn->r.ext_tag); + rtlabel_ref(kn->r.rtlabel); + + /* update if label changed */ + if (kn->r.rtlabel != label) { + if (send_rtmsg(kr_state.fd, RTM_CHANGE, + &kn->r) == -1) + return (-1); } - } else - /* modify first entry */ - kn = kr; - - /* create new entry unless we are changing the first entry */ - if (action == RTM_ADD) - if ((kn = calloc(1, sizeof(*kn))) == NULL) - fatal(NULL); + continue; + } + + /* create new entry */ + if ((kn = calloc(1, sizeof(*kn))) == NULL) + fatal(NULL); kn->r.prefix.s_addr = kroute[i].prefix.s_addr; kn->r.prefixlen = kroute[i].prefixlen; @@ -286,35 +304,45 @@ kr_change_fib(struct kroute_node *kr, st kn->r.flags = kroute[i].flags | F_OSPFD_INSERTED; kn->r.ext_tag = kroute[i].ext_tag; - rtlabel_unref(kn->r.rtlabel); /* for RTM_CHANGE */ kn->r.rtlabel = rtlabel_tag2id(kn->r.ext_tag); rtlabel_ref(kn->r.rtlabel); - if (action == RTM_ADD) - kroute_insert(kn); + kroute_insert(kn); /* send update */ - if (send_rtmsg(kr_state.fd, action, &kn->r) == -1) + if (send_rtmsg(kr_state.fd, RTM_ADD, &kn->r) == -1) return (-1); - - action = RTM_ADD; } + return (0); } int -kr_change(struct kroute *kroute, int krcount) +kr_change(struct imsg *imsg) { + struct ibuf ibuf; + struct kroute kroute; struct kroute_node *kr; - int action = RTM_ADD; - kr = kroute_find(kroute->prefix.s_addr, kroute->prefixlen, + if (imsg_get_ibuf(imsg, &ibuf) == -1) + fatalx("bad KROUTE_CHANGE imsg received"); + + if (ibuf_size(&ibuf) % sizeof(kroute) != 0) + fatalx("bad KROUTE_CHANGE imsg received"); + + if (ibuf_get(&ibuf, &kroute, sizeof(kroute)) == -1) + fatalx("bad KROUTE_CHANGE imsg received"); + + kr = kroute_find(kroute.prefix.s_addr, kroute.prefixlen, kr_state.fib_prio); - if (kr != NULL && kr->next == NULL && krcount == 1) - /* single path OSPF route */ - action = RTM_CHANGE; - return (kr_change_fib(kr, kroute, krcount, action)); + if (kr != NULL && kr->next == NULL && ibuf_size(&ibuf) == 0) { + /* single path OSPF route, do inline change */ + return (kr_change_one(kr, &kroute)); + } + + ibuf_rewind(&ibuf); + return (kr_change_fib(kr, &ibuf)); } int @@ -334,11 +362,15 @@ kr_delete_fib(struct kroute_node *kr) } int -kr_delete(struct kroute *kroute) +kr_delete(struct imsg *imsg) { + struct kroute kroute; struct kroute_node *kr, *nkr; - if ((kr = kroute_find(kroute->prefix.s_addr, kroute->prefixlen, + if (imsg_get_data(imsg, &kroute, sizeof(kroute)) == -1) + fatalx("bad KROUTE_DELETE imsg received"); + + if ((kr = kroute_find(kroute.prefix.s_addr, kroute.prefixlen, kr_state.fib_prio)) == NULL) return (0); Index: ospfd.c =================================================================== RCS file: /cvs/src/usr.sbin/ospfd/ospfd.c,v diff -u -p -r1.128 ospfd.c --- ospfd.c 17 Aug 2026 08:58:47 -0000 1.128 +++ ospfd.c 8 Sep 2026 08:22:39 -0000 @@ -446,7 +446,7 @@ main_dispatch_rde(int fd, short event, v struct imsgev *iev = bula; struct imsgbuf *ibuf; struct imsg imsg; - int n, count, shut = 0; + int n, shut = 0; ibuf = &iev->ibuf; @@ -473,14 +473,12 @@ main_dispatch_rde(int fd, short event, v switch (imsg.hdr.type) { case IMSG_KROUTE_CHANGE: - count = (imsg.hdr.len - IMSG_HEADER_SIZE) / - sizeof(struct kroute); - if (kr_change(imsg.data, count)) + if (kr_change(&imsg)) log_warn("main_dispatch_rde: error changing " "route"); break; case IMSG_KROUTE_DELETE: - if (kr_delete(imsg.data)) + if (kr_delete(&imsg)) log_warn("main_dispatch_rde: error deleting " "route"); break; Index: ospfd.h =================================================================== RCS file: /cvs/src/usr.sbin/ospfd/ospfd.h,v diff -u -p -r1.110 ospfd.h --- ospfd.h 27 Aug 2026 21:18:41 -0000 1.110 +++ ospfd.h 8 Sep 2026 08:21:31 -0000 @@ -580,8 +580,8 @@ u_int16_t iso_cksum(void *, u_int16_t, int kif_init(void); void kif_clear(void); int kr_init(int, u_int, int, u_int8_t); -int kr_change(struct kroute *, int); -int kr_delete(struct kroute *); +int kr_change(struct imsg *); +int kr_delete(struct imsg *); void kr_shutdown(void); void kr_fib_couple(void); void kr_fib_decouple(void);