On Wed, Apr 24, 2024 at 11:55:12AM +0200, Stefan Sperling wrote:
> On Sat, Apr 13, 2024 at 02:16:42AM +0200, Uwe Stuehler wrote:
> > Here's an updated diff that checks if the information in rxd->sd_tdes1
> > is valid and then whether (RDES1_IP_HDR_ERROR | RDES1_IP_CSUM_ERROR) is
> > set before marking the packet as hardware checksum failed and that all
> > three bits are clear before it marks the packet as hardware checksummed.
> 
> I would like to start with a simpler change: Let's adjust our macros
> for Rx descriptors. They are currently a confusing mix between the
> read and writeback formats. And they are incomplete for purposes
> such as checksum offloading.
> 
> This patch does not change the driver's behaviour but should help
> with making sense of things going forward.

New diff, built on top of the previous dwqereg.h diff, which
adds checksumming offload support.

Seems to work in my testing, based on debug prints I added locally.
I did not try to trigger the error path yet.

dwqe_rx_csum: UDP payload detected
dwqe_rx_csum: IPv4 checksum OK
dwqe_rx_csum: UDP checksum OK
dwqe_rx_csum: UDP payload detected
dwqe_rx_csum: IPv4 checksum OK
dwqe_rx_csum: UDP checksum OK
dwqe_rx_csum: checksumming engine bypassed
dwqe_rx_csum: UDP payload detected
dwqe_rx_csum: IPv4 checksum OK
dwqe_rx_csum: UDP checksum OK
dwqe_rx_csum: checksumming engine bypassed

 M  sys/dev/ic/dwqe.c     |  67+  0-
 M  sys/dev/ic/dwqereg.h  |   3+  0-

2 files changed, 70 insertions(+), 0 deletions(-)

diff refs/heads/dwqe-rxbits 454ea1e979b249c662541436b74f4168b86c9ab0
commit - d5cd332729e0a5bb8c36ede324cb8340cfa43a8f
commit + 454ea1e979b249c662541436b74f4168b86c9ab0
blob - 0467a4c72472b70f889e8fcdced0660fa6665a8f
blob + d06711527881f5a175d360e1e77b6bf47790dce6
--- sys/dev/ic/dwqe.c
+++ sys/dev/ic/dwqe.c
@@ -641,7 +641,67 @@ dwqe_tx_proc(struct dwqe_softc *sc)
 	}
 }
 
+int
+dwqe_have_rx_csum_offload(struct dwqe_softc *sc)
+{
+	return (sc->sc_hw_feature[0] & GMAC_MAC_HW_FEATURE0_RXCOESEL);
+}
+
 void
+dwqe_rx_csum(struct dwqe_softc *sc, struct mbuf *m, struct dwqe_desc *rxd)
+{
+	uint16_t csum_flags = 0;
+
+	/*
+	 * Checksum offload must be supported, the Last-Descriptor bit
+	 * must be set, RDES1 must be valid, and checksumming must not
+	 * have been bypassed (happens for unknown packet types), and
+	 * an IP header must have been detected.
+	 */
+	if (!dwqe_have_rx_csum_offload(sc) ||
+	    (rxd->sd_tdes3 & RDES3_LD) == 0 ||
+	    (rxd->sd_tdes3 & RDES3_RDES1_VALID) == 0 ||
+	    (rxd->sd_tdes1 & RDES1_IP_CSUM_BYPASS) ||
+	    (rxd->sd_tdes1 & (RDES1_IPV4_HDR | RDES1_IPV6_HDR)) == 0)
+		return;
+
+	/* If the IP header checksum is invalid then the payload is ignored. */
+	if (rxd->sd_tdes1 & RDES1_IP_HDR_ERROR) {
+		if (rxd->sd_tdes1 & RDES1_IPV4_HDR)
+			csum_flags |= M_IPV4_CSUM_IN_BAD;
+	} else {
+		if (rxd->sd_tdes1 & RDES1_IPV4_HDR)
+			csum_flags |= M_IPV4_CSUM_IN_OK;
+
+		/* Detect payload type and corresponding checksum errors. */
+		switch (rxd->sd_tdes1 & RDES1_IP_PAYLOAD_TYPE) {
+		case RDES1_IP_PAYLOAD_UDP:
+			if (rxd->sd_tdes1 & RDES1_IP_PAYLOAD_ERROR)
+				csum_flags |= M_UDP_CSUM_IN_BAD;
+			else
+				csum_flags |= M_UDP_CSUM_IN_OK;
+			break;
+		case RDES1_IP_PAYLOAD_TCP:
+			if (rxd->sd_tdes1 & RDES1_IP_PAYLOAD_ERROR)
+				csum_flags |= M_TCP_CSUM_IN_BAD;
+			else
+				csum_flags |= M_TCP_CSUM_IN_OK;
+			break;
+		case RDES1_IP_PAYLOAD_ICMP:
+			if (rxd->sd_tdes1 & RDES1_IP_PAYLOAD_ERROR)
+				csum_flags |= M_ICMP_CSUM_IN_BAD;
+			else
+				csum_flags |= M_ICMP_CSUM_IN_OK;
+			break;
+		default:
+			break;
+		}
+	}
+
+	m->m_pkthdr.csum_flags |= csum_flags;
+}
+
+void
 dwqe_rx_proc(struct dwqe_softc *sc)
 {
 	struct ifnet *ifp = &sc->sc_ac.ac_if;
@@ -689,6 +749,7 @@ dwqe_rx_proc(struct dwqe_softc *sc)
 
 			m->m_pkthdr.len = m->m_len = len;
 
+			dwqe_rx_csum(sc, m, rxd);
 			ml_enqueue(&ml, m);
 		}
 
@@ -864,6 +925,12 @@ dwqe_up(struct dwqe_softc *sc)
 
 	if (!sc->sc_fixed_link)
 		timeout_add_sec(&sc->sc_phy_tick, 1);
+
+	if (dwqe_have_rx_csum_offload(sc)) {
+		reg = dwqe_read(sc, GMAC_MAC_CONF);
+		reg |= GMAC_MAC_CONF_IPC;
+		dwqe_write(sc, GMAC_MAC_CONF, reg);
+	}
 }
 
 void
blob - f51d3b0a4717e4e0fd6dd1dd33b9301e27ff261f
blob + 532cbe3c60f46e507b7a1fd1770a0dc1744eb2d3
--- sys/dev/ic/dwqereg.h
+++ sys/dev/ic/dwqereg.h
@@ -17,6 +17,7 @@
  */
 
 #define GMAC_MAC_CONF		0x0000
+#define  GMAC_MAC_CONF_IPC		(1 << 27)
 #define  GMAC_MAC_CONF_CST		(1 << 21)
 #define  GMAC_MAC_CONF_ACS		(1 << 20)
 #define  GMAC_MAC_CONF_BE		(1 << 18)
@@ -61,6 +62,8 @@
 #define GMAC_VERSION		0x0110
 #define  GMAC_VERSION_SNPS_MASK		0xff
 #define GMAC_MAC_HW_FEATURE(x)	(0x011c + (x) * 0x4)
+#define  GMAC_MAC_HW_FEATURE0_TXCOESEL	(1 << 14)
+#define  GMAC_MAC_HW_FEATURE0_RXCOESEL	(1 << 16)
 #define  GMAC_MAC_HW_FEATURE1_TXFIFOSIZE(x) (((x) >> 6) & 0x1f)
 #define  GMAC_MAC_HW_FEATURE1_RXFIFOSIZE(x) (((x) >> 0) & 0x1f)
 #define GMAC_MAC_MDIO_ADDR	0x0200