From: Alexander Bluhm <bluhm@openbsd.org>
Subject: struct rtentry reorder
To: tech@openbsd.org
Date: Thu, 13 Mar 2025 21:19:06 +0100

Hi,

I see races in our routing code.  The union of rt_gwroute and
rt_cachecnt makes debugging harder.  In theory the RTF_GATEWAY flag
distinguishes which field is relevant, but that does not help when
hunting bugs in this area.  The benefit of saving 4 bytes in struct
rtentry is minimal.

Break up the RT_gw union.  To keep memory compact, sort pointers
to the beginning and integres to the end of struct rtentry.

ok?

bluhm

Index: net/route.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/route.h,v
diff -u -p -r1.215 route.h
--- net/route.h	7 Mar 2025 10:18:15 -0000	1.215
+++ net/route.h	13 Mar 2025 20:07:28 -0000
@@ -116,25 +116,21 @@ struct rttimer;
  */
 
 struct rtentry {
-	struct mutex	 rt_mtx;
 	struct sockaddr	*rt_dest;	/* [I] destination */
 	SRPL_ENTRY(rtentry) rt_next;	/* [R] next mpath entry to our dst */
 	struct sockaddr	*rt_gateway;	/* [X] gateway address */
 	struct ifaddr	*rt_ifa;	/* [N] interface addr to use */
 	caddr_t		 rt_llinfo;	/* [L] pointer to link level info or
 					   an MPLS structure */
-	union {
-		struct rtentry	*_nh;	/* [X] rtentry for rt_gateway */
-		unsigned int	 _ref;	/* [r] # gateway rtentry refs */
-	} RT_gw;
-#define rt_gwroute	 RT_gw._nh
-#define rt_cachecnt	 RT_gw._ref
+	struct rtentry	*rt_gwroute;	/* [X] rtentry for rt_gateway */
 	struct rtentry	*rt_parent;	/* [N] if cloned, parent rtentry */
 	LIST_HEAD(, rttimer) rt_timer;  /* queue of timeouts for misc funcs */
+	struct mutex	 rt_mtx;	/* lock members of this struct */
+	struct refcnt	 rt_refcnt;	/* # held references */
 	struct rt_kmetrics rt_rmx;	/* metrics used by rx'ing protocols */
+	unsigned int	 rt_cachecnt;	/* [r] # gateway rtentry refs */
 	unsigned int	 rt_ifidx;	/* [N] interface to use */
 	unsigned int	 rt_flags;	/* [X] up/down?, host/net */
-	struct refcnt	 rt_refcnt;	/* # held references */
 	int		 rt_plen;	/* [I] prefix length */
 	uint16_t	 rt_labelid;	/* [N] route label ID */
 	uint8_t		 rt_priority;	/* [N] routing priority to use */