From: Claudio Jeker Subject: ospfd: rewrite handling of LS_UPD, LS_FLOOD and LS_SNAP in the ospf engine To: tech@openbsd.org Date: Mon, 31 Aug 2026 11:35:16 +0200 The code handling LSA updates and flooding in the ospf engine is doing a lot more work than needed. Also it fumbles inside of the imsg. Instead of passing imsg data around pass the lsa cache ref to lsa_flood and ls_retrans_list_add. The ref holds all the data needed for those calls. lsa_cache_add is also changed to pass an ibuf instead of imsg->data and it is more careful at extracting the data. On top of this lsa_cache_get becomes unused and is replaced by lsa_cache_ref which is used in ls_retrans_list_add to increase the refcount on the lsa_ref. -- :wq Claudio Index: lsupdate.c =================================================================== RCS file: /cvs/src/usr.sbin/ospfd/lsupdate.c,v diff -u -p -r1.56 lsupdate.c --- lsupdate.c 31 Aug 2026 07:57:09 -0000 1.56 +++ lsupdate.c 31 Aug 2026 09:11:10 -0000 @@ -41,12 +41,14 @@ int send_ls_update(struct ibuf *, struct void ls_retrans_list_insert(struct nbr *, struct lsa_entry *); void ls_retrans_list_remove(struct nbr *, struct lsa_entry *); +struct lsa_ref *lsa_cache_ref(struct lsa_ref *); + /* link state update packet handling */ int -lsa_flood(struct iface *iface, struct nbr *originator, struct lsa_hdr *lsa_hdr, - void *data) +lsa_flood(struct iface *iface, struct nbr *originator, struct lsa_ref *ref) { struct nbr *nbr; + struct lsa_hdr *lsa_hdr = &ref->hdr; struct lsa_entry *le = NULL; int queued = 0, dont_ack = 0; int r; @@ -89,11 +91,11 @@ lsa_flood(struct iface *iface, struct nb /* non DR or BDR router keep all lsa in one retrans list */ if (iface->state & IF_STA_DROTHER) { if (!queued) - ls_retrans_list_add(iface->self, data, + ls_retrans_list_add(iface->self, ref, iface->rxmt_interval, 0); queued = 1; } else { - ls_retrans_list_add(nbr, data, iface->rxmt_interval, 0); + ls_retrans_list_add(nbr, ref, iface->rxmt_interval, 0); queued = 1; } } @@ -117,7 +119,7 @@ lsa_flood(struct iface *iface, struct nb switch (iface->type) { case IF_TYPE_POINTOPOINT: case IF_TYPE_BROADCAST: - ls_retrans_list_add(iface->self, data, 0, 1); + ls_retrans_list_add(iface->self, ref, 0, 1); break; case IF_TYPE_NBMA: case IF_TYPE_POINTOMULTIPOINT: @@ -135,7 +137,7 @@ lsa_flood(struct iface *iface, struct nb lsa_hdr->adv_rtr != le->le_lsa->adv_rtr) continue; } - ls_retrans_list_add(nbr, data, 0, 1); + ls_retrans_list_add(nbr, ref, 0, 1); } break; default: @@ -296,20 +298,16 @@ recv_ls_update(struct nbr *nbr, char *bu /* link state retransmit list */ void -ls_retrans_list_add(struct nbr *nbr, struct lsa_hdr *lsa, +ls_retrans_list_add(struct nbr *nbr, struct lsa_ref *ref, unsigned short timeout, unsigned short oneshot) { struct timeval tv; struct lsa_entry *le; - struct lsa_ref *ref; - - if ((ref = lsa_cache_get(lsa)) == NULL) - fatalx("King Bula sez: somebody forgot to lsa_cache_add"); if ((le = calloc(1, sizeof(*le))) == NULL) fatal("ls_retrans_list_add"); - le->le_ref = ref; + le->le_ref = lsa_cache_ref(ref); le->le_when = timeout; le->le_oneshot = oneshot; @@ -465,8 +463,7 @@ ls_retrans_timer(int fd, short event, vo * old retransmission needs to be converted into * flood by rerunning the lsa_flood. */ - lsa_flood(nbr->iface, nbr, &le->le_ref->hdr, - le->le_ref->data); + lsa_flood(nbr->iface, nbr, le->le_ref); ls_retrans_list_free(nbr, le); /* ls_retrans_list_free retriggers the timer */ return; @@ -565,15 +562,18 @@ lsa_hash_hdr(const struct lsa_hdr *hdr) } struct lsa_ref * -lsa_cache_add(void *data, u_int16_t len) +lsa_cache_add(struct ibuf *buf) { struct lsa_cache_head *head; struct lsa_ref *ref, *old; struct timespec tp; + size_t len; if ((ref = calloc(1, sizeof(*ref))) == NULL) fatal("lsa_cache_add"); - memcpy(&ref->hdr, data, sizeof(ref->hdr)); + if (ibuf_get(buf, &ref->hdr, sizeof(ref->hdr)) == -1) + fatal("lsa_cache_add"); + ibuf_rewind(buf); if ((old = lsa_cache_look(&ref->hdr))) { free(ref); @@ -581,9 +581,10 @@ lsa_cache_add(void *data, u_int16_t len) return (old); } + len = ibuf_size(buf); if ((ref->data = malloc(len)) == NULL) fatal("lsa_cache_add"); - memcpy(ref->data, data, len); + memcpy(ref->data, ibuf_data(buf), len); clock_gettime(CLOCK_MONOTONIC, &tp); ref->stamp = tp.tv_sec; @@ -596,14 +597,9 @@ lsa_cache_add(void *data, u_int16_t len) } struct lsa_ref * -lsa_cache_get(struct lsa_hdr *lsa_hdr) +lsa_cache_ref(struct lsa_ref *ref) { - struct lsa_ref *ref; - - ref = lsa_cache_look(lsa_hdr); - if (ref) - ref->refcnt++; - + ref->refcnt++; return (ref); } Index: ospfe.c =================================================================== RCS file: /cvs/src/usr.sbin/ospfd/ospfe.c,v diff -u -p -r1.123 ospfe.c --- ospfe.c 28 Jul 2026 11:48:14 -0000 1.123 +++ ospfe.c 31 Aug 2026 09:17:50 -0000 @@ -489,6 +489,7 @@ ospfe_dispatch_main(int fd, short event, void ospfe_dispatch_rde(int fd, short event, void *bula) { + struct ibuf buf; struct lsa_hdr lsa_hdr; struct imsgev *iev = bula; struct imsgbuf *ibuf = &iev->ibuf; @@ -501,7 +502,7 @@ ospfe_dispatch_rde(int fd, short event, struct imsg imsg; struct abr_rtr ar; int n, noack = 0, shut = 0; - u_int16_t l, age; + u_int16_t age; if (event & EV_READ) { if ((n = imsgbuf_read(ibuf)) == -1) @@ -601,15 +602,12 @@ ospfe_dispatch_rde(int fd, short event, if (nbr == NULL) break; - l = imsg.hdr.len - IMSG_HEADER_SIZE; - if (l < sizeof(lsa_hdr)) - fatalx("ospfe_dispatch_rde: " - "bad imsg size"); - memcpy(&lsa_hdr, imsg.data, sizeof(lsa_hdr)); + if (imsg_get_ibuf(&imsg, &buf) == -1) + fatalx("bad LS_FLOOD imsg received"); - ref = lsa_cache_add(imsg.data, l); + ref = lsa_cache_add(&buf); - if (lsa_hdr.type == LSA_TYPE_EXTERNAL) { + if (ref->hdr.type == LSA_TYPE_EXTERNAL) { /* * flood on all areas but stub areas and * virtual links @@ -619,16 +617,14 @@ ospfe_dispatch_rde(int fd, short event, continue; LIST_FOREACH(iface, &area->iface_list, entry) { - noack += lsa_flood(iface, nbr, - &lsa_hdr, imsg.data); + noack += lsa_flood(iface, nbr, ref); } } - } else if (lsa_hdr.type == LSA_TYPE_LINK_OPAQ) { + } else if (ref->hdr.type == LSA_TYPE_LINK_OPAQ) { /* * Flood on interface only */ - noack += lsa_flood(nbr->iface, nbr, - &lsa_hdr, imsg.data); + noack += lsa_flood(nbr->iface, nbr, ref); } else { /* * Flood on all area interfaces. For @@ -636,14 +632,13 @@ ospfe_dispatch_rde(int fd, short event, */ area = nbr->iface->area; LIST_FOREACH(iface, &area->iface_list, entry) { - noack += lsa_flood(iface, nbr, - &lsa_hdr, imsg.data); + noack += lsa_flood(iface, nbr, ref); } /* XXX virtual links */ } /* remove from ls_req_list */ - le = ls_req_list_get(nbr, &lsa_hdr); + le = ls_req_list_get(nbr, &ref->hdr); if (!(nbr->state & NBR_STA_FULL) && le != NULL) { ls_req_list_free(nbr, le); /* @@ -660,7 +655,7 @@ ospfe_dispatch_rde(int fd, short event, nbr->iface->dr == nbr) { /* delayed ack */ lhp = lsa_hdr_new(); - memcpy(lhp, &lsa_hdr, sizeof(*lhp)); + memcpy(lhp, &ref->hdr, sizeof(*lhp)); ls_ack_list_add(nbr->iface, lhp); } } @@ -677,10 +672,8 @@ ospfe_dispatch_rde(int fd, short event, * IMSG_LS_SNAP is used in one case: * in EXSTART when the LSA has age MaxAge */ - l = imsg.hdr.len - IMSG_HEADER_SIZE; - if (l < sizeof(lsa_hdr)) - fatalx("ospfe_dispatch_rde: " - "bad imsg size"); + if (imsg_get_ibuf(&imsg, &buf) == -1) + fatalx("bad LS_UPD/SNAP imsg received"); nbr = nbr_find_peerid(imsg.hdr.peerid); if (nbr == NULL) @@ -693,13 +686,14 @@ ospfe_dispatch_rde(int fd, short event, nbr->state != NBR_STA_SNAP) break; - memcpy(&age, imsg.data, sizeof(age)); - ref = lsa_cache_add(imsg.data, l); + ref = lsa_cache_add(&buf); + age = ref->hdr.age; + if (ntohs(age) >= MAX_AGE) /* add to retransmit list */ - ls_retrans_list_add(nbr, imsg.data, 0, 0); + ls_retrans_list_add(nbr, ref, 0, 0); else - ls_retrans_list_add(nbr, imsg.data, 0, 1); + ls_retrans_list_add(nbr, ref, 0, 1); lsa_cache_put(ref, nbr); break; Index: ospfe.h =================================================================== RCS file: /cvs/src/usr.sbin/ospfd/ospfe.h,v diff -u -p -r1.49 ospfe.h --- ospfe.h 18 May 2024 11:17:30 -0000 1.49 +++ ospfe.h 31 Aug 2026 09:11:10 -0000 @@ -180,11 +180,10 @@ void start_ls_req_tx_timer(struct nbr * void stop_ls_req_tx_timer(struct nbr *); /* lsupdate.c */ -int lsa_flood(struct iface *, struct nbr *, struct lsa_hdr *, - void *); +int lsa_flood(struct iface *, struct nbr *, struct lsa_ref *); void recv_ls_update(struct nbr *, char *, u_int16_t); -void ls_retrans_list_add(struct nbr *, struct lsa_hdr *, +void ls_retrans_list_add(struct nbr *, struct lsa_ref *, unsigned short, unsigned short); int ls_retrans_list_del(struct nbr *, struct lsa_hdr *); struct lsa_entry *ls_retrans_list_get(struct nbr *, struct lsa_hdr *); @@ -193,8 +192,7 @@ void ls_retrans_list_clr(struct nbr *) void ls_retrans_timer(int, short, void *); void lsa_cache_init(u_int32_t); -struct lsa_ref *lsa_cache_add(void *, u_int16_t); -struct lsa_ref *lsa_cache_get(struct lsa_hdr *); +struct lsa_ref *lsa_cache_add(struct ibuf *); void lsa_cache_put(struct lsa_ref *, struct nbr *); /* neighbor.c */