* Re: [PATCH bpf-next 1/5] bpf: block bpf_get_[stack|stackid] on perf_event with PEBS entries
[not found] ` <20200711012639.3429622-2-songliubraving@fb.com>
@ 2020-07-11 3:53 ` Andrii Nakryiko
2020-07-11 6:28 ` Song Liu
0 siblings, 1 reply; 4+ messages in thread
From: Andrii Nakryiko @ 2020-07-11 3:53 UTC (permalink / raw)
To: Song Liu
Cc: open list, bpf, Networking, Alexei Starovoitov, Daniel Borkmann,
Kernel Team, john fastabend, KP Singh, Jesper Dangaard Brouer,
Peter Ziljstra
On Fri, Jul 10, 2020 at 6:30 PM Song Liu <songliubraving@fb.com> wrote:
>
> Calling get_perf_callchain() on perf_events from PEBS entries may cause
> unwinder errors. To fix this issue, the callchain is fetched early. Such
> perf_events are marked with __PERF_SAMPLE_CALLCHAIN_EARLY.
>
> Similarly, calling bpf_get_[stack|stackid] on perf_events from PEBS may
> also cause unwinder errors. To fix this, block bpf_get_[stack|stackid] on
> these perf_events. Unfortunately, bpf verifier cannot tell whether the
> program will be attached to perf_event with PEBS entries. Therefore,
> block such programs during ioctl(PERF_EVENT_IOC_SET_BPF).
>
> Signed-off-by: Song Liu <songliubraving@fb.com>
> ---
Perhaps it's a stupid question, but why bpf_get_stack/bpf_get_stackid
can't figure out automatically that they are called from
__PERF_SAMPLE_CALLCHAIN_EARLY perf event and use different callchain,
if necessary?
It is quite suboptimal from a user experience point of view to require
two different BPF helpers depending on PEBS or non-PEBS perf events.
[...]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH bpf-next 1/5] bpf: block bpf_get_[stack|stackid] on perf_event with PEBS entries
2020-07-11 3:53 ` [PATCH bpf-next 1/5] bpf: block bpf_get_[stack|stackid] on perf_event with PEBS entries Andrii Nakryiko
@ 2020-07-11 6:28 ` Song Liu
2020-07-12 5:06 ` Andrii Nakryiko
0 siblings, 1 reply; 4+ messages in thread
From: Song Liu @ 2020-07-11 6:28 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: open list, bpf, Networking, Alexei Starovoitov, Daniel Borkmann,
Kernel Team, john fastabend, KP Singh, Jesper Dangaard Brouer,
Peter Ziljstra
> On Jul 10, 2020, at 8:53 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>
> On Fri, Jul 10, 2020 at 6:30 PM Song Liu <songliubraving@fb.com> wrote:
>>
>> Calling get_perf_callchain() on perf_events from PEBS entries may cause
>> unwinder errors. To fix this issue, the callchain is fetched early. Such
>> perf_events are marked with __PERF_SAMPLE_CALLCHAIN_EARLY.
>>
>> Similarly, calling bpf_get_[stack|stackid] on perf_events from PEBS may
>> also cause unwinder errors. To fix this, block bpf_get_[stack|stackid] on
>> these perf_events. Unfortunately, bpf verifier cannot tell whether the
>> program will be attached to perf_event with PEBS entries. Therefore,
>> block such programs during ioctl(PERF_EVENT_IOC_SET_BPF).
>>
>> Signed-off-by: Song Liu <songliubraving@fb.com>
>> ---
>
> Perhaps it's a stupid question, but why bpf_get_stack/bpf_get_stackid
> can't figure out automatically that they are called from
> __PERF_SAMPLE_CALLCHAIN_EARLY perf event and use different callchain,
> if necessary?
>
> It is quite suboptimal from a user experience point of view to require
> two different BPF helpers depending on PEBS or non-PEBS perf events.
I am not aware of an easy way to tell the difference in bpf_get_stack.
But I do agree that would be much better.
Thanks,
Song
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH bpf-next 1/5] bpf: block bpf_get_[stack|stackid] on perf_event with PEBS entries
2020-07-11 6:28 ` Song Liu
@ 2020-07-12 5:06 ` Andrii Nakryiko
2020-07-12 6:34 ` Song Liu
0 siblings, 1 reply; 4+ messages in thread
From: Andrii Nakryiko @ 2020-07-12 5:06 UTC (permalink / raw)
To: Song Liu
Cc: open list, bpf, Networking, Alexei Starovoitov, Daniel Borkmann,
Kernel Team, john fastabend, KP Singh, Jesper Dangaard Brouer,
Peter Ziljstra
On Fri, Jul 10, 2020 at 11:28 PM Song Liu <songliubraving@fb.com> wrote:
>
>
>
> > On Jul 10, 2020, at 8:53 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> >
> > On Fri, Jul 10, 2020 at 6:30 PM Song Liu <songliubraving@fb.com> wrote:
> >>
> >> Calling get_perf_callchain() on perf_events from PEBS entries may cause
> >> unwinder errors. To fix this issue, the callchain is fetched early. Such
> >> perf_events are marked with __PERF_SAMPLE_CALLCHAIN_EARLY.
> >>
> >> Similarly, calling bpf_get_[stack|stackid] on perf_events from PEBS may
> >> also cause unwinder errors. To fix this, block bpf_get_[stack|stackid] on
> >> these perf_events. Unfortunately, bpf verifier cannot tell whether the
> >> program will be attached to perf_event with PEBS entries. Therefore,
> >> block such programs during ioctl(PERF_EVENT_IOC_SET_BPF).
> >>
> >> Signed-off-by: Song Liu <songliubraving@fb.com>
> >> ---
> >
> > Perhaps it's a stupid question, but why bpf_get_stack/bpf_get_stackid
> > can't figure out automatically that they are called from
> > __PERF_SAMPLE_CALLCHAIN_EARLY perf event and use different callchain,
> > if necessary?
> >
> > It is quite suboptimal from a user experience point of view to require
> > two different BPF helpers depending on PEBS or non-PEBS perf events.
>
> I am not aware of an easy way to tell the difference in bpf_get_stack.
> But I do agree that would be much better.
>
Hm... Looking a bit more how all this is tied together in the kernel,
I think it's actually quite easy. So, for perf_event BPF program type:
1. return a special prototype for bpf_get_stack/bpf_get_stackid, which
will have this extra bit of logic for callchain. All other program
types with access to bpf_get_stack/bpf_get_stackid should use the
current one, probably.
2. For that special program, just like for bpf_read_branch_records(),
we know that context is actually `struct bpf_perf_event_data_kern *`,
and it has pt_regs, perf_sample_data and perf_event itself.
3. With that, it seems like you'll have everything you need to
automatically choose a proper callchain.
All this absolutely transparently to the BPF program.
Am I missing something?
> Thanks,
> Song
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH bpf-next 1/5] bpf: block bpf_get_[stack|stackid] on perf_event with PEBS entries
2020-07-12 5:06 ` Andrii Nakryiko
@ 2020-07-12 6:34 ` Song Liu
0 siblings, 0 replies; 4+ messages in thread
From: Song Liu @ 2020-07-12 6:34 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: open list, bpf, Networking, Alexei Starovoitov, Daniel Borkmann,
Kernel Team, john fastabend, KP Singh, Jesper Dangaard Brouer,
Peter Ziljstra
> On Jul 11, 2020, at 10:06 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>
> On Fri, Jul 10, 2020 at 11:28 PM Song Liu <songliubraving@fb.com> wrote:
>>
>>
>>
>>> On Jul 10, 2020, at 8:53 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>>>
>>> On Fri, Jul 10, 2020 at 6:30 PM Song Liu <songliubraving@fb.com> wrote:
>>>>
>>>> Calling get_perf_callchain() on perf_events from PEBS entries may cause
>>>> unwinder errors. To fix this issue, the callchain is fetched early. Such
>>>> perf_events are marked with __PERF_SAMPLE_CALLCHAIN_EARLY.
>>>>
>>>> Similarly, calling bpf_get_[stack|stackid] on perf_events from PEBS may
>>>> also cause unwinder errors. To fix this, block bpf_get_[stack|stackid] on
>>>> these perf_events. Unfortunately, bpf verifier cannot tell whether the
>>>> program will be attached to perf_event with PEBS entries. Therefore,
>>>> block such programs during ioctl(PERF_EVENT_IOC_SET_BPF).
>>>>
>>>> Signed-off-by: Song Liu <songliubraving@fb.com>
>>>> ---
>>>
>>> Perhaps it's a stupid question, but why bpf_get_stack/bpf_get_stackid
>>> can't figure out automatically that they are called from
>>> __PERF_SAMPLE_CALLCHAIN_EARLY perf event and use different callchain,
>>> if necessary?
>>>
>>> It is quite suboptimal from a user experience point of view to require
>>> two different BPF helpers depending on PEBS or non-PEBS perf events.
>>
>> I am not aware of an easy way to tell the difference in bpf_get_stack.
>> But I do agree that would be much better.
>>
>
> Hm... Looking a bit more how all this is tied together in the kernel,
> I think it's actually quite easy. So, for perf_event BPF program type:
>
> 1. return a special prototype for bpf_get_stack/bpf_get_stackid, which
> will have this extra bit of logic for callchain. All other program
> types with access to bpf_get_stack/bpf_get_stackid should use the
> current one, probably.
> 2. For that special program, just like for bpf_read_branch_records(),
> we know that context is actually `struct bpf_perf_event_data_kern *`,
> and it has pt_regs, perf_sample_data and perf_event itself.
> 3. With that, it seems like you'll have everything you need to
> automatically choose a proper callchain.
>
> All this absolutely transparently to the BPF program.
>
> Am I missing something?
Good idea! A separate prototype should work here.
Thanks,
Song
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-07-12 6:35 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20200711012639.3429622-1-songliubraving@fb.com>
[not found] ` <20200711012639.3429622-2-songliubraving@fb.com>
2020-07-11 3:53 ` [PATCH bpf-next 1/5] bpf: block bpf_get_[stack|stackid] on perf_event with PEBS entries Andrii Nakryiko
2020-07-11 6:28 ` Song Liu
2020-07-12 5:06 ` Andrii Nakryiko
2020-07-12 6:34 ` Song Liu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).