From: Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject: Re: simplify icmp6_reflect()
To: David Gwynne <david@gwynne.id.au>
Cc: tech@openbsd.org
Date: Thu, 17 Jul 2025 08:29:36 +0200

On Thu, Jul 17, 2025 at 04:00:14PM +1000, David Gwynne wrote:
> firstly, icmp6_reflect is only called from inside
> src/sys/netinet6/icmp6.c, so this moves the prototype from a public
> header into that file. secondly, the struct sockaddr *sa argument is
> always NULL, so we can omit it.
> 
> ok?

OK claudio@
 
> Index: netinet/icmp6.h
> ===================================================================
> RCS file: /cvs/src/sys/netinet/icmp6.h,v
> diff -u -p -r1.56 icmp6.h
> --- netinet/icmp6.h	19 May 2025 06:50:00 -0000	1.56
> +++ netinet/icmp6.h	17 Jul 2025 05:57:01 -0000
> @@ -593,7 +593,6 @@ void		 icmp6_error(struct mbuf *, int, i
>  int		 icmp6_input(struct mbuf **, int *, int, int,
>  		    struct netstack *);
>  void		 icmp6_fasttimo(void);
> -int		 icmp6_reflect(struct mbuf **, size_t, struct sockaddr *);
>  void		 icmp6_redirect_input(struct mbuf *, int);
>  void		 icmp6_redirect_output(struct mbuf *, struct rtentry *);
>  int		 icmp6_sysctl(int *, u_int, void *, size_t *, void *, size_t);
> Index: netinet6/icmp6.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/icmp6.c,v
> diff -u -p -r1.269 icmp6.c
> --- netinet6/icmp6.c	17 Jul 2025 03:00:45 -0000	1.269
> +++ netinet6/icmp6.c	17 Jul 2025 05:57:01 -0000
> @@ -125,6 +125,7 @@ static int icmp6_mtudisc_lowat = 256;
>   */
>  struct rttimer_queue icmp6_redirect_timeout_q;
>  
> +int	icmp6_reflect(struct mbuf **, size_t);
>  void	icmp6_errcount(int, int);
>  int	icmp6_ratelimit(const struct in6_addr *, const int, const int);
>  int	icmp6_notify_error(struct mbuf *, int, int, int);
> @@ -366,7 +367,7 @@ icmp6_error(struct mbuf *m, int type, in
>  	n = icmp6_do_error(m, type, code, param);
>  	if (n != NULL) {
>  		/* header order: IPv6 - ICMPv6 */
> -		if (!icmp6_reflect(&n, sizeof(struct ip6_hdr), NULL))
> +		if (!icmp6_reflect(&n, sizeof(struct ip6_hdr)))
>  			ip6_send(n);
>  	}
>  }
> @@ -591,7 +592,7 @@ icmp6_input(struct mbuf **mp, int *offp,
>  			nicmp6->icmp6_code = 0;
>  			icmp6stat_inc(icp6s_reflect);
>  			icmp6stat_inc(icp6s_outhist + ICMP6_ECHO_REPLY);
> -			if (!icmp6_reflect(&n, noff, NULL))
> +			if (!icmp6_reflect(&n, noff))
>  				ip6_send(n);
>  		}
>  		if (!m)
> @@ -1019,7 +1020,7 @@ icmp6_mtudisc_update(struct ip6ctlparam 
>   * OFF points to the icmp6 header, counted from the top of the mbuf.
>   */
>  int
> -icmp6_reflect(struct mbuf **mp, size_t off, struct sockaddr *sa)
> +icmp6_reflect(struct mbuf **mp, size_t off)
>  {
>  	struct mbuf *m = *mp;
>  	struct rtentry *rt = NULL;
> @@ -1097,25 +1098,22 @@ icmp6_reflect(struct mbuf **mp, size_t o
>  	sa6_dst.sin6_len = sizeof(sa6_dst);
>  	sa6_dst.sin6_addr = t;
>  
> -	if (sa == NULL) {
> -		/*
> -		 * If the incoming packet was addressed directly to us (i.e.
> -		 * unicast), use dst as the src for the reply. The
> -		 * IN6_IFF_TENTATIVE|IN6_IFF_DUPLICATED case would be VERY rare,
> -		 * but is possible (for example) when we encounter an error
> -		 * while forwarding procedure destined to a duplicated address
> -		 * of ours.
> -		 */
> -		rt = rtalloc(sin6tosa(&sa6_dst), 0, rtableid);
> -		if (rtisvalid(rt) && ISSET(rt->rt_flags, RTF_LOCAL) &&
> -		    !ISSET(ifatoia6(rt->rt_ifa)->ia6_flags,
> -		    IN6_IFF_ANYCAST|IN6_IFF_TENTATIVE|IN6_IFF_DUPLICATED)) {
> -			src = &t;
> -		}
> -		rtfree(rt);
> -		rt = NULL;
> -		sa = sin6tosa(&sa6_src);
> +	/*
> +	 * If the incoming packet was addressed directly to us (i.e.
> +	 * unicast), use dst as the src for the reply. The
> +	 * IN6_IFF_TENTATIVE|IN6_IFF_DUPLICATED case would be VERY rare,
> +	 * but is possible (for example) when we encounter an error
> +	 * while forwarding procedure destined to a duplicated address
> +	 * of ours.
> +	 */
> +	rt = rtalloc(sin6tosa(&sa6_dst), 0, rtableid);
> +	if (rtisvalid(rt) && ISSET(rt->rt_flags, RTF_LOCAL) &&
> +	    !ISSET(ifatoia6(rt->rt_ifa)->ia6_flags,
> +	    IN6_IFF_ANYCAST|IN6_IFF_TENTATIVE|IN6_IFF_DUPLICATED)) {
> +		src = &t;
>  	}
> +	rtfree(rt);
> +	rt = NULL;
>  
>  	if (src == NULL) {
>  		struct in6_ifaddr *ia6;
> @@ -1125,7 +1123,7 @@ icmp6_reflect(struct mbuf **mp, size_t o
>  		 * that we do not own.  Select a source address based on the
>  		 * source address of the erroneous packet.
>  		 */
> -		rt = rtalloc(sa, RT_RESOLVE, rtableid);
> +		rt = rtalloc(sin6tosa(&sa6_src), RT_RESOLVE, rtableid);
>  		if (!rtisvalid(rt)) {
>  			rtfree(rt);
>  			goto bad;
> 

-- 
:wq Claudio