Skip to contentSkip to conversation

ifb: fix packets checksum

PATCH Applied Jon Maxwell 2 participants 4 messages May 24, 2018 – May 29, 2018

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ifb: fix packets checksum

Fixup the checksum for CHECKSUM_COMPLETE when pulling skbs on RX path. Otherwise we get splats when tc mirred is used to redirect packets to ifb.

Before fix:

nic: hw csum failure

---
 drivers/net/ifb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/ifb.c +1 -1
index 5f2897ec0edc..d345c61d476c 100644
@@ -102,7 +102,7 @@ static void ifb_ri_tasklet(unsigned long _txp)
 		if (!skb->tc_from_ingress) {
 			dev_queue_xmit(skb);
 		} else {
-			skb_pull(skb, skb->mac_len);
+			skb_pull_rcsum(skb, skb->mac_len);
 			netif_receive_skb(skb);
 		}
 	}
Signature · 1 line
2.13.6
maintainer
#1 lore raw

From: Jon Maxwell <[email protected]>
Date: Fri, 25 May 2018 07:38:29 +1000

Quoted 8 lines

Fixup the checksum for CHECKSUM_COMPLETE when pulling skbs on RX path. Otherwise we get splats when tc mirred is used to redirect packets to ifb.

Before fix:

nic: hw csum failure

This definitely seems correct, but I am really surprised a bug like this has lasted as long as it has.

So I'll let this sit for another day or two for review.

#2 lore raw
Quoted 6 lines · David Miller

From: Jon Maxwell <[email protected]>
Date: Fri, 25 May 2018 07:38:29 +1000

Quoted 3 lines

Fixup the checksum for CHECKSUM_COMPLETE when pulling skbs on RX path. Otherwise we get splats when tc mirred is used to redirect packets to ifb.

Quoted 9 lines
Quoted 5 lines

Before fix:

nic: hw csum failure

This definitely seems correct, but I am really surprised a bug like this has lasted as long as it has.

Sorry for the late reply I have been away for a few days. The customer never saw this on bnx2x. Then they switched to the mlx5 driver and it started happening continuously when doing iperf3 tests and also for other TCP traffic.

mlx5 uses CHECKSUM_COMPLETE. I think that bnx2x uses CHECKSUM UNNECESSARY which avoided the code path that triggers the csum failure message in __skb_checksum_complete(). It only logs the message for CHECKSUM_COMPLETE skbs. Probably few Linux users are using a combination of NIC drivers that use CHECKSUM_COMPLETE, tc mirred and ifb, which is why this has never been reported before.

This is very similar to commit 7be709af2b65.

Quoted 1 line

So I'll let this sit for another day or two for review.