On Fri, Jun 16, 2023 at 12:06:08PM +0200, Alexander Bluhm wrote:
> On Mon, Jun 12, 2023 at 03:46:28PM +0200, Alexander Bluhm wrote:
> > I found a little inconsistency in IPv6 forwarding with TSO.
> >
> > Sending with TSO should only done if the large packet does not fit
> > in the interface MTU. In case tcp_if_output_tso() does not process
> > the packet, we should send an ICMP6 error. Rearrange the code that
> > it looks more like other calls to tcp_if_output_tso().
> >
> > All these cases can only be reached when LRO is turned on for IPv6
> > which none of our drivers currently supports.
>
> jan@ pointed out that reordering TSO in ip6 forward breaks path MTU
> discovery. So lets only fix the forward counters, icmp6 packet too
> big and icmp6 redirect.
>
> First try to send with TSO. The goto senderr handles icmp6 redirect
> and other errors.
>
> If TSO is not necessary and the interface MTU fits, just send the
> packet. Again goto senderr handles icmp6.
>
> Finally care about icmp6 packet too big.
Works fine in my setup.
> ok?
ok jan@
> Index: netinet6/ip6_forward.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_forward.c,v
> retrieving revision 1.110
> diff -u -p -r1.110 ip6_forward.c
> --- netinet6/ip6_forward.c 1 Jun 2023 09:05:33 -0000 1.110
> +++ netinet6/ip6_forward.c 16 Jun 2023 08:55:43 -0000
> @@ -321,35 +321,30 @@ reroute:
>
> error = tcp_if_output_tso(ifp, &m, sin6tosa(sin6), rt, IFCAP_TSOv6,
> ifp->if_mtu);
> + if (error)
> + ip6stat_inc(ip6s_cantforward);
> + else if (m == NULL)
> + ip6stat_inc(ip6s_forward);
> if (error || m == NULL)
> - goto freecopy;
> + goto senderr;
>
> /* Check the size after pf_test to give pf a chance to refragment. */
> - if (m->m_pkthdr.len > ifp->if_mtu) {
> - if (mcopy)
> - icmp6_error(mcopy, ICMP6_PACKET_TOO_BIG, 0,
> - ifp->if_mtu);
> - m_freem(m);
> - goto out;
> + if (m->m_pkthdr.len <= ifp->if_mtu) {
> + in6_proto_cksum_out(m, ifp);
> + error = ifp->if_output(ifp, m, sin6tosa(sin6), rt);
> + if (error)
> + ip6stat_inc(ip6s_cantforward);
> + else
> + ip6stat_inc(ip6s_forward);
> + goto senderr;
> }
>
> - in6_proto_cksum_out(m, ifp);
> - error = ifp->if_output(ifp, m, sin6tosa(sin6), rt);
> - if (error) {
> - ip6stat_inc(ip6s_cantforward);
> - } else {
> - ip6stat_inc(ip6s_forward);
> - if (type)
> - ip6stat_inc(ip6s_redirectsent);
> - else {
> - if (mcopy)
> - goto freecopy;
> - }
> - }
> + if (mcopy != NULL)
> + icmp6_error(mcopy, ICMP6_PACKET_TOO_BIG, 0, ifp->if_mtu);
> + m_freem(m);
> + goto out;
>
> -#if NPF > 0 || defined(IPSEC)
> senderr:
> -#endif
> if (mcopy == NULL)
> goto out;
>
> @@ -357,6 +352,7 @@ senderr:
> case 0:
> if (type == ND_REDIRECT) {
> icmp6_redirect_output(mcopy, rt);
> + ip6stat_inc(ip6s_redirectsent);
> goto out;
> }
> goto freecopy;
>
