All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yonghong Song <yonghong.song@linux.dev>
To: bot+bpf-ci@kernel.org, bpf@vger.kernel.org
Cc: ast@kernel.org, andrii@kernel.org, daniel@iogearbox.net,
	eddyz87@gmail.com, kernel-team@fb.com, martin.lau@kernel.org,
	mason@kernel.org, ihor.solodrai@linux.dev
Subject: Re: [PATCH bpf-next v3 02/15] selftests/bpf: Add a test for an __int128 by-value argument
Date: Sat, 12 Sep 2026 10:07:06 -0700	[thread overview]
Message-ID: <b2ad4fe4-6c25-4614-b07c-8f078d81e455@linux.dev> (raw)
In-Reply-To: <034515b18553e09288479d75768b362839532503a5ca15889c035acdcdf41b06@mail.kernel.org>



On 9/11/26 9:47 AM, bot+bpf-ci@kernel.org wrote:
>> diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
>> index f7f94ccebce2..3c7ded537314 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/verifier.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
>> @@ -5,6 +5,7 @@
>>   #include "arena_kfunc.skel.h"
>>   #include "arena_kfunc_jit.skel.h"
>>   #include "cap_helpers.h"
>> +#include "verifier_aggregate_arg.skel.h"
>>   #include "verifier_aggregate_ret.skel.h"
>>   #include "verifier_align.skel.h"
>>   #include "verifier_and.skel.h"
>> @@ -171,6 +172,7 @@ void test_arena_kfunc(void)                   { RUN_TESTS(arena_kfunc); }
>>
>>   void test_arena_kfunc_jit(void)               { RUN_TESTS(arena_kfunc_jit); }
>>
>> +void test_verifier_aggregate_arg(void)        { RUN_TESTS(verifier_aggregate_arg); }
>>   void test_verifier_aggregate_ret(void)        { RUN_TESTS(verifier_aggregate_ret); }
>>   void test_verifier_align(void)                { RUN(verifier_align); }
>>   void test_verifier_and(void)                  { RUN(verifier_and); }
>> diff --git a/tools/testing/selftests/bpf/progs/verifier_aggregate_arg.c b/tools/testing/selftests/bpf/progs/verifier_aggregate_arg.c
>> new file mode 100644
>> index 000000000000..d90f754396d0
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/progs/verifier_aggregate_arg.c
>> @@ -0,0 +1,40 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */
>> +#include <vmlinux.h>
>> +#include <bpf/bpf_helpers.h>
>> +#include "bpf_misc.h"
>> +
>> +#define MIX_A	0xdeadbeefcafef00dULL
>> +#define MIX_B	0x0123456789abcdefULL
>> +
>> +#ifdef __SIZEOF_INT128__
>> +
>> +typedef unsigned __int128 u128;
>> +
>> +__noinline __u64 take_i128_global(int a, u128 v, int c)
>> +{
>> +	return (__u64)a + (__u64)(v >> 64) + (__u64)v + c;
>> +}
>> +
>> +SEC("tc")
>> +/*
>> + * The verifier counts one argument register for the __int128 and marks only
>> + * R1 through R3 at the entry of take_i128_global(), while the compiler passed
>> + * a in R1, v in R2:R3 and c in R4.
>> + */
>> +__failure __msg("R4 !read_ok")
>> +int aggregate_arg_int128_c_test(struct __sk_buff *skb)
>> +{
>> +	__u64 a = skb->len ^ MIX_A;
>> +	__u64 b = skb->len ^ MIX_B;
>> +	u128 v = ((u128)a << 64) | b;
>> +
>> +	if (take_i128_global(1, v, 2) != a + b + 3)
>> +		return 1;
>> +
>> +	return 0;
>> +}
>> +
>> +#endif /* __SIZEOF_INT128__ */
>> +
>> +char _license[] SEC("license") = "GPL";
> A subsystem pattern flags this as potentially concerning: every program
> in the new file is wrapped in '#ifdef __SIZEOF_INT128__' with no '#else'
> branch.
>
> When a BPF compiler does not define __SIZEOF_INT128__ (for example, when
> building with bpf-gcc via BPF_GCC=1), the generated verifier_aggregate_arg.bpf.o
> contains no programs at all. The skeleton still compiles because bpftool
> guards maps/progs/links members on their counts, but test_loader__run_subtests()
> then iterates zero programs and test_progs.c reports the test as passing with
> no indication that the __int128 coverage was compiled out.
>
> Every sibling prog file in progs/ that wraps all of its tests in a feature
> #ifdef adds an '#else' dummy carrying __description()/__skip() so the result
> is a visible SKIP instead. For instance, verifier_aggregate_ret.c (from the
> same series) includes such a dummy at lines 167-175 with the message "needs
> LLVM 23, dummy test". The same pattern appears in verifier_gotol.c,
> verifier_bswap.c, verifier_ldsx.c, verifier_movsx.c, verifier_sdiv.c,
> verifier_private_stack.c, verifier_stack_arg_order.c, verifier_load_acquire.c,
> verifier_store_release.c, arena_kfunc.c, stack_arg_precision.c, and
> stack_arg_fail.c.
>
> Should this file include an '#else' dummy with __skip() to match the
> pattern used by its sibling tests, so that builds without __SIZEOF_INT128__
> report SKIP rather than silently passing?
>
> Note: __SIZEOF_INT128__ is defined by every clang for the bpf target, so
> the normal CI matrix (clang test_progs, no_alu32, cpuv4) does compile and
> run the test. Only the optional bpf-gcc flavor would be affected.

