On Mon, Apr 08, 2024 at 05:20:51PM +0200, Stefan Sperling wrote:
> If someone has the means to test bad IP checksums easily I would
> appreciate help and/or hints.

I was able to trigger the "unhappy" path with scapy from igc0 connected
directly to dwqe0:

    send(IP(dst="10.0.1.1",chksum=0xdead)/TCP(dport=12345,flags="S"))

However, I am also still unable to trigger the (RDES1_IP_HDR_ERROR |
RDES1_IP_CSUM_ERROR) path.

> I am not sure which of (RDES1_IP_HDR_ERROR | RDES1_IP_CSUM_ERROR) will
> be set in the error case, so for now I am checking both bits.

For the generated packet above only RDES1_IP_CSUM_BYPASS was set, but
that bit only seems to indicate that hardware checksumming was skipped
and not that it has failed?

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.


diff --git a/sys/dev/ic/dwqe.c b/sys/dev/ic/dwqe.c
index 0467a4c7247..f9d1d63906a 100644
--- a/sys/dev/ic/dwqe.c
+++ b/sys/dev/ic/dwqe.c
@@ -641,6 +641,12 @@ 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_proc(struct dwqe_softc *sc)
 {
@@ -689,6 +695,15 @@ dwqe_rx_proc(struct dwqe_softc *sc)
 
 			m->m_pkthdr.len = m->m_len = len;
 
+			if (dwqe_have_rx_csum_offload(sc) &&
+			    (rxd->sd_tdes3 & RDES3_RDES1_VALID)) {
+				if (rxd->sd_tdes1 &
+				    (RDES1_IP_HDR_ERROR | RDES1_IP_CSUM_ERROR))
+					m->m_pkthdr.csum_flags |= M_IPV4_CSUM_IN_BAD;
+				else if (!(rxd->sd_tdes1 & RDES1_IP_CSUM_BYPASS))
+					m->m_pkthdr.csum_flags |= M_IPV4_CSUM_IN_OK;
+			}
+
 			ml_enqueue(&ml, m);
 		}
 
@@ -864,6 +879,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
diff --git a/sys/dev/ic/dwqereg.h b/sys/dev/ic/dwqereg.h
index bc6de32a5a5..c2a4c6f946e 100644
--- a/sys/dev/ic/dwqereg.h
+++ b/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
@@ -234,6 +237,12 @@ struct dwqe_desc {
 #define TDES3_OWN		(1U << 31)
 
 /* Rx bits */
+#define RDES1_IP_PAYLOAD_TYPE	0x7
+#define RDES1_IP_HDR_ERROR	(1 << 3)
+#define RDES1_IPV4_HDR		(1 << 4)
+#define RDES1_IPV6_HDR		(1 << 5)
+#define RDES1_IP_CSUM_BYPASS	(1 << 6)
+#define RDES1_IP_CSUM_ERROR	(1 << 7)
 #define RDES3_ES		(1 << 15)
 #define RDES3_DE		(1 << 19)
 #define RDES3_RE		(1 << 20)
@@ -241,6 +250,7 @@ struct dwqe_desc {
 #define RDES3_RWT		(1 << 22)
 #define RDES3_CE		(1 << 24)
 #define RDES3_BUF1V		(1 << 24)
+#define RDES3_RDES1_VALID	(1 << 26)
 #define RDES3_IC		(1 << 30)
 #define RDES3_OWN		(1U << 31)
 #define RDES3_LENGTH		(0x7fff << 0)