Re: [PATCH 2/3] perf: collect BPF metadata from existing BPF programs

[Blake Jones]

Hi Namhyung,

On Tue, Jun 3, 2025 at 1:15 PM Namhyung Kim <namhyung@xxxxxxxxxx> wrote:
>
> On Wed, May 21, 2025 at 03:27:24PM -0700, Blake Jones wrote:
> > Look for .rodata maps, find ones with 'bpf_metadata_' variables, extract
> > their values as strings, and create a new PERF_RECORD_BPF_METADATA
> > synthetic event using that data. The code gets invoked from the existing
> > routine perf_event__synthesize_one_bpf_prog().
>
> It would be great if you can show an example how those metadata is
> constructed and shared between BPF programs.

I've added the following to my commit message:

| For example, a BPF program with the following variables:
|
|     const char bpf_metadata_version[] SEC(".rodata") = "3.14159";
|     int bpf_metadata_value[] SEC(".rodata") = 42;
|
| would generate a PERF_RECORD_BPF_METADATA record with:
|
|     .prog_name        = <BPF program name, e.g. "bpf_prog_a1b2c3_foo">
|     .nr_entries       = 2
|     .entries[0].key   = "version"
|     .entries[0].value = "3.14159"
|     .entries[1].key   = "value"
|     .entries[1].value = "42"
|
| Each of the BPF programs and subprograms that share those variables would
| get a distinct PERF_RECORD_BPF_METADATA record, with the ".prog_name" showing
| the name of each program or subprogram. The prog_name is deliberately the
| same as the ".name" field in the corresponding PERF_RECORD_KSYMBOL record.

> IIUC the metadata is collected for each BPF program which may have
> multiple subprograms.  Then this patch creates multiple PERF_RECORD_
> BPF_METADATA for each subprogram, right?
>
> Can it be shared using the BPF program ID?

In theory, yes, it could be shared. But I want to be able to correlate them
with the corresponding PERF_RECORD_KSYMBOL events, and KSYMBOL events for
subprograms don't have the full-program ID, so I wouldn't be able to do that.

> > +     rodata = calloc(1, map_info.value_size);
>
> You can use 'zalloc()' instead, in other places too.

Fixed, thanks.

> > +void bpf_metadata_free(struct bpf_metadata *metadata)
> > +{
> > +     if (metadata == NULL)
> > +             return;
> > +     for (__u32 index = 0; index < metadata->nr_prog_names; index++)
> > +             free(metadata->prog_names[index]);
> > +     if (metadata->prog_names != NULL)
> > +             free(metadata->prog_names);
> > +     if (metadata->event != NULL)
> > +             free(metadata->event);
>
> No need to NULL change for free().

I've removed the NULL checks.

> > +static int synthesize_perf_record_bpf_metadata(
> > [...]
> > +     for (__u32 index = 0; index < metadata->nr_prog_names; index++) {
> > +             memcpy(event->bpf_metadata.prog_name,
> > +                    metadata->prog_names[index], BPF_PROG_NAME_LEN);
>
> Is it possible to call synthesize_bpf_prog_name() directly to the
> event->bpf_metadata.prog_name instead of saving it metadata->prog_names?

Not with the way the code is currently structured - we need the BTF data
to call synthesize_bpf_prog_name(), and that's allocated and freed inside
of bpf_metadata_create().

Blake