From: Alexander Bluhm <bluhm@openbsd.org>
Subject: Re: use struct route in ip input
To: tech@openbsd.org
Date: Tue, 16 Apr 2024 00:02:10 +0200

On Tue, Apr 09, 2024 at 03:58:05PM +0200, Alexander Bluhm wrote:
> Hi,
> 
> Instaed of passing a struct rtentry from ip_input() to ip_forward()
> and then embed it into a struct route for ip_output(), start with
> struct route and pass it along.  Then the route cache is used
> consistently.  Also the route cache hit and missed counters should
> reflect reality with this diff.
> 
> There is a small difference in the code.  in_ouraddr() checks for
> NULL and not rtisvalid().  Previous discussion showed that the route
> UP flag should only be considered for multipath routing.  Otherwise
> it does not mean anything.  Especially the local and broadcast check
> should not be affected by interface link status.
> 
> When doing cache lookups route must be valid, but after rtalloc_mpath()
> lookup, we use any route we get.
> 
> ok?
> 
> bluhm

Anyone?

Index: netinet/ip_input.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_input.c,v
diff -u -p -r1.392 ip_input.c
--- netinet/ip_input.c	14 Apr 2024 20:46:27 -0000	1.392
+++ netinet/ip_input.c	15 Apr 2024 21:59:44 -0000
@@ -138,7 +138,7 @@ extern struct niqueue		arpinq;
 
 int	ip_ours(struct mbuf **, int *, int, int);
 int	ip_dooptions(struct mbuf *, struct ifnet *);
-int	in_ouraddr(struct mbuf *, struct ifnet *, struct rtentry **);
+int	in_ouraddr(struct mbuf *, struct ifnet *, struct route *);
 
 int		ip_fragcheck(struct mbuf **, int *);
 struct mbuf *	ip_reass(struct ipqent *, struct ipq *);
