Re: [PATCH] selftests/bpf: fix tunnel option size in ip6geneve_set_tunnel()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The test constructs a malformed Geneve packet where the option size field exceeds the total options length.
- [High] The receiver test silently fails to read the Geneve options, meaning the BPF tunnel option feature remains unverified.
- [Medium] The commit message incorrectly claims to fix an incorrect option length, but causes no functional change.
--

commit 41bdc36eb9d63a2614970dded5866e42f459a85a
Author: Faisal Bukhari <faisalbukhari523@gmail.com>

selftests/bpf: fix tunnel option size in ip6geneve_set_tunnel()

Pass the correct size to bpf_skb_set_tunnel_opt() in
ip6geneve_set_tunnel().

> sizeof(gopt) returns the size of the pointer instead of the
> actual Geneve option structure, resulting in an incorrect
> option length being passed to the helper. Use
> sizeof(local_gopt) to provide the proper structure size.

This isn't a bug, but does this actually change the size passed to the helper?

On the 64-bit eBPF architecture, pointers are exactly 8 bytes.
struct local_geneve_opt consists of a 4-byte struct geneve_opt and a 4-byte
int, totaling exactly 8 bytes. Thus, the exact same length (8) is passed to
the helper before and after the patch. The patch improves semantics but does
not appear to fix an incorrect length.

> diff --git a/tools/testing/selftests/bpf/progs/test_tunnel_kern.c b/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
> index 32127f1cd6872..b53d367451b2c 100644
> --- a/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
> +++ b/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
> @@ -675,7 +675,7 @@ int ip6geneve_set_tunnel(struct __sk_buff *skb)
>  	gopt->length = 2; /* 4-byte multiple */
>  	*(int *) &gopt->opt_data = bpf_htonl(0xfeedbeef);

Is it possible this creates a malformed Geneve packet?

According to the Geneve protocol (RFC 8926), the length field defines the
option data length in 4-byte multiples, excluding the 4-byte header.
Thus, length = 2 indicates 8 bytes of data (12 bytes total option length).

However, only 8 bytes total are passed to bpf_skb_set_tunnel_opt() via
sizeof(local_gopt). This results in an option that claims a length exceeding
the available option space.

Should the length field be set to 1 instead?

> -	ret = bpf_skb_set_tunnel_opt(skb, gopt, sizeof(gopt));
> +	ret = bpf_skb_set_tunnel_opt(skb, gopt, sizeof(local_gopt));

While looking at this, I noticed an issue in the receiver function
ip6geneve_get_tunnel() which calls bpf_skb_get_tunnel_opt() with a 4-byte
local struct geneve_opt:

tools/testing/selftests/bpf/progs/test_tunnel_kern.c:ip6geneve_get_tunnel() {
	struct geneve_opt gopt;
...
	ret = bpf_skb_get_tunnel_opt(skb, &gopt, sizeof(gopt));
	if (ret < 0)
		gopt.opt_class = 0;
...
}

Does this silently fail to read the Geneve options?

The kernel helper enforces that the buffer size must be at least the size of
the options present on the packet. Since the sender set 8 bytes of options,
bpf_skb_get_tunnel_opt() with a 4-byte buffer will fail with -ENOMEM.

It seems the test might be ignoring this error and bypassing the verification
of the Geneve options entirely.

>  	if (ret < 0) {
>  		log_err(ret);
>  		return TC_ACT_SHOT;

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