Download raw body.
igc(4) tso
On Tue, Jan 09, 2024 at 11:22:37PM +0100, Moritz Buhl wrote:
> Here is a diff that implements tso for igc(4).
> It looks very similar to the other intel cards but it was missing
> VLAN_HWTAGGING.
I suggest to separate the vlan tagging part and commit it upfront of the
tso part. Thus, your diff is shorter and we could better identify
regressions in one of it.
> I am not too happy how olinfo_status is initialized now so I am
> open to suggestions.
Some comments below.
bye,
Jan
> I tested the diff on I225-V and I225-LM in various configurations.
> More testing welcome.
>
> mbuhl
>
> Index: dev/pci//if_igc.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/if_igc.c,v
> diff -u -p -r1.14 if_igc.c
> --- dev/pci//if_igc.c 10 Nov 2023 15:51:20 -0000 1.14
> +++ dev/pci//if_igc.c 9 Jan 2024 18:46:50 -0000
> @@ -44,12 +44,16 @@
>
> #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/ip.h>
> #include <netinet/ip6.h>
> +#include <netinet/tcp.h>
> +#include <netinet/tcp_timer.h>
> +#include <netinet/tcp_var.h>
>
> #if NBPFILTER > 0
> #include <net/bpf.h>
> @@ -117,7 +121,8 @@ int igc_media_change(struct ifnet *);
> void igc_iff(struct igc_softc *);
> void igc_update_link_status(struct igc_softc *);
> int igc_get_buf(struct rx_ring *, int);
> -int igc_tx_ctx_setup(struct tx_ring *, struct mbuf *, int, uint32_t *);
> +int igc_tx_ctx_setup(struct tx_ring *, struct mbuf *, int, uint32_t *,
> + uint32_t *);
>
> void igc_configure_queues(struct igc_softc *);
> void igc_set_queues(struct igc_softc *, uint32_t, uint32_t, int);
> @@ -790,15 +795,14 @@ igc_setup_interface(struct igc_softc *sc
>
> ifp->if_capabilities = IFCAP_VLAN_MTU;
>
> -#ifdef notyet
> #if NVLAN > 0
> ifp->if_capabilities |= IFCAP_VLAN_HWTAGGING;
> #endif
> -#endif
>
> 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);
> @@ -954,7 +958,8 @@ igc_start(struct ifqueue *ifq)
> struct mbuf *m;
> unsigned int prod, free, last, i;
> unsigned int mask;
> - uint32_t cmd_type_len;
> + uint32_t cmd_type_len = IGC_ADVTXD_DCMD_IFCS | IGC_ADVTXD_DTYP_DATA |
> + IGC_ADVTXD_DCMD_DEXT;
> uint32_t olinfo_status;
> int post = 0;
> #if NBPFILTER > 0
> @@ -996,29 +1001,31 @@ 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);
>
> - if (igc_tx_ctx_setup(txr, m, prod, &olinfo_status)) {
> + if (ISSET(m->m_flags, M_VLANTAG))
> + cmd_type_len |= IGC_ADVTXD_DCMD_VLE;
> +
> + if (igc_tx_ctx_setup(txr, m, prod, &cmd_type_len,
> + &olinfo_status)) {
> /* Consume the first descriptor */
> prod++;
> prod &= mask;
> free--;
> + } else {
> + olinfo_status =
> + m->m_pkthdr.len << IGC_ADVTXD_PAYLEN_SHIFT;
> }
>
> for (i = 0; i < map->dm_nsegs; i++) {
> txdesc = &txr->tx_base[prod];
>
> - cmd_type_len = IGC_ADVTXD_DCMD_IFCS | IGC_ADVTXD_DTYP_DATA |
> - IGC_ADVTXD_DCMD_DEXT | map->dm_segs[i].ds_len;
> - if (i == map->dm_nsegs - 1)
> - cmd_type_len |= IGC_ADVTXD_DCMD_EOP |
> - IGC_ADVTXD_DCMD_RS;
> -
> - htolem64(&txdesc->read.buffer_addr, map->dm_segs[i].ds_addr);
> - htolem32(&txdesc->read.cmd_type_len, cmd_type_len);
> + htolem64(&txdesc->read.buffer_addr,
> + map->dm_segs[i].ds_addr);
> + htolem32(&txdesc->read.cmd_type_len, cmd_type_len |
> + map->dm_segs[i].ds_len | ((i == map->dm_nsegs - 1)?
> + IGC_ADVTXD_DCMD_EOP | IGC_ADVTXD_DCMD_RS : 0));
> htolem32(&txdesc->read.olinfo_status, olinfo_status);
>
> last = prod;
> @@ -1999,23 +2006,26 @@ igc_free_transmit_buffers(struct tx_ring
>
> int
> igc_tx_ctx_setup(struct tx_ring *txr, struct mbuf *mp, int prod,
> - uint32_t *olinfo_status)
> + uint32_t *cmd_type_len, uint32_t *olinfo_status)
> {
> 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;
> - uint32_t iphlen;
> + uint32_t ethlen, iphlen;
> int off = 0;
>
> - vlan_macip_lens |= (sizeof(*ext.eh) << IGC_ADVTXD_MACLEN_SHIFT);
Please call ether_extract_headers() before this line above. It looks
super wired, if you use a struct member before initialization. Also,
if its save for sizeof.
Btw. we should avoid calling ether_extract_headers() and the rest of
this function, when no offloading flags are used in this mbuf.
> + if (mp->m_pkthdr.csum_flags & M_TCP_TSO)
> + *olinfo_status = 0;
> + else
Remove this part and use CLR() below to override this value, like its
done in ix(4).
> + *olinfo_status = mp->m_pkthdr.len << IGC_ADVTXD_PAYLEN_SHIFT;
>
> /*
> * In advanced descriptors the vlan tag must
> * be placed into the context descriptor. Hence
> * we need to make one even if not doing offloads.
> */
> -#ifdef notyet
> #if NVLAN > 0
> if (ISSET(mp->m_flags, M_VLANTAG)) {
> uint32_t vtag = mp->m_pkthdr.ether_vtag;
> @@ -2023,8 +2033,9 @@ igc_tx_ctx_setup(struct tx_ring *txr, st
> off = 1;
> }
> #endif
> -#endif
>
> + ethlen = sizeof(*ext.eh);
> + vlan_macip_lens |= (ethlen << IGC_ADVTXD_MACLEN_SHIFT);
> ether_extract_headers(mp, &ext);
>
> if (ext.ip4) {
> @@ -2042,6 +2053,8 @@ igc_tx_ctx_setup(struct tx_ring *txr, st
> type_tucmd_mlhl |= IGC_ADVTXD_TUCMD_IPV6;
> #endif
> } else {
> + if (mp->m_pkthdr.csum_flags & M_TCP_TSO)
> + tcpstat_inc(tcps_outbadtso);
> return 0;
> }
>
> @@ -2065,6 +2078,29 @@ igc_tx_ctx_setup(struct tx_ring *txr, st
> if (off == 0)
> return 0;
>
> + if (mp->m_pkthdr.csum_flags & M_TCP_TSO) {
> + if (ext.tcp) {
> + uint32_t hdrlen, thlen, paylen, outlen;
> +
> + thlen = ext.tcp->th_off << 2;
> +
> + outlen = mp->m_pkthdr.ph_mss;
> + mss_l4len_idx |= outlen << IGC_ADVTXD_MSS_SHIFT;
> + mss_l4len_idx |= thlen << IGC_ADVTXD_L4LEN_SHIFT;
> +
> + hdrlen = ethlen + iphlen + thlen;
> + paylen = mp->m_pkthdr.len - hdrlen;
> + *olinfo_status |= paylen << IGC_ADVTXD_PAYLEN_SHIFT;
Do here some like this in ix(4):
CLR(*olinfo_status, IXGBE_ADVTXD_PAYLEN_MASK
<< IXGBE_ADVTXD_PAYLEN_SHIFT);
*olinfo_status |= paylen << IXGBE_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);
> + }
> +
> /* Now ready a context descriptor */
> txdesc = (struct igc_adv_tx_context_desc *)&txr->tx_base[prod];
>
> @@ -2072,7 +2108,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;
> }
>
igc(4) tso