* Re: [PATCH] selftests/bpf: fix tunnel option size in ip6geneve_set_tunnel()
2026-05-15 15:18 [PATCH] selftests/bpf: fix tunnel option size in ip6geneve_set_tunnel() Faisal Bukhari
@ 2026-05-15 15:37 ` sashiko-bot
2026-05-15 16:01 ` bot+bpf-ci
1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-05-15 15:37 UTC (permalink / raw)
To: Faisal Bukhari; +Cc: bpf
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
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] selftests/bpf: fix tunnel option size in ip6geneve_set_tunnel()
2026-05-15 15:18 [PATCH] selftests/bpf: fix tunnel option size in ip6geneve_set_tunnel() Faisal Bukhari
2026-05-15 15:37 ` sashiko-bot
@ 2026-05-15 16:01 ` bot+bpf-ci
1 sibling, 0 replies; 3+ messages in thread
From: bot+bpf-ci @ 2026-05-15 16:01 UTC (permalink / raw)
To: faisalbukhari523, ast, daniel, andrii, martin.lau, eddyz87,
memxor, shuah
Cc: bpf, linux-kselftest, linux-kernel, ast, andrii, daniel,
martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 3077 bytes --]
> commit 5f750f9a85fff89d7c4960e06d838efe58b8a805
> 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.
>
> Signed-off-by: Faisal Bukhari <faisalbukhari523@gmail.com>
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.
This looks like a bug fix. Should this include:
Fixes: 5ddafcc377f9 ("selftests/bpf: Fix a few tests for GCC related warnings.")
> diff --git a/tools/testing/selftests/bpf/progs/test_tunnel_kern.c b/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
> index 30f1de458669..ca04d1d1ade4 100644
> --- a/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
> +++ b/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
> @@ -674,7 +674,7 @@ int ip6geneve_set_tunnel(struct __sk_buff *skb)
> gopt->length = 2; /* 4-byte multiple */
> *(int *) &gopt->opt_data = bpf_htonl(0xfeedbeef);
>
> - ret = bpf_skb_set_tunnel_opt(skb, gopt, sizeof(gopt));
> + ret = bpf_skb_set_tunnel_opt(skb, gopt, sizeof(local_gopt));
> if (ret < 0) {
> log_err(ret);
> return TC_ACT_SHOT;
Does this construct a malformed Geneve packet? According to 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?
Looking at ip6geneve_get_tunnel(), does the receiver test 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:
ip6geneve_get_tunnel() {
struct geneve_opt gopt;
ret = bpf_skb_get_tunnel_opt(skb, &gopt, sizeof(gopt));
if (ret < 0)
gopt.opt_class = 0;
}
It seems the test might be ignoring this error and bypassing the verification
of the Geneve options entirely.
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/25926759989
^ permalink raw reply [flat|nested] 3+ messages in thread