Index | Thread | Search

From:
Claudio Jeker <cjeker@diehard.n-r-g.com>
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

Download raw body.

Thread
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 */