[1/3] leds: trigger: legtrig-bpf: Add ledtrig-bpf module

Commit Message

Daniel Hodges March 22, 2024, 2:08 p.m. UTC

This patch adds a led trigger that interfaces with the bpf subsystem. It
allows for BPF programs to control LED activity through calling bpf
kfuncs. This functionality is useful in giving users a physical
indication that a BPF program has performed an operation such as
handling a packet or probe point.

Signed-off-by: Daniel Hodges <[e-mail]>
---
 drivers/leds/trigger/Kconfig       | 10 +++++
 drivers/leds/trigger/Makefile      |  1 +
 drivers/leds/trigger/ledtrig-bpf.c | 72 ++++++++++++++++++++++++++++++
 3 files changed, 83 insertions(+)
 create mode 100644 drivers/leds/trigger/ledtrig-bpf.c

Comments

Yonghong Song March 22, 2024, 3:24 p.m. UTC | #1

On 3/22/24 7:08 AM, Daniel Hodges wrote:
> This patch adds a led trigger that interfaces with the bpf subsystem. It
> allows for BPF programs to control LED activity through calling bpf
> kfuncs. This functionality is useful in giving users a physical
> indication that a BPF program has performed an operation such as
> handling a packet or probe point.
>
> Signed-off-by: Daniel Hodges <[e-mail]>
> ---
>   drivers/leds/trigger/Kconfig       | 10 +++++
>   drivers/leds/trigger/Makefile      |  1 +
>   drivers/leds/trigger/ledtrig-bpf.c | 72 ++++++++++++++++++++++++++++++
>   3 files changed, 83 insertions(+)
>   create mode 100644 drivers/leds/trigger/ledtrig-bpf.c
>
> diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig
> index d11d80176fc0..30b0fd3847be 100644
> --- a/drivers/leds/trigger/Kconfig
> +++ b/drivers/leds/trigger/Kconfig
> @@ -152,4 +152,14 @@ config LEDS_TRIGGER_TTY
>   
>   	  When build as a module this driver will be called ledtrig-tty.
>   
> +config LEDS_TRIGGER_BPF
> +	tristate "LED BPF Trigger"
> +	depends on BPF
> +	depends on BPF_SYSCALL
> +	help
> +	  This allows LEDs to be controlled by the BPF subsystem. This trigger
> +	  must be used with a loaded BPF program in order to control LED state.
> +	  BPF programs can control LED state with kfuncs.
> +	  If unsure, say N.
> +
>   endif # LEDS_TRIGGERS
> diff --git a/drivers/leds/trigger/Makefile b/drivers/leds/trigger/Makefile
> index 25c4db97cdd4..ac47128d406c 100644
> --- a/drivers/leds/trigger/Makefile
> +++ b/drivers/leds/trigger/Makefile
> @@ -16,3 +16,4 @@ obj-$(CONFIG_LEDS_TRIGGER_NETDEV)	+= ledtrig-netdev.o
>   obj-$(CONFIG_LEDS_TRIGGER_PATTERN)	+= ledtrig-pattern.o
>   obj-$(CONFIG_LEDS_TRIGGER_AUDIO)	+= ledtrig-audio.o
>   obj-$(CONFIG_LEDS_TRIGGER_TTY)		+= ledtrig-tty.o
> +obj-$(CONFIG_LEDS_TRIGGER_BPF)		+= ledtrig-bpf.o
> diff --git a/drivers/leds/trigger/ledtrig-bpf.c b/drivers/leds/trigger/ledtrig-bpf.c
> new file mode 100644
> index 000000000000..e3b0b8281b70
> --- /dev/null
> +++ b/drivers/leds/trigger/ledtrig-bpf.c
> @@ -0,0 +1,72 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * LED BPF Trigger
> + *
> + * Author: Daniel Hodges <[e-mail]>
> + */
> +
> +#include <linux/bpf.h>
> +#include <linux/btf.h>
> +#include <linux/btf_ids.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/leds.h>
> +
> +
> +DEFINE_LED_TRIGGER(ledtrig_bpf);
> +
> +__bpf_hook_start()
> +
> +__bpf_kfunc void bpf_ledtrig_blink(unsigned long delay_on, unsigned long delay_off, int invert)
> +{
> +	led_trigger_blink_oneshot(ledtrig_bpf, delay_on, delay_off, invert);
> +}
> +__bpf_hook_end();
> +
> +BTF_SET8_START(ledtrig_bpf_kfunc_ids)
> +BTF_ID_FLAGS(func, bpf_ledtrig_blink)
> +BTF_SET8_END(ledtrig_bpf_kfunc_ids)
> +
> +static const struct btf_kfunc_id_set ledtrig_bpf_kfunc_set = {
> +	.owner = THIS_MODULE,
> +	.set   = &ledtrig_bpf_kfunc_ids,
> +};
> +
> +static int init_bpf(void)
> +{
> +	int ret;
> +
> +	ret = register_btf_kfunc_id_set(BPF_PROG_TYPE_UNSPEC,
> +			&ledtrig_bpf_kfunc_set);

If you add the kfunc to BPF_PROG_TYPE_UNSPEC, then
you do not need to add to the following prog types
since the kernel will search BPF_PROG_TYPE_UNSPEC
for all prog types.

But if the kfunc only intends to be used for the following
program types, I suggest removing the above BPF_PROG_TYPE_UNSPEC
registration.

> +	ret = register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING,
> +			&ledtrig_bpf_kfunc_set);
> +	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SYSCALL,
> +			&ledtrig_bpf_kfunc_set);
> +	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SK_SKB,
> +			&ledtrig_bpf_kfunc_set);
> +	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_CLS,
> +			&ledtrig_bpf_kfunc_set);
> +	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_ACT,
> +			&ledtrig_bpf_kfunc_set);
> +	return ret;
> +}
> +
> +static int ledtrig_bpf_init(void)
> +{
> +	led_trigger_register_simple("bpf", &ledtrig_bpf);
> +
> +	return init_bpf();
> +}
> +
> +static void __exit ledtrig_bpf_exit(void)
> +{
> +	led_trigger_unregister_simple(ledtrig_bpf);
> +}
> +
> +module_init(ledtrig_bpf_init);
> +module_exit(ledtrig_bpf_exit);
> +
> +MODULE_AUTHOR("Daniel Hodges <[e-mail]>");
> +MODULE_DESCRIPTION("BPF LED trigger");
> +MODULE_LICENSE("GPL v2");

Pavel Machek March 22, 2024, 9:52 p.m. UTC | #2

Hi!

> This patch adds a led trigger that interfaces with the bpf subsystem. It
> allows for BPF programs to control LED activity through calling bpf
> kfuncs. This functionality is useful in giving users a physical
> indication that a BPF program has performed an operation such as
> handling a packet or probe point.
> 
> Signed-off-by: Daniel Hodges <[e-mail]>

> +static int init_bpf(void)
> +{
> +	int ret;
> +
> +	ret = register_btf_kfunc_id_set(BPF_PROG_TYPE_UNSPEC,
> +			&ledtrig_bpf_kfunc_set);
> +	ret = register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING,
> +			&ledtrig_bpf_kfunc_set);

Should have ret ?: here, too?

> +static int ledtrig_bpf_init(void)
> +{
> +	led_trigger_register_simple("bpf", &ledtrig_bpf);
> +
> +	return init_bpf();
> +}

Is it somehow possible to have multiple LEDs hooked to bpf
functionality? I guess someone will want that...

Best regards,
								Pavel

Alexei Starovoitov March 23, 2024, 7:19 p.m. UTC | #3

On Fri, Mar 22, 2024 at 7:08 AM Daniel Hodges
<[e-mail]> wrote:
>
> This patch adds a led trigger that interfaces with the bpf subsystem. It
> allows for BPF programs to control LED activity through calling bpf
> kfuncs. This functionality is useful in giving users a physical
> indication that a BPF program has performed an operation such as
> handling a packet or probe point.
>
> Signed-off-by: Daniel Hodges <[e-mail]>
> ---
>  drivers/leds/trigger/Kconfig       | 10 +++++
>  drivers/leds/trigger/Makefile      |  1 +
>  drivers/leds/trigger/ledtrig-bpf.c | 72 ++++++++++++++++++++++++++++++
>  3 files changed, 83 insertions(+)
>  create mode 100644 drivers/leds/trigger/ledtrig-bpf.c
>
> diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig
> index d11d80176fc0..30b0fd3847be 100644
> --- a/drivers/leds/trigger/Kconfig
> +++ b/drivers/leds/trigger/Kconfig
> @@ -152,4 +152,14 @@ config LEDS_TRIGGER_TTY
>
>           When build as a module this driver will be called ledtrig-tty.
>
> +config LEDS_TRIGGER_BPF
> +       tristate "LED BPF Trigger"
> +       depends on BPF
> +       depends on BPF_SYSCALL
> +       help
> +         This allows LEDs to be controlled by the BPF subsystem. This trigger
> +         must be used with a loaded BPF program in order to control LED state.
> +         BPF programs can control LED state with kfuncs.
> +         If unsure, say N.
> +
>  endif # LEDS_TRIGGERS
> diff --git a/drivers/leds/trigger/Makefile b/drivers/leds/trigger/Makefile
> index 25c4db97cdd4..ac47128d406c 100644
> --- a/drivers/leds/trigger/Makefile
> +++ b/drivers/leds/trigger/Makefile
> @@ -16,3 +16,4 @@ obj-$(CONFIG_LEDS_TRIGGER_NETDEV)     += ledtrig-netdev.o
>  obj-$(CONFIG_LEDS_TRIGGER_PATTERN)     += ledtrig-pattern.o
>  obj-$(CONFIG_LEDS_TRIGGER_AUDIO)       += ledtrig-audio.o
>  obj-$(CONFIG_LEDS_TRIGGER_TTY)         += ledtrig-tty.o
> +obj-$(CONFIG_LEDS_TRIGGER_BPF)         += ledtrig-bpf.o
> diff --git a/drivers/leds/trigger/ledtrig-bpf.c b/drivers/leds/trigger/ledtrig-bpf.c
> new file mode 100644
> index 000000000000..e3b0b8281b70
> --- /dev/null
> +++ b/drivers/leds/trigger/ledtrig-bpf.c
> @@ -0,0 +1,72 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * LED BPF Trigger
> + *
> + * Author: Daniel Hodges <[e-mail]>
> + */
> +
> +#include <linux/bpf.h>
> +#include <linux/btf.h>
> +#include <linux/btf_ids.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/leds.h>
> +
> +
> +DEFINE_LED_TRIGGER(ledtrig_bpf);
> +
> +__bpf_hook_start()
> +
> +__bpf_kfunc void bpf_ledtrig_blink(unsigned long delay_on, unsigned long delay_off, int invert)
> +{
> +       led_trigger_blink_oneshot(ledtrig_bpf, delay_on, delay_off, invert);

A new kernel module just to call this helper?
Feels like overkill. Can it be a part of generic led bits?
btw, have you looked at net/netfilter/xt_LED.c ?
netfilter had the ability to blink led for a long time.
I'm curious whether folks found it useful.
It can also do led_trigger_event().
Should that be another kfunc?

Andrew Lunn March 23, 2024, 10:18 p.m. UTC | #4

On Fri, Mar 22, 2024 at 10:08:14AM -0400, Daniel Hodges wrote:
> This patch adds a led trigger that interfaces with the bpf subsystem. It
> allows for BPF programs to control LED activity through calling bpf
> kfuncs. This functionality is useful in giving users a physical
> indication that a BPF program has performed an operation such as
> handling a packet or probe point.

Don't you want a mechanism to say which LED it should blink? Say you
have a BPF IR receiver, a BPF kernel scheduler and a BPF firewall. All
sharing the same LED is probably not what you want. Each probably
wants its own LED.

      Andrew

Andrew Lunn March 23, 2024, 10:25 p.m. UTC | #5

> A new kernel module just to call this helper?
> Feels like overkill. Can it be a part of generic led bits?
> btw, have you looked at net/netfilter/xt_LED.c ?
> netfilter had the ability to blink led for a long time.
> I'm curious whether folks found it useful.

This might become more useful now that we have support for PHY & MAC
LEDs. You can use the netdev trigger for the usual things an RJ45 LED
shows: link, rx/tx activity, link speed etc. But they are just Linux
LEDs, you can also use them for heartbeat, disc IO, tty IO, or xt_LED.
xt_LED would actually make sense for an LED in a RJ45 socket.

       Andrew

Daniel Hodges March 24, 2024, 2:15 a.m. UTC | #6

> On Fri, Mar 22, 2024 at 10:08:14AM -0400, Daniel Hodges wrote:
> > This patch adds a led trigger that interfaces with the bpf subsystem. It
> > allows for BPF programs to control LED activity through calling bpf
> > kfuncs. This functionality is useful in giving users a physical
> > indication that a BPF program has performed an operation such as
> > handling a packet or probe point.
>
> Don't you want a mechanism to say which LED it should blink? Say you
> have a BPF IR receiver, a BPF kernel scheduler and a BPF firewall. All
> sharing the same LED is probably not what you want. Each probably
> wants its own LED.
>
>       Andrew

Yeah, I think that makes sense. I guess the idea with this patch set was
more or less to show the simple implementation. Since there can be
multiple devices the trigger probably needs a registry of the active
led_classdev names or other id. Alexei mentioned the xt_led which looks
like it is rather similar as well. I can give this another shot, but I
didn't want to go too far down the rabbit hole without getting some
signal on the idea in general.

PS Sorry to folks if I've directly replied to your comments. I'm still in the
process of setting up my email workflow.

- Daniel

Patch

diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig
index d11d80176fc0..30b0fd3847be 100644
--- a/drivers/leds/trigger/Kconfig
+++ b/drivers/leds/trigger/Kconfig
@@ -152,4 +152,14 @@  config LEDS_TRIGGER_TTY
 
 	  When build as a module this driver will be called ledtrig-tty.
 
+config LEDS_TRIGGER_BPF
+	tristate "LED BPF Trigger"
+	depends on BPF
+	depends on BPF_SYSCALL
+	help
+	  This allows LEDs to be controlled by the BPF subsystem. This trigger
+	  must be used with a loaded BPF program in order to control LED state.
+	  BPF programs can control LED state with kfuncs.
+	  If unsure, say N.
+
 endif # LEDS_TRIGGERS
diff --git a/drivers/leds/trigger/Makefile b/drivers/leds/trigger/Makefile
index 25c4db97cdd4..ac47128d406c 100644
--- a/drivers/leds/trigger/Makefile
+++ b/drivers/leds/trigger/Makefile
@@ -16,3 +16,4 @@  obj-$(CONFIG_LEDS_TRIGGER_NETDEV)	+= ledtrig-netdev.o
 obj-$(CONFIG_LEDS_TRIGGER_PATTERN)	+= ledtrig-pattern.o
 obj-$(CONFIG_LEDS_TRIGGER_AUDIO)	+= ledtrig-audio.o
 obj-$(CONFIG_LEDS_TRIGGER_TTY)		+= ledtrig-tty.o
+obj-$(CONFIG_LEDS_TRIGGER_BPF)		+= ledtrig-bpf.o
diff --git a/drivers/leds/trigger/ledtrig-bpf.c b/drivers/leds/trigger/ledtrig-bpf.c
new file mode 100644
index 000000000000..e3b0b8281b70
--- /dev/null
+++ b/drivers/leds/trigger/ledtrig-bpf.c
@@ -0,0 +1,72 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * LED BPF Trigger
+ *
+ * Author: Daniel Hodges <[e-mail]>
+ */
+
+#include <linux/bpf.h>
+#include <linux/btf.h>
+#include <linux/btf_ids.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/leds.h>
+
+
+DEFINE_LED_TRIGGER(ledtrig_bpf);
+
+__bpf_hook_start()
+
+__bpf_kfunc void bpf_ledtrig_blink(unsigned long delay_on, unsigned long delay_off, int invert)
+{
+	led_trigger_blink_oneshot(ledtrig_bpf, delay_on, delay_off, invert);
+}
+__bpf_hook_end();
+
+BTF_SET8_START(ledtrig_bpf_kfunc_ids)
+BTF_ID_FLAGS(func, bpf_ledtrig_blink)
+BTF_SET8_END(ledtrig_bpf_kfunc_ids)
+
+static const struct btf_kfunc_id_set ledtrig_bpf_kfunc_set = {
+	.owner = THIS_MODULE,
+	.set   = &ledtrig_bpf_kfunc_ids,
+};
+
+static int init_bpf(void)
+{
+	int ret;
+
+	ret = register_btf_kfunc_id_set(BPF_PROG_TYPE_UNSPEC,
+			&ledtrig_bpf_kfunc_set);
+	ret = register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING,
+			&ledtrig_bpf_kfunc_set);
+	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SYSCALL,
+			&ledtrig_bpf_kfunc_set);
+	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SK_SKB,
+			&ledtrig_bpf_kfunc_set);
+	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_CLS,
+			&ledtrig_bpf_kfunc_set);
+	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_ACT,
+			&ledtrig_bpf_kfunc_set);
+	return ret;
+}
+
+static int ledtrig_bpf_init(void)
+{
+	led_trigger_register_simple("bpf", &ledtrig_bpf);
+
+	return init_bpf();
+}
+
+static void __exit ledtrig_bpf_exit(void)
+{
+	led_trigger_unregister_simple(ledtrig_bpf);
+}
+
+module_init(ledtrig_bpf_init);
+module_exit(ledtrig_bpf_exit);
+
+MODULE_AUTHOR("Daniel Hodges <[e-mail]>");
+MODULE_DESCRIPTION("BPF LED trigger");
+MODULE_LICENSE("GPL v2");