Re: [PATCH] tcp: Encode checksum computation flags in a single parameter

On Tue, Mar 24, 2026 at 12:01:23PM +0100, Laurent Vivier wrote:

tcp_fill_headers() takes a pointer to a previously computed IPv4 header checksum to avoid recalculating it when the payload length doesn't change, and a separate bool to skip TCP checksum computation.

Replace both parameters with a single uint32_t csum_flags that encodes: - IP4_CSUM (bit 31): compute IPv4 header checksum from scratch - TCP_CSUM (bit 30): compute TCP checksum - IP4_CMASK (low 16 bits): cached IPv4 header checksum value

When IP4_CSUM is not set, the cached checksum is extracted from the low 16 bits. This is cleaner than the pointer-based approach, and also avoids a potential dangling pointer issue: a subsequent patch makes tcp_fill_headers() access ip4h via with_header(), which scopes it to a temporary variable, so a pointer to ip4h->check would become invalid after the with_header() block.

Suggested-by: David Gibson Signed-off-by: Laurent Vivier

When I suggested this, I'd missed the fact that @ip4_check and @no_tcp_csum were talking about different checksums, oops. Nonetheless you've made it work :). Reviewed-by: David Gibson [snip]

if (ip4h) - *check = &ip4h->check; + *csum_flags = (*csum_flags & TCP_CSUM) | ip4h->check;

Now that I've realised my mistake, I'm pretty neutral on whether we include the TCP checksum control in this parameter. I still think avoiding the pointer is a significant win - not referencing one packet's buffer when we're working on another means less non-obvious constraints in how we organise those buffers. -- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson