From: jan@openbsd.org
Subject: Re: igc(4) tso
To: Moritz Buhl <mbuhl@openbsd.org>
Cc: tech@openbsd.org, bket@openbsd.org, bluhm@openbsd.org
Date: Thu, 2 May 2024 10:08:39 +0200

On Wed, May 01, 2024 at 06:24:36PM +0200, Moritz Buhl wrote:
> The VLAN_HWTAGGING diff is in snapshots for a while, I saw no
> additional feedback.  How about adding TSO to igc(4) next?
> 
> I ran thorough tests on bluhms' setup:
> http://bluhm.genua.de/netlink/results/2024-04-30T00%3A37%3A41Z/netlink.html
> and additional tests on my home router using pppoe(4) and vlan(4).
> 
> ok?

comment below.

> Index: dev/pci/if_igc.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/if_igc.c,v
> diff -u -p -r1.20 if_igc.c
> --- dev/pci/if_igc.c	12 Apr 2024 19:27:43 -0000	1.20
> +++ dev/pci/if_igc.c	1 May 2024 16:16:29 -0000
> @@ -44,10 +44,14 @@
>  
>  #include <net/if.h>
>  #include <net/if_media.h>
> +#include <net/route.h>
>  #include <net/toeplitz.h>
>  
>  #include <netinet/in.h>
>  #include <netinet/if_ether.h>
> +#include <netinet/tcp.h>
> +#include <netinet/tcp_timer.h>
> +#include <netinet/tcp_var.h>
>  
>  #if NBPFILTER > 0
>  #include <net/bpf.h>
> @@ -796,6 +800,7 @@ igc_setup_interface(struct igc_softc *sc
>  	ifp->if_capabilities |= IFCAP_CSUM_IPv4;
>  	ifp->if_capabilities |= IFCAP_CSUM_TCPv4 | IFCAP_CSUM_UDPv4;
>  	ifp->if_capabilities |= IFCAP_CSUM_TCPv6 | IFCAP_CSUM_UDPv6;
> +	ifp->if_capabilities |= IFCAP_TSOv4 | IFCAP_TSOv6;
>  
>  	/* Initialize ifmedia structures. */
>  	ifmedia_init(&sc->media, IFM_IMASK, igc_media_change, igc_media_status);
> @@ -993,8 +998,6 @@ igc_start(struct ifqueue *ifq)
>  			continue;
>  		}
>  
> -		olinfo_status = m->m_pkthdr.len << IGC_ADVTXD_PAYLEN_SHIFT;
> -
>  		bus_dmamap_sync(txr->txdma.dma_tag, map, 0,
>  		    map->dm_mapsize, BUS_DMASYNC_PREWRITE);
>  
> @@ -1007,6 +1010,9 @@ igc_start(struct ifqueue *ifq)
>  			prod++;
>  			prod &= mask;
>  			free--;
> +		} else {
> +			olinfo_status =
> +			    m->m_pkthdr.len << IGC_ADVTXD_PAYLEN_SHIFT;

This is useless In my opinion.  igc_tx_ctx_setup() already runs this
code before:

...
	if (mp->m_pkthdr.csum_flags & M_TCP_TSO)
		*olinfo_status = 0;
	else
		*olinfo_status = mp->m_pkthdr.len << IGC_ADVTXD_PAYLEN_SHIFT;
...

>  		}
>  
>  		for (i = 0; i < map->dm_nsegs; i++) {
> @@ -2025,12 +2031,15 @@ igc_tx_ctx_setup(struct tx_ring *txr, st
>  {
>  	struct ether_extracted ext;
>  	struct igc_adv_tx_context_desc *txdesc;
> +	uint32_t mss_l4len_idx = 0;
>  	uint32_t type_tucmd_mlhl = 0;
>  	uint32_t vlan_macip_lens = 0;
>  	int off = 0;
>  
> -	ether_extract_headers(mp, &ext);
> -	vlan_macip_lens |= (sizeof(*ext.eh) << IGC_ADVTXD_MACLEN_SHIFT);
> +	if (mp->m_pkthdr.csum_flags & M_TCP_TSO)
> +		*olinfo_status = 0;
> +	else
> +		*olinfo_status = mp->m_pkthdr.len << IGC_ADVTXD_PAYLEN_SHIFT;
>  
>  	/*
>  	 * In advanced descriptors the vlan tag must
> @@ -2046,6 +2055,10 @@ igc_tx_ctx_setup(struct tx_ring *txr, st
>  	}
>  #endif
>  
> +	ether_extract_headers(mp, &ext);
> +
> +	vlan_macip_lens |= (sizeof(*ext.eh) << IGC_ADVTXD_MACLEN_SHIFT);
> +
>  	if (ext.ip4) {
>  		type_tucmd_mlhl |= IGC_ADVTXD_TUCMD_IPV4;
>  		if (ISSET(mp->m_pkthdr.csum_flags, M_IPV4_CSUM_OUT)) {
> @@ -2056,6 +2069,9 @@ igc_tx_ctx_setup(struct tx_ring *txr, st
>  	} else if (ext.ip6) {
>  		type_tucmd_mlhl |= IGC_ADVTXD_TUCMD_IPV6;
>  #endif
> +	} else {
> +		if (mp->m_pkthdr.csum_flags & M_TCP_TSO)
> +			tcpstat_inc(tcps_outbadtso);
>  	}
>  
>  	vlan_macip_lens |= ext.iphlen;
> @@ -2075,6 +2091,29 @@ igc_tx_ctx_setup(struct tx_ring *txr, st
>  		}
>  	}
>  
> +	if (mp->m_pkthdr.csum_flags & M_TCP_TSO) {
> +		if (ext.tcp) {
> +			uint32_t hdrlen, thlen, paylen, outlen;
> +
> +			thlen = ext.tcphlen;
> +
> +			outlen = mp->m_pkthdr.ph_mss;
> +			mss_l4len_idx |= outlen << IGC_ADVTXD_MSS_SHIFT;
> +			mss_l4len_idx |= thlen << IGC_ADVTXD_L4LEN_SHIFT;
> +
> +			hdrlen = sizeof(*ext.eh) + ext.iphlen + thlen;
> +			paylen = mp->m_pkthdr.len - hdrlen;
> +			*olinfo_status |= paylen << IGC_ADVTXD_PAYLEN_SHIFT;
> +
> +			*cmd_type_len |= IGC_ADVTXD_DCMD_TSE;
> +			off = 1;
> +
> +			tcpstat_add(tcps_outpkttso,
> +			    (paylen + outlen - 1) / outlen);
> +		} else
> +			tcpstat_inc(tcps_outbadtso);
> +	}
> +
>  	if (off == 0)
>  		return 0;
>  
> @@ -2085,7 +2124,7 @@ igc_tx_ctx_setup(struct tx_ring *txr, st
>  	htolem32(&txdesc->vlan_macip_lens, vlan_macip_lens);
>  	htolem32(&txdesc->type_tucmd_mlhl, type_tucmd_mlhl);
>  	htolem32(&txdesc->seqnum_seed, 0);
> -	htolem32(&txdesc->mss_l4len_idx, 0);
> +	htolem32(&txdesc->mss_l4len_idx, mss_l4len_idx);
>  
>  	return 1;
>  }
>