This should be okay. Later selftest will have more contents in file verifier_aggregate_arg.c
which has some not-guarded bpf programs.

>
>
> ---
> 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/34620351527


  reply	other threads:[~2026-09-12 17:07 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-11 15:49 [PATCH bpf-next v3 00/15] bpf: Support by-value struct and __int128 arguments Yonghong Song
2026-09-11 15:49 ` [PATCH bpf-next v3 01/15] bpf: Read a kfunc's __sz argument only when it is in a register Yonghong Song
2026-09-11 15:49 ` [PATCH bpf-next v3 02/15] selftests/bpf: Add a test for an __int128 by-value argument Yonghong Song
2026-09-11 16:47   ` bot+bpf-ci
2026-09-12 17:07     ` Yonghong Song [this message]
2026-09-11 15:49 ` [PATCH bpf-next v3 03/15] bpf: Rename bpf_subprog_info::arg_cnt to arg_slot_cnt Yonghong Song
2026-09-11 15:49 ` [PATCH bpf-next v3 04/15] bpf: Index global function arguments by argument slot Yonghong Song
2026-09-11 15:49 ` [PATCH bpf-next v3 05/15] bpf: Support by-value struct arguments up to 16 bytes Yonghong Song
2026-09-11 15:49 ` [PATCH bpf-next v3 06/15] bpf: Support __int128 as a by-value function argument Yonghong Song
2026-09-11 15:49 ` [PATCH bpf-next v3 07/15] bpf: Rename bpf_call_summary::num_params to arg_slot_cnt Yonghong Song
2026-09-11 15:49 ` [PATCH bpf-next v3 08/15] bpf: Recognize by-value struct and __int128 kfunc arguments Yonghong Song
2026-09-11 16:47   ` bot+bpf-ci
2026-09-12 17:13     ` Yonghong Song
2026-09-11 15:50 ` [PATCH bpf-next v3 09/15] bpf: Prepare kfunc arguments for the JIT from an ABI description Yonghong Song
2026-09-11 15:50 ` [PATCH bpf-next v3 10/15] bpf, x86: Move kfunc arguments into the x86-64 calling convention Yonghong Song
2026-09-11 16:47   ` bot+bpf-ci
2026-09-12 17:14     ` Yonghong Song
2026-09-11 15:50 ` [PATCH bpf-next v3 11/15] bpf, arm64: Move kfunc arguments into the arm64 " Yonghong Song
2026-09-11 16:19   ` sashiko-bot
2026-09-12 17:16     ` Yonghong Song
2026-09-11 16:47   ` bot+bpf-ci
2026-09-12 17:19     ` Yonghong Song
2026-09-11 15:50 ` [PATCH bpf-next v3 12/15] selftests/bpf: Add C tests for by-value arguments up to 16 bytes Yonghong Song
2026-09-11 15:50 ` [PATCH bpf-next v3 13/15] selftests/bpf: Add inline-asm tests for by-value arguments Yonghong Song
2026-09-11 16:06   ` sashiko-bot
2026-09-11 15:50 ` [PATCH bpf-next v3 14/15] selftests/bpf: Add tests for by-value kfunc arguments Yonghong Song
2026-09-11 16:47   ` bot+bpf-ci
2026-09-12 17:24     ` Yonghong Song
2026-09-11 15:50 ` [PATCH bpf-next v3 15/15] selftests/bpf: Temporary hack to disable register mismatch in arm64 Yonghong Song
2026-09-11 16:47   ` bot+bpf-ci
2026-09-12  3:57   ` Alexei Starovoitov
2026-09-12 17:30     ` Yonghong Song

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b2ad4fe4-6c25-4614-b07c-8f078d81e455@linux.dev \
    --to=yonghong.song@linux.dev \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bot+bpf-ci@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=ihor.solodrai@linux.dev \
    --cc=kernel-team@fb.com \
    --cc=martin.lau@kernel.org \
    --cc=mason@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.