Re: [PATCH bpf-next v3] selftests/bpf: add XDP test for per-flow LRU_HASH window updates

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] bpf: multi-line comments violate subsystem style
--

commit 3357ebe262845fd222dfb8c2602be7eaa9237aad
Author: Anil Kaushik <anilkaushikwireless@gmail.com>

selftests/bpf: add XDP test for per-flow LRU_HASH window updates

This commit adds an XDP program and a test_progs case to verify per-flow key
isolation in a BPF_MAP_TYPE_LRU_HASH map. It stresses the verifier's bounds
checking of a value-internal array when indexed by a runtime value.

> diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_lru_window.c b/tools/testing/selftests/bpf/prog_tests/xdp_lru_window.c
> new file mode 100644
> index 0000000000000..5e353fb1425eb
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/xdp_lru_window.c

[ ... ]

> +/* Like inject(), but sends frames of a chosen on-wire length (>= the
> + * TCP/IPv4 headers) so the recorded pkt_len differs from the default.
> + */
> +static int inject_len(int n, __u16 dport, __u32 len)

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

[ ... ]

> +static void test_one_and_wrap(void)
> +{

[ ... ]

> +	ASSERT_EQ(st.pkt_len[0], base_len, "len0");
> +
> +	/* Fill the whole window with base-length packets (slots 0..W-1),
> +	 * then send 5 more of a different length so they wrap into slots
> +	 * 0..4. Distinct lengths let the checks below catch a wrong, but
> +	 * still in-range, post-wrap index.
> +	 */
> +	if (inject(AGGREGATION_WINDOW - 1, DST_PORT))

[Severity: Low]
Are we missing a standalone /* line for this comment to match the BPF
subsystem multi-line comment formatting rules?

[ ... ]

> +static void test_trunc(void)
> +{

[ ... ]

> +	fill_pkt(&pkt, DST_PORT);
> +	reset_map();
> +
> +	/* Valid Ethernet/IP ethertype, but the IPv4 header is cut off:
> +	 * exercises the program's IPv4 length check.
> +	 */
> +	if (run_pkt(&pkt, ip_trunc, &retval))

[Severity: Low]
Could the opening /* be placed on its own line here as well, keeping the
comment block consistent with modern BPF style?

[ ... ]

> +	err = bpf_map_get_next_key(map_fd, NULL, &next);
> +	ASSERT_EQ(err, -ENOENT, "ip trunc no insert");
> +
> +	/* Full Ethernet + IPv4 header, but the TCP header is cut off:
> +	 * exercises the program's TCP length check.
> +	 */
> +	if (run_pkt(&pkt, tcp_trunc, &retval))

[Severity: Low]
Does this comment also need to be updated so the opening /* is on its own
line, adhering to the same formatting standard?

[ ... ]

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