Re: [PATCH bpf v2 3/4] selftests/bpf: Adapt sockmap update error handling

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Low] Usage of legacy FAIL_ERRNO / CHECK_FAIL macros in updated tests instead of modern ASSERT_* macros.
--

commit bc0e5d743f397cf1347e049495ba4a7950323606
Author: Michal Luczaj <mhal@rbox.co>

selftests/bpf: Adapt sockmap update error handling

This patch updates sockmap_listen to accommodate recent changes in sockmap
that reject unbound UDP sockets.

> diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
> index cc0c68bab9079..6ee1bc6b3b235 100644
> --- a/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
> +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_listen.c
> @@ -63,11 +63,8 @@ static void test_insert_opened(struct test_sockmap_listen *skel __always_unused,
>  	errno = 0;
>  	value = s;
>  	err = bpf_map_update_elem(mapfd, &key, &value, BPF_NOEXIST);
> -	if (sotype == SOCK_STREAM) {
> -		if (!err || errno != EOPNOTSUPP)
> -			FAIL_ERRNO("map_update: expected EOPNOTSUPP");
> -	} else if (err)
> -		FAIL_ERRNO("map_update: expected success");
> +	if (!err || errno != EOPNOTSUPP)
> +		FAIL_ERRNO("map_update: expected EOPNOTSUPP");

[Severity: Low]
This is a pre-existing issue, but since this block in test_insert_opened() is
being updated, should these deprecated FAIL_ERRNO() macros be migrated to the
modern ASSERT_*() macros? The BPF selftests subsystem guidelines prefer using
ASSERT_* macros when updating existing tests.

[ ... ]

> @@ -93,8 +90,12 @@ static void test_insert_bound(struct test_sockmap_listen *skel __always_unused,
>  	errno = 0;
>  	value = s;
>  	err = bpf_map_update_elem(mapfd, &key, &value, BPF_NOEXIST);
> -	if (!err || errno != EOPNOTSUPP)
> -		FAIL_ERRNO("map_update: expected EOPNOTSUPP");
> +	if (sotype == SOCK_STREAM) {
> +		if (!err || errno != EOPNOTSUPP)
> +			FAIL_ERRNO("map_update: expected EOPNOTSUPP");
> +	} else if (err) {
> +		FAIL_ERRNO("map_update: expected success");
> +	}

[Severity: Low]
This isn't a bug introduced by this patch, but would it make sense to replace
these FAIL_ERRNO() checks in test_insert_bound() with ASSERT_ERR() or similar
modern macros while refactoring this logic?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260626-sockmap-lookup-udp-leak-v2-0-7e7e201c951a@rbox.co?part=3