@@ -424,9 +424,9 @@ bad:
 int
 ip_input_if(struct mbuf **mp, int *offp, int nxt, int af, struct ifnet *ifp)
 {
-	struct mbuf	*m;
-	struct rtentry	*rt = NULL;
-	struct ip	*ip;
+	struct route ro;
+	struct mbuf *m;
+	struct ip *ip;
 	int hlen;
 #if NPF > 0
 	struct in_addr odst;
@@ -435,6 +435,7 @@ ip_input_if(struct mbuf **mp, int *offp,
 
 	KASSERT(*offp == 0);
 
+	ro.ro_rt = NULL;
 	ipstat_inc(ips_total);
 	m = *mp = ipv4_check(ifp, *mp);
 	if (m == NULL)
@@ -482,7 +483,7 @@ ip_input_if(struct mbuf **mp, int *offp,
 		goto out;
 	}
 
-	switch(in_ouraddr(m, ifp, &rt)) {
+	switch(in_ouraddr(m, ifp, &ro)) {
 	case 2:
 		goto bad;
 	case 1:
@@ -584,14 +585,14 @@ ip_input_if(struct mbuf **mp, int *offp,
 	}
 #endif /* IPSEC */
 
-	ip_forward(m, ifp, rt, pfrdr);
+	ip_forward(m, ifp, &ro, pfrdr);
 	*mp = NULL;
 	return IPPROTO_DONE;
  bad:
 	nxt = IPPROTO_DONE;
 	m_freemp(mp);
  out:
-	rtfree(rt);
+	rtfree(ro.ro_rt);
 	return nxt;
 }
 
@@ -805,11 +806,10 @@ ip_deliver(struct mbuf **mp, int *offp, 
 #undef IPSTAT_INC
 
 int
-in_ouraddr(struct mbuf *m, struct ifnet *ifp, struct rtentry **prt)
+in_ouraddr(struct mbuf *m, struct ifnet *ifp, struct route *ro)
 {
 	struct rtentry		*rt;
 	struct ip		*ip;
-	struct sockaddr_in	 sin;
 	int			 match = 0;
 
 #if NPF > 0
@@ -826,13 +826,8 @@ in_ouraddr(struct mbuf *m, struct ifnet 
 
 	ip = mtod(m, struct ip *);
 
-	memset(&sin, 0, sizeof(sin));
-	sin.sin_len = sizeof(sin);
-	sin.sin_family = AF_INET;
-	sin.sin_addr = ip->ip_dst;
-	rt = rtalloc_mpath(sintosa(&sin), &ip->ip_src.s_addr,
-	    m->m_pkthdr.ph_rtableid);
-	if (rtisvalid(rt)) {
+	rt = route_mpath(ro, &ip->ip_dst, &ip->ip_src, m->m_pkthdr.ph_rtableid);
+	if (rt != NULL) {
 		if (ISSET(rt->rt_flags, RTF_LOCAL))
 			match = 1;
 
@@ -848,7 +843,6 @@ in_ouraddr(struct mbuf *m, struct ifnet 
 			m->m_flags |= M_BCAST;
 		}
 	}
-	*prt = rt;
 
 	if (!match) {
 		struct ifaddr *ifa;
@@ -1527,11 +1521,12 @@ const u_char inetctlerrmap[PRC_NCMDS] = 
  * via a source route.
  */
 void
-ip_forward(struct mbuf *m, struct ifnet *ifp, struct rtentry *rt, int srcrt)
+ip_forward(struct mbuf *m, struct ifnet *ifp, struct route *ro, int srcrt)
 {
 	struct mbuf mfake, *mcopy;
 	struct ip *ip = mtod(m, struct ip *);
-	struct route ro;
+	struct route iproute;
+	struct rtentry *rt;
 	int error = 0, type = 0, code = 0, destmtu = 0, fake = 0, len;
 	u_int32_t dest;
 
@@ -1546,19 +1541,16 @@ ip_forward(struct mbuf *m, struct ifnet 
 		goto done;
 	}
 
-	ro.ro_rt = NULL;
-	route_cache(&ro, &ip->ip_dst, &ip->ip_src, m->m_pkthdr.ph_rtableid);
-	if (!rtisvalid(rt)) {
-		rtfree(rt);
-		rt = rtalloc_mpath(&ro.ro_dstsa, &ip->ip_src.s_addr,
-		    m->m_pkthdr.ph_rtableid);
-		if (rt == NULL) {
-			ipstat_inc(ips_noroute);
-			icmp_error(m, ICMP_UNREACH, ICMP_UNREACH_HOST, dest, 0);
-			return;
-		}
+	if (ro == NULL) {
+		ro = &iproute;
+		ro->ro_rt = NULL;
+	}
+	rt = route_mpath(ro, &ip->ip_dst, &ip->ip_src, m->m_pkthdr.ph_rtableid);
+	if (rt == NULL) {
+		ipstat_inc(ips_noroute);
+		icmp_error(m, ICMP_UNREACH, ICMP_UNREACH_HOST, dest, 0);
+		goto done;
 	}
-	ro.ro_rt = rt;
 
 	/*
 	 * Save at most 68 bytes of the packet in case
@@ -1609,10 +1601,10 @@ ip_forward(struct mbuf *m, struct ifnet 
 		}
 	}
 
-	error = ip_output(m, NULL, &ro,
+	error = ip_output(m, NULL, ro,
 	    (IP_FORWARDING | (ip_directedbcast ? IP_ALLOWBROADCAST : 0)),
 	    NULL, NULL, 0);
-	rt = ro.ro_rt;
+	rt = ro->ro_rt;
 	if (error)
 		ipstat_inc(ips_cantforward);
 	else {
@@ -1680,9 +1672,10 @@ ip_forward(struct mbuf *m, struct ifnet 
 		icmp_error(mcopy, type, code, dest, destmtu);
 
  done:
+	if (ro == &iproute)
+		rtfree(ro->ro_rt);
 	if (fake)
 		m_tag_delete_chain(&mfake);
-	rtfree(rt);
 }
 
 int
Index: netinet/ip_var.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_var.h,v
diff -u -p -r1.115 ip_var.h
--- netinet/ip_var.h	14 Apr 2024 20:46:27 -0000	1.115
+++ netinet/ip_var.h	15 Apr 2024 21:59:44 -0000
@@ -260,7 +260,7 @@ void	 ip_savecontrol(struct inpcb *, str
 	    struct mbuf *);
 int	 ip_input_if(struct mbuf **, int *, int, int, struct ifnet *);
 int	 ip_deliver(struct mbuf **, int *, int, int, int);
-void	 ip_forward(struct mbuf *, struct ifnet *, struct rtentry *, int);
+void	 ip_forward(struct mbuf *, struct ifnet *, struct route *, int);
 int	 rip_ctloutput(int, struct socket *, int, int, struct mbuf *);
 void	 rip_init(void);
 int	 rip_input(struct mbuf **, int *, int, int);
Index: netinet6/ip6_forward.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_forward.c,v
diff -u -p -r1.116 ip6_forward.c
--- netinet6/ip6_forward.c	28 Feb 2024 10:57:20 -0000	1.116
+++ netinet6/ip6_forward.c	15 Apr 2024 21:59:44 -0000
@@ -82,11 +82,12 @@
  */
 
 void
-ip6_forward(struct mbuf *m, struct rtentry *rt, int srcrt)
+ip6_forward(struct mbuf *m, struct route *ro, int srcrt)
 {
 	struct ip6_hdr *ip6 = mtod(m, struct ip6_hdr *);
+	struct route iproute;
+	struct rtentry *rt;
 	struct sockaddr *dst;
-	struct route ro;
 	struct ifnet *ifp = NULL;
 	int error = 0, type = 0, code = 0, destmtu = 0;
 	struct mbuf *mcopy;
@@ -165,25 +166,22 @@ reroute:
 	}
 #endif /* IPSEC */
 
-	ro.ro_rt = NULL;
-	route6_cache(&ro, &ip6->ip6_dst, &ip6->ip6_src,
+	if (ro == NULL) {
+		ro = &iproute;
+		ro->ro_rt = NULL;
+	}
+	rt = route6_mpath(ro, &ip6->ip6_dst, &ip6->ip6_src,
 	    m->m_pkthdr.ph_rtableid);
-	dst = &ro.ro_dstsa;
-	if (!rtisvalid(rt)) {
-		rtfree(rt);
-		rt = rtalloc_mpath(dst, &ip6->ip6_src.s6_addr32[0],
-		    m->m_pkthdr.ph_rtableid);
-		if (rt == NULL) {
-			ip6stat_inc(ip6s_noroute);
-			if (mcopy != NULL) {
-				icmp6_error(mcopy, ICMP6_DST_UNREACH,
-					    ICMP6_DST_UNREACH_NOROUTE, 0);
-			}
-			m_freem(m);
-			goto done;
+	if (rt == NULL) {
+		ip6stat_inc(ip6s_noroute);
+		if (mcopy != NULL) {
+			icmp6_error(mcopy, ICMP6_DST_UNREACH,
+				    ICMP6_DST_UNREACH_NOROUTE, 0);
 		}
+		m_freem(m);
+		goto done;
 	}
-	ro.ro_rt = rt;
+	dst = &ro->ro_dstsa;
 
 	/*
 	 * Scope check: if a packet can't be delivered to its destination
@@ -225,8 +223,8 @@ reroute:
 	 */
 	if (tdb != NULL) {
 		/* Callee frees mbuf */
-		error = ip6_output_ipsec_send(tdb, m, &ro, 0, 1);
-		rt = ro.ro_rt;
+		error = ip6_output_ipsec_send(tdb, m, ro, 0, 1);
+		rt = ro->ro_rt;
 		if (error)
 			goto senderr;
 		goto freecopy;
@@ -254,7 +252,7 @@ reroute:
 	    ip6_sendredirects &&
 	    (rt->rt_flags & (RTF_DYNAMIC|RTF_MODIFIED)) == 0) {
 		if ((ifp->if_flags & IFF_POINTOPOINT) &&
-		    nd6_is_addr_neighbor(&ro.ro_dstsin6, ifp)) {
+		    nd6_is_addr_neighbor(&ro->ro_dstsin6, ifp)) {
 			/*
 			 * If the incoming interface is equal to the outgoing
 			 * one, the link attached to the interface is
@@ -308,8 +306,9 @@ reroute:
 		/* tag as generated to skip over pf_test on rerun */
 		m->m_pkthdr.pf.flags |= PF_TAG_GENERATED;
 		srcrt = 1;
-		rtfree(rt);
-		rt = NULL;
+		if (ro == &iproute)
+			rtfree(ro->ro_rt);
+		ro = NULL;
 		if_put(ifp);
 		ifp = NULL;
 		goto reroute;
@@ -388,7 +387,8 @@ senderr:
  freecopy:
 	m_freem(mcopy);
  done:
-	rtfree(rt);
+	if (ro == &iproute)
+		rtfree(ro->ro_rt);
 	if_put(ifp);
 #ifdef IPSEC
 	tdb_unref(tdb);
Index: netinet6/ip6_input.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_input.c,v
diff -u -p -r1.260 ip6_input.c
--- netinet6/ip6_input.c	14 Apr 2024 20:46:27 -0000	1.260
+++ netinet6/ip6_input.c	15 Apr 2024 21:59:44 -0000
@@ -356,10 +356,10 @@ bad:
 int
 ip6_input_if(struct mbuf **mp, int *offp, int nxt, int af, struct ifnet *ifp)
 {
+	struct route ro;
 	struct mbuf *m;
 	struct ip6_hdr *ip6;
-	struct sockaddr_in6 sin6;
-	struct rtentry *rt = NULL;
+	struct rtentry *rt;
 	int ours = 0;
 	u_int16_t src_scope, dst_scope;
 #if NPF > 0
@@ -369,8 +369,8 @@ ip6_input_if(struct mbuf **mp, int *offp
 
 	KASSERT(*offp == 0);
 
+	ro.ro_rt = NULL;
 	ip6stat_inc(ip6s_total);
-
 	m = *mp = ipv6_check(ifp, *mp);
 	if (m == NULL)
 		goto bad;
@@ -516,18 +516,14 @@ ip6_input_if(struct mbuf **mp, int *offp
 	/*
 	 *  Unicast check
 	 */
-	memset(&sin6, 0, sizeof(struct sockaddr_in6));
-	sin6.sin6_len = sizeof(struct sockaddr_in6);
-	sin6.sin6_family = AF_INET6;
-	sin6.sin6_addr = ip6->ip6_dst;
-	rt = rtalloc_mpath(sin6tosa(&sin6), &ip6->ip6_src.s6_addr32[0],
+	rt = route6_mpath(&ro, &ip6->ip6_dst, &ip6->ip6_src,
 	    m->m_pkthdr.ph_rtableid);
 
 	/*
 	 * Accept the packet if the route to the destination is marked
 	 * as local.
 	 */
-	if (rtisvalid(rt) && ISSET(rt->rt_flags, RTF_LOCAL)) {
+	if (rt != NULL && ISSET(rt->rt_flags, RTF_LOCAL)) {
 		struct in6_ifaddr *ia6 = ifatoia6(rt->rt_ifa);
 
 		if (ip6_forwarding == 0 && rt->rt_ifidx != ifp->if_index &&
@@ -617,14 +613,14 @@ ip6_input_if(struct mbuf **mp, int *offp
 	}
 #endif /* IPSEC */
 
-	ip6_forward(m, rt, pfrdr);
+	ip6_forward(m, &ro, pfrdr);
 	*mp = NULL;
 	return IPPROTO_DONE;
  bad:
 	nxt = IPPROTO_DONE;
 	m_freemp(mp);
  out:
-	rtfree(rt);
+	rtfree(ro.ro_rt);
 	return nxt;
 }
 
Index: netinet6/ip6_output.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_output.c,v
diff -u -p -r1.289 ip6_output.c
--- netinet6/ip6_output.c	9 Apr 2024 11:05:05 -0000	1.289
+++ netinet6/ip6_output.c	15 Apr 2024 21:59:44 -0000
@@ -391,7 +391,7 @@ reroute:
 	/* initialize cached route */
 	if (ro == NULL) {
 		ro = &iproute;
-		bzero((caddr_t)ro, sizeof(*ro));
+		ro->ro_rt = NULL;
 	}
 	ro_pmtu = ro;
 	if (opt && opt->ip6po_rthdr)
Index: netinet6/ip6_var.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_var.h,v
diff -u -p -r1.114 ip6_var.h
--- netinet6/ip6_var.h	14 Feb 2024 13:18:21 -0000	1.114
+++ netinet6/ip6_var.h	15 Apr 2024 21:59:44 -0000
@@ -320,7 +320,7 @@ int	ip6_process_hopopts(struct mbuf **, 
 void	ip6_savecontrol(struct inpcb *, struct mbuf *, struct mbuf **);
 int	ip6_sysctl(int *, u_int, void *, size_t *, void *, size_t);
 
-void	ip6_forward(struct mbuf *, struct rtentry *, int);
+void	ip6_forward(struct mbuf *, struct route *, int);
 
 void	ip6_mloopback(struct ifnet *, struct mbuf *, struct sockaddr_in6 *);
 int	ip6_output(struct mbuf *, struct ip6_pktopts *, struct route *, int,