On Thu, Jul 06, 2023 at 08:49:03PM +0200, Jan Klemkow wrote:
> > @@ -109,6 +109,9 @@
> >  #include <netinet/tcp.h>
> >  #include <netinet/tcp_timer.h>
> >  #include <netinet/tcp_var.h>
> 
> I think is a merge bug, isn't it?
> 
> > +#include <netinet/tcp.h>
> > +#include <netinet/tcp_timer.h>
> > +#include <netinet/tcp_var.h>

Right.

> > +   error = tcp_if_output_tso(ifp, mp, dst, rt, ifcap, mtu);
> > +   if (error || *mp == NULL)
> > +           return error;
> > +
> > +   if ((*mp)->m_pkthdr.len <= mtu) {
> 
> I may miss something but...
> 
> Couldn't you move the *_cksum_out() calls above the upper
> tcp_if_output_tso() call?  And than remove the *_cksum_out() calls
> inside of tcp_if_output_tso()?
> 
> Thus, there is just one place where we call them.
> 
> > +           switch (dst->sa_family) {
> > +           case AF_INET:
> > +                   in_hdr_cksum_out(*mp, ifp);
> > +                   in_proto_cksum_out(*mp, ifp);
> > +                   break;
> > +#ifdef INET6
> > +           case AF_INET6:
> > +                   in6_proto_cksum_out(*mp, ifp);
> > +                   break;
> > +#endif

There is the case in tcp_if_output_tso() where we call tcp_chopper().
Then checksum has to be calcualted after chopping.  If I do it
always before tcp_if_output_tso(), we may caluclate it twice.  Once
for the large packet and once for the small ones.

New diff without duplicate includes.

bluhm

Index: net/if.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/if.c,v
retrieving revision 1.704
diff -u -p -r1.704 if.c
--- net/if.c    6 Jul 2023 04:55:04 -0000       1.704
+++ net/if.c    6 Jul 2023 19:15:00 -0000
@@ -886,6 +886,57 @@ if_output_ml(struct ifnet *ifp, struct m
 }
 
 int
+if_output_tso(struct ifnet *ifp, struct mbuf **mp, struct sockaddr *dst,
+    struct rtentry *rt, u_int mtu)
+{
+       uint32_t ifcap;
+       int error;
+
+       switch (dst->sa_family) {
+       case AF_INET:
+               ifcap = IFCAP_TSOv4;
+               break;
+#ifdef INET6
+       case AF_INET6:
+               ifcap = IFCAP_TSOv6;
+               break;
+#endif
+       default:
+               unhandled_af(dst->sa_family);
+       }
+
+       /*
+        * Try to send with TSO first.  When forwarding LRO may set
+        * maximium segment size in mbuf header.  Chop TCP segment
+        * even if it would fit interface MTU to preserve maximum
+        * path MTU.
+        */
+       error = tcp_if_output_tso(ifp, mp, dst, rt, ifcap, mtu);
+       if (error || *mp == NULL)
+               return error;
+
+       if ((*mp)->m_pkthdr.len <= mtu) {
+               switch (dst->sa_family) {
+               case AF_INET:
+                       in_hdr_cksum_out(*mp, ifp);
+                       in_proto_cksum_out(*mp, ifp);
+                       break;
+#ifdef INET6
+               case AF_INET6:
+                       in6_proto_cksum_out(*mp, ifp);
+                       break;
+#endif
+               }
+               error = ifp->if_output(ifp, *mp, dst, rt);
+               *mp = NULL;
+               return error;
+       }
+
+       /* mp still contains mbuf that has to be fragmented or dropped. */
+       return 0;
+}
+
+int
 if_output_mq(struct ifnet *ifp, struct mbuf_queue *mq, unsigned int *total,
     struct sockaddr *dst, struct rtentry *rt)
 {
Index: net/if_var.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/if_var.h,v
retrieving revision 1.128
diff -u -p -r1.128 if_var.h
--- net/if_var.h        28 Jun 2023 11:49:49 -0000      1.128
+++ net/if_var.h        6 Jul 2023 19:12:39 -0000
@@ -329,6 +329,8 @@ int if_output_ml(struct ifnet *, struct 
            struct sockaddr *, struct rtentry *);
 int    if_output_mq(struct ifnet *, struct mbuf_queue *, unsigned int *,
            struct sockaddr *, struct rtentry *);
+int    if_output_tso(struct ifnet *, struct mbuf **, struct sockaddr *,
+           struct rtentry *, u_int);
 int    if_output_local(struct ifnet *, struct mbuf *, sa_family_t);
 void   if_rtrequest_dummy(struct ifnet *, int, struct rtentry *);
 void   p2p_rtrequest(struct ifnet *, int, struct rtentry *);
Index: net/pf.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
retrieving revision 1.1182
diff -u -p -r1.1182 pf.c
--- net/pf.c    6 Jul 2023 04:55:05 -0000       1.1182
+++ net/pf.c    6 Jul 2023 19:12:39 -0000
@@ -6610,15 +6610,8 @@ pf_route(struct pf_pdesc *pd, struct pf_
                ip = mtod(m0, struct ip *);
        }
 
-       if (ntohs(ip->ip_len) <= ifp->if_mtu) {
-               in_hdr_cksum_out(m0, ifp);
-               in_proto_cksum_out(m0, ifp);
-               ifp->if_output(ifp, m0, sintosa(dst), rt);
-               goto done;
-       }
-
-       if (tcp_if_output_tso(ifp, &m0, sintosa(dst), rt,
-           IFCAP_TSOv4, ifp->if_mtu) || m0 == NULL)
+       if (if_output_tso(ifp, &m0, sintosa(dst), rt, ifp->if_mtu) ||
+           m0 == NULL)
                goto done;
 
        /*
@@ -6745,14 +6738,8 @@ pf_route6(struct pf_pdesc *pd, struct pf
                goto done;
        }
 
-       if (m0->m_pkthdr.len <= ifp->if_mtu) {
-               in6_proto_cksum_out(m0, ifp);
-               ifp->if_output(ifp, m0, sin6tosa(dst), rt);
-               goto done;
-       }
-
-       if (tcp_if_output_tso(ifp, &m0, sin6tosa(dst), rt,
-           IFCAP_TSOv6, ifp->if_mtu) || m0 == NULL)
+       if (if_output_tso(ifp, &m0, sin6tosa(dst), rt, ifp->if_mtu) ||
+           m0 == NULL)
                goto done;
 
        ip6stat_inc(ip6s_cantfrag);
Index: netinet/ip_output.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_output.c,v
retrieving revision 1.389
diff -u -p -r1.389 ip_output.c
--- netinet/ip_output.c 4 Jul 2023 10:48:19 -0000       1.389
+++ netinet/ip_output.c 6 Jul 2023 19:12:39 -0000
@@ -451,17 +451,9 @@ sendit:
 #endif
 
        /*
-        * If small enough for interface, can just send directly.
+        * If TSO or small enough for interface, can just send directly.
         */
-       if (ntohs(ip->ip_len) <= mtu) {
-               in_hdr_cksum_out(m, ifp);
-               in_proto_cksum_out(m, ifp);
-               error = ifp->if_output(ifp, m, sintosa(dst), ro->ro_rt);
-               goto done;
-       }
-
-       error = tcp_if_output_tso(ifp, &m, sintosa(dst), ro->ro_rt,
-           IFCAP_TSOv4, mtu);
+       error = if_output_tso(ifp, &m, sintosa(dst), ro->ro_rt, mtu);
        if (error || m == NULL)
                goto done;
 
Index: netinet6/ip6_forward.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_forward.c,v
retrieving revision 1.111
diff -u -p -r1.111 ip6_forward.c
--- netinet6/ip6_forward.c      16 Jun 2023 19:18:56 -0000      1.111
+++ netinet6/ip6_forward.c      6 Jul 2023 19:12:39 -0000
@@ -319,25 +319,13 @@ reroute:
        }
 #endif
 
-       error = tcp_if_output_tso(ifp, &m, sin6tosa(sin6), rt, IFCAP_TSOv6,
-           ifp->if_mtu);
+       error = if_output_tso(ifp, &m, sin6tosa(sin6), rt, ifp->if_mtu);
        if (error)
                ip6stat_inc(ip6s_cantforward);
        else if (m == NULL)
                ip6stat_inc(ip6s_forward);
        if (error || m == NULL)
                goto senderr;
-
-       /* Check the size after pf_test to give pf a chance to refragment. */
-       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;
-       }
 
        if (mcopy != NULL)
                icmp6_error(mcopy, ICMP6_PACKET_TOO_BIG, 0, ifp->if_mtu);
Index: netinet6/ip6_output.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_output.c,v
retrieving revision 1.278
diff -u -p -r1.278 ip6_output.c
--- netinet6/ip6_output.c       13 Jun 2023 19:34:12 -0000      1.278
+++ netinet6/ip6_output.c       6 Jul 2023 19:15:00 -0000
@@ -677,7 +677,8 @@ reroute:
         * 2-a: send as is if tlen <= interface mtu
         * 2-b: error if tlen > interface mtu
         */
-       tlen = m->m_pkthdr.len;
+       tlen = ISSET(m->m_pkthdr.csum_flags, M_TCP_TSO) ?
+           m->m_pkthdr.ph_mss : m->m_pkthdr.len;
 
        if (ISSET(m->m_pkthdr.csum_flags, M_IPV6_DF_OUT)) {
                CLR(m->m_pkthdr.csum_flags, M_IPV6_DF_OUT);
@@ -686,9 +687,8 @@ reroute:
                dontfrag = 1;
        else
                dontfrag = 0;
-       if (dontfrag &&                                 /* case 2-b */
-           (ISSET(m->m_pkthdr.csum_flags, M_TCP_TSO) ?
-           m->m_pkthdr.ph_mss : tlen) > ifp->if_mtu) {
+
+       if (dontfrag && tlen > ifp->if_mtu) {           /* case 2-b */
 #ifdef IPSEC
                if (ip_mtudisc)
                        ipsec_adjust_mtu(m, mtu);
@@ -701,15 +701,12 @@ reroute:
         * transmit packet without fragmentation
         */
        if (dontfrag || tlen <= mtu) {                  /* case 1-a and 2-a */
-               in6_proto_cksum_out(m, ifp);
-               error = ifp->if_output(ifp, m, sin6tosa(dst), ro->ro_rt);
-               goto done;
+               error = if_output_tso(ifp, &m, sin6tosa(dst), ro->ro_rt,
+                   ifp->if_mtu);
+               if (error || m == NULL)
+                       goto done;
+               goto bad;                               /* should not happen */
        }
-
-       error = tcp_if_output_tso(ifp, &m, sin6tosa(dst), ro->ro_rt,
-           IFCAP_TSOv6, mtu);
-       if (error || m == NULL)
-               goto done;
 
        /*
         * try to fragment the packet.  case 1-b

Reply via email to