From: Alexander Bluhm Subject: Re: Remove upper layer IPv6 neighbor reachability hints. To: tech Date: Mon, 15 Sep 2025 18:43:11 +0200 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.