linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Oleksij Rempel <o.rempel@pengutronix.de>
Cc: "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Paolo Abeni <pabeni@redhat.com>, Simon Horman <horms@kernel.org>,
	kernel@pengutronix.de, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org,
	Maxime Chevallier <maxime.chevallier@bootlin.com>
Subject: Re: [PATCH net-next v4 0/4] net: selftest: improve test string formatting and checksum handling
Date: Sat, 21 Jun 2025 06:46:00 -0700	[thread overview]
Message-ID: <20250621064600.035b83b3@kernel.org> (raw)
In-Reply-To: <aFU9o5F4RG3QVygb@pengutronix.de>

On Fri, 20 Jun 2025 12:53:23 +0200 Oleksij Rempel wrote:
> > 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.

Interesting, that sounds like the smoking gun. When rx-checksumming 
is enabled the packet still reaches the stack right?
If so does the frame enter the stack with CHECKSUM_COMPLETE or
UNNECESSARY?

> 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.

stmmac shouldn't touch the frame, note that skb_checksum_help() sets
skb->ip_summed = CHECKSUM_NONE; so the skb should no longer be considered
for csum offload.

> 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.

Not yet, at least. Once we figure out the problem you're seeing we can
decide whether we should adjust the tests or the tests are failing
because they are doing their job.

  reply	other threads:[~2025-06-21 13:46 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-15  8:30 [PATCH net-next v4 0/4] net: selftest: improve test string formatting and checksum handling Oleksij Rempel
2025-05-15  8:30 ` [PATCH net-next v4 1/4] net: selftests: drop test index from net_selftest_get_strings() Oleksij Rempel
2025-05-15  8:30 ` [PATCH net-next v4 2/4] net: selftests: prepare for detailed error handling in net_test_get_skb() Oleksij Rempel
2025-05-15  8:30 ` [PATCH net-next v4 3/4] net: selftests: add checksum mode support and SW checksum handling Oleksij Rempel
2025-05-16 12:57   ` Simon Horman
2025-05-17  1:48   ` Jakub Kicinski
2025-05-15  8:31 ` [PATCH net-next v4 4/4] net: selftests: add PHY loopback tests with HW checksum offload Oleksij Rempel
2025-05-17  1:45 ` [PATCH net-next v4 0/4] net: selftest: improve test string formatting and checksum handling Jakub Kicinski
2025-06-20 10:53   ` Oleksij Rempel
2025-06-21 13:46     ` Jakub Kicinski [this message]
2025-06-23 11:45       ` Oleksij Rempel
2025-06-23 17:19         ` Jakub Kicinski
2025-06-24  8:26           ` Oleksij Rempel
2025-06-24 16:09             ` Jakub Kicinski
2025-06-25  5:07               ` Oleksij Rempel
2025-06-25 20:21                 ` Jakub Kicinski
2025-07-11  8:42               ` Marc Kleine-Budde
2025-07-11 22:36                 ` Jakub Kicinski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250621064600.035b83b3@kernel.org \
    --to=kuba@kernel.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=kernel@pengutronix.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maxime.chevallier@bootlin.com \
    --cc=netdev@vger.kernel.org \
    --cc=o.rempel@pengutronix.de \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).