On Thu, Jan 23, 2020 at 11:44:53PM +0100, Daniel Borkmann wrote: > On 1/23/20 11:30 PM, Daniel Xu wrote: > > On Thu Jan 23, 2020 at 11:23 PM, Daniel Borkmann wrote: > > [...] > > > > > > Yes, so we've been following this practice for all the BPF helpers no > > > matter > > > which program type. Though for tracing it may be up to debate whether it > > > makes > > > still sense given there's nothing to be leaked here since you can read > > > this data > > > anyway via probe read if you'd wanted to. So we might as well get rid of > > > the > > > clearing for all tracing helpers. > > > > Right, that makes sense. Do you want me to leave it in for this patchset > > and then remove all of them in a followup patchset? > > Lets leave it in and in a different set, we can clean this up for all tracing > related helpers at once. > > > > Different question related to your set. It looks like br_stack is only > > > available > > > on x86, is that correct? For other archs this will always bail out on > > > !br_stack > > > test. Perhaps we should document this fact so users are not surprised > > > why their > > > prog using this helper is not working on !x86. Wdyt? > > > > I think perf_event_open() should fail on !x86 if a user tries to configure > > it with branch stack collection. So there would not be the opportunity for > > the bpf prog to be attached and run. I haven't tested this, though. I'll > > look through the code / install a VM and test it. > > As far as I can see the prog would still be attachable and runnable, just that > the helper always will return -EINVAL on these archs. Maybe error code should be > changed into -ENOENT to avoid confusion wrt whether user provided some invalid +1 on -ENOENT. > input args. Should this actually bail out with -EINVAL if size is not a multiple > of sizeof(struct perf_branch_entry) as otherwise we'd end up copying half broken > branch entry information?
