Index | Thread | Search

From:
Bjorn Ketelaars <bket@openbsd.org>
Subject:
Re: igc(4) tso
To:
Moritz Buhl <mbuhl@openbsd.org>
Cc:
tech@openbsd.org, j.klemkow@wemelug.de
Date:
Fri, 1 Mar 2024 07:42:59 +0100

Download raw body.

Thread
  • Moritz Buhl:

    igc(4) tso

    • Jan Klemkow:

      igc(4) tso

    • Bjorn Ketelaars:

      igc(4) tso

On Tue 09/01/2024 23:22, 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 am not too happy how olinfo_status is initialized now so I am
> open to suggestions.
> 
> I tested the diff on I225-V and I225-LM in various configurations.
> More testing welcome.

As privately discussed, I rebased your diff on top of recent changes to
igc(4). Initially the interface locked up after about an hour, which was
fixed by copying a recent change to ix(4), em(4), and others (from
bluhm@ [0]). With this I have been running your diff for the last couple
of days without any issues.

$ dmesg | grep igc0
igc0 at pci2 dev 0 function 0 "Intel I225-V" rev 0x03, msix, 4 queues, address 00:e2:69:5b:a6:aa

$ ifconfig igc0 hwfeatures
igc0: flags=8a43<UP,BROADCAST,RUNNING,ALLMULTI,SIMPLEX,MULTICAST> mtu 1500
        hwfeatures=31b7<CSUM_IPv4,CSUM_TCPv4,CSUM_UDPv4,VLAN_MTU,VLAN_HWTAGGING,CSUM_TCPv6,CSUM_UDPv6,TSOv4,TSOv6> hardmtu 9216
        lladdr 00:e2:69:5b:a6:aa
        index 1 priority 0 llprio 3
        media: Ethernet autoselect (2500baseT full-duplex)
        status: active

I refactored your diff to make it look more similar to the approach in
ix(4), and split it up in 1.) a vlan hwtagging part and 2.) a tso part,
as has been suggested by jan@.

Included with this mail is the hwtagging part, which has been tested
successfully on a system with multiple igc(4) interfaces. I will send
the tso part later. BTW, the issue with locking that has been resolved
by picking a change from [0] is in the tso part.


[0] https://codeberg.org/OpenBSD/src/commit/e78a66e5775d3e9cf59b16f193be327ae6d56426


diff --git sys/dev/pci/if_igc.c sys/dev/pci/if_igc.c
index b88644ade21..159051d8124 100644
--- sys/dev/pci/if_igc.c
+++ sys/dev/pci/if_igc.c
@@ -117,7 +117,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,10 +791,8 @@ 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;
@@ -1001,7 +1000,11 @@ igc_start(struct ifqueue *ifq)
 		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)) {
+		cmd_type_len = IGC_ADVTXD_DCMD_IFCS | IGC_ADVTXD_DTYP_DATA |
+		    IGC_ADVTXD_DCMD_DEXT;
+
+		if (igc_tx_ctx_setup(txr, m, prod, &cmd_type_len,
+		    &olinfo_status)) {
 			/* Consume the first descriptor */
 			prod++;
 			prod &= mask;
@@ -1011,14 +1014,11 @@ igc_start(struct ifqueue *ifq)
 		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;
@@ -2019,7 +2019,7 @@ igc_free_transmit_buffers(struct tx_ring *txr)
 
 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;
@@ -2027,6 +2027,7 @@ igc_tx_ctx_setup(struct tx_ring *txr, struct mbuf *mp, int prod,
 	uint32_t vlan_macip_lens = 0;
 	int off = 0;
 
+	ether_extract_headers(mp, &ext);
 	vlan_macip_lens |= (sizeof(*ext.eh) << IGC_ADVTXD_MACLEN_SHIFT);
 
 	/*
@@ -2034,18 +2035,15 @@ igc_tx_ctx_setup(struct tx_ring *txr, struct mbuf *mp, int prod,
 	 * 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;
 		vlan_macip_lens |= (vtag << IGC_ADVTXD_VLAN_SHIFT);
+		*cmd_type_len |= IGC_ADVTXD_DCMD_VLE;
 		off = 1;
 	}
-#endif
 #endif
 
-	ether_extract_headers(mp, &ext);
-
 	if (ext.ip4) {
 		type_tucmd_mlhl |= IGC_ADVTXD_TUCMD_IPV4;
 		if (ISSET(mp->m_pkthdr.csum_flags, M_IPV4_CSUM_OUT)) {
@@ -2056,8 +2054,6 @@ igc_tx_ctx_setup(struct tx_ring *txr, struct mbuf *mp, int prod,
 	} else if (ext.ip6) {
 		type_tucmd_mlhl |= IGC_ADVTXD_TUCMD_IPV6;
 #endif
-	} else {
-		return 0;
 	}
 
 	vlan_macip_lens |= ext.iphlen;