From: Florian Obser Subject: Re: ip6 forwarding MP consistent read To: Alexander Bluhm Cc: tech@openbsd.org Date: Thu, 20 Jun 2024 11:05:36 +0200 Works on my NAT64 gateway and reads OK On 2024-06-17 14:19 +02, Alexander Bluhm wrote: > Hi, > > In IPv4 we use IP_FORWARDING to pass down a consistent value of > net.inet.ip.forwarding down the stack. This is needed for unlocking > sysctl. I would like to do the same for IPv6. > > Read ip6_forwarding once in ip6_input_if() and pass down IPV6_FORWARDING > as flags to ip6_ours(), ip6_hbhchcheck(), ip6_forward(). Replace > the srcrt value with IPV6_REDIRECT flag for consistency with IPv4. > > To have consistent syntax with IPv4, use ip6_forwarding == 0 checks > instead of !ip6_forwarding. This will also make it easier to > implement net.inet6.ip6.forwarding=2 for IPsec only forwarding > later. > > There is a little change for IPv4 behavior. Before we did accept > redirects if ip_forwarding == 2 in icmp_input_if(), now we reject > them for 1 and 2. Claudio had some concerns here and I think he > is right. I have to make the same decision for IPv6. > > In nd6_ns_input() and nd6_na_input() I read ip6_forwarding once and > store it in i_am_router. The variable name is chosen to avoid > confusion with is_router, which indicates router flag of the packet. > Reading of ip6_forwarding is done independently from ip6_input_if(), > consistency does not really matter. One is for ND router befavior > the other for forwarding. Again I use the ip6_forwarding != 0 > check, so when ip6_forwarding IPsec only value 2 gets implemented, > it will behave like a router. > > ok? > > bluhm > > Index: net/if.c > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/net/if.c,v > diff -u -p -r1.718 if.c > --- net/if.c 6 Feb 2024 00:18:53 -0000 1.718 > +++ net/if.c 17 Jun 2024 11:22:18 -0000 > @@ -3378,7 +3378,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) { > + if (ip6_forwarding == 0) { > 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.1197 pf.c > --- net/pf.c 7 Jun 2024 18:24:16 -0000 1.1197 > +++ net/pf.c 17 Jun 2024 11:22:18 -0000 > @@ -7974,12 +7974,15 @@ done: > break; > case AF_INET6: > if (pd.dir == PF_IN) { > + int flags; > + > if (ip6_forwarding == 0) { > ip6stat_inc(ip6s_cantforward); > action = PF_DROP; > break; > } > - ip6_forward(pd.m, NULL, 1); > + flags = IPV6_FORWARDING | IPV6_REDIRECT; > + ip6_forward(pd.m, NULL, flags); > } else > ip6_output(pd.m, NULL, NULL, 0, NULL, NULL); > break; > Index: net/pf_norm.c > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf_norm.c,v > diff -u -p -r1.230 pf_norm.c > --- net/pf_norm.c 22 Apr 2024 13:30:22 -0000 1.230 > +++ net/pf_norm.c 17 Jun 2024 11:57:31 -0000 > @@ -1011,7 +1011,7 @@ pf_refragment6(struct mbuf **m0, struct > while ((m = ml_dequeue(&ml)) != NULL) { > m->m_pkthdr.pf.flags |= PF_TAG_REFRAGMENTED; > if (ifp == NULL) { > - ip6_forward(m, NULL, 0); > + ip6_forward(m, NULL, IPV6_FORWARDING); > } else if ((u_long)m->m_pkthdr.len <= ifp->if_mtu) { > ifp->if_output(ifp, m, sin6tosa(dst), rt); > } else { > Index: netinet/ip_carp.c > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_carp.c,v > diff -u -p -r1.361 ip_carp.c > --- netinet/ip_carp.c 13 Feb 2024 12:22:09 -0000 1.361 > +++ netinet/ip_carp.c 17 Jun 2024 11:22:18 -0000 > @@ -1287,6 +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 flags = ND_NA_FLAG_OVERRIDE; > + > + if (ip6_forwarding != 0) > + flags |= ND_NA_FLAG_ROUTER; > > TAILQ_FOREACH(ifa, &sc->sc_if.if_addrlist, ifa_list) { > > @@ -1294,9 +1298,7 @@ carp_send_na(struct carp_softc *sc) > continue; > > in6 = &ifatoia6(ifa)->ia_addr.sin6_addr; > - nd6_na_output(&sc->sc_if, &mcast, in6, > - ND_NA_FLAG_OVERRIDE | > - (ip6_forwarding ? ND_NA_FLAG_ROUTER : 0), 1, NULL); > + nd6_na_output(&sc->sc_if, &mcast, in6, flags, 1, NULL); > } > } > #endif /* INET6 */ > Index: netinet/ip_icmp.c > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_icmp.c,v > diff -u -p -r1.193 ip_icmp.c > --- netinet/ip_icmp.c 7 Jun 2024 18:24:16 -0000 1.193 > +++ netinet/ip_icmp.c 17 Jun 2024 11:22:18 -0000 > @@ -589,7 +589,7 @@ reflect: > struct sockaddr_in ssrc; > struct rtentry *newrt = NULL; > > - if (icmp_rediraccept == 0 || ip_forwarding == 1) > + if (icmp_rediraccept == 0 || ip_forwarding != 0) > goto freeit; > if (code > 3) > goto badcode; > Index: netinet6/icmp6.c > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/icmp6.c,v > diff -u -p -r1.252 icmp6.c > --- netinet6/icmp6.c 21 Apr 2024 17:32:10 -0000 1.252 > +++ netinet6/icmp6.c 17 Jun 2024 11:22:18 -0000 > @@ -1240,8 +1240,8 @@ icmp6_redirect_input(struct mbuf *m, int > if (ifp == NULL) > return; > > - /* XXX if we are router, we don't update route by icmp6 redirect */ > - if (ip6_forwarding) > + /* if we are router, we don't update route by icmp6 redirect */ > + if (ip6_forwarding != 0) > goto freeit; > if (!(ifp->if_xflags & IFXF_AUTOCONF6)) > goto freeit; > @@ -1442,7 +1442,7 @@ icmp6_redirect_output(struct mbuf *m0, s > icmp6_errcount(ND_REDIRECT, 0); > > /* if we are not router, we don't send icmp6 redirect */ > - if (!ip6_forwarding) > + if (ip6_forwarding == 0) > goto fail; > > /* sanity check */ > Index: netinet6/ip6_forward.c > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_forward.c,v > diff -u -p -r1.118 ip6_forward.c > --- netinet6/ip6_forward.c 7 Jun 2024 18:24:16 -0000 1.118 > +++ netinet6/ip6_forward.c 17 Jun 2024 11:22:18 -0000 > @@ -82,7 +82,7 @@ > */ > > void > -ip6_forward(struct mbuf *m, struct route *ro, int srcrt) > +ip6_forward(struct mbuf *m, struct route *ro, int flags) > { > struct ip6_hdr *ip6 = mtod(m, struct ip6_hdr *); > struct route iproute; > @@ -248,8 +248,8 @@ reroute: > m_freem(m); > goto freecopy; > } > - if (rt->rt_ifidx == m->m_pkthdr.ph_ifidx && !srcrt && > - ip6_sendredirects && > + if (rt->rt_ifidx == m->m_pkthdr.ph_ifidx && > + ip6_sendredirects && !ISSET(flags, IPV6_REDIRECT) && > (rt->rt_flags & (RTF_DYNAMIC|RTF_MODIFIED)) == 0) { > if ((ifp->if_flags & IFF_POINTOPOINT) && > nd6_is_addr_neighbor(&ro->ro_dstsin6, ifp)) { > @@ -305,7 +305,7 @@ reroute: > } else if (m->m_pkthdr.pf.flags & PF_TAG_REROUTE) { > /* tag as generated to skip over pf_test on rerun */ > m->m_pkthdr.pf.flags |= PF_TAG_GENERATED; > - srcrt = 1; > + SET(flags, IPV6_REDIRECT); > if (ro == &iproute) > rtfree(ro->ro_rt); > ro = NULL; > Index: netinet6/ip6_input.c > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_input.c,v > diff -u -p -r1.262 ip6_input.c > --- netinet6/ip6_input.c 8 May 2024 13:01:30 -0000 1.262 > +++ netinet6/ip6_input.c 17 Jun 2024 11:22:18 -0000 > @@ -119,9 +119,9 @@ struct cpumem *ip6counters; > > uint8_t ip6_soiikey[IP6_SOIIKEY_LEN]; > > -int ip6_ours(struct mbuf **, int *, int, int); > +int ip6_ours(struct mbuf **, int *, int, int, int); > int ip6_check_rh0hdr(struct mbuf *, int *); > -int ip6_hbhchcheck(struct mbuf **, int *, int *); > +int ip6_hbhchcheck(struct mbuf **, int *, int *, int); > int ip6_hopopts_input(struct mbuf **, int *, u_int32_t *, u_int32_t *); > struct mbuf *ip6_pullexthdr(struct mbuf *, size_t, int); > int ip6_sysctl_soiikey(void *, size_t *, void *, size_t); > @@ -172,11 +172,11 @@ ip6_init(void) > * NET_LOCK_SHARED() and the transport layer needing it exclusively. > */ > int > -ip6_ours(struct mbuf **mp, int *offp, int nxt, int af) > +ip6_ours(struct mbuf **mp, int *offp, int nxt, int af, int flags) > { > /* ip6_hbhchcheck() may be run before, then off and nxt are set */ > if (*offp == 0) { > - nxt = ip6_hbhchcheck(mp, offp, NULL); > + nxt = ip6_hbhchcheck(mp, offp, NULL, flags); > if (nxt == IPPROTO_DONE) > return IPPROTO_DONE; > } > @@ -365,7 +365,7 @@ ip6_input_if(struct mbuf **mp, int *offp > #if NPF > 0 > struct in6_addr odst; > #endif > - int pfrdr = 0; > + int flags = 0; > > KASSERT(*offp == 0); > > @@ -412,9 +412,13 @@ ip6_input_if(struct mbuf **mp, int *offp > goto bad; > > ip6 = mtod(m, struct ip6_hdr *); > - pfrdr = !IN6_ARE_ADDR_EQUAL(&odst, &ip6->ip6_dst); > + if (!IN6_ARE_ADDR_EQUAL(&odst, &ip6->ip6_dst)) > + SET(flags, IPV6_REDIRECT); > #endif > > + if (ip6_forwarding != 0) > + SET(flags, IPV6_FORWARDING); > + > /* > * Without embedded scope ID we cannot find link-local > * addresses in the routing table. > @@ -445,7 +449,7 @@ ip6_input_if(struct mbuf **mp, int *offp > > #if NPF > 0 > if (pf_ouraddr(m) == 1) { > - nxt = ip6_ours(mp, offp, nxt, af); > + nxt = ip6_ours(mp, offp, nxt, af, flags); > goto out; > } > #endif > @@ -472,7 +476,7 @@ ip6_input_if(struct mbuf **mp, int *offp > if (ip6_mforwarding && ip6_mrouter[ifp->if_rdomain]) { > int error; > > - nxt = ip6_hbhchcheck(&m, offp, &ours); > + nxt = ip6_hbhchcheck(&m, offp, &ours, flags); > if (nxt == IPPROTO_DONE) > goto out; > > @@ -496,7 +500,8 @@ ip6_input_if(struct mbuf **mp, int *offp > > if (ours) { > if (af == AF_UNSPEC) > - nxt = ip6_ours(mp, offp, nxt, af); > + nxt = ip6_ours(mp, offp, nxt, af, > + flags); > goto out; > } > goto bad; > @@ -508,7 +513,7 @@ ip6_input_if(struct mbuf **mp, int *offp > ip6stat_inc(ip6s_cantforward); > goto bad; > } > - nxt = ip6_ours(mp, offp, nxt, af); > + nxt = ip6_ours(mp, offp, nxt, af, flags); > goto out; > } > > @@ -526,7 +531,8 @@ ip6_input_if(struct mbuf **mp, int *offp > if (rt != NULL && ISSET(rt->rt_flags, RTF_LOCAL)) { > struct in6_ifaddr *ia6 = ifatoia6(rt->rt_ifa); > > - if (ip6_forwarding == 0 && rt->rt_ifidx != ifp->if_index && > + if (!ISSET(flags, IPV6_FORWARDING) && > + rt->rt_ifidx != ifp->if_index && > !((ifp->if_flags & IFF_LOOPBACK) || > (ifp->if_type == IFT_ENC) || > (m->m_pkthdr.pf.flags & PF_TAG_TRANSLATE_LOCALHOST))) { > @@ -567,7 +573,7 @@ ip6_input_if(struct mbuf **mp, int *offp > > goto bad; > } else { > - nxt = ip6_ours(mp, offp, nxt, af); > + nxt = ip6_ours(mp, offp, nxt, af, flags); > goto out; > } > } > @@ -582,18 +588,18 @@ ip6_input_if(struct mbuf **mp, int *offp > * Now there is no reason to process the packet if it's not our own > * and we're not a router. > */ > - if (!ip6_forwarding) { > + if (!ISSET(flags, IPV6_FORWARDING)) { > ip6stat_inc(ip6s_cantforward); > goto bad; > } > > - nxt = ip6_hbhchcheck(&m, offp, &ours); > + nxt = ip6_hbhchcheck(&m, offp, &ours, flags); > if (nxt == IPPROTO_DONE) > goto out; > > if (ours) { > if (af == AF_UNSPEC) > - nxt = ip6_ours(mp, offp, nxt, af); > + nxt = ip6_ours(mp, offp, nxt, af, flags); > goto out; > } > > @@ -613,7 +619,7 @@ ip6_input_if(struct mbuf **mp, int *offp > } > #endif /* IPSEC */ > > - ip6_forward(m, &ro, pfrdr); > + ip6_forward(m, &ro, flags); > *mp = NULL; > rtfree(ro.ro_rt); > return IPPROTO_DONE; > @@ -627,7 +633,7 @@ ip6_input_if(struct mbuf **mp, int *offp > > /* On error free mbuf and return IPPROTO_DONE. */ > int > -ip6_hbhchcheck(struct mbuf **mp, int *offp, int *oursp) > +ip6_hbhchcheck(struct mbuf **mp, int *offp, int *oursp, int flags) > { > struct ip6_hdr *ip6; > u_int32_t plen, rtalert = ~0; > @@ -680,7 +686,8 @@ ip6_hbhchcheck(struct mbuf **mp, int *of > * accept the packet if a router alert option is included > * and we act as an IPv6 router. > */ > - if (rtalert != ~0 && ip6_forwarding && oursp != NULL) > + if (rtalert != ~0 && ISSET(flags, IPV6_FORWARDING) && > + oursp != NULL) > *oursp = 1; > } else > nxt = ip6->ip6_nxt; > Index: netinet6/ip6_var.h > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_var.h,v > diff -u -p -r1.117 ip6_var.h > --- netinet6/ip6_var.h 13 May 2024 01:15:53 -0000 1.117 > +++ netinet6/ip6_var.h 17 Jun 2024 11:22:18 -0000 > @@ -265,10 +265,11 @@ ip6stat_add(enum ip6stat_counters c, uin > counters_add(ip6counters, c, v); > } > > -/* flags passed to ip6_output as last parameter */ > -#define IPV6_UNSPECSRC 0x01 /* allow :: as the source address */ > -#define IPV6_FORWARDING 0x02 /* most of IPv6 header exists */ > -#define IPV6_MINMTU 0x04 /* use minimum MTU (IPV6_USE_MIN_MTU) */ > +/* flags passed to ip6_output or ip6_forward as last parameter */ > +#define IPV6_UNSPECSRC 0x01 /* allow :: as the source address */ > +#define IPV6_FORWARDING 0x02 /* most of IPv6 header exists */ > +#define IPV6_MINMTU 0x04 /* use minimum MTU (IPV6_USE_MIN_MTU) */ > +#define IPV6_REDIRECT 0x08 /* redirected by pf */ > > extern int ip6_mtudisc_timeout; /* mtu discovery */ > extern struct rttimer_queue icmp6_mtudisc_timeout_q; > Index: netinet6/nd6.c > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/nd6.c,v > diff -u -p -r1.280 nd6.c > --- netinet6/nd6.c 13 May 2023 16:27:59 -0000 1.280 > +++ netinet6/nd6.c 17 Jun 2024 11:22:18 -0000 > @@ -671,7 +671,7 @@ nd6_free(struct rtentry *rt) > > ifp = if_get(rt->rt_ifidx); > > - if (!ip6_forwarding) { > + if (ip6_forwarding == 0) { > if (ln->ln_router) { > /* > * rt6_flush must be called whether or not the neighbor > Index: netinet6/nd6_nbr.c > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/nd6_nbr.c,v > diff -u -p -r1.151 nd6_nbr.c > --- netinet6/nd6_nbr.c 30 Jul 2023 12:52:03 -0000 1.151 > +++ netinet6/nd6_nbr.c 17 Jun 2024 11:22:18 -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 router = ip6_forwarding; > + int i_am_router = (ip6_forwarding != 0); > int tlladdr; > struct nd_opts ndopts; > struct sockaddr_dl *proxydl = NULL; > @@ -244,7 +244,7 @@ nd6_ns_input(struct mbuf *m, int off, in > if (ifa) { > proxy = 1; > proxydl = satosdl(rt->rt_gateway); > - router = 0; /* XXX */ > + i_am_router = 0; /* XXX */ > } > } > if (rt) > @@ -317,7 +317,7 @@ nd6_ns_input(struct mbuf *m, int off, in > saddr6.s6_addr16[1] = htons(ifp->if_index); > nd6_na_output(ifp, &saddr6, &taddr6, > ((anycast || proxy || !tlladdr) ? 0 : ND_NA_FLAG_OVERRIDE) | > - (router ? ND_NA_FLAG_ROUTER : 0), > + (i_am_router ? ND_NA_FLAG_ROUTER : 0), > tlladdr, sdltosa(proxydl)); > goto freeit; > } > @@ -327,7 +327,7 @@ nd6_ns_input(struct mbuf *m, int off, in > > nd6_na_output(ifp, &saddr6, &taddr6, > ((anycast || proxy || !tlladdr) ? 0 : ND_NA_FLAG_OVERRIDE) | > - (router ? ND_NA_FLAG_ROUTER : 0) | ND_NA_FLAG_SOLICITED, > + (i_am_router ? ND_NA_FLAG_ROUTER : 0) | ND_NA_FLAG_SOLICITED, > tlladdr, sdltosa(proxydl)); > freeit: > m_freem(m); > @@ -559,6 +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); > struct ifaddr *ifa; > struct in6_ifaddr *ifa6; > struct llinfo_nd6 *ln; > @@ -684,7 +685,7 @@ nd6_na_input(struct mbuf *m, int off, in > * If we are a router, we may create new stale cache entries upon > * receiving Unsolicited Neighbor Advertisements. > */ > - if (rt == NULL && ip6_forwarding == 1) { > + if (rt == NULL && i_am_router) { > rt = nd6_lookup(&taddr6, 1, ifp, ifp->if_rdomain); > if (rt == NULL || lladdr == NULL || > ((sdl = satosdl(rt->rt_gateway)) == NULL)) > @@ -837,7 +838,7 @@ nd6_na_input(struct mbuf *m, int off, in > } > > if (ln->ln_router && !is_router) { > - if (!ip6_forwarding) { > + if (!i_am_router) { > /* > * The neighbor may be used > * as a next hop for some destinations > -- In my defence, I have been left unsupervised.