Re: [PATCH bpf v1 2/8] selftests/bpf: Test timer field on recycled rhtab element

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Redundant file descriptor validation after successful BPF skeleton open_and_load
--

commit 3c5ef82656b9b8e3dd5798348adbe7dae8162f8d
Author: Kumar Kartikeya Dwivedi <memxor@gmail.com>

selftests/bpf: Test timer field on recycled rhtab element

This commit adds a selftest to exercise the rhtab special-field lifecycle
using a one-element map. It verifies that deleting a map element and then
replacing it preserves the timer field and successfully recycles the element.

> diff --git a/tools/testing/selftests/bpf/prog_tests/rhash_timer.c b/tools/testing/selftests/bpf/prog_tests/rhash_timer.c
> new file mode 100644
> index 0000000000000..3aad9fc02e063
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/rhash_timer.c

[ ... ]

> +void test_rhash_timer(void)
> +{
> +	LIBBPF_OPTS(bpf_test_run_opts, opts);
> +	struct rhash_timer *skel = NULL;

[ ... ]

> +	skel = rhash_timer__open_and_load();
> +	if (!ASSERT_OK_PTR(skel, "open_and_load"))
> +		goto out;
> +
> +	map_fd = bpf_map__fd(skel->maps.timer_map);
> +	if (!ASSERT_GE(map_fd, 0, "timer_map fd"))
> +		goto out;

[Severity: Low]
According to the BPF subsystem guidelines for the skeleton API, after a
successful rhash_timer__open_and_load() call, the skeleton pointer is valid
and all maps are guaranteed to be created with valid FDs. Is it necessary
to manually extract and validate map_fd using ASSERT_GE() here?

> +	arm_fd = bpf_program__fd(skel->progs.arm_deleted_timer);
> +	if (!ASSERT_GE(arm_fd, 0, "arm_deleted_timer fd"))
> +		goto out;
> +	cancel_fd = bpf_program__fd(skel->progs.cancel_recycled_timer);
> +	if (!ASSERT_GE(cancel_fd, 0, "cancel_recycled_timer fd"))
> +		goto out;

[Severity: Low]
Similarly, is this program FD validation needed? The skeleton guarantees
that all programs are loaded with valid FDs after open_and_load succeeds,
so bpf_program__fd() cannot return a negative value here.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904104203.345917-1-memxor@gmail.com?part=2