Index | Thread | Search

From:
Alexander Bluhm <bluhm@openbsd.org>
Subject:
Re: Remove upper layer IPv6 neighbor reachability hints.
To:
tech <tech@openbsd.org>
Date:
Mon, 15 Sep 2025 18:43:11 +0200

Download raw body.

Thread
On Mon, Sep 15, 2025 at 04:58:07PM +0200, Florian Obser wrote:
> Running neighbor discovery once in a while is better than touching the
> neighbor cache for (nearly) every TCP packet.
> 
> This functionality was disabled by default, however due to a bug /
> missing optimization, the code did a bunch of work before figuring out
> that the feature is disabled.
> 
> With this, net.inet6.icmp6.nd6_maxnudhint can be removed.
> 
> OK?

OK bluhm@

> diff --git lib/libc/sys/sysctl.2 lib/libc/sys/sysctl.2
> index 412ead4fa01..30499de85e4 100644
> --- lib/libc/sys/sysctl.2
> +++ lib/libc/sys/sysctl.2
> @@ -1838,7 +1838,6 @@ The currently defined protocols and names are:
>  .It icmp6 Ta mtudisc_hiwat Ta integer Ta yes
>  .It icmp6 Ta mtudisc_lowat Ta integer Ta yes
>  .It icmp6 Ta nd6_delay Ta integer Ta yes
> -.It icmp6 Ta nd6_maxnudhint Ta integer Ta yes
>  .It icmp6 Ta nd6_mmaxtries Ta integer Ta yes
>  .It icmp6 Ta nd6_umaxtries Ta integer Ta yes
>  .It icmp6 Ta redirtimeout Ta integer Ta yes
> @@ -1892,16 +1891,6 @@ timing constant in IPv6 neighbor discovery specification
>  .Pq RFC 4861 ,
>  in seconds.
>  .Pp
> -.It Li icmp6.nd6_maxnudhint Pq Va net.inet6.icmp6.nd6_maxnudhint
> -IPv6 neighbor discovery permits upper layer protocols to supply reachability
> -hints, to avoid unnecessary neighbor discovery exchanges.
> -This variable defines the number of consecutive hints the neighbor discovery
> -layer will take.
> -For example, by setting the variable to 3, neighbor discovery will take
> -a maximum of 3 consecutive hints.
> -After receiving 3 hints, the neighbor discovery layer will instead perform
> -the normal neighbor discovery process.
> -.Pp
>  .It Li icmp6.nd6_mmaxtries Pq Va net.inet6.icmp6.nd6_mmaxtries
>  This variable specifies the
>  .Dv MAX_MULTICAST_SOLICIT
> diff --git sys/netinet/icmp6.h sys/netinet/icmp6.h
> index 20e8761d8a1..48c26d30c91 100644
> --- sys/netinet/icmp6.h
> +++ sys/netinet/icmp6.h
> @@ -508,7 +508,6 @@ struct icmp6stat {
>  #define ICMPV6CTL_ND6_QUEUED	11
>  #define ICMPV6CTL_NODEINFO	13
>  #define ICMPV6CTL_ERRPPSLIMIT	14	/* ICMPv6 error pps limitation */
> -#define ICMPV6CTL_ND6_MAXNUDHINT	15
>  #define ICMPV6CTL_MTUDISC_HIWAT	16
>  #define ICMPV6CTL_MTUDISC_LOWAT	17
>  #define ICMPV6CTL_MAXID		18
> @@ -529,7 +528,7 @@ struct icmp6stat {
>  	{ 0, 0 }, \
>  	{ 0, 0 }, \
>  	{ "errppslimit", CTLTYPE_INT }, \
> -	{ "nd6_maxnudhint", CTLTYPE_INT }, \
> +	{ 0, 0 }, \
>  	{ "mtudisc_hiwat", CTLTYPE_INT }, \
>  	{ "mtudisc_lowat", CTLTYPE_INT }, \
>  }
> diff --git sys/netinet/tcp_input.c sys/netinet/tcp_input.c
> index bd61054df4b..732ad9f477e 100644
> --- sys/netinet/tcp_input.c
> +++ sys/netinet/tcp_input.c
> @@ -125,22 +125,6 @@ struct timeval tcp_ackdrop_ppslim_last;
>  #define	SEQ_MIN(a,b)	(SEQ_LT(a,b) ? (a) : (b))
>  #define	SEQ_MAX(a,b)	(SEQ_GT(a,b) ? (a) : (b))
>  
> -/*
> - * Neighbor Discovery, Neighbor Unreachability Detection Upper layer hint.
> - */
> -#ifdef INET6
> -#define ND6_HINT(tp, hint) \
> -do { \
> -	if (tp && tp->t_inpcb &&					\
> -	    ISSET(tp->t_inpcb->inp_flags, INP_IPV6) &&			\
> -	    rtisvalid(tp->t_inpcb->inp_route.ro_rt)) {			\
> -		nd6_nud_hint(tp->t_inpcb->inp_route.ro_rt, hint);	\
> -	} \
> -} while (0)
> -#else
> -#define ND6_HINT(tp, hint)
> -#endif
> -
>  #ifdef TCP_ECN
>  /*
>   * ECN (Explicit Congestion Notification) support based on RFC3168
> @@ -311,9 +295,6 @@ tcp_flush_queue(struct tcpcb *tp)
>  {
>  	struct socket *so = tp->t_inpcb->inp_socket;
>  	struct tcpqent *q, *nq;
> -#ifdef INET6
> -	int nd6_maxnudhint_local = atomic_load_int(&nd6_maxnudhint);
> -#endif
>  	int flags;
>  
>  	/*
> @@ -333,7 +314,6 @@ tcp_flush_queue(struct tcpcb *tp)
>  
>  		nq = TAILQ_NEXT(q, tcpqe_q);
>  		TAILQ_REMOVE(&tp->t_segq, q, tcpqe_q);
> -		ND6_HINT(tp, nd6_maxnudhint_local);
>  		if (so->so_rcv.sb_state & SS_CANTRCVMORE)
>  			m_freem(q->tcpqe_m);
>  		else {
> @@ -427,9 +407,6 @@ tcp_input_solocked(struct mbuf **mp, int *offp, int proto, int af,
>  	struct ip6_hdr *ip6 = NULL;
>  #endif /* INET6 */
>  	int do_ecn = 0;
> -#ifdef INET6
> -	int nd6_maxnudhint_local = atomic_load_int(&nd6_maxnudhint);
> -#endif
>  #ifdef TCP_ECN
>  	u_char iptos;
>  #endif
> @@ -973,7 +950,6 @@ findpcb:
>  				tcpstat_pkt(tcps_rcvackpack, tcps_rcvackbyte,
>  				    acked);
>  				tp->t_rcvacktime = now;
> -				ND6_HINT(tp, nd6_maxnudhint_local);
>  
>  				mtx_enter(&so->so_snd.sb_mtx);
>  				sbdrop(&so->so_snd, acked);
> @@ -1058,7 +1034,6 @@ findpcb:
>  			/* Packet has most recent segment, no urgent exists. */
>  			tp->rcv_up = tp->rcv_nxt;
>  			tcpstat_pkt(tcps_rcvpack, tcps_rcvbyte, tlen);
> -			ND6_HINT(tp, nd6_maxnudhint_local);
>  
>  			TCP_SETUP_ACK(tp, tiflags, m);
>  			/*
> @@ -1763,7 +1738,6 @@ trimthenstep6:
>  			tp->snd_cwnd = ulmin(cw + incr,
>  			    TCP_MAXWIN << tp->snd_scale);
>  		}
> -		ND6_HINT(tp, nd6_maxnudhint_local);
>  		if (acked > so->so_snd.sb_cc) {
>  			if (tp->snd_wnd > so->so_snd.sb_cc)
>  				tp->snd_wnd -= so->so_snd.sb_cc;
> @@ -1993,7 +1967,6 @@ dodata:							/* XXX */
>  			tp->rcv_nxt += tlen;
>  			tiflags = th->th_flags & TH_FIN;
>  			tcpstat_pkt(tcps_rcvpack, tcps_rcvbyte, tlen);
> -			ND6_HINT(tp, nd6_maxnudhint_local);
>  			if (so->so_rcv.sb_state & SS_CANTRCVMORE)
>  				m_freem(m);
>  			else {
> diff --git sys/netinet6/icmp6.c sys/netinet6/icmp6.c
> index 6eb4ebf90f6..23a35fbf2f4 100644
> --- sys/netinet6/icmp6.c
> +++ sys/netinet6/icmp6.c
> @@ -1781,7 +1781,6 @@ const struct sysctl_bounded_args icmpv6ctl_vars_unlocked[] = {
>  	{ ICMPV6CTL_ND6_UMAXTRIES, &nd6_umaxtries, 0, INT_MAX },
>  	{ ICMPV6CTL_ND6_MMAXTRIES, &nd6_mmaxtries, 0, INT_MAX },
>  	{ ICMPV6CTL_MTUDISC_HIWAT, &icmp6_mtudisc_hiwat, 0, INT_MAX },
> -	{ ICMPV6CTL_ND6_MAXNUDHINT, &nd6_maxnudhint, 0, INT_MAX },
>  	{ ICMPV6CTL_MTUDISC_LOWAT, &icmp6_mtudisc_lowat, 0, INT_MAX },
>  };
>  
> @@ -1851,7 +1850,6 @@ icmp6_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp,
>  	case ICMPV6CTL_ND6_DELAY:
>  	case ICMPV6CTL_ND6_UMAXTRIES:
>  	case ICMPV6CTL_ND6_MMAXTRIES:
> -	case ICMPV6CTL_ND6_MAXNUDHINT:
>  	case ICMPV6CTL_MTUDISC_HIWAT:
>  	case ICMPV6CTL_MTUDISC_LOWAT:
>  		error = sysctl_bounded_arr(icmpv6ctl_vars_unlocked,
> diff --git sys/netinet6/nd6.c sys/netinet6/nd6.c
> index 80ef6a28506..cc52d7d487b 100644
> --- sys/netinet6/nd6.c
> +++ sys/netinet6/nd6.c
> @@ -83,8 +83,6 @@ const int nd6_gctimer	= (60 * 60 * 24); /* 1 day: garbage collection timer */
>  /* preventing too many loops in ND option parsing */
>  int nd6_maxndopt = 10;	/* max # of ND options allowed */
>  
> -int nd6_maxnudhint = 0;	/* [a] max # of subsequent upper layer hints */
> -
>  /* llinfo_nd6 live time, rt_llinfo and RTF_LLINFO are protected by nd6_mtx */
>  struct mutex nd6_mtx = MUTEX_INITIALIZER(IPL_SOFTNET);
>  
> @@ -715,55 +713,6 @@ nd6_free(struct rtentry *rt, struct ifnet *ifp, int i_am_router)
>  		rtdeletemsg(rt, ifp, ifp->if_rdomain);
>  }
>  
> -/*
> - * Upper-layer reachability hint for Neighbor Unreachability Detection.
> - *
> - * XXX cost-effective methods?
> - */
> -void
> -nd6_nud_hint(struct rtentry *rt, int maxnudhint)
> -{
> -	struct llinfo_nd6 *ln;
> -	struct ifnet *ifp;
> -
> -	NET_ASSERT_LOCKED();
> -
> -	if ((rt->rt_flags & RTF_GATEWAY) != 0 ||
> -	    (rt->rt_flags & RTF_LLINFO) == 0 ||
> -	    rt->rt_gateway == NULL ||
> -	    rt->rt_gateway->sa_family != AF_LINK) {
> -		/* This is not a host route. */
> -		return;
> -	}
> -
> -	ifp = if_get(rt->rt_ifidx);
> -	if (ifp == NULL)
> -		return;
> -
> -	mtx_enter(&nd6_mtx);
> -
> -	ln = (struct llinfo_nd6 *)rt->rt_llinfo;
> -	if (ln == NULL)
> -		goto out;
> -	if (ln->ln_state < ND6_LLINFO_REACHABLE)
> -		goto out;
> -
> -	/*
> -	 * if we get upper-layer reachability confirmation many times,
> -	 * it is possible we have false information.
> -	 */
> -	ln->ln_byhint++;
> -	if (ln->ln_byhint > maxnudhint)
> -		goto out;
> -
> -	ln->ln_state = ND6_LLINFO_REACHABLE;
> -	if (!ND6_LLINFO_PERMANENT(ln))
> -		nd6_llinfo_settimer(ln, ifp->if_nd->reachable);
> -out:
> -	mtx_leave(&nd6_mtx);
> -	if_put(ifp);
> -}
> -
>  void
>  nd6_rtrequest(struct ifnet *ifp, int req, struct rtentry *rt)
>  {
> @@ -855,7 +804,6 @@ nd6_rtrequest(struct ifnet *ifp, int req, struct rtentry *rt)
>  			 * which is specified by ndp command.
>  			 */
>  			ln->ln_state = ND6_LLINFO_REACHABLE;
> -			ln->ln_byhint = 0;
>  		} else {
>  			/*
>  			 * When req == RTM_RESOLVE, rt is created and
> @@ -907,7 +855,6 @@ nd6_rtrequest(struct ifnet *ifp, int req, struct rtentry *rt)
>  		if (ifa != NULL ||
>  		    (rt->rt_flags & RTF_ANNOUNCE)) {
>  			ln->ln_state = ND6_LLINFO_REACHABLE;
> -			ln->ln_byhint = 0;
>  			rt->rt_expire = 0;
>  		}
>  		mtx_leave(&nd6_mtx);
> diff --git sys/netinet6/nd6.h sys/netinet6/nd6.h
> index 4967266f46a..eb7001a4560 100644
> --- sys/netinet6/nd6.h
> +++ sys/netinet6/nd6.h
> @@ -87,7 +87,6 @@ struct llinfo_nd6 {
>  	struct	mbuf_queue ln_mq;	/* hold packets until resolved */
>  	struct	in6_addr ln_saddr6;	/* source of prompting packet */
>  	long	ln_asked;	/* number of queries already sent for addr */
> -	int	ln_byhint;	/* # of times we made it reachable by UL hint */
>  	short	ln_state;	/* reachability state */
>  	short	ln_router;	/* 2^0: ND6 router bit */
>  };
> @@ -116,7 +115,6 @@ extern unsigned int ln_hold_total;
>  extern int nd6_delay;
>  extern int nd6_umaxtries;
>  extern int nd6_mmaxtries;
> -extern int nd6_maxnudhint;
>  extern const int nd6_gctimer;
>  
>  struct nd_opts {
> @@ -133,7 +131,6 @@ struct	rtentry *nd6_lookup(const struct in6_addr *, int, struct ifnet *,
>      u_int);
>  void nd6_llinfo_settimer(const struct llinfo_nd6 *, unsigned int);
>  void nd6_purge(struct ifnet *);
> -void nd6_nud_hint(struct rtentry *, int);
>  void nd6_rtrequest(struct ifnet *, int, struct rtentry *);
>  int nd6_ioctl(u_long, caddr_t, struct ifnet *);
>  void nd6_cache_lladdr(struct ifnet *, const struct in6_addr *, char *,
> diff --git sys/netinet6/nd6_nbr.c sys/netinet6/nd6_nbr.c
> index 2e09e591a73..489016b42dd 100644
> --- sys/netinet6/nd6_nbr.c
> +++ sys/netinet6/nd6_nbr.c
> @@ -670,7 +670,6 @@ nd6_na_input(struct mbuf *m, int off, int icmp6len)
>  		bcopy(lladdr, LLADDR(sdl), ifp->if_addrlen);
>  		if (is_solicited) {
>  			ln->ln_state = ND6_LLINFO_REACHABLE;
> -			ln->ln_byhint = 0;
>  			/* Notify userland that a new ND entry is reachable. */
>  			rtm_send(rt, RTM_RESOLVE, 0, ifp->if_rdomain);
>  			if (!ND6_LLINFO_PERMANENT(ln)) {
> @@ -764,7 +763,6 @@ nd6_na_input(struct mbuf *m, int off, int icmp6len)
>  			 */
>  			if (is_solicited) {
>  				ln->ln_state = ND6_LLINFO_REACHABLE;
> -				ln->ln_byhint = 0;
>  				if (!ND6_LLINFO_PERMANENT(ln)) {
>  					nd6_llinfo_settimer(ln,
>  					    ifp->if_nd->reachable);
> 
> -- 
> In my defence, I have been left unsupervised.