netdev - Re: [PATCH net-next v1 3/4] net: selftest: add checksum mode support and SW checksum handling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250421143246.GK2789685@horms.kernel.org>
Date: Mon, 21 Apr 2025 15:32:46 +0100
From: Simon Horman <horms@...nel.org>
To: Oleksij Rempel <o.rempel@...gutronix.de>
Cc: "David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	kernel@...gutronix.de, linux-kernel@...r.kernel.org,
	netdev@...r.kernel.org,
	Maxime Chevallier <maxime.chevallier@...tlin.com>
Subject: Re: [PATCH net-next v1 3/4] net: selftest: add checksum mode support
 and SW checksum handling

On Wed, Apr 16, 2025 at 06:14:38PM +0200, Oleksij Rempel wrote:
> Introduce `enum net_test_checksum_mode` to support both CHECKSUM_COMPLETE
> and CHECKSUM_PARTIAL modes in selftest packet generation.
> 
> Add helpers to calculate and apply software checksums for TCP/UDP in
> CHECKSUM_COMPLETE mode, and refactor checksum handling into a dedicated
> function `net_test_set_checksum()`.
> 
> Update PHY loopback tests to use CHECKSUM_COMPLETE by default to avoid
> hardware offload dependencies and improve reliability.
> 
> Also rename loopback test names to clarify checksum type and transport.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@...gutronix.de>

Unfortunately this patch does not apply against net-next
(or at any rate, the series did not at the time it was submitted).
Please rebase and repost.

> ---
>  net/core/selftests.c | 218 ++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 206 insertions(+), 12 deletions(-)
> 
> diff --git a/net/core/selftests.c b/net/core/selftests.c

...

> +/**
> + * net_test_setup_sw_csum - Compute and apply software checksum
> + *                          (CHECKSUM_COMPLETE)
> + * @skb: Socket buffer with transport header set
> + * @iph: Pointer to IPv4 header inside skb
> + *
> + * This function computes and fills the transport layer checksum (TCP or UDP),
> + * and sets skb->ip_summed = CHECKSUM_COMPLETE.
> + *
> + * Returns 0 on success, or negative error code on failure.
> + */
> +static int net_test_setup_sw_csum(struct sk_buff *skb,
> +				  struct iphdr *iph)
> +{
> +	int transport_offset = skb_transport_offset(skb);
> +	int transport_len = skb->len - transport_offset;
> +	__be16 final_csum;
> +	__wsum csum;
> +
> +	switch (iph->protocol) {
> +	case IPPROTO_TCP:
> +		if (!pskb_may_pull(skb,
> +				   transport_offset + sizeof(struct tcphdr)))
> +			return -EFAULT;
> +
> +		tcp_hdr(skb)->check = 0;
> +		break;
> +	case IPPROTO_UDP:
> +		if (!pskb_may_pull(skb,
> +				   transport_offset + sizeof(struct udphdr)))
> +			return -EFAULT;
> +
> +		udp_hdr(skb)->check = 0;
> +		break;
> +	default:
> +		pr_err("net_selftest: unsupported proto for sw csum: %u\n",
> +		       iph->protocol);
> +		return -EINVAL;
> +	}
> +
> +	csum = skb_checksum(skb, transport_offset, transport_len, 0);
> +	final_csum = csum_tcpudp_magic(iph->saddr, iph->daddr, transport_len,
> +				       iph->protocol, csum);

Sparse is unhappy about integer type annotations around here.
The 'final_csum =' line above is line number 101.

  .../selftests.c:101:20: warning: incorrect type in assignment (different base types)
  .../selftests.c:101:20:    expected restricted __be16 [usertype] final_csum
  .../selftests.c:101:20:    got restricted __sum16
  .../selftests.c:105:28: warning: incorrect type in assignment (different base types)
  .../selftests.c:105:28:    expected restricted __be16 [usertype] final_csum
  .../selftests.c:105:28:    got restricted __sum16 [usertype]
  .../selftests.c:108:37: warning: incorrect type in assignment (different base types)
  .../selftests.c:108:37:    expected restricted __sum16 [usertype] check
  .../selftests.c:108:37:    got restricted __be16 [usertype] final_csum
  .../selftests.c:110:37: warning: incorrect type in assignment (different base types)
  .../selftests.c:110:37:    expected restricted __sum16 [usertype] check
  .../selftests.c:110:37:    got restricted __be16 [usertype] final_csum

> +
> +	if (iph->protocol == IPPROTO_UDP && final_csum == 0)
> +		final_csum = CSUM_MANGLED_0;
> +
> +	if (iph->protocol == IPPROTO_TCP)
> +		tcp_hdr(skb)->check = final_csum;
> +	else
> +		udp_hdr(skb)->check = final_csum;
> +
> +	skb->ip_summed = CHECKSUM_COMPLETE;
> +
> +	return 0;
> +}

...

-- 
pw-bot: changes-requested