On Tue, Nov 1, 2022 at 3:17 PM Namhyung Kim <namhyung@xxxxxxxxxx> wrote: > > > > > > > > > > +1 > > > > > Let's avoid new stable helpers for this. > > > > > Pls use CORE and read perf_sample_data directly. > > > > > > > > We have legacy ways to access sample_period and addr with > > > > struct bpf_perf_event_data and struct bpf_perf_event_data_kern. I > > > > think mixing that > > > > with CORE makes it confusing for the user. And a helper or a kfunc would make it > > > > easier to follow. perf_btf might also be a good approach for this. > > > > > > imo that's a counter argument to non-CORE style. > > > struct bpf_perf_event_data has sample_period and addr, > > > and as soon as we pushed the boundaries it turned out it's not enough. > > > Now we're proposing to extend uapi a bit with sample_ip. > > > That will repeat the same mistake. > > > Just use CORE and read everything that is there today > > > and will be there in the future. > > > > Another work of this effort is that we need the perf_event to prepare > > required fields before calling the BPF program. I think we will need > > some logic in addition to CORE to get that right. How about we add > > perf_btf where the perf_event prepare all fields before calling the > > BPF program? perf_btf + CORE will be able to read all fields in the > > sample. > > IIUC we want something like below to access sample data directly, > right? > > BPF_CORE_READ(ctx, data, ip); > I haven't tried this, but I guess we may need something like data = ctx->data; BPF_CORE_READ(data, ip); > Some fields like raw and callchains will have variable length data > so it'd be hard to check the boundary at load time. I think we are fine as long as we can check boundaries at run time. > Also it's possible > that some fields are not set (according to sample type), and it'd be > the user's (or programmer's) responsibility to check if the data is > valid. If these are not the concerns, I think I'm good. So we still need 1/3 of the set to make sure the data is valid? Thanks, Song
