From: Alexander Bluhm <bluhm@openbsd.org>
Subject: Re: sysctl ip6 forwarding unlock
To: Vitaliy Makkoveev <otto@bsdbox.dev>
Cc: tech@openbsd.org
Date: Sun, 14 Jul 2024 18:04:46 +0200

On Sun, Jul 14, 2024 at 06:53:06PM +0300, Vitaliy Makkoveev wrote:
> Hi,
> 
> Personally I prefer ???_local??? postfix like we already used for
> unlocked sysctls like kern.somaxconn, kern.sominconn or
> net.pipex.enable

Yes.  But here I also make the difference between forwarding policy
and the IPv6 router bahavior based on forwarding.  That's why
i_am_router = (ip6_forwarding != 0) makes more sense than using
ip6_forwarding_local != 0 everywhere.

bluhm

> solisten(struct socket *so, int backlog)
> {
>         int somaxconn_local = READ_ONCE(somaxconn);
>         int sominconn_local = READ_ONCE(sominconn); 
> 
> pppx_if_output(struct ifnet *ifp, struct mbuf *m, struct sockaddr *dst,
>     struct rtentry *rt)
> {
>         struct pppx_if *pxi = (struct pppx_if *)ifp->if_softc;
>         struct pppx_hdr *th;
>         int error = 0;
>         int pipex_enable_local, proto;
> 
>         pipex_enable_local = atomic_load_int(&pipex_enable);
> 
> The rest of the diff is good for me.
> 
> > On 14 Jul 2024, at 18:13, Alexander Bluhm <bluhm@openbsd.org> wrote:
> > 
> > Hi,
> > 
> > Like already done for IPv4, also unlock ip6_forwarding from net
> > lock.
> > 
> > To make clear where actually the router property is needed, I added
> > the i_am_router variable.  It already existed in nd6_nbr.  Move
> > i_am_router setting up the call stack until all users seem to be
> > independent.
> > 
> > The forwarding decisions in pf_test, pf_refragment6, ip6_input do
> > also not interfere.
> > 
> > Use a new array ipv6ctl_vars_unlocked to make transition of all the
> > integer sysctls easier.  Adapt IPv4 to the new style.
> > 
> > ok?
> > 
> > bluhm
> > 
> > Index: net/if.c
> > ===================================================================
> > RCS file: /data/mirror/openbsd/cvs/src/sys/net/if.c,v
> > diff -u -p -r1.719 if.c
> > --- net/if.c	20 Jun 2024 19:25:42 -0000	1.719
> > +++ net/if.c	14 Jul 2024 08:52:35 -0000
> > @@ -3353,6 +3353,7 @@ ifnewlladdr(struct ifnet *ifp)
> > {
> > #ifdef INET6
> > 	struct ifaddr *ifa;
> > +	int i_am_router = (atomic_load_int(&ip6_forwarding) != 0);
> > #endif
> > 	struct ifreq ifrq;
> > 	short up;
> > @@ -3378,7 +3379,7 @@ ifnewlladdr(struct ifnet *ifp)
> > 	 * Update the link-local address.  Don't do it if we're
> > 	 * a router to avoid confusing hosts on the network.
> > 	 */
> > -	if (ip6_forwarding == 0) {
> > +	if (!i_am_router) {
> > 		ifa = &in6ifa_ifpforlinklocal(ifp, 0)->ia_ifa;
> > 		if (ifa) {
> > 			in6_purgeaddr(ifa);
> > Index: net/pf.c
> > ===================================================================
> > RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
> > diff -u -p -r1.1202 pf.c
> > --- net/pf.c	12 Jul 2024 09:25:27 -0000	1.1202
> > +++ net/pf.c	14 Jul 2024 08:52:35 -0000
> > @@ -7988,7 +7988,7 @@ done:
> > 			if (pd.dir == PF_IN) {
> > 				int flags = IPV6_REDIRECT;
> > 
> > -				switch (ip6_forwarding) {
> > +				switch (atomic_load_int(&ip6_forwarding)) {
> > 				case 2:
> > 					SET(flags, IPV6_FORWARDING_IPSEC);
> > 					/* FALLTHROUGH */
> > Index: net/pf_norm.c
> > ===================================================================
> > RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf_norm.c,v
> > diff -u -p -r1.232 pf_norm.c
> > --- net/pf_norm.c	4 Jul 2024 12:50:08 -0000	1.232
> > +++ net/pf_norm.c	14 Jul 2024 08:52:35 -0000
> > @@ -1013,7 +1013,7 @@ pf_refragment6(struct mbuf **m0, struct 
> > 		if (ifp == NULL) {
> > 			int flags = 0;
> > 
> > -			switch (ip6_forwarding) {
> > +			switch (atomic_load_int(&ip6_forwarding)) {
> > 			case 2:
> > 				SET(flags, IPV6_FORWARDING_IPSEC);
> > 				/* FALLTHROUGH */
> > Index: netinet/ip_carp.c
> > ===================================================================
> > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_carp.c,v
> > diff -u -p -r1.362 ip_carp.c
> > --- netinet/ip_carp.c	20 Jun 2024 19:25:42 -0000	1.362
> > +++ netinet/ip_carp.c	14 Jul 2024 08:52:35 -0000
> > @@ -1287,9 +1287,10 @@ carp_send_na(struct carp_softc *sc)
> > 	struct ifaddr *ifa;
> > 	struct in6_addr *in6;
> > 	static struct in6_addr mcast = IN6ADDR_LINKLOCAL_ALLNODES_INIT;
> > +	int i_am_router = (atomic_load_int(&ip6_forwarding) != 0);
> > 	int flags = ND_NA_FLAG_OVERRIDE;
> > 
> > -	if (ip6_forwarding != 0)
> > +	if (i_am_router)
> > 		flags |= ND_NA_FLAG_ROUTER;
> > 
> > 	TAILQ_FOREACH(ifa, &sc->sc_if.if_addrlist, ifa_list) {
> > Index: netinet/ip_icmp.c
> > ===================================================================
> > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_icmp.c,v
> > diff -u -p -r1.195 ip_icmp.c
> > --- netinet/ip_icmp.c	12 Jul 2024 09:25:27 -0000	1.195
> > +++ netinet/ip_icmp.c	14 Jul 2024 14:54:13 -0000
> > @@ -588,9 +588,9 @@ reflect:
> > 		struct sockaddr_in sgw;
> > 		struct sockaddr_in ssrc;
> > 		struct rtentry *newrt = NULL;
> > +		int i_am_router = (atomic_load_int(&ip_forwarding) != 0);
> > 
> > -		if (icmp_rediraccept == 0 ||
> > -		    atomic_load_int(&ip_forwarding) != 0)
> > +		if (icmp_rediraccept == 0 || i_am_router)
> > 			goto freeit;
> > 		if (code > 3)
> > 			goto badcode;
> > Index: netinet/ip_input.c
> > ===================================================================
> > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_input.c,v
> > diff -u -p -r1.398 ip_input.c
> > --- netinet/ip_input.c	12 Jul 2024 09:25:27 -0000	1.398
> > +++ netinet/ip_input.c	14 Jul 2024 09:01:22 -0000
> > @@ -111,6 +111,10 @@ LIST_HEAD(, ipq) ipq;
> > int	ip_maxqueue = 300;
> > int	ip_frags = 0;
> > 
> > +const struct sysctl_bounded_args ipctl_vars_unlocked[] = {
> > +	{ IPCTL_FORWARDING, &ip_forwarding, 0, 2 },
> > +};
> > +
> > const struct sysctl_bounded_args ipctl_vars[] = {
> > #ifdef MROUTING
> > 	{ IPCTL_MRTPROTO, &ip_mrtproto, SYSCTL_INT_READONLY },
> > @@ -1799,8 +1803,9 @@ ip_sysctl(int *name, u_int namelen, void
> > 		NET_UNLOCK();
> > 		return (error);
> > 	case IPCTL_FORWARDING:
> > -		return (sysctl_int_bounded(oldp, oldlenp, newp, newlen,
> > -		    &ip_forwarding, 0, 2));
> > +		return (sysctl_bounded_arr(
> > +		    ipctl_vars_unlocked, nitems(ipctl_vars_unlocked),
> > +		    name, namelen, oldp, oldlenp, newp, newlen));
> > 	default:
> > 		NET_LOCK();
> > 		error = sysctl_bounded_arr(ipctl_vars, nitems(ipctl_vars),
> > Index: netinet6/icmp6.c
> > ===================================================================
> > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/icmp6.c,v
> > diff -u -p -r1.253 icmp6.c
> > --- netinet6/icmp6.c	20 Jun 2024 19:25:42 -0000	1.253
> > +++ netinet6/icmp6.c	14 Jul 2024 14:44:30 -0000
> > @@ -1228,6 +1228,7 @@ icmp6_redirect_input(struct mbuf *m, int
> > 	char *lladdr = NULL;
> > 	int lladdrlen = 0;
> > 	struct rtentry *rt = NULL;
> > +	int i_am_router = (atomic_load_int(&ip6_forwarding) != 0);
> > 	int is_router;
> > 	int is_onlink;
> > 	struct in6_addr src6 = ip6->ip6_src;
> > @@ -1241,7 +1242,7 @@ icmp6_redirect_input(struct mbuf *m, int
> > 		return;
> > 
> > 	/* if we are router, we don't update route by icmp6 redirect */
> > -	if (ip6_forwarding != 0)
> > +	if (i_am_router)
> > 		goto freeit;
> > 	if (!(ifp->if_xflags & IFXF_AUTOCONF6))
> > 		goto freeit;
> > @@ -1366,7 +1367,7 @@ icmp6_redirect_input(struct mbuf *m, int
> > 
> > 	/* RFC 2461 8.3 */
> > 	nd6_cache_lladdr(ifp, &redtgt6, lladdr, lladdrlen, ND_REDIRECT,
> > -			 is_onlink ? ND_REDIRECT_ONLINK : ND_REDIRECT_ROUTER);
> > +	    is_onlink ? ND_REDIRECT_ONLINK : ND_REDIRECT_ROUTER, i_am_router);
> > 
> > 	if (!is_onlink) {	/* better router case.  perform rtredirect. */
> > 		/* perform rtredirect */
> > @@ -1438,11 +1439,12 @@ icmp6_redirect_output(struct mbuf *m0, s
> > 	size_t maxlen;
> > 	u_char *p;
> > 	struct sockaddr_in6 src_sa;
> > +	int i_am_router = (atomic_load_int(&ip6_forwarding) != 0);
> > 
> > 	icmp6_errcount(ND_REDIRECT, 0);
> > 
> > 	/* if we are not router, we don't send icmp6 redirect */
> > -	if (ip6_forwarding == 0)
> > +	if (!i_am_router)
> > 		goto fail;
> > 
> > 	/* sanity check */
> > Index: netinet6/ip6_input.c
> > ===================================================================
> > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_input.c,v
> > diff -u -p -r1.264 ip6_input.c
> > --- netinet6/ip6_input.c	4 Jul 2024 12:50:08 -0000	1.264
> > +++ netinet6/ip6_input.c	14 Jul 2024 09:03:09 -0000
> > @@ -416,7 +416,7 @@ ip6_input_if(struct mbuf **mp, int *offp
> > 		SET(flags, IPV6_REDIRECT);
> > #endif
> > 
> > -	switch (ip6_forwarding) {
> > +	switch (atomic_load_int(&ip6_forwarding)) {
> > 	case 2:
> > 		SET(flags, IPV6_FORWARDING_IPSEC);
> > 		/* FALLTHROUGH */
> > @@ -1443,12 +1443,15 @@ const u_char inet6ctlerrmap[PRC_NCMDS] =
> > extern int ip6_mrtproto;
> > #endif
> > 
> > +const struct sysctl_bounded_args ipv6ctl_vars_unlocked[] = {
> > +	{ IPV6CTL_FORWARDING, &ip6_forwarding, 0, 2 },
> > +};
> > +
> > const struct sysctl_bounded_args ipv6ctl_vars[] = {
> > 	{ IPV6CTL_DAD_PENDING, &ip6_dad_pending, SYSCTL_INT_READONLY },
> > #ifdef MROUTING
> > 	{ IPV6CTL_MRTPROTO, &ip6_mrtproto, SYSCTL_INT_READONLY },
> > #endif
> > -	{ IPV6CTL_FORWARDING, &ip6_forwarding, 0, 2 },
> > 	{ IPV6CTL_SENDREDIRECTS, &ip6_sendredirects, 0, 1 },
> > 	{ IPV6CTL_DEFHLIM, &ip6_defhlim, 0, 255 },
> > 	{ IPV6CTL_MAXFRAGPACKETS, &ip6_maxfragpackets, 0, 1000 },
> > @@ -1568,6 +1571,10 @@ ip6_sysctl(int *name, u_int namelen, voi
> > 			atomic_inc_long(&rtgeneration);
> > 		NET_UNLOCK();
> > 		return (error);
> > +	case IPV6CTL_FORWARDING:
> > +		return (sysctl_bounded_arr(
> > +		    ipv6ctl_vars_unlocked, nitems(ipv6ctl_vars_unlocked),
> > +		    name, namelen, oldp, oldlenp, newp, newlen));
> > 	default:
> > 		NET_LOCK();
> > 		error = sysctl_bounded_arr(ipv6ctl_vars, nitems(ipv6ctl_vars),
> > Index: netinet6/nd6.c
> > ===================================================================
> > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/nd6.c,v
> > diff -u -p -r1.281 nd6.c
> > --- netinet6/nd6.c	20 Jun 2024 19:25:42 -0000	1.281
> > +++ netinet6/nd6.c	14 Jul 2024 14:44:51 -0000
> > @@ -107,8 +107,8 @@ void nd6_slowtimo(void *);
> > void nd6_expire(void *);
> > void nd6_expire_timer(void *);
> > void nd6_invalidate(struct rtentry *);
> > -void nd6_free(struct rtentry *);
> > -int nd6_llinfo_timer(struct rtentry *);
> > +void nd6_free(struct rtentry *, int);
> > +int nd6_llinfo_timer(struct rtentry *, int);
> > 
> > struct timeout nd6_timer_to;
> > struct timeout nd6_slowtimo_ch;
> > @@ -264,6 +264,7 @@ nd6_timer(void *unused)
> > {
> > 	struct llinfo_nd6 *ln, *nln;
> > 	time_t uptime, expire;
> > +	int i_am_router = (atomic_load_int(&ip6_forwarding) != 0);
> > 	int secs;
> > 
> > 	NET_LOCK();
> > @@ -276,7 +277,7 @@ nd6_timer(void *unused)
> > 		struct rtentry *rt = ln->ln_rt;
> > 
> > 		if (rt->rt_expire && rt->rt_expire <= uptime)
> > -			if (nd6_llinfo_timer(rt))
> > +			if (nd6_llinfo_timer(rt, i_am_router))
> > 				continue;
> > 
> > 		if (rt->rt_expire && rt->rt_expire < expire)
> > @@ -300,7 +301,7 @@ nd6_timer(void *unused)
> >  * Returns 1 if `rt' should no longer be used, 0 otherwise.
> >  */
> > int
> > -nd6_llinfo_timer(struct rtentry *rt)
> > +nd6_llinfo_timer(struct rtentry *rt, int i_am_router)
> > {
> > 	struct llinfo_nd6 *ln = (struct llinfo_nd6 *)rt->rt_llinfo;
> > 	struct sockaddr_in6 *dst = satosin6(rt_key(rt));
> > @@ -346,7 +347,7 @@ nd6_llinfo_timer(struct rtentry *rt)
> > 			} else
> > 				atomic_sub_int(&ln_hold_total, len);
> > 
> > -			nd6_free(rt);
> > +			nd6_free(rt, i_am_router);
> > 			ln = NULL;
> > 		}
> > 		break;
> > @@ -362,7 +363,7 @@ nd6_llinfo_timer(struct rtentry *rt)
> > 	case ND6_LLINFO_PURGE:
> > 		/* Garbage Collection(RFC 2461 5.3) */
> > 		if (!ND6_LLINFO_PERMANENT(ln)) {
> > -			nd6_free(rt);
> > +			nd6_free(rt, i_am_router);
> > 			ln = NULL;
> > 		}
> > 		break;
> > @@ -383,7 +384,7 @@ nd6_llinfo_timer(struct rtentry *rt)
> > 			nd6_ns_output(ifp, &dst->sin6_addr, &dst->sin6_addr,
> > 			    &ln->ln_saddr6, 0);
> > 		} else {
> > -			nd6_free(rt);
> > +			nd6_free(rt, i_am_router);
> > 			ln = NULL;
> > 		}
> > 		break;
> > @@ -477,6 +478,7 @@ void
> > nd6_purge(struct ifnet *ifp)
> > {
> > 	struct llinfo_nd6 *ln, *nln;
> > +	int i_am_router = (atomic_load_int(&ip6_forwarding) != 0);
> > 
> > 	NET_ASSERT_LOCKED_EXCLUSIVE();
> > 
> > @@ -492,7 +494,7 @@ nd6_purge(struct ifnet *ifp)
> > 		    rt->rt_gateway->sa_family == AF_LINK) {
> > 			sdl = satosdl(rt->rt_gateway);
> > 			if (sdl->sdl_index == ifp->if_index)
> > -				nd6_free(rt);
> > +				nd6_free(rt, i_am_router);
> > 		}
> > 	}
> > }
> > @@ -661,7 +663,7 @@ nd6_invalidate(struct rtentry *rt)
> >  * Free an nd6 llinfo entry.
> >  */
> > void
> > -nd6_free(struct rtentry *rt)
> > +nd6_free(struct rtentry *rt, int i_am_router)
> > {
> > 	struct llinfo_nd6 *ln = (struct llinfo_nd6 *)rt->rt_llinfo;
> > 	struct in6_addr in6 = satosin6(rt_key(rt))->sin6_addr;
> > @@ -671,7 +673,7 @@ nd6_free(struct rtentry *rt)
> > 
> > 	ifp = if_get(rt->rt_ifidx);
> > 
> > -	if (ip6_forwarding == 0) {
> > +	if (!i_am_router) {
> > 		if (ln->ln_router) {
> > 			/*
> > 			 * rt6_flush must be called whether or not the neighbor
> > @@ -1031,7 +1033,7 @@ nd6_ioctl(u_long cmd, caddr_t data, stru
> >  */
> > void
> > nd6_cache_lladdr(struct ifnet *ifp, const struct in6_addr *from, char *lladdr,
> > -    int lladdrlen, int type, int code)
> > +    int lladdrlen, int type, int code, int i_am_router)
> > {
> > 	struct rtentry *rt;
> > 	struct llinfo_nd6 *ln;
> > @@ -1080,7 +1082,7 @@ nd6_cache_lladdr(struct ifnet *ifp, cons
> > 		return;
> > 	if ((rt->rt_flags & (RTF_GATEWAY | RTF_LLINFO)) != RTF_LLINFO) {
> > fail:
> > -		nd6_free(rt);
> > +		nd6_free(rt, i_am_router);
> > 		rtfree(rt);
> > 		return;
> > 	}
> > Index: netinet6/nd6.h
> > ===================================================================
> > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/nd6.h,v
> > diff -u -p -r1.99 nd6.h
> > --- netinet6/nd6.h	4 May 2023 06:56:56 -0000	1.99
> > +++ netinet6/nd6.h	14 Jul 2024 14:38:25 -0000
> > @@ -134,7 +134,7 @@ void nd6_nud_hint(struct rtentry *);
> > 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 *,
> > -    int, int, int);
> > +    int, int, int, int);
> > int nd6_resolve(struct ifnet *, struct rtentry *, struct mbuf *,
> > 	 struct sockaddr *, u_char *);
> > int nd6_need_cache(struct ifnet *);
> > Index: netinet6/nd6_nbr.c
> > ===================================================================
> > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/nd6_nbr.c,v
> > diff -u -p -r1.152 nd6_nbr.c
> > --- netinet6/nd6_nbr.c	20 Jun 2024 19:25:42 -0000	1.152
> > +++ netinet6/nd6_nbr.c	14 Jul 2024 14:39:34 -0000
> > @@ -108,7 +108,7 @@ nd6_ns_input(struct mbuf *m, int off, in
> > 	struct ifaddr *ifa = NULL;
> > 	int lladdrlen = 0;
> > 	int anycast = 0, proxy = 0, tentative = 0;
> > -	int i_am_router = (ip6_forwarding != 0);
> > +	int i_am_router = (atomic_load_int(&ip6_forwarding) != 0);
> > 	int tlladdr;
> > 	struct nd_opts ndopts;
> > 	struct sockaddr_dl *proxydl = NULL;
> > @@ -323,7 +323,7 @@ nd6_ns_input(struct mbuf *m, int off, in
> > 	}
> > 
> > 	nd6_cache_lladdr(ifp, &saddr6, lladdr, lladdrlen, ND_NEIGHBOR_SOLICIT,
> > -	    0);
> > +	    0, i_am_router);
> > 
> > 	nd6_na_output(ifp, &saddr6, &taddr6,
> > 	    ((anycast || proxy || !tlladdr) ? 0 : ND_NA_FLAG_OVERRIDE) |
> > @@ -559,7 +559,7 @@ nd6_na_input(struct mbuf *m, int off, in
> > 	int is_override;
> > 	char *lladdr = NULL;
> > 	int lladdrlen = 0;
> > -	int i_am_router = (ip6_forwarding != 0);
> > +	int i_am_router = (atomic_load_int(&ip6_forwarding) != 0);
> > 	struct ifaddr *ifa;
> > 	struct in6_ifaddr *ifa6;
> > 	struct llinfo_nd6 *ln;
> > Index: netinet6/nd6_rtr.c
> > ===================================================================
> > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/nd6_rtr.c,v
> > diff -u -p -r1.170 nd6_rtr.c
> > --- netinet6/nd6_rtr.c	31 Mar 2023 19:43:33 -0000	1.170
> > +++ netinet6/nd6_rtr.c	14 Jul 2024 14:42:55 -0000
> > @@ -73,6 +73,7 @@ nd6_rtr_cache(struct mbuf *m, int off, i
> > 	struct in6_addr saddr6 = ip6->ip6_src;
> > 	char *lladdr = NULL;
> > 	int lladdrlen = 0;
> > +	int i_am_router = (atomic_load_int(&ip6_forwarding) != 0);
> > 	struct nd_opts ndopts;
> > 	char src[INET6_ADDRSTRLEN], dst[INET6_ADDRSTRLEN];
> > 
> > @@ -157,7 +158,8 @@ nd6_rtr_cache(struct mbuf *m, int off, i
> > 		goto bad;
> > 	}
> > 
> > -	nd6_cache_lladdr(ifp, &saddr6, lladdr, lladdrlen, icmp6_type, 0);
> > +	nd6_cache_lladdr(ifp, &saddr6, lladdr, lladdrlen, icmp6_type, 0,
> > +	    i_am_router);
> > 	if_put(ifp);
> > 
> >  freeit:
> > 
>