Re: [PATCH v2 10/15] tools/rv: Add BPF monitors

[Gabriele Monaco <gmonaco@redhat.com>]

On Mon, 2026-10-19 at 13:05 +0200, Tomas Glozar wrote:
> On Mon, Oct 19, 2026 at 12:43 PM Gabriele Monaco <gmonaco@redhat.com> wrote:
> > 
> > > One disadvantage of using bpf_trace_printk() for RV violation
> > > reporting is that the event will be caught by every instance that
> > > enables bpf_trace_printk() and does not explicitly filter rv out.
> > 
> > Yes, that should somewhat be mitigated by the glob filter, though events are
> > still generated.
> > 
> 
> Do I understand it correctly that the idea is to modify existing
> tracefs instances when enabling an RV monitor to filter our RV-related
> bpf_trace_printk events?

Not quite. The initial issue is that the current iteration of RV BPF monitor
don't cooperate well with trace-cmd/perf. Alexei suggested we use bpf_printk and
filter out the desired events with plain ftrace glob filters (from trace-
cmd/perf).

I believe this would be usually equivalent to having a dedicated tracepoint for
RV BPF monitors.

E.g. you'd do (roughly):

  trace-cmd -e bpf_prink --filter 'bpf_string ~ "rv: sleep: * violation"

instead of (with a new kfunc)

  trace-cmd -e rv:error_bpf --filter 'monitor = "sleep"'

Now, I didn't try either way and don't know what implications there are either
way (you seem to know better about this).


> > > I can imagine an RV-specific trace event for that, however, it would
> > > require a separate kfunc to generate it, and ideally also distinct
> > > labeling of a BPF program eligible to call that kfunc. The "rv"
> > > userspace tool could then iterate over these labels so the user knows
> > > which violation events can be expected to occur on the system.
> > 
> > That'd work too. It's what I initially envisioned but I'm not sure a kfunc
> > producing a tracepoint is something BPF folks would really like (and
> > technically
> > any other unrelated BPF program could call that, unless there's some access
> > control mechanism I'm not aware of).
> > 
> > Not sure how much it matters in practice against the glob filter, it
> > probably
> > really depends on the other bpf_printk users (and a mildly better
> > usability).
> > 
> 
> bpf_trace_printk is already a BPF helper producing a tracepoint, this
> would be equivalent.

Fair point.

> Of course, that does not mean we want it, but it
> is something we already have in at least one instance. I know of at
> least one quirk: you can have a BPF program attached to a tracepoint
> that will call bpf_trace_printk(), and a tracefs instance tracing both
> bpf_trace_printk() and the original tracepoint. That leads to a new
> tracepoint triggering while another tracepoint is already in progress.
> I don't know how it is currently handled; from what I recall when
> testing RTLA's BPF actions, the bpf_trace_printk() event might get
> dropped by the tracefs instance, so I had to use a different way to
> signal to userspace in BPF actions test.
> 
> As I said, to prevent unrelated BPF programs from calling the new
> tracepoint, you'd need to recognize and track RV BPF programs as a
> special type of program, similarly to what your original RFC did. I'm
> not sure if that's what we want, but I see some advantages.

Yeah, this last step is probably something we don't want.

Although it may not be such a bad thing: if we require enough privileges to
trigger this tracepoint, then the worst we get is root trying to mess up with
monitoring, where it could easily mess up with everything else already.
But it definitely needs more thinking.

> In addition to better filtering of reactor events, you could separate
> reactors from the monitors entirely: you could trigger the tracepoint
> unconditionally, and attach reactors as tracepoint triggers to the new
> tracepoint.

Something like this was already brought up actually and isn't a bad idea, it has
its strengths and issues but would somewhat work in-kernel as well.
But as you said, that's a bigger reactor refactoring and beyond the scope of
this submission.

Thanks,
Gabriele

> 
> All of that adds complexity, though, and requires separate discussion,
> so it's probably not something relevant for the initial
> implementation. I just wanted to mention it so that it doesn't get
> lost.
> 
> Tomas