[PATCH net-next v2 1/1] net: selftests: add PHY-loopback test for bad TCP checksums

[PATCH net-next v2 1/1] net: selftests: add PHY-loopback test for bad TCP checksums

[Oleksij Rempel]

Detect NICs and drivers that either drop frames with a corrupted TCP
checksum or, worse, pass them up as valid.  The test flips one bit in
the checksum, transmits the packet in internal loopback, and fails when
the driver reports CHECKSUM_UNNECESSARY.

Discussed at:
https://lore.kernel.org/all/20250625132117.1b3264e8@kernel.org/

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
changes v2:
- Replaced manual calculation of TCP checksum with standard kernel helper
  skb_checksum_help().
- add test documentation
---
 net/core/selftests.c | 84 ++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 82 insertions(+), 2 deletions(-)

diff --git a/net/core/selftests.c b/net/core/selftests.c
index 406faf8e5f3f..513c8b5f7c46 100644
--- a/net/core/selftests.c
+++ b/net/core/selftests.c
@@ -27,6 +27,7 @@ struct net_packet_attrs {
 	int max_size;
 	u8 id;
 	u16 queue_mapping;
+	bool bad_csum;
 };
 
 struct net_test_priv {
@@ -165,6 +166,37 @@ static struct sk_buff *net_test_get_skb(struct net_device *ndev,
 		thdr->check = ~tcp_v4_check(l4len, ihdr->saddr, ihdr->daddr, 0);
 		skb->csum_start = skb_transport_header(skb) - skb->head;
 		skb->csum_offset = offsetof(struct tcphdr, check);
+
+		if (attr->bad_csum) {
+			u16 csum;
+
+			/* Force mangled checksum */
+			if (skb_checksum_help(skb)) {
+				kfree_skb(skb);
+				return NULL;
+			}
+
+			/* To avoid sparse warnings about operating on
+			 * restricted __sum16/__be16 types, explicitly cast the
+			 * checksum to a plain u16, perform the manipulation,
+			 * and then cast the result back.
+			 */
+			csum = (__force u16)thdr->check;
+
+			/* Mangle the checksum by flipping the LSB. */
+			csum ^= 1;
+			/* If mangling resulted in 0, use the raw value for a
+			 * mangled-zero checksum. We use the literal 0xffff
+			 * because CSUM_MANGLED_0 has a restricted type.
+			 */
+			if (!csum)
+				csum = 0xffff;
+
+			/* Cast the final integer value back to the restricted
+			 * type
+			 */
+			thdr->check = (__force __sum16)csum;
+		}
 	} else {
 		udp4_hwcsum(skb, ihdr->saddr, ihdr->daddr);
 	}
@@ -239,7 +271,11 @@ static int net_test_loopback_validate(struct sk_buff *skb,
 	if (tpriv->packet->id != shdr->id)
 		goto out;
 
-	tpriv->ok = true;
+	if (tpriv->packet->bad_csum && skb->ip_summed == CHECKSUM_UNNECESSARY)
+		tpriv->ok = -EIO;
+	else
+		tpriv->ok = true;
+
 	complete(&tpriv->comp);
 out:
 	kfree_skb(skb);
@@ -285,7 +321,12 @@ static int __net_test_loopback(struct net_device *ndev,
 		attr->timeout = NET_LB_TIMEOUT;
 
 	wait_for_completion_timeout(&tpriv->comp, attr->timeout);
-	ret = tpriv->ok ? 0 : -ETIMEDOUT;
+	if (tpriv->ok < 0)
+		ret = tpriv->ok;
+	else if (!tpriv->ok)
+		ret = -ETIMEDOUT;
+	else
+		ret = 0;
 
 cleanup:
 	dev_remove_pack(&tpriv->pt);
@@ -345,6 +386,42 @@ static int net_test_phy_loopback_tcp(struct net_device *ndev)
 	return __net_test_loopback(ndev, &attr);
 }
 
+/**
+ * net_test_phy_loopback_tcp_bad_csum - PHY loopback test with a deliberately
+ *					corrupted TCP checksum
+ * @ndev: the network device to test
+ *
+ * Builds the same minimal Ethernet/IPv4/TCP frame as
+ * net_test_phy_loopback_tcp(), then flips the least-significant bit of the TCP
+ * checksum so the resulting value is provably invalid (neither 0 nor 0xFFFF).
+ * The frame is transmitted through the device’s internal PHY loopback path:
+ *
+ *   test code -> MAC driver -> MAC HW -> xMII -> PHY ->
+ *   internal PHY loopback -> xMII -> MAC HW -> MAC driver -> test code
+ *
+ * Result interpretation
+ * ---------------------
+ *  0            The frame is delivered to the stack and the driver reports
+ *               ip_summed as CHECKSUM_NONE or CHECKSUM_COMPLETE - both are
+ *               valid ways to indicate “bad checksum, let the stack verify.”
+ *  -ETIMEDOUT   The MAC/PHY silently dropped the frame; hardware checksum
+ *               verification filtered it out before the driver saw it.
+ *  -EIO         The driver returned the frame with ip_summed ==
+ *               CHECKSUM_UNNECESSARY, falsely claiming a valid checksum and
+ *               indicating a serious RX-path defect.
+ *
+ * Return: 0 on success or a negative error code on failure.
+ */
+static int net_test_phy_loopback_tcp_bad_csum(struct net_device *ndev)
+{
+	struct net_packet_attrs attr = { };
+
+	attr.dst = ndev->dev_addr;
+	attr.tcp = true;
+	attr.bad_csum = true;
+	return __net_test_loopback(ndev, &attr);
+}
+
 static const struct net_test {
 	char name[ETH_GSTRING_LEN];
 	int (*fn)(struct net_device *ndev);
@@ -368,6 +445,9 @@ static const struct net_test {
 	}, {
 		.name = "PHY internal loopback, TCP    ",
 		.fn = net_test_phy_loopback_tcp,
+	}, {
+		.name = "PHY loopback, bad TCP csum    ",
+		.fn = net_test_phy_loopback_tcp_bad_csum,
 	}, {
 		/* This test should be done after all PHY loopback test */
 		.name = "PHY internal loopback, disable",
-- 
2.39.5

Re: [PATCH net-next v2 1/1] net: selftests: add PHY-loopback test for bad TCP checksums

[Simon Horman]

On Fri, Jul 11, 2025 at 09:24:49AM +0200, Oleksij Rempel wrote:
> Detect NICs and drivers that either drop frames with a corrupted TCP
> checksum or, worse, pass them up as valid.  The test flips one bit in
> the checksum, transmits the packet in internal loopback, and fails when
> the driver reports CHECKSUM_UNNECESSARY.
> 
> Discussed at:
> https://lore.kernel.org/all/20250625132117.1b3264e8@kernel.org/
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
> changes v2:
> - Replaced manual calculation of TCP checksum with standard kernel helper
>   skb_checksum_help().
> - add test documentation

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net-next v2 1/1] net: selftests: add PHY-loopback test for bad TCP checksums

[Jakub Kicinski]

On Fri, 11 Jul 2025 09:24:49 +0200 Oleksij Rempel wrote:
> +			/* To avoid sparse warnings about operating on
> +			 * restricted __sum16/__be16 types, explicitly cast the
> +			 * checksum to a plain u16, perform the manipulation,
> +			 * and then cast the result back.
> +			 */
> +			csum = (__force u16)thdr->check;
> +
> +			/* Mangle the checksum by flipping the LSB. */
> +			csum ^= 1;
> +			/* If mangling resulted in 0, use the raw value for a
> +			 * mangled-zero checksum. We use the literal 0xffff
> +			 * because CSUM_MANGLED_0 has a restricted type.
> +			 */
> +			if (!csum)
> +				csum = 0xffff;
> +
> +			/* Cast the final integer value back to the restricted
> +			 * type
> +			 */
> +			thdr->check = (__force __sum16)csum;

Way to manny lines of code for something this simple.
Can csum_add() help you get rid of all these casts and comments?
-- 
pw-bot: cr