Re: [PATCH bpf-next v3 2/2] selftests/bpf: verify syncookie statistics in tcp_custom_syncookie

[sashiko-bot@kernel.org]

Sashiko AI review found 1 potential issue(s):
- [Medium] nstat command in read_tcpext_snmp() clobbers global history file due to missing -s flag
--

commit 6d6b9a4b92f87f759fc396b394eb46513c637ea5
Author: Jiayuan Chen <jiayuan.chen@linux.dev>

selftests/bpf: verify syncookie statistics in tcp_custom_syncookie

This commit adds a read_tcpext_snmp() helper to network_helpers.c to read
TcpExt SNMP counters via nstat. It uses this helper in the tcp_custom_syncookie
test to verify that LINUX_MIB_SYNCOOKIESRECV increments appropriately.

> diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
> --- a/tools/testing/selftests/bpf/network_helpers.c
> +++ b/tools/testing/selftests/bpf/network_helpers.c

[ ... ]

> @@ -621,6 +621,28 @@ int get_socket_local_port(int sock_fd)
>  	return -1;
>  }
>  
> +int read_tcpext_snmp(const char *name, unsigned long *val)
> +{
> +	char cmd[128], buf[128];
> +	int ret = 0;
> +	FILE *f;
> +
> +	snprintf(cmd, sizeof(cmd),
> +		 "nstat -az TcpExt%s | awk '/TcpExt/ {print $2}'", name);

Does invoking nstat without the -s flag clobber the global history file?

By default, nstat writes its state to a shared history file in the
temporary directory. Because BPF selftests run in parallel and share the
/tmp filesystem, multiple threads executing read_tcpext_snmp() might race
to write to this file.

Can this also corrupt the baseline for any concurrent networking tests that
rely on nstat delta calculations?

Would it be safer to invoke this as nstat -asz instead?

> +	f = popen(cmd, "r");
> +	if (!f)
> +		return -errno;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260427001006.14465-1-jiayuan.chen@linux.dev?part=2