Index | Thread | Search

From:
Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject:
Re: route cache return hit or miss
To:
Alexander Bluhm <alexander.bluhm@gmx.net>
Cc:
tech@openbsd.org
Date:
Fri, 9 Feb 2024 08:39:32 +0100

Download raw body.

Thread
On Thu, Feb 08, 2024 at 11:15:40PM +0100, Alexander Bluhm wrote:
> Hi,
> 
> The route_cache() function can easily return whether it was a cache
> hit or miss.  Then the logic to perform a route lookup gets a bit
> simpler.  Some more complicated if (ro->ro_rt == NULL) checks still
> exist elsewhere.
> 
> Also use route cache in in_pcbselsrc() instead of filling struct
> route manually.
> 
> ok?

Why return ESRCH instead of simply -1?
Apart from that I like this a lot.

> Index: net/route.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/net/route.c,v
> diff -u -p -r1.430 route.c
> --- net/route.c	7 Feb 2024 23:52:20 -0000	1.430
> +++ net/route.c	8 Feb 2024 14:43:01 -0000
> @@ -201,7 +201,7 @@ route_init(void)
>  #endif
>  }
>  
> -void
> +int
>  route_cache(struct route *ro, struct in_addr addr, u_int rtableid)
>  {
>  	u_long gen;
> @@ -215,7 +215,7 @@ route_cache(struct route *ro, struct in_
>  	    ro->ro_dst.sa_family == AF_INET &&
>  	    satosin(&ro->ro_dst)->sin_addr.s_addr == addr.s_addr) {
>  		ipstat_inc(ips_rtcachehit);
> -		return;
> +		return (0);
>  	}
>  
>  	ipstat_inc(ips_rtcachemiss);
> @@ -228,10 +228,12 @@ route_cache(struct route *ro, struct in_
>  	satosin(&ro->ro_dst)->sin_family = AF_INET;
>  	satosin(&ro->ro_dst)->sin_len = sizeof(struct sockaddr_in);
>  	satosin(&ro->ro_dst)->sin_addr = addr;
> +
> +	return (ESRCH);
>  }
>  
>  #ifdef INET6
> -void
> +int
>  route6_cache(struct route_in6 *ro, const struct in6_addr *addr,
>      u_int rtableid)
>  {
> @@ -246,7 +248,7 @@ route6_cache(struct route_in6 *ro, const
>  	    ro->ro_dst.sin6_family == AF_INET6 &&
>  	    IN6_ARE_ADDR_EQUAL(&ro->ro_dst.sin6_addr, addr)) {
>  		ip6stat_inc(ip6s_rtcachehit);
> -		return;
> +		return (0);
>  	}
>  
>  	ip6stat_inc(ip6s_rtcachemiss);
> @@ -259,6 +261,8 @@ route6_cache(struct route_in6 *ro, const
>  	ro->ro_dst.sin6_family = AF_INET6;
>  	ro->ro_dst.sin6_len = sizeof(struct sockaddr_in6);
>  	ro->ro_dst.sin6_addr = *addr;
> +
> +	return (ESRCH);
>  }
>  #endif
>  
> Index: netinet/in.h
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/in.h,v
> diff -u -p -r1.146 in.h
> --- netinet/in.h	5 Feb 2024 12:52:11 -0000	1.146
> +++ netinet/in.h	8 Feb 2024 14:37:42 -0000
> @@ -789,7 +789,7 @@ void	   in_len2mask(struct in_addr *, in
>  int	   in_nam2sin(const struct mbuf *, struct sockaddr_in **);
>  int	   in_sa2sin(struct sockaddr *, struct sockaddr_in **);
>  
> -void	   route_cache(struct route *, struct in_addr, u_int);
> +int	   route_cache(struct route *, struct in_addr, u_int);
>  
>  char	  *inet_ntoa(struct in_addr);
>  int	   inet_nat64(int, const void *, void *, const void *, u_int8_t);
> Index: netinet/in_pcb.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/in_pcb.c,v
> diff -u -p -r1.290 in_pcb.c
> --- netinet/in_pcb.c	7 Feb 2024 23:40:40 -0000	1.290
> +++ netinet/in_pcb.c	8 Feb 2024 14:44:49 -0000
> @@ -918,8 +918,7 @@ in_pcbrtentry(struct inpcb *inp)
>  
>  	if (inp->inp_faddr.s_addr == INADDR_ANY)
>  		return (NULL);
> -	route_cache(ro, inp->inp_faddr, inp->inp_rtableid);
> -	if (ro->ro_rt == NULL) {
> +	if (route_cache(ro, inp->inp_faddr, inp->inp_rtableid)) {
>  		ro->ro_rt = rtalloc_mpath(&ro->ro_dst,
>  		    &inp->inp_laddr.s_addr, ro->ro_tableid);
>  	}
> @@ -941,8 +940,6 @@ in_pcbselsrc(struct in_addr *insrc, stru
>  	const struct in_addr *laddr = &inp->inp_laddr;
>  	u_int rtableid = inp->inp_rtableid;
>  	struct sockaddr	*ip4_source = NULL;
> -
> -	struct sockaddr_in *sin2;
>  	struct in_ifaddr *ia = NULL;
>  
>  	/*
> @@ -984,25 +981,9 @@ in_pcbselsrc(struct in_addr *insrc, stru
>  	 * If route is known or can be allocated now,
>  	 * our src addr is taken from the i/f, else punt.
>  	 */
> -	if (!rtisvalid(ro->ro_rt) || (ro->ro_tableid != rtableid) ||
> -	    (satosin(&ro->ro_dst)->sin_addr.s_addr != sin->sin_addr.s_addr)) {
> -		rtfree(ro->ro_rt);
> -		ro->ro_rt = NULL;
> -	}
> -	if (ro->ro_rt == NULL) {
> +	if (route_cache(ro, sin->sin_addr, rtableid)) {
>  		/* No route yet, so try to acquire one */
> -		ro->ro_dst.sa_family = AF_INET;
> -		ro->ro_dst.sa_len = sizeof(struct sockaddr_in);
> -		satosin(&ro->ro_dst)->sin_addr = sin->sin_addr;
> -		ro->ro_tableid = rtableid;
>  		ro->ro_rt = rtalloc_mpath(&ro->ro_dst, NULL, ro->ro_tableid);
> -
> -		/*
> -		 * It is important to zero out the rest of the
> -		 * struct sockaddr_in when mixing v6 & v4!
> -		 */
> -		sin2 = satosin(&ro->ro_dst);
> -		memset(sin2->sin_zero, 0, sizeof(sin2->sin_zero));
>  	}
>  
>  	/*
> Index: netinet6/in6.h
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/in6.h,v
> diff -u -p -r1.114 in6.h
> --- netinet6/in6.h	7 Feb 2024 23:40:40 -0000	1.114
> +++ netinet6/in6.h	8 Feb 2024 14:38:13 -0000
> @@ -428,7 +428,7 @@ int	in6_mask2len(struct in6_addr *, u_ch
>  int	in6_nam2sin6(const struct mbuf *, struct sockaddr_in6 **);
>  int	in6_sa2sin6(struct sockaddr *, struct sockaddr_in6 **);
>  
> -void	route6_cache(struct route_in6 *, const struct in6_addr *, u_int);
> +int	route6_cache(struct route_in6 *, const struct in6_addr *, u_int);
>  
>  struct ip6_pktopts;
>  struct ip6_moptions;
> Index: netinet6/in6_pcb.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/in6_pcb.c,v
> diff -u -p -r1.135 in6_pcb.c
> --- netinet6/in6_pcb.c	7 Feb 2024 23:40:40 -0000	1.135
> +++ netinet6/in6_pcb.c	8 Feb 2024 14:47:09 -0000
> @@ -568,8 +568,7 @@ in6_pcbrtentry(struct inpcb *inp)
>  
>  	if (IN6_IS_ADDR_UNSPECIFIED(&inp->inp_faddr6))
>  		return (NULL);
> -	route6_cache(ro, &inp->inp_faddr6, inp->inp_rtableid);
> -	if (ro->ro_rt == NULL) {
> +	if (route6_cache(ro, &inp->inp_faddr6, inp->inp_rtableid)) {
>  		ro->ro_rt = rtalloc_mpath(sin6tosa(&ro->ro_dst),
>  		    &inp->inp_laddr6.s6_addr32[0], ro->ro_tableid);
>  	}
> Index: netinet6/in6_src.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/in6_src.c,v
> diff -u -p -r1.92 in6_src.c
> --- netinet6/in6_src.c	7 Feb 2024 23:40:40 -0000	1.92
> +++ netinet6/in6_src.c	8 Feb 2024 14:48:05 -0000
> @@ -179,8 +179,7 @@ in6_pcbselsrc(const struct in6_addr **in
>  	 * If route is known or can be allocated now,
>  	 * our src addr is taken from the i/f, else punt.
>  	 */
> -	route6_cache(ro, dst, rtableid);
> -	if (ro->ro_rt == NULL) {
> +	if (route6_cache(ro, dst, rtableid)) {
>  		ro->ro_rt = rtalloc(sin6tosa(&ro->ro_dst),
>  		    RT_RESOLVE, ro->ro_tableid);
>  	}
> @@ -306,8 +305,7 @@ in6_selectroute(const struct in6_addr *d
>  	 * a new one.
>  	 */
>  	if (ro) {
> -		route6_cache(ro, dst, rtableid);
> -		if (ro->ro_rt == NULL) {
> +		if (route6_cache(ro, dst, rtableid)) {
>  			/* No route yet, so try to acquire one */
>  			ro->ro_rt = rtalloc_mpath(sin6tosa(&ro->ro_dst),
>  			    NULL, ro->ro_tableid);
> 

-- 
:wq Claudio