[PATCH net-next v2] selftests: net: csum: filter packets by source address and port

[PATCH net-next v2] selftests: net: csum: filter packets by source address and port

[Willem de Bruijn]

From: Willem de Bruijn <willemb@google.com>

Add source address and port verification in recv_verify_packet_* and
recv_udp: non-matching packets are skipped.

The receiver reads from PF_PACKET and UDP sockets to verify incoming
packets to test hardware checksum offload. It did not validate the
packet source address or source port.

During tests expecting an invalid checksum (-E), background packets
(with a valid checksum) fail the test.

Also
- add an inter-packet delay to avoid drops from bursts.
- remove a comment that is no longer correct.

Fixes: 91a7de85600d ("selftests/net: add csum offload test")
Signed-off-by: Willem de Bruijn <willemb@google.com>

---

Changes
  v1 -> v2
    - also check udp socket, with recvfrom

v1: https://lore.kernel.org/netdev/20260831210128.1359978-1-willemdebruijn.kernel@gmail.com/
---
 tools/testing/selftests/net/lib/csum.c | 43 ++++++++++++++++++++++++--
 1 file changed, 40 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/net/lib/csum.c b/tools/testing/selftests/net/lib/csum.c
index e28884ce3ab3..5c8f9d175dd3 100644
--- a/tools/testing/selftests/net/lib/csum.c
+++ b/tools/testing/selftests/net/lib/csum.c
@@ -2,8 +2,7 @@
 
 /* Test hardware checksum offload: Rx + Tx, IPv4 + IPv6, TCP + UDP.
  *
- * The test runs on two machines to exercise the NIC. For this reason it
- * is not integrated in kselftests.
+ * The test runs on two machines to exercise the NIC.
  *
  *     CMD=$((./csum -[46] -[tu] -S $SADDR -D $DADDR -[RT] -r 1 $EXTRA_ARGS))
  *
@@ -571,15 +570,38 @@ static int recv_prepare_packet(void)
 static int recv_udp(int fd)
 {
 	static char buf[MAX_PAYLOAD_LEN];
+	struct sockaddr_storage addr;
+	socklen_t addrlen;
 	int ret, count = 0;
 
 	while (1) {
-		ret = recv(fd, buf, sizeof(buf), MSG_DONTWAIT);
+		addrlen = sizeof(addr);
+		ret = recvfrom(fd, buf, sizeof(buf), MSG_DONTWAIT,
+			       (struct sockaddr *)&addr, &addrlen);
 		if (ret == -1 && errno == EAGAIN)
 			break;
 		if (ret == -1)
 			error(1, errno, "recv r");
 
+		if (cfg_family == PF_INET) {
+			struct sockaddr_in *sin = (void *)&addr;
+
+			if (sin->sin_addr.s_addr != cfg_saddr4.sin_addr.s_addr)
+				continue;
+			if (!cfg_zero_sum &&
+			    sin->sin_port != htons(cfg_port_src))
+				continue;
+		} else {
+			struct sockaddr_in6 *sin6 = (void *)&addr;
+
+			if (memcmp(&sin6->sin6_addr, &cfg_saddr6.sin6_addr,
+				   sizeof(sin6->sin6_addr)))
+				continue;
+			if (!cfg_zero_sum &&
+			    sin6->sin6_port != htons(cfg_port_src))
+				continue;
+		}
+
 		fprintf(stderr, "rx: udp: len=%u\n", ret);
 		count++;
 	}
@@ -620,6 +642,9 @@ static int recv_verify_packet_tcp(void *th, int len)
 	if (len < sizeof(*tcph) || tcph->dest != htons(cfg_port_dst))
 		return -1;
 
+	if (tcph->source != htons(cfg_port_src))
+		return -1;
+
 	return recv_verify_csum(th, len, ntohs(tcph->source), tcph->check);
 }
 
@@ -647,6 +672,9 @@ static int recv_verify_packet_udp(void *th, int len)
 		return recv_verify_packet_udp_encap(udph + 1,
 						    len - sizeof(*udph));
 
+	if (!cfg_zero_sum && udph->source != htons(cfg_port_src))
+		return -1;
+
 	return recv_verify_csum(th, len, ntohs(udph->source), udph->check);
 }
 
@@ -659,6 +687,9 @@ static int recv_verify_packet_ipv4(void *nh, int len)
 	if (len < sizeof(*iph) || iph->protocol != proto)
 		return -1;
 
+	if (iph->saddr != cfg_saddr4.sin_addr.s_addr)
+		return -1;
+
 	ip_len = ntohs(iph->tot_len);
 	if (ip_len > len || ip_len < sizeof(*iph))
 		return -1;
@@ -680,6 +711,9 @@ static int recv_verify_packet_ipv6(void *nh, int len)
 	if (len < sizeof(*ip6h) || ip6h->nexthdr != proto)
 		return -1;
 
+	if (memcmp(&ip6h->saddr, &cfg_saddr6.sin6_addr, sizeof(ip6h->saddr)))
+		return -1;
+
 	payload_len = ntohs(ip6h->payload_len);
 	if (payload_len > len - sizeof(*ip6h))
 		return -1;
@@ -940,6 +974,9 @@ static void do_tx(void)
 			cfg_payload_len = rand() % MAX_PAYLOAD_LEN;
 			buf = build_packet(_buf, sizeof(_buf), &len);
 		}
+
+		/* avoid bursting */
+		usleep(20);
 	}
 
 	if (close(fd))
-- 
2.55.0.979.g7e5102b832-goog

Re: [PATCH net-next v2] selftests: net: csum: filter packets by source address and port

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 0

- [Medium] The newly added source-port filter is silently disabled for
  the `-Z` (zero-checksum) configuration in both receive paths, leaving…

--

selftests: net: csum: filter packets by source address and port

The csum receiver now verifies the source address (and for TCP/UDP the
source port) of packets read from the PF_PACKET and UDP sockets, skipping
non-matching packets so background traffic does not fail the test. It also
adds a 20us inter-packet delay on transmit and drops a stale comment.

v1: https://lore.kernel.org/netdev/20260831210128.1359978-1-willemdebruijn.kernel@gmail.com/

> diff --git a/tools/testing/selftests/net/lib/csum.c b/tools/testing/selftests/net/lib/csum.c
> index e28884ce3ab39..5c8f9d175dd32 100644
> --- a/tools/testing/selftests/net/lib/csum.c
> +++ b/tools/testing/selftests/net/lib/csum.c

[ ... ]

> @@ -571,15 +570,38 @@ static int recv_prepare_packet(void)
>  static int recv_udp(int fd)
>  {
>  	static char buf[MAX_PAYLOAD_LEN];
> +	struct sockaddr_storage addr;
> +	socklen_t addrlen;
>  	int ret, count = 0;
>  
>  	while (1) {
> -		ret = recv(fd, buf, sizeof(buf), MSG_DONTWAIT);
> +		addrlen = sizeof(addr);
> +		ret = recvfrom(fd, buf, sizeof(buf), MSG_DONTWAIT,
> +			       (struct sockaddr *)&addr, &addrlen);
>  		if (ret == -1 && errno == EAGAIN)
>  			break;
>  		if (ret == -1)
>  			error(1, errno, "recv r");
>  
> +		if (cfg_family == PF_INET) {
> +			struct sockaddr_in *sin = (void *)&addr;
> +
> +			if (sin->sin_addr.s_addr != cfg_saddr4.sin_addr.s_addr)
> +				continue;
> +			if (!cfg_zero_sum &&
> +			    sin->sin_port != htons(cfg_port_src))
> +				continue;
> +		} else {
> +			struct sockaddr_in6 *sin6 = (void *)&addr;
> +
> +			if (memcmp(&sin6->sin6_addr, &cfg_saddr6.sin6_addr,
> +				   sizeof(sin6->sin6_addr)))
> +				continue;
> +			if (!cfg_zero_sum &&
> +			    sin6->sin6_port != htons(cfg_port_src))
> +				continue;
> +		}
> +
>  		fprintf(stderr, "rx: udp: len=%u\n", ret);
>  		count++;
>  	}

[ ... ]

> @@ -647,6 +672,9 @@ static int recv_verify_packet_udp(void *th, int len)
>  		return recv_verify_packet_udp_encap(udph + 1,
>  						    len - sizeof(*udph));
>  
> +	if (!cfg_zero_sum && udph->source != htons(cfg_port_src))
> +		return -1;
> +

[Severity: Medium]
Could the !cfg_zero_sum condition in recv_udp() and in
recv_verify_packet_udp() get a comment, or be replaced by computing the
expected port on the receiver?

As written, a -Z run keeps the old behaviour for UDP: the only remaining
filter on the PF_PACKET path is the source address checked in
recv_verify_packet_ipv4()/recv_verify_packet_ipv6(), and the BPF program
installed by recv_prepare_packet() only matches pkttype, nexthdr and the
destination port. A foreign UDP packet from the peer address to the
destination port therefore still reaches:

	if (cfg_zero_sum && csum_field != 0xFFFF) {
		fprintf(stderr, "pkt: zero csum: field should be 0xFFFF, is 0x%hx\n", csum_field);
		return 1;
	}

which bumps bad_csums in recv_packet() and aborts the run through
error(1, 0, "rx: errors at pf_packet: ..."), i.e. the same background
traffic failure the commit message describes as fixed ("non-matching
packets are skipped").

The reason for the guard looks real but is not visible here: cfg_port_src
is only rewritten to the derived zero-sum port inside build_packet_udp():

	if (cfg_zero_sum) {
		uh->source = 0;
		uh->source = checksum(uh, IPPROTO_UDP, sizeof(*uh) + cfg_payload_len);
		...
		cfg_port_src = ntohs(uh->source);
	}

and build_packet() is only reached from do_tx(), so a -R receiver keeps
the default cfg_port_src while the wire packets carry the computed port.

The receiver has the same inputs as the sender for the -Z case (same -S/-D,
same -l, fixed payload character, and randomization is rejected by
"Cannot combine zero checksum conversion with randomization"), so it could
derive the same port instead of skipping the comparison. Failing that,
would it be worth noting the -Z limitation in the commit message and next
to both !cfg_zero_sum guards?

>  	return recv_verify_csum(th, len, ntohs(udph->source), udph->check);
>  }
>  

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260904030624.3833721-1-willemdebruijn.kernel%40gmail.com

Re: [PATCH net-next v2] selftests: net: csum: filter packets by source address and port

[Willem de Bruijn]

netdev-bot+sashiko@ wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
> 
> Critical: 0 · High: 0 · Medium: 1 · Low: 0
> 
> - [Medium] The newly added source-port filter is silently disabled for
>   the `-Z` (zero-checksum) configuration in both receive paths, leaving…
> 
> 
> [Severity: Medium]
> Could the !cfg_zero_sum condition in recv_udp() and in
> recv_verify_packet_udp() get a comment, or be replaced by computing the
> expected port on the receiver?
 
Fair. For -Z, I'll have the receiver call build_packet() too, so that
build_packet_udp() updates cfg_port_src to the expected value.