Created attachment 216815 [details] Fix The mvneta device requires MVNETA_TX_CMD_L4_CHECKSUM_NONE bit to be set in the tx descriptor is checksum not required. However, mvneta_tx_set_csumflag() is not setting this flag currently, causing the hardware to randomly corrupt IP header during transmission. This affects injected IPv4 packets that skips kernel IP stack processing (e.g. DHCP), as well as all IPv6 packets, since the driver currently does not offload csum for IPv6. The fix is to remove all the early return paths from mvneta_tx_set_csumflag() which do not set the MVNETA_TX_CMD_L4_CHECKSUM_NONE flag.
Oh, wow, this might explain some corrupted IPv6 RA packets I have seen. Are you sure that for the VLAN case we don't have to distinguish anything further?
The patch looks right to me. cc'ed mw@ since this looks like a regression from r357676.
(In reply to Mark Johnston from comment #2) Yes, I read the code in the mean time and it seems it is correct currently. The comment disabling IFCAP_HWCSUM_IPV6 made me wonder, but that's a different issue to the one here.
(In reply to Mark Johnston from comment #2) Even before r357676, the early return in the switch block is corrupting IPv6 packets. This patch fixes both.
(In reply to Mike Cui from comment #4) Thanks for the patch, LGTM. Do you want me to commit it?
(In reply to mw from comment #5) Yes, please. I don't know how to commit it myself. Side question: what's the process to get the latest version of this driver, as well as the accompanying e6000sw etherswitch driver merged into stable/12 so they make it into 12.2? I manually cherry-picked these changes into 12-STABLE and I've been running with them. They work fine.
I can commit the patch then. About updating e6000sw & if_mvneta on stable/12, do you have maybe a list of commit that you locally backported and tested?
(In reply to mw from comment #7) Here's a git tree with my backports: https://github.com/iucoen/freebsd-espressobin/commits/stable/12 There is one patch, titled "Set the allocated memory as coherent where is necessary", that I got from pfsense's tree based on 11.2. I'm not sure if that's strictly needed. You can ignore my own patch "Build espressobin-v7.dts", or maybe rework it to build the v7 DTS in addition. I've tested on on espressobin-v7, currently running as my home dual-stack router, so I've tested both IPv4 and IPv6.
A commit references this bug: Author: mw Date: Sat Aug 1 09:40:19 UTC 2020 New revision: 363759 URL: https://svnweb.freebsd.org/changeset/base/363759 Log: Fix TX csum handling in if_mvneta The mvneta device requires MVNETA_TX_CMD_L4_CHECKSUM_NONE bit to be set in the tx descriptor is checksum not required. However, mvneta_tx_set_csumflag() is not setting this flag currently, causing the hardware to randomly corrupt IP header during transmission. This affects injected IPv4 packets that skips kernel IP stack processing (e.g. DHCP), as well as all IPv6 packets, since the driver currently does not offload csum for IPv6. The fix is to remove all the early return paths from mvneta_tx_set_csumflag() which do not set the MVNETA_TX_CMD_L4_CHECKSUM_NONE flag. PR: 248306 Submitted by: Mike Cui <cuicui@gmail.com> Reported by: Mike Cui <cuicui@gmail.com> Changes: head/sys/dev/neta/if_mvneta.c