Index | Thread | Search

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

Download raw body.

Thread
  • Alexander Bluhm:

    igc(4) tso

  • 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).
    
    I did test it with more options.  With jumbo frames I see this crash.
    Next I will test igc with jumbos, but without diff.
    
    panic: kernel diagnostic assertion "M_DATABUF(m) + M_SIZE(m) >= (m->m_data + m->m_len)" failed: file "/usr/src/sys/kern/uipc_mbuf.c", line 1364
    Stopped at      db_enter+0x14:  popq    %rbp
        TID    PID    UID     PRFLAGS     PFLAGS  CPU  COMMAND
     487658  86452   1000  0x18100002          0    3  tcpbench
     258955  92782      0     0x14000      0x200    1  softnet2
    *238262  73418      0     0x14000      0x200    2  softnet0
    db_enter() at db_enter+0x14
    panic(ffffffff822047c4) at panic+0xc3
    __assert(ffffffff821bfc8a,ffffffff8214da2f,554,ffffffff821999f7) at __assert+0x29
    m_trailingspace(fffffd80bb7f6500) at m_trailingspace+0xab
    sbcompress(ffffffff82533bc0,fffffd9b1007a0c0,fffffd80b4428f00,fffffd80bb7f6500) at sbcompress+0x102
    sbappendstream(fffffd9b1007a020,fffffd9b1007a0c0,fffffd80b4428f00) at sbappendstream+0x5a
    tcp_input(ffff80005c0098c0,ffff80005c0098cc,6,2) at tcp_input+0x2e5d
    ip_deliver(ffff80005c0098c0,ffff80005c0098cc,6,2,0) at ip_deliver+0xf8
    ipintr() at ipintr+0xae
    if_netisr(0) at if_netisr+0xc0
    taskq_thread(ffff80000002f000) at taskq_thread+0x100
    end trace frame: 0x0, count: 4
    https://www.openbsd.org/ddb.html describes the minimum info required in bug
    reports.  Insufficient info makes it difficult to find and fix bugs.
    
    ddb{2}> show panic
    *cpu2: kernel diagnostic assertion "M_DATABUF(m) + M_SIZE(m) >= (m->m_data + m->m_len)" failed: file "/usr/src/sys/kern/uipc_mbuf.c", line 1364
    
    ddb{2}> show register
    rdi                                0
    rsi                             0x10
    rbp               0xffff80005c0094b0
    rbx                              0x4
    rdx               0xfe000000000000ff
    rcx                            0x202
    rax                             0x90
    r8                 0x101010101010101
    r9                                 0
    r10                 0x15237e8f9fbfea
    r11               0xc5ba19ba5c94ed07
    r12               0xffff80005a5f3b78
    r13                                0
    r14                                0
    r15               0xffffffff822047c4    substchar+0x4ca68
    rip               0xffffffff81799754    db_enter+0x14
    cs                               0x8
    rflags                         0x282
    rsp               0xffff80005c0094b0
    ss                              0x10
    db_enter+0x14:  popq    %rbp
    
    > 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;
    >  		}
    >  
    >  		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;
    >  }
    
    
    
  • Alexander Bluhm:

    igc(4) tso