Hi Blake,
On Tue, Jun 03, 2025 at 02:27:53PM -0700, Blake Jones wrote:
> 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.
Thanks!
>
> > 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.
It's unfortunate that KSYMBOL doesn't have the program ID, but IIRC the
following BPF_EVENT should have it. I think it's safe to think KSYMBOLs
belong to the BPF_EVENT when they are from the same thread.
>
> > > + 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().
I see. You already freed the map data and BTF. Ok, it's not a big deal
and probably not needed if we can switch to BPF ID.
Thanks,
Namhyung
