Index | Thread | Search

From:
Jonathan Matthew <jonathan@d14n.org>
Subject:
em(4): fix I210/I350 handling of crc stripping
To:
tech@openbsd.org
Date:
Mon, 7 Jul 2025 20:03:33 +1000

Download raw body.

Thread
dtucker@ reported that I210 em(4) interfaces behaved weirdly at specific
larger-than-default-MTU packet sizes.

The handling of packets with very short final segments for I210/I350 looks
fishy to me. On these chips the CRC is always stripped (see if_em.c r1.378 line 2850),
but the code in em_rxeof() dealing with stripping the CRC will still remove the
final 4 bytes of the packet if they happen to be split across segments.

The diff below, which reorders the tests in the if-else block so we never
truncate packets in the name of CRC stripping on I210/I350, reportedly fixes this.
Darren's test for reproducing the problem was just using ping:

  $ for i in `seq 64 9000`; do ping -c1 -w1 -s$i 192.168.32.248 >/dev/null || printf "$i "; done; echo
  2007 2008 2009 4055 4056 4057 6103 6104 6105 8151 8152 8153 

If you use large packets with em(4) interfaces, or would like to but haven't
been able to, you might want to test this yourself.

ok?

Index: if_em.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/if_em.c,v
diff -u -p -r1.378 if_em.c
--- if_em.c	31 Aug 2024 16:23:09 -0000	1.378
+++ if_em.c	7 Jul 2025 03:31:22 -0000
@@ -3116,13 +3116,14 @@ em_rxeof(struct em_queue *que)
 
 		if (status & E1000_RXD_STAT_EOP) {
 			eop = 1;
-			if (desc_len < ETHER_CRC_LEN) {
+			if (sc->hw.mac_type == em_i210 ||
+			    sc->hw.mac_type == em_i350) {
+				/* crc has already been stripped */
+				len = desc_len;
+			} else if (desc_len < ETHER_CRC_LEN) {
 				len = 0;
 				prev_len_adj = ETHER_CRC_LEN - desc_len;
-			} else if (sc->hw.mac_type == em_i210 ||
-			    sc->hw.mac_type == em_i350)
-				len = desc_len;
-			else
+			} else
 				len = desc_len - ETHER_CRC_LEN;
 		} else {
 			eop = 0;