Re: [PATCH net-next v4 0/4] net: selftest: improve test string formatting and checksum handling

[Oleksij Rempel <o.rempel@pengutronix.de>]

Hi Jakub,

Sorry for the delay in getting back to you.

On Fri, May 16, 2025 at 06:45:10PM -0700, Jakub Kicinski wrote:
> On Thu, 15 May 2025 10:30:56 +0200 Oleksij Rempel wrote:
> > - Inconsistent checksum behavior: On DSA setups and similar
> >   environments, checksum offloading is not always available or
> >   appropriate. The previous selftests did not distinguish between software
> >   and hardware checksum modes, leading to unreliable results. This
> >   patchset introduces explicit csum_mode handling and adds separate tests
> >   for both software and hardware checksum validation.
> 
> What device are you talking about? How is this a problem with 
> the selftest and not with the stack? If the test is flaky I'd 
> think real traffic will suffer too. We pass these selftest packets
> thru xmit validation AFAICT, so the stack should compute checksum
> for the if the device can't.
> 

Let me first describe the setup where this issue was observed and my findings.
The problem occurs on a system utilizing a Microchip DSA driver with an STMMAC
Ethernet controller attached to the CPU port.

In the current selftest implementation, the TCP checksum validation fails,
while the UDP test passes. The existing code prepares the skb for hardware
checksum offload by setting skb->ip_summed = CHECKSUM_PARTIAL. For TCP, it sets
the thdr->check field to the complement of the pseudo-header checksum, and for
UDP, it uses udp4_hwcsum. If I understand it correct, this configuration tells
the kernel that the hardware should perform the checksum calculation.

However, during testing, I noticed that "rx-checksumming" is enabled by default
on the CPU port, and this leads to the TCP test failure.  Only after disabling
"rx-checksumming" on the CPU port did the selftest pass. This suggests that the
issue is specifically related to the hardware checksum offload mechanism in
this particular setup. The behavior indicates that something on the path
recalculated the checksum incorrectly.

When examining the loopbacked frames, I observed that the TCP checksum was
incorrect. Upon further investigation, the xmit helper in net/dsa/tag_ksz.c
includes the following:

if (skb->ip_summed == CHECKSUM_PARTIAL && skb_checksum_help(skb))
    return NULL;

I assume skb_checksum_help() is intended to calculate the proper checksum when
CHECKSUM_PARTIAL is set, indicating that the software should complete the
checksum before handing it to the hardware. My understanding is that the STMMAC
hardware then calculates the checksum for egress frames if CHECKSUM_PARTIAL is
used. Since these egress frames are passed from the DSA framework with a
tailtag, the checksum calculated by the hardware would then be incorrect for
the original packet. The STMMAC then seems to drop ingress packets if they have
an incorrect checksum.

I'm still trying to grasp the full picture of checksumming in such complex
environments. I would be grateful for your guidance on how this problem should
be addressed properly.

Regarding the current patch series, do these tests and the csum_mode
implementation make sense to you in this context? I believe it would be good
practice to have selftests that can detect these kinds of checksum
inconsistencies in drivers.

Best Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |