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

[Nam Cao <namcao@linutronix.de>]

Gabriele Monaco <gmonaco@redhat.com> writes:
> Well, this would be yet another hybrid requiring BPF monitors registration in
> the kernel like I proposed in the RFC [1], which was the main point of
> contention (that fully shared reactors and sysfs interface, by the way).
>
> For this I would argue, though, that such monitors would be BPF just on paper,
> we wouldn't be able to use many advantages BPF provides (e.g. BPF maps).
>
> The current implementation already shares the header with in-kernel monitors, so
> the actual logic is shared. What isn't shared is the storage type and some glue
> around it.
> I used BPF maps because they offer some nice flexibility especially around per-
> task and per-obj monitors. On top of using more battle tested code.
> I would like to keep that.
>
> Besides, we'd need to be extra careful on controlling the callers. Right now
> handlers are static, so it's easier to make sure no rogue caller messes up with
> monitoring.
>
> What else definitely isn't shared, which is the main point of this whole
> conversation, is trace events.
> Those could be kfuncs. And no matter what you do, BPF monitors will need to
> share the trace events (unlike in-kernel that have separate events per monitor).
>
> In short, I don't think this would be easy to achieve and wouldn't earn us that
> much. (Plus I'd lose the hardly earned Alexei's blessing ;) )
>
> What do you think?

What about we refactor the in-kernel monitors' trace events into a
single one. I am picturing two functions for ALL classes and ALL
monitors:

    void rv_error(const char *monitor_name, const char *message);
    void rv_event(const char *monitor_name, const char *message);

which trigger the tracepoints. And we make those kfuncs. This allows
sharing trace events between in-kernel monitors and BPF monitors.

Then everything else (kernel reactors, users, perhaps even eBPF reactors
in the future) hook into those tracepoints.

Nam