Re: [EXT.MSG] Re: [PATCH RFC bpf-next 0/2] bpf: Do not let preemption cause false recursion drops in trampoline progs

[Andrii Nakryiko]

(resending as plain text, hopefully)

On Thu, Nov 5, 2026 at 3:35 PM Mike Bazov <mikeba@xxxxxxxxxxxxxxx> wrote:
>
> On Thu Nov 5, 2026 at 5:14 AM UTC, Alexei Starovoitov wrote:
>    > Also try NO_HRTICK in /sys/kernel/debug/sched/features.
>    > That may be decent workaround for now.
>
>    The 7.0 numbers I posted were already with NO_HRTICK, and either way
>    our agent runs on customer machines, so changing scheduler settings
>    there isn't an option for us.
>
>    > hmm. you really cannot tolerate that small drop? it's tracing at the end.
>
>    For security monitoring each skip is an event we never see, and the one
>    we miss can be exactly the one that shows an attack. For stateful
>    protocol parsing, one skipped sendmsg/recvmsg can desync the parser for
>    the rest of that connection.
>
>    > let's try to think how per-task counter would work.
>    > I'm guessing 128 connections is 128 threads servicing them,
>    > so 200 cpus will be hammering the same per-task counter which will
>    > cause real perf loss.
>    > Gotta do better than per-task.
>    > Also it needs to be per-task + per-prog. How will it look like?
>
>    On Thu, Nov 05, 2026 at 01:26 PM Peter Zijlstra wrote:
>    > I would strongly prefer all BPF programs that attach to known
>    > preemptible sites to use a per-task recursion counter or something of
>    > that sort.
>

I agree, per-cpu recursion counter doesn't work given most of BPF
programs only disable migration, not preemption (even if
non-sleepable).

>    This is what I have in mind for v2:
>
>    - Every trampoline and raw_tp run already has a run ctx on the running
>      task's stack. Add the prog pointer to it, and link the task's active
>      run ctxs through one new pointer in task_struct (other prog types
>      don't touch it), so each task has a short list of the progs it is
>      currently inside.

hm, funny how AI figured out run_ctx, but didn't figure out that we
already chain them and we do have that pointer in current->bpf_ctx...

but yes, I think this is the only practical approach for per-task
recursion check. performance-wise it should be ok as normally we
shouldn't have nested bpf program executions, so the hot path will be
quick.

>    - On entry, walk only the current task's list. If the prog is already
>      there, it's real recursion (the same task, or an irq/nmi on top of it,
>      since current doesn't change there): skip it, as today. Otherwise push
>      the entry, run the prog, and pop it on exit.
>    - A preempted task's entry stays on its own list, so another task on
>      the same cpu is no longer blocked by it.
>    - Cost: the list is empty unless the task is genuinely nested, so the
>      check is one load and one predictable branch, with no atomics and
>      nothing shared between tasks or cpus. The walk is capped (e.g. 8),
>      which also bounds nesting depth. It also works across sleep for
>      sleepable progs.
>
>    To keep the per-cpu guarantee for existing progs:

I don't think per-cpu "guarantee" was ever there. per-program/per-cpu
recursion protection only "protected" (by dropping event, effectively,
so-so tradeoff) a case when per-cpu map is used exclusively from
single BPF program. But even then, there are BPF program types (like,
I believe raw tracepoint programs) that intentionally didn't have
recursion protection.

Realistically, you have to have some allowance for having nested
per-cpu map uses, e.g., in wprof I maintain a small stack of per-cpu
values, see stack_trace_scratch and embedded atomic st_cnt counter in
[0].

That's all to say that I don't think there is a need to keep
guarantees. If per-cpu map is used naively today it works ok most of
the time, but not all of the time.

  [0] https://github.com/anakryiko/wprof/blob/master/src/wprof.bpf.c#L574

>    - Make it opt-in: a prog load flag for fentry/fexit and raw_tp progs
>      (like BPF_F_SLEEPABLE) saying the prog may run concurrently on one
>      cpu, and that it doesn't rely on per-cpu exclusivity for its own data
>      (per-cpu scratch buffers etc.).
>    - Flagged progs get no private stack. The verifier could also enforce
>      parts of the per-cpu promise if you prefer.
>    - Unflagged progs keep today's prog->active check, so existing progs
>      see no change.
>
>    What do you think?
>

I'm actually wondering if we should re-evaluate the whole per-program
recursion protection. I think this was added to prevent deadlocks
inside bpf's maps and other internals. but we have switched to
rq_spin_lock since then, so that shouldn't be a reason.

Perhaps we should just get rid of that whole per-program recursion
protection and just make sure that a) BPF program can protect itself,
if necessary (with naive per-cpu check, I did that trick in retsnoop,
[1], because it was, and probably still is, possible to attach BPF
program to kernel functions that shouldn't be attachable by BPF,
leading to all sorts of issues; but I digress), and b) just make sure
that BPF maps and other functionality doesn't get deadlocked if
another BPF program got preempted in the same code path.

I didn't do any analysis if that's already the case (for b)), but
let's discuss if this is an option at all?

  [1] https://github.com/anakryiko/retsnoop/blob/master/src/mass_attach.bpf.c#L229-L243

>    Mike