[PATCH net 0/1] Prevent DSA tags from breaking COE

[Vladimir Oltean]

Hi Lucas,

On Thu, Dec 21, 2023 at 02:40:34AM -0500, Household Cang wrote:
> > On Dec 18, 2023, at 11:23 AM, Romain Gantois <romain.gantois at bootlin.com> wrote:
> > 
> > This is a bugfix for an issue that was recently brought up in two
> > reports:
> > 
> > https://lore.kernel.org/netdev/c57283ed-6b9b-b0e6-ee12-5655c1c54495@bootlin.com/
> > https://lore.kernel.org/netdev/e5c6c75f-2dfa-4e50-a1fb-6bf4cdb617c2@electromag.com.au/
> > 
> Add me in to be the 3rd report...
> RK3568 GMAC0 (eth1) to MT7531BE (CPU port)
> Current workaround for me is ethtool -K eth1 rx off tx off

Is "rx off" actually required, or just "tx off"?

> https://lore.kernel.org/netdev/m3clft2k7umjtny546ot3ayebattksibty3yyttpffvdixl65p@7dpqsr5nisbk/T/#t
> 
> Question on the patch to be built: how would I know if my setup could
> take advantage of the HW checksum offload? RK3658’s eth0 on stmmac is
> doing fine, and eth0 is not on a DSA switch. Does this mean eth1
> should be able to do hw checksum offload once the stmmac driver is
> fixed?

The MT7531BE switch requires transmitted packets to have an additional
header which indicates what switch port is targeted. So the packet
structure is not the same as what eth0 transmits.

Your GMAC datasheet should explain what packets it is able to offload
L4 checksumming for, quite plainly. Probably MAC + IP + TCP yes, but
MAC + MTK DSA + IP + TCP no.

The bug is that the network stack thinks that the GMAC is able to
offload TX checksums for these MTK DSA tagged packets, so it does not
calculate the checksum in software, leaving the task up to the hardware.
My guess is that the hardware doesn't recognize the packets as something
that is offloadable, so it doesn't calculate the checksum either, and
that's the story of how you end up with packets with bad checksums.

The patch to be built should analyze the packet before passing it to a
hardware offload engine which will do nothing. The driver still declares
the NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM features because it is able to
offload checksumming for _some_ packets, but it falls back to the
software checksum helper for the rest. This includes your MTK DSA tagged
packets. They can be checksummed in software even with the DSA tag added,
because that uses the more generic mechanism with skb->csum_start and
skb->csum_offset, which DSA is compatible with, just fine. The GMAC
driver, most likely because of the lack of hardware support, does not
look at skb->csum_start and skb->csum_offset (aka it does not declare
the NETIF_F_HW_CSUM feature), so it cannot offload checksumming for your
switch traffic unless that was specifically baked into the RTL.

More details in the "Switch tagging protocols" section of
Documentation/networking/dsa/dsa.rst.