Index | Thread | Search

From:
Jan Klemkow <jan@openbsd.org>
Subject:
Re: names for TSO and LRO
To:
tech@openbsd.org, Alexander Bluhm <bluhm@openbsd.org>
Date:
Mon, 21 Apr 2025 07:05:15 +0200

Download raw body.

Thread
On April 20, 2025 10:34:47 PM GMT+02:00, Alexander Bluhm <bluhm@openbsd.org> 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 {
>