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

[bot+bpf-ci@kernel.org]

[-- 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