From: KaFai Wan <kafai.wan@linux.dev>
To: Yonghong Song <yonghong.song@linux.dev>,
ast@kernel.org, daniel@iogearbox.net, john.fastabend@gmail.com,
andrii@kernel.org, martin.lau@linux.dev, eddyz87@gmail.com,
song@kernel.org, kpsingh@kernel.org, sdf@fomichev.me,
haoluo@google.com, jolsa@kernel.org, mrpre@163.com,
bpf@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: Felix Fietkau <nbd@nbd.name>
Subject: Re: [PATCH bpf-next 1/1] bpf: Allow fall back to interpreter for programs with stack size <= 512
Date: Wed, 06 Aug 2025 18:57:43 +0800 [thread overview]
Message-ID: <c8c870e25c07aee5c84c84aa62cebd655ff53f50.camel@linux.dev> (raw)
In-Reply-To: <401418b7-248c-42a3-ba74-9b2b2959e36c@linux.dev>
On Tue, 2025-08-05 at 10:45 -0700, Yonghong Song wrote:
>
>
> On 8/5/25 4:55 AM, KaFai Wan wrote:
> > OpenWRT users reported regression on ARMv6 devices after updating
> > to latest
> > HEAD, where tcpdump filter:
> >
> > tcpdump -i mon1 \
> > "not wlan addr3 3c37121a2b3c and not wlan addr2 184ecbca2a3a \
> > and not wlan addr2 14130b4d3f47 and not wlan addr2 f0f61cf440b7 \
> > and not wlan addr3 a84b4dedf471 and not wlan addr3 d022be17e1d7 \
> > and not wlan addr3 5c497967208b and not wlan addr2 706655784d5b"
> >
> > fails with warning: "Kernel filter failed: No error information"
> > when using config:
> > # CONFIG_BPF_JIT_ALWAYS_ON is not set
> > CONFIG_BPF_JIT_DEFAULT_ON=y
> >
> > The issue arises because commits:
> > 1. "bpf: Fix array bounds error with may_goto" changed default
> > runtime to
> > __bpf_prog_ret0_warn when jit_requested = 1
> > 2. "bpf: Avoid __bpf_prog_ret0_warn when jit fails" returns error
> > when
> > jit_requested = 1 but jit fails
> >
> > This change restores interpreter fallback capability for BPF
> > programs with
> > stack size <= 512 bytes when jit fails.
> >
> > Reported-by: Felix Fietkau <nbd@nbd.name>
> > Closes:
> > https://lore.kernel.org/bpf/2e267b4b-0540-45d8-9310-e127bf95fc63@nbd.name/
> > Fixes: 6ebc5030e0c5 ("bpf: Fix array bounds error with may_goto")
> > Fixes: 86bc9c742426 ("bpf: Avoid __bpf_prog_ret0_warn when jit
> > fails")
> > Signed-off-by: KaFai Wan <kafai.wan@linux.dev>
> > ---
> > kernel/bpf/core.c | 12 +++++++-----
> > 1 file changed, 7 insertions(+), 5 deletions(-)
> >
> > diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> > index 5d1650af899d..2d86bd4b0b97 100644
> > --- a/kernel/bpf/core.c
> > +++ b/kernel/bpf/core.c
> > @@ -2366,8 +2366,8 @@ static unsigned int
> > __bpf_prog_ret0_warn(const void *ctx,
> > const struct bpf_insn
> > *insn)
> > {
> > /* If this handler ever gets executed, then
> > BPF_JIT_ALWAYS_ON
> > - * is not working properly, or interpreter is being used
> > when
> > - * prog->jit_requested is not 0, so warn about it!
> > + * or may_goto may cause stack size > 512 is not working
> > properly,
> > + * so warn about it!
> > */
> > WARN_ON_ONCE(1);
> > return 0;
> > @@ -2478,10 +2478,10 @@ static void bpf_prog_select_func(struct
> > bpf_prog *fp)
> > * But for non-JITed programs, we don't need bpf_func, so
> > no bounds
> > * check needed.
> > */
> > - if (!fp->jit_requested &&
> > - !WARN_ON_ONCE(idx >= ARRAY_SIZE(interpreters))) {
> > + if (idx < ARRAY_SIZE(interpreters)) {
> > fp->bpf_func = interpreters[idx];
> > } else {
> > + WARN_ON_ONCE(!fp->jit_requested);
> > fp->bpf_func = __bpf_prog_ret0_warn;
> > }
>
> Your logic here is to do interpreter even if fp->jit_requested is
> true.
> This is different from the current implementation.
>
> Also see below code:
>
> static unsigned int __bpf_prog_ret0_warn(const void *ctx,
> const struct bpf_insn
> *insn)
> {
> /* If this handler ever gets executed, then
> BPF_JIT_ALWAYS_ON
> * is not working properly, or interpreter is being used
> when
> * prog->jit_requested is not 0, so warn about it!
> */
> WARN_ON_ONCE(1);
> return 0;
> }
>
>
> It mentions to warn if the interpreter is being used when
> prog->jit_requested is not 0.
>
> So if prog->jit_requested is not 0, it is expected not to use
> interpreter.
>
The commit 6ebc5030e0c5 ("bpf: Fix array bounds error with may_goto")
[1] this patch fix change the code to that, before this commit it was:
static unsigned int __bpf_prog_ret0_warn(const void *ctx,
const struct bpf_insn *insn)
{
/* If this handler ever gets executed, then BPF_JIT_ALWAYS_ON
* is not working properly, so warn about it!
*/
WARN_ON_ONCE(1);
return 0;
}
And
static void bpf_prog_select_func(struct bpf_prog *fp)
{
#ifndef CONFIG_BPF_JIT_ALWAYS_ON
u32 stack_depth = max_t(u32, fp->aux->stack_depth, 1);
fp->bpf_func = interpreters[(round_up(stack_depth, 32) / 32) -
1];
#else
fp->bpf_func = __bpf_prog_ret0_warn;
#endif
}
so it can fall back to the interpreter when jit fails. And this fit the
intent of bpf_prog_select_runtime(), see comment:
/**
* bpf_prog_select_runtime - select exec runtime for BPF program
* @fp: bpf_prog populated with BPF program
* @err: pointer to error variable
*
* Try to JIT eBPF program, if JIT is not available, use interpreter.
* The BPF program will be executed via bpf_prog_run() function.
*
* Return: the &fp argument along with &err set to 0 for success or
* a negative errno code on failure
*/
struct bpf_prog *bpf_prog_select_runtime(struct bpf_prog *fp, int *err)
And this:
bpf_prog_select_func(fp);
/* eBPF JITs can rewrite the program in case constant
* blinding is active. However, in case of error during
* blinding, bpf_int_jit_compile() must always return a
* valid program, which in this case would simply not
* be JITed, but falls back to the interpreter.
*/
if (!bpf_prog_is_offloaded(fp->aux)) {
The commit [1] mismatch the intent of bpf_prog_select_runtime(), so it
should be fixed.
[1] https://lore.kernel.org/all/20250214091823.46042-2-mrpre@163.com/
>
> > #else
> > @@ -2505,7 +2505,7 @@ struct bpf_prog
> > *bpf_prog_select_runtime(struct bpf_prog *fp, int *err)
> > /* In case of BPF to BPF calls, verifier did all the prep
> > * work with regards to JITing, etc.
> > */
> > - bool jit_needed = fp->jit_requested;
> > + bool jit_needed = false;
> >
> > if (fp->bpf_func)
> > goto finalize;
> > @@ -2515,6 +2515,8 @@ struct bpf_prog
> > *bpf_prog_select_runtime(struct bpf_prog *fp, int *err)
> > jit_needed = true;
> >
> > bpf_prog_select_func(fp);
> > + if (fp->bpf_func == __bpf_prog_ret0_warn)
> > + jit_needed = true;
> >
> > /* eBPF JITs can rewrite the program in case constant
> > * blinding is active. However, in case of error during
>
--
Thanks,
KaFai
next prev parent reply other threads:[~2025-08-06 10:57 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-08-05 11:55 [PATCH bpf-next 1/1] bpf: Allow fall back to interpreter for programs with stack size <= 512 KaFai Wan
2025-08-05 17:45 ` Yonghong Song
2025-08-06 10:57 ` KaFai Wan [this message]
2025-08-06 17:37 ` Yonghong Song
2025-08-07 16:50 ` Alexei Starovoitov
2025-08-11 11:28 ` KaFai Wan
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=c8c870e25c07aee5c84c84aa62cebd655ff53f50.camel@linux.dev \
--to=kafai.wan@linux.dev \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=eddyz87@gmail.com \
--cc=haoluo@google.com \
--cc=john.fastabend@gmail.com \
--cc=jolsa@kernel.org \
--cc=kpsingh@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=martin.lau@linux.dev \
--cc=mrpre@163.com \
--cc=nbd@nbd.name \
--cc=sdf@fomichev.me \
--cc=song@kernel.org \
--cc=yonghong.song@linux.dev \
/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.