linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [FIX bpf,perf] bpf,perf: return EOPNOTSUPP for bpf handler on PERF_COUNT_SW_DUMMY
@ 2020-11-16 18:37 Florian Lehner
  2020-11-16 21:02 ` Martin KaFai Lau
  0 siblings, 1 reply; 4+ messages in thread
From: Florian Lehner @ 2020-11-16 18:37 UTC (permalink / raw)
  To: bpf
  Cc: netdev, linux-kernel, ast, daniel, andrii, john.fastabend,
	peterz, mingo, acme, Florian Lehner

bpf handlers for perf events other than tracepoints, kprobes or uprobes
are attached to the overflow_handler of the perf event.

Perf events of type software/dummy are placeholder events. So when
attaching a bpf handle to an overflow_handler of such an event, the bpf
handler will not be triggered.

This fix returns the error EOPNOTSUPP to indicate that attaching a bpf
handler to a perf event of type software/dummy is not supported.

Signed-off-by: Florian Lehner <dev@der-flo.net>
---
 kernel/events/core.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index da467e1dd49a..4e8846b7ceda 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -9668,6 +9668,10 @@ static int perf_event_set_bpf_handler(struct perf_event *event, u32 prog_fd)
 	if (event->prog)
 		return -EEXIST;
 
+	if (event->attr.type == PERF_TYPE_SOFTWARE &&
+	    event->attr.config == PERF_COUNT_SW_DUMMY)
+		return -EOPNOTSUPP;
+
 	prog = bpf_prog_get_type(prog_fd, BPF_PROG_TYPE_PERF_EVENT);
 	if (IS_ERR(prog))
 		return PTR_ERR(prog);
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [FIX bpf,perf] bpf,perf: return EOPNOTSUPP for bpf handler on PERF_COUNT_SW_DUMMY
  2020-11-16 18:37 [FIX bpf,perf] bpf,perf: return EOPNOTSUPP for bpf handler on PERF_COUNT_SW_DUMMY Florian Lehner
@ 2020-11-16 21:02 ` Martin KaFai Lau
  2020-11-17  7:53   ` Peter Zijlstra
  0 siblings, 1 reply; 4+ messages in thread
From: Martin KaFai Lau @ 2020-11-16 21:02 UTC (permalink / raw)
  To: Florian Lehner
  Cc: bpf, netdev, linux-kernel, ast, daniel, andrii, john.fastabend,
	peterz, mingo, acme

On Mon, Nov 16, 2020 at 07:37:52PM +0100, Florian Lehner wrote:
> bpf handlers for perf events other than tracepoints, kprobes or uprobes
> are attached to the overflow_handler of the perf event.
> 
> Perf events of type software/dummy are placeholder events. So when
> attaching a bpf handle to an overflow_handler of such an event, the bpf
> handler will not be triggered.
> 
> This fix returns the error EOPNOTSUPP to indicate that attaching a bpf
> handler to a perf event of type software/dummy is not supported.
> 
> Signed-off-by: Florian Lehner <dev@der-flo.net>
It is missing a Fixes tag.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [FIX bpf,perf] bpf,perf: return EOPNOTSUPP for bpf handler on PERF_COUNT_SW_DUMMY
  2020-11-16 21:02 ` Martin KaFai Lau
@ 2020-11-17  7:53   ` Peter Zijlstra
  2020-11-17 15:39     ` Florian Lehner
  0 siblings, 1 reply; 4+ messages in thread
From: Peter Zijlstra @ 2020-11-17  7:53 UTC (permalink / raw)
  To: Martin KaFai Lau
  Cc: Florian Lehner, bpf, netdev, linux-kernel, ast, daniel, andrii,
	john.fastabend, mingo, acme

On Mon, Nov 16, 2020 at 01:02:09PM -0800, Martin KaFai Lau wrote:
> On Mon, Nov 16, 2020 at 07:37:52PM +0100, Florian Lehner wrote:
> > bpf handlers for perf events other than tracepoints, kprobes or uprobes
> > are attached to the overflow_handler of the perf event.
> > 
> > Perf events of type software/dummy are placeholder events. So when
> > attaching a bpf handle to an overflow_handler of such an event, the bpf
> > handler will not be triggered.
> > 
> > This fix returns the error EOPNOTSUPP to indicate that attaching a bpf
> > handler to a perf event of type software/dummy is not supported.
> > 
> > Signed-off-by: Florian Lehner <dev@der-flo.net>
> It is missing a Fixes tag.

I don't think it actually fixes anything. worse it could break things.

Atatching a bpf filter to a dummy event is pointless, but harmless. We
allow it now, disallowing it will break whatever programs out there are
doing harmless silly things.

I really don't see the point of this patch. It grows the kernel code for
absolutely no distinguishable benefit.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [FIX bpf,perf] bpf,perf: return EOPNOTSUPP for bpf handler on PERF_COUNT_SW_DUMMY
  2020-11-17  7:53   ` Peter Zijlstra
@ 2020-11-17 15:39     ` Florian Lehner
  0 siblings, 0 replies; 4+ messages in thread
From: Florian Lehner @ 2020-11-17 15:39 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: acme, andrii, ast, bpf, daniel, john.fastabend, kafai,
	linux-kernel, mingo, netdev

On Tue, Nov 17, 2020 at 08:53:34AM +0100, Peter Zijlstra wrote:
> On Mon, Nov 16, 2020 at 01:02:09PM -0800, Martin KaFai Lau wrote:
> > On Mon, Nov 16, 2020 at 07:37:52PM +0100, Florian Lehner wrote:
> > > bpf handlers for perf events other than tracepoints, kprobes or uprobes
> > > are attached to the overflow_handler of the perf event.
> > > 
> > > Perf events of type software/dummy are placeholder events. So when
> > > attaching a bpf handle to an overflow_handler of such an event, the bpf
> > > handler will not be triggered.
> > > 
> > > This fix returns the error EOPNOTSUPP to indicate that attaching a bpf
> > > handler to a perf event of type software/dummy is not supported.
> > > 
> > > Signed-off-by: Florian Lehner <dev@der-flo.net>
> > It is missing a Fixes tag.
> 
> I don't think it actually fixes anything. worse it could break things.
> 
> Atatching a bpf filter to a dummy event is pointless, but harmless. We
> allow it now, disallowing it will break whatever programs out there are
> doing harmless silly things.
> 
> I really don't see the point of this patch. It grows the kernel code for
> absolutely no distinguishable benefit.

I agree, this fix does not implement the functionality of attaching a
bpf handler to a perf event of type software/dummy. Instead it returns
an error code and let the user know that this kind of action is not
supported (yet).
As a user I would prefer to get an error for something that is pointless
than needing to debug why an attached bpf handler is never execute.

Do you think it would be better to improve documentation to point this
out? And if so, which documentation would be best to update?

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-11-17 15:41 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-16 18:37 [FIX bpf,perf] bpf,perf: return EOPNOTSUPP for bpf handler on PERF_COUNT_SW_DUMMY Florian Lehner
2020-11-16 21:02 ` Martin KaFai Lau
2020-11-17  7:53   ` Peter Zijlstra
2020-11-17 15:39     ` Florian Lehner

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).