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.

ok?

bluhm

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;

Reply via email to