[PATCH net] selftests: net: Fix checksums in xdp_native

[PATCH net] selftests: net: Fix checksums in xdp_native

[Nimrod Oren]

Data adjustment cases failed with "Data exchange failed" when using IPv4
because the program did not update the IP and UDP checksums in the IPv4
branch. The issue was masked when both IPv4 and IPv6 were configured,
since the test harness prefers IPv6.

Fixes: 0b65cfcef9c5 ("selftests: drv-net: Test tail-adjustment support")
Reviewed-by: Carolina Jubran <cjubran@nvidia.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
Signed-off-by: Nimrod Oren <noren@nvidia.com>
---
 .../selftests/net/lib/xdp_native.bpf.c        | 54 ++++++++++---------
 1 file changed, 29 insertions(+), 25 deletions(-)

diff --git a/tools/testing/selftests/net/lib/xdp_native.bpf.c b/tools/testing/selftests/net/lib/xdp_native.bpf.c
index 64f05229ab24..eaecfa58b967 100644
--- a/tools/testing/selftests/net/lib/xdp_native.bpf.c
+++ b/tools/testing/selftests/net/lib/xdp_native.bpf.c
@@ -268,6 +268,16 @@ static int xdp_mode_tx_handler(struct xdp_md *ctx, __u16 port)
 	return XDP_PASS;
 }
 
+static __always_inline __u16 csum_fold_helper(__u32 csum)
+{
+	return ~((csum & 0xffff) + (csum >> 16));
+}
+
+static __always_inline __u16 csum_fold_udp_helper(__u32 csum)
+{
+	return csum_fold_helper(csum) ? : 0xffff;
+}
+
 static void *update_pkt(struct xdp_md *ctx, __s16 offset, __u32 *udp_csum)
 {
 	void *data_end = (void *)(long)ctx->data_end;
@@ -281,21 +291,22 @@ static void *update_pkt(struct xdp_md *ctx, __s16 offset, __u32 *udp_csum)
 
 	if (eth->h_proto == bpf_htons(ETH_P_IP)) {
 		struct iphdr *iph = data + sizeof(*eth);
-		__u16 total_len;
 
 		if (iph + 1 > (struct iphdr *)data_end)
 			return NULL;
 
-		iph->tot_len = bpf_htons(bpf_ntohs(iph->tot_len) + offset);
-
 		udph = (void *)eth + sizeof(*iph) + sizeof(*eth);
 		if (!udph || udph + 1 > (struct udphdr *)data_end)
 			return NULL;
 
-		len_new = bpf_htons(bpf_ntohs(udph->len) + offset);
+		len = iph->tot_len;
+		len_new = bpf_htons(bpf_ntohs(len) + offset);
+		iph->tot_len = len_new;
+		iph->check = csum_fold_helper(
+			bpf_csum_diff(&len, sizeof(len), &len_new,
+				      sizeof(len_new), ~((__u32)iph->check)));
 	} else if (eth->h_proto  == bpf_htons(ETH_P_IPV6)) {
 		struct ipv6hdr *ipv6h = data + sizeof(*eth);
-		__u16 payload_len;
 
 		if (ipv6h + 1 > (struct ipv6hdr *)data_end)
 			return NULL;
@@ -304,33 +315,27 @@ static void *update_pkt(struct xdp_md *ctx, __s16 offset, __u32 *udp_csum)
 		if (!udph || udph + 1 > (struct udphdr *)data_end)
 			return NULL;
 
-		*udp_csum = ~((__u32)udph->check);
-
 		len = ipv6h->payload_len;
 		len_new = bpf_htons(bpf_ntohs(len) + offset);
 		ipv6h->payload_len = len_new;
-
-		*udp_csum = bpf_csum_diff(&len, sizeof(len), &len_new,
-					  sizeof(len_new), *udp_csum);
-
-		len = udph->len;
-		len_new = bpf_htons(bpf_ntohs(udph->len) + offset);
-		*udp_csum = bpf_csum_diff(&len, sizeof(len), &len_new,
-					  sizeof(len_new), *udp_csum);
 	} else {
 		return NULL;
 	}
 
+	len = udph->len;
+	len_new = bpf_htons(bpf_ntohs(len) + offset);
+
+	*udp_csum = ~((__u32)udph->check);
+	*udp_csum = bpf_csum_diff(&len, sizeof(len), &len_new,
+				  sizeof(len_new), *udp_csum);
+	*udp_csum = bpf_csum_diff(&len, sizeof(len), &len_new,
+				  sizeof(len_new), *udp_csum);
+
 	udph->len = len_new;
 
 	return udph;
 }
 
