From: Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject: Re: split in_pcbrtentry() and in6_pcbnotify()
To: Alexander Bluhm <alexander.bluhm@gmx.net>
Cc: tech@openbsd.org
Date: Wed, 31 Jan 2024 10:58:49 +0100

On Tue, Jan 30, 2024 at 06:29:27PM +0100, Alexander Bluhm wrote:
> Hi,
> 
> Can we split in_pcbrtentry() and in6_pcbnotify() with the usual
> INP_IPV6 check?
> 
> This means less #ifdef INET6 in the function.
> 
> Also struct route_in6 *ro is of the correct type and we don't rely
> on the fact that inp_route and inp_route6 are pointers to the same
> union.
> 
> ok?

I'm fine with this split. I got lost in here before and see my other mail
about why I despise struct route and struct route_in6.
 
> bluhm
> 
> Index: netinet/in_pcb.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/in_pcb.c,v
> diff -u -p -r1.287 in_pcb.c
> --- netinet/in_pcb.c	28 Jan 2024 20:34:25 -0000	1.287
> +++ netinet/in_pcb.c	30 Jan 2024 17:21:45 -0000
> @@ -909,6 +909,11 @@ in_pcbrtentry(struct inpcb *inp)
>  {
>  	struct route *ro;
>  
> +#ifdef INET6
> +	if (ISSET(inp->inp_flags, INP_IPV6))
> +		in6_pcbrtentry(inp);
> +#endif
> +
>  	ro = &inp->inp_route;
>  
>  	/* check if route is still valid */
> @@ -921,34 +926,16 @@ in_pcbrtentry(struct inpcb *inp)
>  	 * No route yet, so try to acquire one.
>  	 */
>  	if (ro->ro_rt == NULL) {
> -#ifdef INET6
> -		memset(ro, 0, sizeof(struct route_in6));
> -#else
>  		memset(ro, 0, sizeof(struct route));
> -#endif
>  
> -#ifdef INET6
> -		if (ISSET(inp->inp_flags, INP_IPV6)) {
> -			if (IN6_IS_ADDR_UNSPECIFIED(&inp->inp_faddr6))
> -				return (NULL);
> -			ro->ro_dst.sa_family = AF_INET6;
> -			ro->ro_dst.sa_len = sizeof(struct sockaddr_in6);
> -			satosin6(&ro->ro_dst)->sin6_addr = inp->inp_faddr6;
> -			ro->ro_tableid = inp->inp_rtableid;
> -			ro->ro_rt = rtalloc_mpath(&ro->ro_dst,
> -			    &inp->inp_laddr6.s6_addr32[0], ro->ro_tableid);
> -		} else
> -#endif /* INET6 */
> -		{
> -			if (inp->inp_faddr.s_addr == INADDR_ANY)
> -				return (NULL);
> -			ro->ro_dst.sa_family = AF_INET;
> -			ro->ro_dst.sa_len = sizeof(struct sockaddr_in);
> -			satosin(&ro->ro_dst)->sin_addr = inp->inp_faddr;
> -			ro->ro_tableid = inp->inp_rtableid;
> -			ro->ro_rt = rtalloc_mpath(&ro->ro_dst,
> -			    &inp->inp_laddr.s_addr, ro->ro_tableid);
> -		}
> +		if (inp->inp_faddr.s_addr == INADDR_ANY)
> +			return (NULL);
> +		ro->ro_dst.sa_family = AF_INET;
> +		ro->ro_dst.sa_len = sizeof(struct sockaddr_in);
> +		satosin(&ro->ro_dst)->sin_addr = inp->inp_faddr;
> +		ro->ro_tableid = inp->inp_rtableid;
> +		ro->ro_rt = rtalloc_mpath(&ro->ro_dst,
> +		    &inp->inp_laddr.s_addr, ro->ro_tableid);
>  	}
>  	return (ro->ro_rt);
>  }
> Index: netinet/in_pcb.h
> ===================================================================
> RCS file: /cvs/src/sys/netinet/in_pcb.h,v
> diff -u -p -r1.149 in_pcb.h
> --- netinet/in_pcb.h	28 Jan 2024 20:34:25 -0000	1.149
> +++ netinet/in_pcb.h	30 Jan 2024 17:21:45 -0000
> @@ -367,6 +367,8 @@ struct rtentry *
>  	in_pcbrtentry(struct inpcb *);
>  
>  /* INET6 stuff */
> +struct rtentry *
> +	in6_pcbrtentry(struct inpcb *);
>  void	in6_pcbnotify(struct inpcbtable *, const struct sockaddr_in6 *,
>  	u_int, const struct sockaddr_in6 *, u_int, u_int, int, void *,
>  	void (*)(struct inpcb *, int));
> Index: netinet6/in6_pcb.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/in6_pcb.c,v
> diff -u -p -r1.133 in6_pcb.c
> --- netinet6/in6_pcb.c	28 Jan 2024 20:34:25 -0000	1.133
> +++ netinet6/in6_pcb.c	30 Jan 2024 17:21:46 -0000
> @@ -561,6 +561,35 @@ in6_pcbnotify(struct inpcbtable *table, 
>  	rw_exit_write(&table->inpt_notify);
>  }
>  
> +struct rtentry *
> +in6_pcbrtentry(struct inpcb *inp)
> +{
> +	struct route_in6 *ro = &inp->inp_route6;
> +
> +	/* check if route is still valid */
> +	if (!rtisvalid(ro->ro_rt)) {
> +		rtfree(ro->ro_rt);
> +		ro->ro_rt = NULL;
> +	}
> +
> +	/*
> +	 * No route yet, so try to acquire one.
> +	 */
> +	if (ro->ro_rt == NULL) {
> +		memset(ro, 0, sizeof(struct route_in6));
> +
> +		if (IN6_IS_ADDR_UNSPECIFIED(&inp->inp_faddr6))
> +			return (NULL);
> +		ro->ro_dst.sin6_family = AF_INET6;
> +		ro->ro_dst.sin6_len = sizeof(struct sockaddr_in6);
> +		ro->ro_dst.sin6_addr = inp->inp_faddr6;
> +		ro->ro_tableid = inp->inp_rtableid;
> +		ro->ro_rt = rtalloc_mpath(sin6tosa(&ro->ro_dst),
> +		    &inp->inp_laddr6.s6_addr32[0], ro->ro_tableid);
> +	}
> +	return (ro->ro_rt);
> +}
> +
>  struct inpcb *
>  in6_pcbhash_lookup(struct inpcbtable *table, uint64_t hash, u_int rdomain,
>      const struct in6_addr *faddr, u_short fport,
> Index: netinet6/ip6_output.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/ip6_output.c,v
> diff -u -p -r1.283 ip6_output.c
> --- netinet6/ip6_output.c	18 Jan 2024 11:03:16 -0000	1.283
> +++ netinet6/ip6_output.c	30 Jan 2024 17:21:47 -0000
> @@ -1486,7 +1486,7 @@ do { \
>  			if (!(so->so_state & SS_ISCONNECTED))
>  				return (ENOTCONN);
>  
> -			rt = in_pcbrtentry(inp);
> +			rt = in6_pcbrtentry(inp);
>  			if (!rtisvalid(rt))
>  				return (EHOSTUNREACH);
>  
> 

-- 
:wq Claudio