From: Jan Klemkow Subject: Re: names for TSO and LRO To: tech@openbsd.org, Alexander Bluhm Date: Mon, 21 Apr 2025 07:05:15 +0200 On April 20, 2025 10:34:47 PM GMT+02:00, Alexander Bluhm wrote: > On Thu, Apr 17, 2025 at 03:25:46PM +0200, Jan Klemkow wrote: > > On Wed, Apr 16, 2025 at 11:59:43PM +0200, Jan Klemkow wrote: > > > On Wed, Apr 16, 2025 at 11:39:21PM GMT, Alexander Bluhm wrote: > > > > Now that we have software LRO, we need consistent naming with TSO. > > > > One is called tcp_softlro and the other tcp_copper. As claudio@ > > > > suggested the descriptive name "copper", I would like to have it > > > > this way for LRO. tcp_glue_enqueue() came to my mind. > > > > > > In my opinion we should not make up new names for already established > > > things. LRO und TSO are established names [1]. If you google "tcp > > > glue" you will very misleading results. Looks like its a product name > > > for some glue. Also "tcp copper" won't get better search results. > > > > > > Thus, I suggest to rename tcp_copper() to tcp_softtso() to get things > > > consistent here. Or, tcp_tso() and tcp_lro() to keep it shorter. > > > > > > [1]: https://en.wikipedia.org/wiki/TCP_offload_engine > > > > As explained in me message before. I suggest this diff to > > make this naming consistent. > > Naming is hard. > > Can we keep the suffix _chop and _glue? Then the function name > says what it is doing. Also use the same order for the parameter. > > ok? ok jan@ > bluhm > > > Index: dev/pci/if_ixl.c > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/dev/pci/if_ixl.c,v > diff -u -p -r1.104 if_ixl.c > --- dev/pci/if_ixl.c 16 Apr 2025 17:17:06 -0000 1.104 > +++ dev/pci/if_ixl.c 20 Apr 2025 20:28:47 -0000 > @@ -3342,7 +3342,7 @@ ixl_rxeof(struct ixl_softc *sc, struct i > if (ISSET(ifp->if_xflags, IFXF_LRO) && > (ptype == IXL_RX_DESC_PTYPE_MAC_IPV4_TCP || > ptype == IXL_RX_DESC_PTYPE_MAC_IPV6_TCP)) > - tcp_softlro_enqueue(ifp, &mltcp, m); > + tcp_softlro_glue(&mltcp, m, ifp); > else > ml_enqueue(&ml, m); > } else { > Index: netinet/ip_output.c > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_output.c,v > diff -u -p -r1.407 ip_output.c > --- netinet/ip_output.c 12 Mar 2025 01:44:27 -0000 1.407 > +++ netinet/ip_output.c 20 Apr 2025 20:26:34 -0000 > @@ -634,7 +634,7 @@ ip_output_ipsec_send(struct tdb *tdb, st > m->m_flags &= ~(M_MCAST | M_BCAST); > > if (tso) { > - error = tcp_chopper(m, &ml, encif, len); > + error = tcp_softtso_chop(&ml, m, encif, len); > if (error) > goto done; > } else { > Index: netinet/tcp_input.c > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_input.c,v > diff -u -p -r1.436 tcp_input.c > --- netinet/tcp_input.c 16 Apr 2025 17:17:06 -0000 1.436 > +++ netinet/tcp_input.c 20 Apr 2025 20:26:56 -0000 > @@ -4452,7 +4452,7 @@ tcp_softlro(struct mbuf *mhead, struct m > } > > void > -tcp_softlro_enqueue(struct ifnet *ifp, struct mbuf_list *ml, struct mbuf *mtail) > +tcp_softlro_glue(struct mbuf_list *ml, struct mbuf *mtail, struct ifnet *ifp) > { > struct mbuf *mhead; > > Index: netinet/tcp_output.c > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_output.c,v > diff -u -p -r1.153 tcp_output.c > --- netinet/tcp_output.c 17 Feb 2025 12:46:02 -0000 1.153 > +++ netinet/tcp_output.c 20 Apr 2025 20:27:49 -0000 > @@ -1199,7 +1199,7 @@ tcp_setpersist(struct tcpcb *tp) > } > > int > -tcp_chopper(struct mbuf *m0, struct mbuf_list *ml, struct ifnet *ifp, > +tcp_softtso_chop(struct mbuf_list *ml, struct mbuf *m0, struct ifnet *ifp, > u_int mss) > { > struct ip *ip = NULL; > @@ -1394,7 +1394,7 @@ tcp_if_output_tso(struct ifnet *ifp, str > } > > /* as fallback do TSO in software */ > - if ((error = tcp_chopper(*mp, &ml, ifp, (*mp)->m_pkthdr.ph_mss)) || > + if ((error = tcp_softtso_chop(&ml, *mp, ifp, (*mp)->m_pkthdr.ph_mss)) || > (error = if_output_ml(ifp, &ml, dst, rt))) > goto done; > tcpstat_inc(tcps_outswtso); > Index: netinet/tcp_var.h > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_var.h,v > diff -u -p -r1.187 tcp_var.h > --- netinet/tcp_var.h 16 Apr 2025 17:17:06 -0000 1.187 > +++ netinet/tcp_var.h 20 Apr 2025 20:28:29 -0000 > @@ -720,7 +720,7 @@ void tcp_init(void); > int tcp_input(struct mbuf **, int *, int, int, struct netstack *); > int tcp_mss(struct tcpcb *, int); > void tcp_mss_update(struct tcpcb *); > -void tcp_softlro_enqueue(struct ifnet *, struct mbuf_list *, struct mbuf *); > +void tcp_softlro_glue(struct mbuf_list *, struct mbuf *, struct ifnet *); > u_int tcp_hdrsz(struct tcpcb *); > void tcp_mtudisc(struct inpcb *, int); > void tcp_mtudisc_increase(struct inpcb *, int); > @@ -731,7 +731,8 @@ struct tcpcb * > tcp_newtcpcb(struct inpcb *, int); > void tcp_notify(struct inpcb *, int); > int tcp_output(struct tcpcb *); > -int tcp_chopper(struct mbuf *, struct mbuf_list *, struct ifnet *, u_int); > +int tcp_softtso_chop(struct mbuf_list *, struct mbuf *, struct ifnet *, > + u_int); > int tcp_if_output_tso(struct ifnet *, struct mbuf **, struct sockaddr *, > struct rtentry *, uint32_t, u_int); > void tcp_pulloutofband(struct socket *, u_int, struct mbuf *, int); > Index: netinet6/ip6_output.c > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_output.c,v > diff -u -p -r1.297 ip6_output.c > --- netinet6/ip6_output.c 2 Mar 2025 21:28:32 -0000 1.297 > +++ netinet6/ip6_output.c 20 Apr 2025 20:28:37 -0000 > @@ -2899,7 +2899,7 @@ ip6_output_ipsec_send(struct tdb *tdb, s > m->m_flags &= ~(M_BCAST | M_MCAST); > > if (tso) { > - error = tcp_chopper(m, &ml, encif, len); > + error = tcp_softtso_chop(&ml, m, encif, len); > if (error) > goto done; > } else { >