Re: [PATCH net-next v1 1/1] net: selftests: add PHY-loopback test for bad TCP checksums

[Jakub Kicinski <kuba@kernel.org>]

On Tue,  8 Jul 2025 14:28:23 +0200 Oleksij Rempel wrote:
>  	if (attr->tcp) {
>  		int l4len = skb->len - skb_transport_offset(skb);
>  
> -		thdr->check = ~tcp_v4_check(l4len, ihdr->saddr, ihdr->daddr, 0);
> -		skb->csum_start = skb_transport_header(skb) - skb->head;
> -		skb->csum_offset = offsetof(struct tcphdr, check);
> +		if (attr->bad_csum) {
> +			__sum16 good_csum;
> +			u16 bad_csum;
> +
> +			skb->ip_summed = CHECKSUM_NONE;
> +			thdr->check = 0;
> +			skb->csum = skb_checksum(skb, skb_transport_offset(skb),
> +						 l4len, 0);
> +			good_csum = csum_tcpudp_magic(ihdr->saddr, ihdr->daddr,
> +						      l4len, IPPROTO_TCP,
> +						      skb->csum);
> +
> +			/* Flip the least-significant bit.  This is fast,
> +			 * deterministic, and cannot accidentally turn the
> +			 * checksum back into a value the stack treats as valid
> +			 * (0 or 0xFFFF).
> +			 */
> +			bad_csum = (__force u16)good_csum ^ 0x0001;
> +			if (bad_csum == 0 || bad_csum == 0xFFFF) {
> +				/* If the checksum is 0 or 0xFFFF, flip another
> +				 * bit to ensure it is not valid.
> +				 */
> +				bad_csum ^= 0x0002;
> +			}
> +
> +			thdr->check = (__force __sum16)bad_csum;
> +		} else {
> +			skb->csum = 0;
> +			skb->ip_summed = CHECKSUM_PARTIAL;
> +			thdr->check = ~tcp_v4_check(l4len, ihdr->saddr,
> +						    ihdr->daddr, 0);
> +			skb->csum_start = skb_transport_header(skb) - skb->head;
> +			skb->csum_offset = offsetof(struct tcphdr, check);
> +		}
>  	} else {
> +		skb->csum = 0;
> +		skb->ip_summed = CHECKSUM_PARTIAL;
>  		udp4_hwcsum(skb, ihdr->saddr, ihdr->daddr);
>  	}

I think it'd be simpler if - after setting up CHECKSUM_PARTIAL
we called skb_checksum_help() to get the correct checksum filled in
and then did the bad checksum mangling.

BTW mangling like this should be idiomatic enough to avoid the comment:

	thdr->check = thdr->check ^ 1 ?: CSUM_MANGLED_0;
-- 
pw-bot: cr