Download raw body.
> On 8 Jul 2024, at 22:54, Alexander Bluhm <bluhm@openbsd.org> wrote:
>
> On Mon, Jul 01, 2024 at 03:51:28PM +0200, Alexander Bluhm wrote:
>> On Fri, Jun 28, 2024 at 04:59:40PM +0200, Alexander Bluhm wrote:
>>> What do we want to do?
>>> 1. keep everything as it is
>>> 2. use my diff below
>>> 3. copy ICMPV6_PLD_MAXLEN to stack
>>> 4. replace m_copym() with m_dup_pkt()
>>> 5. someone has a better idea
>>
>> As a small step forward, we could copy only small packets onto the
>> stack. This avoids stack exhaustion and RFC violation. And we
>> save m_copym() overhead for these.
>
> Rebased to current.
>
>> ok?
>
ok mvs@
> Anyone?
>
> Index: netinet6/ip6_forward.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_forward.c,v
> diff -u -p -r1.120 ip6_forward.c
> --- netinet6/ip6_forward.c 4 Jul 2024 12:50:08 -0000 1.120
> +++ netinet6/ip6_forward.c 8 Jul 2024 18:33:57 -0000
> @@ -89,8 +89,16 @@ ip6_forward(struct mbuf *m, struct route
> struct rtentry *rt;
> struct sockaddr *dst;
> struct ifnet *ifp = NULL;
> - int error = 0, type = 0, code = 0, destmtu = 0;
> + u_int rtableid = m->m_pkthdr.ph_rtableid;
> + u_int ifidx = m->m_pkthdr.ph_ifidx;
> + u_int8_t loopcnt = m->m_pkthdr.ph_loopcnt;
> + u_int icmp_len;
> + char icmp_buf[MHLEN];
> + CTASSERT(sizeof(struct ip6_hdr) + sizeof(struct tcphdr) +
> + MAX_TCPOPTLEN <= sizeof(icmp_buf));
> + u_short mflags, pfflags;
> struct mbuf *mcopy;
> + int error = 0, type = 0, code = 0, destmtu = 0;
> #ifdef IPSEC
> struct tdb *tdb = NULL;
> #endif /* IPSEC */
> @@ -117,9 +125,7 @@ ip6_forward(struct mbuf *m, struct route
> log(LOG_DEBUG,
> "cannot forward "
> "from %s to %s nxt %d received on interface %u\n",
> - src6, dst6,
> - ip6->ip6_nxt,
> - m->m_pkthdr.ph_ifidx);
> + src6, dst6, ip6->ip6_nxt, ifidx);
> }
> m_freem(m);
> goto done;
> @@ -137,12 +143,21 @@ ip6_forward(struct mbuf *m, struct route
> * size of IPv6 + ICMPv6 headers) bytes of the packet in case
> * we need to generate an ICMP6 message to the src.
> * Thanks to M_EXT, in most cases copy will not occur.
> + * For small packets copy original onto stack instead of mbuf.
> *
> * It is important to save it before IPsec processing as IPsec
> * processing may modify the mbuf.
> */
> - mcopy = m_copym(m, 0, imin(m->m_pkthdr.len, ICMPV6_PLD_MAXLEN),
> - M_NOWAIT);
> + icmp_len = min(m->m_pkthdr.len, ICMPV6_PLD_MAXLEN);
> + if (icmp_len <= sizeof(icmp_buf)) {
> + mflags = m->m_flags;
> + pfflags = m->m_pkthdr.pf.flags;
> + m_copydata(m, 0, icmp_len, icmp_buf);
> + mcopy = NULL;
> + } else {
> + mcopy = m_copym(m, 0, icmp_len, M_NOWAIT);
> + icmp_len = 0;
> + }
>
> #if NPF > 0
> reroute:
> @@ -174,12 +189,10 @@ reroute:
> m->m_pkthdr.ph_rtableid);
> if (rt == NULL) {
> ip6stat_inc(ip6s_noroute);
> - if (mcopy != NULL) {
> - icmp6_error(mcopy, ICMP6_DST_UNREACH,
> - ICMP6_DST_UNREACH_NOROUTE, 0);
> - }
> + type = ICMP6_DST_UNREACH;
> + code = ICMP6_DST_UNREACH_NOROUTE;
> m_freem(m);
> - goto done;
> + goto icmperror;
> }
> dst = &ro->ro_dstsa;
>
> @@ -190,7 +203,7 @@ reroute:
> * unreachable error with Code 2 (beyond scope of source address).
> * [draft-ietf-ipngwg-icmp-v3-00.txt, Section 3.1]
> */
> - if (in6_addr2scopeid(m->m_pkthdr.ph_ifidx, &ip6->ip6_src) !=
> + if (in6_addr2scopeid(ifidx, &ip6->ip6_src) !=
> in6_addr2scopeid(rt->rt_ifidx, &ip6->ip6_src)) {
> time_t uptime;
>
> @@ -205,15 +218,12 @@ reroute:
> log(LOG_DEBUG,
> "cannot forward "
> "src %s, dst %s, nxt %d, rcvif %u, outif %u\n",
> - src6, dst6,
> - ip6->ip6_nxt,
> - m->m_pkthdr.ph_ifidx, rt->rt_ifidx);
> + src6, dst6, ip6->ip6_nxt, ifidx, rt->rt_ifidx);
> }
> - if (mcopy != NULL)
> - icmp6_error(mcopy, ICMP6_DST_UNREACH,
> - ICMP6_DST_UNREACH_BEYONDSCOPE, 0);
> + type = ICMP6_DST_UNREACH;
> + code = ICMP6_DST_UNREACH_BEYONDSCOPE;
> m_freem(m);
> - goto done;
> + goto icmperror;
> }
>
> #ifdef IPSEC
> @@ -248,7 +258,7 @@ reroute:
> m_freem(m);
> goto freecopy;
> }
> - if (rt->rt_ifidx == m->m_pkthdr.ph_ifidx &&
> + if (rt->rt_ifidx == ifidx &&
> ip6_sendredirects && !ISSET(flags, IPV6_REDIRECT) &&
> (rt->rt_flags & (RTF_DYNAMIC|RTF_MODIFIED)) == 0) {
> if ((ifp->if_flags & IFF_POINTOPOINT) &&
> @@ -268,11 +278,10 @@ reroute:
> * type/code is based on suggestion by Rich Draves.
> * not sure if it is the best pick.
> */
> - if (mcopy != NULL)
> - icmp6_error(mcopy, ICMP6_DST_UNREACH,
> - ICMP6_DST_UNREACH_ADDR, 0);
> + type = ICMP6_DST_UNREACH;
> + code = ICMP6_DST_UNREACH_ADDR;
> m_freem(m);
> - goto done;
> + goto icmperror;
> }
> type = ND_REDIRECT;
> }
> @@ -332,27 +341,41 @@ reroute:
> if (error || m == NULL)
> goto senderr;
>
> - if (mcopy != NULL)
> - icmp6_error(mcopy, ICMP6_PACKET_TOO_BIG, 0, ifp->if_mtu);
> + type = ICMP6_PACKET_TOO_BIG;
> + destmtu = ifp->if_mtu;
> m_freem(m);
> - goto done;
> + goto icmperror;
>
> senderr:
> - if (mcopy == NULL)
> + if (mcopy == NULL && icmp_len == 0)
> goto done;
>
> switch (error) {
> case 0:
> if (type == ND_REDIRECT) {
> - icmp6_redirect_output(mcopy, rt);
> - ip6stat_inc(ip6s_redirectsent);
> + if (icmp_len != 0) {
> + mcopy = m_gethdr(M_DONTWAIT, MT_DATA);
> + if (mcopy == NULL)
> + goto done;
> + mcopy->m_len = mcopy->m_pkthdr.len = icmp_len;
> + mcopy->m_flags |= (mflags & M_COPYFLAGS);
> + mcopy->m_pkthdr.ph_rtableid = rtableid;
> + mcopy->m_pkthdr.ph_ifidx = ifidx;
> + mcopy->m_pkthdr.ph_loopcnt = loopcnt;
> + mcopy->m_pkthdr.pf.flags |=
> + (pfflags & PF_TAG_GENERATED);
> + memcpy(mcopy->m_data, icmp_buf, icmp_len);
> + }
> + if (mcopy != NULL) {
> + icmp6_redirect_output(mcopy, rt);
> + ip6stat_inc(ip6s_redirectsent);
> + }
> goto done;
> }
> goto freecopy;
>
> case EMSGSIZE:
> type = ICMP6_PACKET_TOO_BIG;
> - code = 0;
> if (rt != NULL) {
> if (rt->rt_mtu) {
> destmtu = rt->rt_mtu;
> @@ -390,7 +413,21 @@ senderr:
> code = ICMP6_DST_UNREACH_ADDR;
> break;
> }
> - icmp6_error(mcopy, type, code, destmtu);
> + icmperror:
> + if (icmp_len != 0) {
> + mcopy = m_gethdr(M_DONTWAIT, MT_DATA);
> + if (mcopy == NULL)
> + goto done;
> + mcopy->m_len = mcopy->m_pkthdr.len = icmp_len;
> + mcopy->m_flags |= (mflags & M_COPYFLAGS);
> + mcopy->m_pkthdr.ph_rtableid = rtableid;
> + mcopy->m_pkthdr.ph_ifidx = ifidx;
> + mcopy->m_pkthdr.ph_loopcnt = loopcnt;
> + mcopy->m_pkthdr.pf.flags |= (pfflags & PF_TAG_GENERATED);
> + memcpy(mcopy->m_data, icmp_buf, icmp_len);
> + }
> + if (mcopy != NULL)
> + icmp6_error(mcopy, type, code, destmtu);
> goto done;
>
> freecopy: