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

[netdev-bot+sashiko@kernel.org]

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