Re: [PATCH bpf-next 2/3] selftests/bpf: Keep int return type for tailcall subprogs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Incomplete fix for compiler optimization: `classifier_0` does not consume `subprog_tail0` return value.
--

commit 889c5150d158afd20cd2f2219687b45dbae685e2
Author: Yonghong Song <yonghong.song@linux.dev>

selftests/bpf: Keep int return type for tailcall subprogs

This commit updates selftests to prevent the LLVM23 compiler from
optimizing away the return value of subprogs that issue a bpf_tail_call.
It does this by storing the return value into a global variable so the
compiler cannot change the subprog return type to void.

> diff --git a/tools/testing/selftests/bpf/progs/tailcall_bpf2bpf_hierarchy2.c b/tools/testing/selftests/bpf/progs/tailcall_bpf2bpf_hierarchy2.c
> index ae94c9c70ab7d..8f217bef3d62c 100644
> --- a/tools/testing/selftests/bpf/progs/tailcall_bpf2bpf_hierarchy2.c
> +++ b/tools/testing/selftests/bpf/progs/tailcall_bpf2bpf_hierarchy2.c
[ ... ]
> @@ -41,16 +44,21 @@ int classifier_0(struct __sk_buff *skb)

[Severity: Medium]
Does this miss an update to classifier_0()?

The commit message notes that "Where the caller does not already consume the
return value, store it into a global variable as well".

In this file, classifier_1() and tailcall_bpf2bpf_hierarchy_2() were
updated to store the return values into ret1 and ret2.

However, classifier_0() still calls subprog_tail0(skb) without consuming its
return value:

int classifier_0(struct __sk_buff *skb)
{
        count0++;
        subprog_tail0(skb);
        return 0;
}

Could this allow the compiler to still optimize away the return type of
subprog_tail0?

>  static __noinline
>  int subprog_tail1(struct __sk_buff *skb)
>  {
> +	int ret = 0;
> +
>  	bpf_tail_call_static(skb, &jmp_table, 1);
> -	return 0;
> +	barrier_var(ret);
> +	return ret;
>  }
>  
> +int ret1, ret2;
> +
>  __auxiliary
>  SEC("tc")
>  int classifier_1(struct __sk_buff *skb)
>  {
>  	count1++;
> -	subprog_tail1(skb);
> +	ret1 = subprog_tail1(skb);
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609163947.1717694-1-yonghong.song@linux.dev?part=2