Re: [RFC bpf-next 3/3] selftests/bpf: test BPF_F_COMPARE compare-and-delete

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Multi-line comments do not follow the mandatory BPF subsystem style.
--

commit d9d64a94ef921683b52276aed902be3aa7f68eed
Author: Gyutae Bae <gyutae.bae@navercorp.com>

selftests/bpf: test BPF_F_COMPARE compare-and-delete

This patch introduces a selftest for the compare-and-delete operation on a
hash map. Since there is no libbpf wrapper, it uses the raw bpf syscall to
verify the deletion behavior based on a revision field.

> diff --git a/tools/testing/selftests/bpf/prog_tests/map_cmp_delete.c b/tools/testing/selftests/bpf/prog_tests/map_cmp_delete.c
> new file mode 100644
> index 0000000000000..6d6df13adcc42
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/map_cmp_delete.c

[ ... ]

> +struct val {
> +	__u64 revision;	/* synchronization field we compare on */
> +	__u64 payload;
> +};
> +
> +/* libbpf has no wrapper that passes the compare-and-delete fields, so
> + * issue bpf() directly. 'flags' is a parameter so tests can also
> + * exercise the no-BPF_F_COMPARE case.
> + */

[Severity: Low]
This isn't a bug, but does this multi-line comment follow the preferred BPF
subsystem style? The guidelines specify that the opening /* should be on its
own line, with the text starting on the following line.

> +static int cmp_delete_flags(int fd, const void *key, const void *compare,
> +			    __u32 off, __u32 size, __u64 flags)

[ ... ]

> +static void test_contract(void)
> +{

[ ... ]

> +	/* mismatch -> -EBUSY, entry preserved */
> +	snap.revision = 9;
> +	ASSERT_EQ(cmp_delete(fd, &k, &snap, off, sz), -EBUSY, "mismatch_ebusy");
> +	ASSERT_OK(bpf_map_lookup_elem(fd, &k, &v), "still_present");
> +
> +	/* compare fields set but BPF_F_COMPARE omitted -> -EINVAL, entry preserved
> +	 * (a dropped flag must not silently become an unconditional delete)
> +	 */

[Severity: Low]
This isn't a bug, but is this multi-line comment using the preferred
formatting? Similar to the comment above, the opening /* should typically
be placed on its own empty line.

> +	ASSERT_EQ(cmp_delete_flags(fd, &k, &snap, off, sz, 0), -EINVAL,
> +		  "cmp_fields_no_flag_einval");
> +	ASSERT_OK(bpf_map_lookup_elem(fd, &k, &v), "preserved_no_flag");

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260622071649.31541-1-gyutae.opensource@navercorp.com?part=3