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

[Gabriele Monaco <gmonaco@redhat.com>]

On Tue, 2026-10-06 at 13:29 +0000, Alexei Starovoitov wrote:
> On Thu, Oct 01, 2026 at 05:20 PM Gabriele Monaco <gmonaco@redhat.com> wrote:
> 
> Overall looks much better.
> Some of the code reimplements bpftool, but it's user space.
> It can be cleaned up later.

Thanks for going through these.
Indeed I tried to reduce the code reimplementing these things but probably I can
do better.

> > +$(BPF_DIR)/%.o: $(BPF_DIR)/%.c $(VMLINUX_H)
> > +	$(QUIET_CLANG)$(CLANG) $(BPF_CFLAGS) -c $< -o $@
> > +	$(Q)$(LLVM_STRIP) -g $@
> > +	$(Q)$(LLVM_OBJCOPY) --remove-section=.rel.rodata $@
> 
> What is this for?
> Released libbpf ignores it with "skipping relo section" message.
> Removing the section only hides that some pointer in .rodata is left
> without relocation. What is it? libbpf in bpf-next needs .rel.rodata to
> resolve pointers to functions. See commit b223044a68d5 ("libbpf: Resolve
> pointers to functions in read-only data").

Yeah indeed I used this to silence the warning. Apparently right now the warning
is gone (no relocation section even if I don't trim it), but I went back to the
initial version and it was about a bunch of .rodata.str1.1

I'm going to test a bit more but I can probably remove this.

> > +struct {
> > +	__uint(type, BPF_MAP_TYPE_HASH);
> > +	__uint(max_entries, 10240);
> > +	__type(key, da_id_type);
> > +	__type(value, struct da_monitor_storage_bpf);
> > +} rv_mon_map SEC(".maps");
> 
> Why hash by pid?
> Use BPF_MAP_TYPE_TASK_STORAGE for per-task monitors.
> Once there are 10240 tasks bpf_map_update_elem() in da_create_storage()
> fails, the error is ignored, and the rest of the tasks are silently
> not monitored.
> Task storage is freed with the task. No need for handle_obj_cleanup,
> id, target and PF_EXITING.

Great, I wasn't aware of it and looks much neater. Will use that.

I couldn't catch you yesterday at the conference, but I can summarise here what
I really wanted to ask you:

Reactors in this patch use bpf_printk(), which is in fact a trace_printk()
instead of a real printk, is there a technical limitation forbidding us to
create a kfunc using some flavour of printk_deferred()? Or am I missing an
existing one? (this need was brought up by John, Cc'd here)

Not needed in this patch but potentially required for more complex monitors. In
kernel, when timers are required, we allow monitors to use hrtimers (precise but
heavy), or the timer_wheel (lightweight but less precise).
I'm not aware of any wrapper of the wheel in BPF, but I think a few kfuncs for
these could be useful, in general, also for other programs needing a lot of
timer instances or simply light timeouts.
What do you think?

Thanks,
Gabriele