-static __u16 csum_fold_helper(__u32 csum)
-{
-	return ~((csum & 0xffff) + (csum >> 16)) ? : 0xffff;
-}
-
 static int xdp_adjst_tail_shrnk_data(struct xdp_md *ctx, __u16 offset,
 				     unsigned long hdr_len)
 {
@@ -359,7 +364,7 @@ static int xdp_adjst_tail_shrnk_data(struct xdp_md *ctx, __u16 offset,
 		return -1;
 
 	udp_csum = bpf_csum_diff((__be32 *)tmp_buff, offset, 0, 0, udp_csum);
-	udph->check = (__u16)csum_fold_helper(udp_csum);
+	udph->check = (__u16)csum_fold_udp_helper(udp_csum);
 
 	if (bpf_xdp_adjust_tail(ctx, 0 - offset) < 0)
 		return -1;
@@ -403,7 +408,7 @@ static int xdp_adjst_tail_grow_data(struct xdp_md *ctx, __u16 offset)
 		return -1;
 
 	udp_csum = bpf_csum_diff(0, 0, (__be32 *)tmp_buff, offset, udp_csum);
-	udph->check = (__u16)csum_fold_helper(udp_csum);
+	udph->check = (__u16)csum_fold_udp_helper(udp_csum);
 
 	buff_len = bpf_xdp_get_buff_len(ctx);
 
@@ -484,8 +489,7 @@ static int xdp_adjst_head_shrnk_data(struct xdp_md *ctx, __u64 hdr_len,
 		return -1;
 
 	udp_csum = bpf_csum_diff((__be32 *)tmp_buff, offset, 0, 0, udp_csum);
-
-	udph->check = (__u16)csum_fold_helper(udp_csum);
+	udph->check = (__u16)csum_fold_udp_helper(udp_csum);
 
 	if (bpf_xdp_load_bytes(ctx, 0, tmp_buff, MAX_ADJST_OFFSET) < 0)
 		return -1;
@@ -542,7 +546,7 @@ static int xdp_adjst_head_grow_data(struct xdp_md *ctx, __u64 hdr_len,
 		return -1;
 
 	udp_csum = bpf_csum_diff(0, 0, (__be32 *)data_buff, offset, udp_csum);
-	udph->check = (__u16)csum_fold_helper(udp_csum);
+	udph->check = (__u16)csum_fold_udp_helper(udp_csum);
 
 	if (hdr_len > MAX_ADJST_OFFSET || hdr_len == 0)
 		return -1;
-- 
2.45.0

Re: [PATCH net] selftests: net: Fix checksums in xdp_native

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] Incomplete checksum fold in csum_fold_helper causes persistent off-by-one errors
- [Medium] Unconditional update of IPv4 UDP checksum corrupts packets with no checksum
- [Medium] Offset parameter is used to modify packet headers before it is bounds-checked and clamped
--

commit 2e32120c7c113684b13c8a13f5c0a114c19b097e
Author: Nimrod Oren <noren@nvidia.com>

selftests: net: Fix checksums in xdp_native

This commit fixes data adjustment cases for IPv4 by adding updates to the
IP and UDP checksums in the IPv4 branch.

> diff --git a/tools/testing/selftests/net/lib/xdp_native.bpf.c b/tools/testing/selftests/net/lib/xdp_native.bpf.c
> --- a/tools/testing/selftests/net/lib/xdp_native.bpf.c
> +++ b/tools/testing/selftests/net/lib/xdp_native.bpf.c
> @@ -268,6 +268,16 @@ static int xdp_mode_tx_handler(struct xdp_md *ctx, __u16 port)
>  	return XDP_PASS;
>  }
>  
> +static __always_inline __u16 csum_fold_helper(__u32 csum)
> +{
> +	return ~((csum & 0xffff) + (csum >> 16));
> +}

Does this folding function handle the carry bit completely? Standard 1's
complement checksum folding usually requires a second addition to handle
the carry from the first addition.

Since the 32-bit seed passed to bpf_csum_diff for iph->check is constructed
via ~((__u32)iph->check), the upper 16 bits are 0xFFFF. This guarantees the
intermediate 32-bit sum will exceed 0xFFFF, producing a carry that might
be discarded here, resulting in an off-by-one checksum.

[ ... ]

> @@ -281,21 +291,22 @@ static void *update_pkt(struct xdp_md *ctx, __s16 offset, __u32 *udp_csum)
>  
>  	if (eth->h_proto == bpf_htons(ETH_P_IP)) {
>  		struct iphdr *iph = data + sizeof(*eth);
[ ... ]
> +		len = iph->tot_len;
> +		len_new = bpf_htons(bpf_ntohs(len) + offset);
> +		iph->tot_len = len_new;
> +		iph->check = csum_fold_helper(
> +			bpf_csum_diff(&len, sizeof(len), &len_new,
> +				      sizeof(len_new), ~((__u32)iph->check)));
>  	} else if (eth->h_proto  == bpf_htons(ETH_P_IPV6)) {
[ ... ]
> +	len = udph->len;
> +	len_new = bpf_htons(bpf_ntohs(len) + offset);
> +
> +	*udp_csum = ~((__u32)udph->check);
> +	*udp_csum = bpf_csum_diff(&len, sizeof(len), &len_new,
> +				  sizeof(len_new), *udp_csum);
> +	*udp_csum = bpf_csum_diff(&len, sizeof(len), &len_new,
> +				  sizeof(len_new), *udp_csum);
> +
>  	udph->len = len_new;

If this is an IPv4 packet and the original UDP checksum is 0 (meaning no
checksum was generated by the transmitter), will this differential update
produce a non-zero, invalid checksum?

The receiver might then attempt to validate this new checksum against the
unchecksummed payload, causing the packet to be dropped. Should we skip the
UDP checksum update entirely for IPv4 when udph->check == 0?

[ ... ]

> @@ -359,7 +364,7 @@ static int xdp_adjst_tail_shrnk_data(struct xdp_md *ctx, __u16 offset,
>  		return -1;
>  
>  	udp_csum = bpf_csum_diff((__be32 *)tmp_buff, offset, 0, 0, udp_csum);
> -	udph->check = (__u16)csum_fold_helper(udp_csum);
> +	udph->check = (__u16)csum_fold_udp_helper(udp_csum);

Is it safe that the raw offset parameter is directly passed to update_pkt
(which modifies iph->tot_len and udph->len) before the offset is clamped
to MAX_ADJST_OFFSET (256) in this caller and others?

If the raw offset is larger than 256, the packet headers will be reduced by
the full offset amount, but the actual packet length adjustment via
bpf_xdp_adjust_tail will only use the clamped amount.

Could this result in packets with mismatched logical header lengths and
physical payload sizes?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260513072355.1083381-1-noren@nvidia.com?part=1