[PATCH 0/3] leds: trigger: Add led trigger for bpf

[PATCH 0/3] leds: trigger: Add led trigger for bpf

[Daniel Hodges]

This patch set adds a new led trigger that uses the bpf subsystem for
triggering leds. It is designed to be used in conjunction with a bpf
program(s) that can modify led state through the use of bpf kfuncs. This
is useful for providing a physical indication that a some event has
occurred. In the context of bpf this could range from handling a packet
to hitting a tracepoint.

Daniel Hodges (3):
  leds: trigger: legtrig-bpf: Add ledtrig-bpf module
  selftests/bpf: Add selftests for bpf led programs
  leds: trigger: Add documentation for ledtrig-bpf

 Documentation/leds/index.rst                  |  1 +
 Documentation/leds/ledtrig-bpf.rst            | 13 ++++
 drivers/leds/trigger/Kconfig                  | 10 +++
 drivers/leds/trigger/Makefile                 |  1 +
 drivers/leds/trigger/ledtrig-bpf.c            | 72 +++++++++++++++++++
 tools/testing/selftests/bpf/config            |  1 +
 .../testing/selftests/bpf/progs/ledtrig_bpf.c | 28 ++++++++
 7 files changed, 126 insertions(+)
 create mode 100644 Documentation/leds/ledtrig-bpf.rst
 create mode 100644 drivers/leds/trigger/ledtrig-bpf.c
 create mode 100644 tools/testing/selftests/bpf/progs/ledtrig_bpf.c

-- 
2.43.0

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

[Daniel Hodges]

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 <hodges.daniel.scott@gmail.com>
---
 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 <hodges.daniel.scott@gmail.com>
+ */
+
+#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 <hodges.daniel.scott@gmail.com>");
+MODULE_DESCRIPTION("BPF LED trigger");
+MODULE_LICENSE("GPL v2");
-- 
2.43.0

[PATCH 2/3] selftests/bpf: Add selftests for bpf led programs

[Daniel Hodges]

Add selftests for led trigger bpf programs. To run the tests use the
"test_progs -t ledtrig_bpf" helper.

Signed-off-by: Daniel Hodges <hodges.daniel.scott@gmail.com>
---
 tools/testing/selftests/bpf/config            |  1 +
 .../testing/selftests/bpf/progs/ledtrig_bpf.c | 28 +++++++++++++++++++
 2 files changed, 29 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/ledtrig_bpf.c

diff --git a/tools/testing/selftests/bpf/config b/tools/testing/selftests/bpf/config
index 01f241ea2c67..17a027905dcd 100644
--- a/tools/testing/selftests/bpf/config
+++ b/tools/testing/selftests/bpf/config
@@ -42,6 +42,7 @@ CONFIG_IPV6_SEG6_BPF=y
 CONFIG_IPV6_SIT=y
 CONFIG_IPV6_TUNNEL=y
 CONFIG_KEYS=y
+CONFIG_LEDS_TRIGGER_BPF=y
 CONFIG_LIRC=y
 CONFIG_LWTUNNEL=y
 CONFIG_MODULE_SIG=y
diff --git a/tools/testing/selftests/bpf/progs/ledtrig_bpf.c b/tools/testing/selftests/bpf/progs/ledtrig_bpf.c
new file mode 100644
index 000000000000..8bf1044e6896
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/ledtrig_bpf.c
@@ -0,0 +1,28 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <vmlinux.h>
+
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include <bpf/bpf_core_read.h>
+
+
+extern void bpf_ledtrig_blink(unsigned long delay_on, unsigned long delay_off,
+		int invert) __weak __ksym;
+
+
+SEC("perf_event")
+int perf_blink(void)
+{
+	bpf_ledtrig_blink(30, 30, 0);
+	return 0;
+}
+
+
+SEC("syscall")
+int fork_blink(void)
+{
+	bpf_ledtrig_blink(30, 30, 0);
+	return 0;
+}
+
+char LICENSE[] SEC("license") = "GPL";
-- 
2.43.0

[PATCH 3/3] leds: trigger: Add documentation for ledtrig-bpf

[Daniel Hodges]

Update the documentation for led triggers to include the BPF LED
trigger.

Signed-off-by: Daniel Hodges <hodges.daniel.scott@gmail.com>
---
 Documentation/leds/index.rst       |  1 +
 Documentation/leds/ledtrig-bpf.rst | 13 +++++++++++++
 2 files changed, 14 insertions(+)
 create mode 100644 Documentation/leds/ledtrig-bpf.rst

diff --git a/Documentation/leds/index.rst b/Documentation/leds/index.rst
index 3ade16c18328..2af52a19d6bb 100644
--- a/Documentation/leds/index.rst
+++ b/Documentation/leds/index.rst
@@ -10,6 +10,7 @@ LEDs
    leds-class
    leds-class-flash
    leds-class-multicolor
+   ledtrig-bpf
    ledtrig-oneshot
    ledtrig-transient
    ledtrig-usbport
diff --git a/Documentation/leds/ledtrig-bpf.rst b/Documentation/leds/ledtrig-bpf.rst
new file mode 100644
index 000000000000..391a305f69d1
--- /dev/null
+++ b/Documentation/leds/ledtrig-bpf.rst
@@ -0,0 +1,13 @@
+====================
+BPF LED Trigger
+====================
+
+This LED trigger is useful for triggering LEDs from the BPF subsystem.  This
+trigger is designed to be used in combination with a BPF program that interacts
+with the trigger via a kfunc.  The exported kfuncs will have BTF names that
+start with "bpf_ledtrig_".
+
+The trigger can be activated from user space on led class devices as shown
+below::
+
+  echo bpf > trigger
-- 
2.43.0

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

[Yonghong Song]

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 <hodges.daniel.scott@gmail.com>
> ---
>   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 <hodges.daniel.scott@gmail.com>
> + */
> +
> +#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 <hodges.daniel.scott@gmail.com>");
> +MODULE_DESCRIPTION("BPF LED trigger");
> +MODULE_LICENSE("GPL v2");

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

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1019 bytes --]

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 <hodges.daniel.scott@gmail.com>

> +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
-- 
People of Russia, stop Putin before his war on Ukraine escalates.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

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

[Alexei Starovoitov]

On Fri, Mar 22, 2024 at 7:08 AM Daniel Hodges
<hodges.daniel.scott@gmail.com> 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 <hodges.daniel.scott@gmail.com>
> ---
>  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 <hodges.daniel.scott@gmail.com>
> + */
> +
> +#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?

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

[Andrew Lunn]

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

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

[Andrew Lunn]

> 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

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

[Daniel Hodges]

> 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

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

[Daniel Hodges]

From: Andrew Lunn <andrew@lunn.ch>

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

[PATCH v2 0/3] leds: trigger: legtrig-bpf: Add ledtrig-bpf trigger

[Daniel Hodges]

This patch set adds a led trigger that interfaces with the bpf
subsystem. It allows for BPF programs to control LED activity using 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.

V1: https://lore.kernel.org/bpf/cover.1711113657.git.hodges.daniel.scott@gmail.com/
Changes in V2:
- Change kfunc to trigger based on LED name.
- Fix kfunc registration of BPF_PROG_TYPE_UNSPEC.
- Use new kfunc helpers off bpf-next branch.

Daniel Hodges (3):
  leds: trigger: legtrig-bpf: Add ledtrig-bpf trigger
  selftests/bpf: Add selftests for bpf led programs
  leds: trigger: Add documentation for ledtrig-bpf

 Documentation/leds/index.rst                  |  1 +
 Documentation/leds/ledtrig-bpf.rst            | 13 ++++
 drivers/leds/trigger/Kconfig                  | 10 +++
 drivers/leds/trigger/Makefile                 |  1 +
 drivers/leds/trigger/ledtrig-bpf.c            | 73 +++++++++++++++++++
 tools/testing/selftests/bpf/config            |  1 +
 .../testing/selftests/bpf/progs/ledtrig_bpf.c | 32 ++++++++
 7 files changed, 131 insertions(+)
 create mode 100644 Documentation/leds/ledtrig-bpf.rst
 create mode 100644 drivers/leds/trigger/ledtrig-bpf.c
 create mode 100644 tools/testing/selftests/bpf/progs/ledtrig_bpf.c

-- 
2.43.2

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

[Daniel Hodges]

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 <hodges.daniel.scott@gmail.com>
---
 drivers/leds/trigger/Kconfig       | 10 ++++
 drivers/leds/trigger/Makefile      |  1 +
 drivers/leds/trigger/ledtrig-bpf.c | 73 ++++++++++++++++++++++++++++++
 3 files changed, 84 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..99cabf816da4
--- /dev/null
+++ b/drivers/leds/trigger/ledtrig-bpf.c
@@ -0,0 +1,73 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * LED BPF Trigger
+ *
+ * Author: Daniel Hodges <hodges.daniel.scott@gmail.com>
+ */
+
+#include <linux/bpf.h>
+#include <linux/btf.h>
+#include <linux/btf_ids.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/rcupdate.h>
+
+
+DEFINE_LED_TRIGGER(ledtrig_bpf);
+
+__bpf_kfunc_start_defs();
+__bpf_kfunc void bpf_ledtrig_blink(const char *led_name__str, unsigned long
+		delay_on, unsigned long delay_off, int invert)
+{
+	struct led_classdev *led_cdev;
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(led_cdev, &ledtrig_bpf->led_cdevs, trig_list) {
+		if (strcmp(led_name__str, led_cdev->name) == 0) {
+			led_blink_set_oneshot(led_cdev, &delay_on, &delay_off,
+					invert);
+			break;
+		}
+	}
+	rcu_read_unlock();
+}
+__bpf_kfunc_end_defs();
+
+BTF_KFUNCS_START(ledtrig_bpf_kfunc_ids)
+BTF_ID_FLAGS(func, bpf_ledtrig_blink, KF_TRUSTED_ARGS)
+BTF_KFUNCS_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);
+	return ret;
+}
+
+static int __init 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 <hodges.daniel.scott@gmail.com>");
+MODULE_DESCRIPTION("BPF LED trigger");
+MODULE_LICENSE("GPL v2");
-- 
2.43.2

[PATCH v2 2/3] selftests/bpf: Add selftests for bpf led programs

[Daniel Hodges]

Add selftests for led trigger bpf programs. To run the tests use the
"test_progs -t ledtrig_bpf" helper.

Signed-off-by: Daniel Hodges <hodges.daniel.scott@gmail.com>
---
 tools/testing/selftests/bpf/config            |  1 +
 .../testing/selftests/bpf/progs/ledtrig_bpf.c | 32 +++++++++++++++++++
 2 files changed, 33 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/ledtrig_bpf.c

diff --git a/tools/testing/selftests/bpf/config b/tools/testing/selftests/bpf/config
index 01f241ea2c67..17a027905dcd 100644
--- a/tools/testing/selftests/bpf/config
+++ b/tools/testing/selftests/bpf/config
@@ -42,6 +42,7 @@ CONFIG_IPV6_SEG6_BPF=y
 CONFIG_IPV6_SIT=y
 CONFIG_IPV6_TUNNEL=y
 CONFIG_KEYS=y
+CONFIG_LEDS_TRIGGER_BPF=y
 CONFIG_LIRC=y
 CONFIG_LWTUNNEL=y
 CONFIG_MODULE_SIG=y
diff --git a/tools/testing/selftests/bpf/progs/ledtrig_bpf.c b/tools/testing/selftests/bpf/progs/ledtrig_bpf.c
new file mode 100644
index 000000000000..d2096f13e5b6
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/ledtrig_bpf.c
@@ -0,0 +1,32 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <vmlinux.h>
+
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include <bpf/bpf_core_read.h>
+
+
+extern void bpf_ledtrig_blink(const char *led_name, unsigned long delay_on,
+		unsigned long delay_off, int invert) __weak __ksym;
+
+
+SEC("perf_event")
+int perf_blink(void)
+{
+	const char *led = "test_led";
+
+	bpf_ledtrig_blink(led, 30, 30, 0);
+	return 0;
+}
+
+
+SEC("syscall")
+int fork_blink(void)
+{
+	const char *led = "test_led";
+
+	bpf_ledtrig_blink(led, 30, 30, 0);
+	return 0;
+}
+
+char LICENSE[] SEC("license") = "GPL";
-- 
2.43.2

[PATCH v2 3/3] leds: trigger: Add documentation for ledtrig-bpf

[Daniel Hodges]

Update the documentation for led triggers to include the BPF LED
trigger.

Signed-off-by: Daniel Hodges <hodges.daniel.scott@gmail.com>
---
 Documentation/leds/index.rst       |  1 +
 Documentation/leds/ledtrig-bpf.rst | 13 +++++++++++++
 2 files changed, 14 insertions(+)
 create mode 100644 Documentation/leds/ledtrig-bpf.rst

diff --git a/Documentation/leds/index.rst b/Documentation/leds/index.rst
index 3ade16c18328..2af52a19d6bb 100644
--- a/Documentation/leds/index.rst
+++ b/Documentation/leds/index.rst
@@ -10,6 +10,7 @@ LEDs
    leds-class
    leds-class-flash
    leds-class-multicolor
+   ledtrig-bpf
    ledtrig-oneshot
    ledtrig-transient
    ledtrig-usbport
diff --git a/Documentation/leds/ledtrig-bpf.rst b/Documentation/leds/ledtrig-bpf.rst
new file mode 100644
index 000000000000..391a305f69d1
--- /dev/null
+++ b/Documentation/leds/ledtrig-bpf.rst
@@ -0,0 +1,13 @@
+====================
+BPF LED Trigger
+====================
+
+This LED trigger is useful for triggering LEDs from the BPF subsystem.  This
+trigger is designed to be used in combination with a BPF program that interacts
+with the trigger via a kfunc.  The exported kfuncs will have BTF names that
+start with "bpf_ledtrig_".
+
+The trigger can be activated from user space on led class devices as shown
+below::
+
+  echo bpf > trigger
-- 
2.43.2

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

[Daniel Hodges]

From: Alexei Starovoitov <alexei.starovoitov@gmail.com>


> A new kernel module just to call this helper?
> Feels like overkill. Can it be a part of generic led bits?

I looked and in general most of the other triggers are modules so I was trying
to follow that pattern. I think having a few kfuncs in the generic led core
might be far more flexible. I'd have to defer to someone with more insight on
that though.

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

I briefly looked at it and to be honest not sure of how widely used it is. I
did see some docs in OpenWRT
(https://openwrt.org/docs/guide-user/base-system/led_configuration) so it may
be used in that context. My thought is to have something like bpftrace be able
to trigger LEDs for troubleshooting purposes. Sure, it can already print
counts and sums of counts over intervals, but one thing I've found interesting
is triggering on perf events. For example, triggering on CPU cycles gives a
nice visual indication of the amount of work being done.

> It can also do led_trigger_event().
> Should that be another kfunc?

That's a good question, there seems to be a few other functions that would
maybe be useful kfuncs (led_set_brightness etc). My only thought was some of
them get passed the led_classdev and wasn't sure how difficult it would easily
it would be for callers of the kfuncs to access/pass it through. With the
trigger interface at least it seems rather self contained.

Anyways, part of this exercise was a learning experience for me on getting
some bearings for being able to submit patches properly and learning the
kernel development process. The V2 patchset (which I wasn't sure if I was
supposed to respond to a thread or make a new one from
https://kernelnewbies.org/FirstKernelPatch it made it seem like either was
acceptable), I think correctly implements a per LED version. Thanks all for
your time and insight.

- Daniel

Re: [PATCH v2 1/3] leds: trigger: legtrig-bpf: Add ledtrig-bpf trigger

[Lee Jones]

On Mon, 25 Mar 2024, 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 <hodges.daniel.scott@gmail.com>
> ---
>  drivers/leds/trigger/Kconfig       | 10 ++++
>  drivers/leds/trigger/Makefile      |  1 +
>  drivers/leds/trigger/ledtrig-bpf.c | 73 ++++++++++++++++++++++++++++++
>  3 files changed, 84 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..99cabf816da4
> --- /dev/null
> +++ b/drivers/leds/trigger/ledtrig-bpf.c
> @@ -0,0 +1,73 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * LED BPF Trigger
> + *
> + * Author: Daniel Hodges <hodges.daniel.scott@gmail.com>

Any copyright?

> + */
> +
> +#include <linux/bpf.h>
> +#include <linux/btf.h>
> +#include <linux/btf_ids.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/leds.h>
> +#include <linux/module.h>
> +#include <linux/rcupdate.h>
> +
> +
> +DEFINE_LED_TRIGGER(ledtrig_bpf);
> +
> +__bpf_kfunc_start_defs();

__bpf_hook_start()?

> +__bpf_kfunc void bpf_ledtrig_blink(const char *led_name__str, unsigned long
> +		delay_on, unsigned long delay_off, int invert)

Nit: Try to keep the variable types and names together please.

I.e. break before the 'unsigned' or after 'delay_on'.

> +{
> +	struct led_classdev *led_cdev;
> +
> +	rcu_read_lock();
> +	list_for_each_entry_rcu(led_cdev, &ledtrig_bpf->led_cdevs, trig_list) {
> +		if (strcmp(led_name__str, led_cdev->name) == 0) {
> +			led_blink_set_oneshot(led_cdev, &delay_on, &delay_off,
> +					invert);

Use 100-chars to avoid the break.

> +			break;
> +		}
> +	}
> +	rcu_read_unlock();
> +}
> +__bpf_kfunc_end_defs();

_bpf_hook_end()?

> +BTF_KFUNCS_START(ledtrig_bpf_kfunc_ids)
> +BTF_ID_FLAGS(func, bpf_ledtrig_blink, KF_TRUSTED_ARGS)
> +BTF_KFUNCS_END(ledtrig_bpf_kfunc_ids)

Why not a single macro to output the full struct?

Or better yet, be less opaque and not use a macro at all?

> +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);
> +	return ret;

return register_btf_kfunc_id_set()?

Or again, better yet, put the single function call directly into
ledtrig_bpf_init()?  Creating a whole function just for a single
invocation seems superfluous.

> +}
> +
> +static int __init 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 <hodges.daniel.scott@gmail.com>");
> +MODULE_DESCRIPTION("BPF LED trigger");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.43.2
> 

-- 
Lee Jones [李琼斯]

Re: [PATCH v2 3/3] leds: trigger: Add documentation for ledtrig-bpf

[Lee Jones]

On Mon, 25 Mar 2024, Daniel Hodges wrote:

> Update the documentation for led triggers to include the BPF LED

LED throughout please.

Same in the previous commit please.

> trigger.
> 
> Signed-off-by: Daniel Hodges <hodges.daniel.scott@gmail.com>
> ---
>  Documentation/leds/index.rst       |  1 +
>  Documentation/leds/ledtrig-bpf.rst | 13 +++++++++++++
>  2 files changed, 14 insertions(+)
>  create mode 100644 Documentation/leds/ledtrig-bpf.rst
> 
> diff --git a/Documentation/leds/index.rst b/Documentation/leds/index.rst
> index 3ade16c18328..2af52a19d6bb 100644
> --- a/Documentation/leds/index.rst
> +++ b/Documentation/leds/index.rst
> @@ -10,6 +10,7 @@ LEDs
>     leds-class
>     leds-class-flash
>     leds-class-multicolor
> +   ledtrig-bpf
>     ledtrig-oneshot
>     ledtrig-transient
>     ledtrig-usbport
> diff --git a/Documentation/leds/ledtrig-bpf.rst b/Documentation/leds/ledtrig-bpf.rst
> new file mode 100644
> index 000000000000..391a305f69d1
> --- /dev/null
> +++ b/Documentation/leds/ledtrig-bpf.rst
> @@ -0,0 +1,13 @@
> +====================
> +BPF LED Trigger
> +====================
> +
> +This LED trigger is useful for triggering LEDs from the BPF subsystem.  This
> +trigger is designed to be used in combination with a BPF program that interacts
> +with the trigger via a kfunc.  The exported kfuncs will have BTF names that
> +start with "bpf_ledtrig_".
> +
> +The trigger can be activated from user space on led class devices as shown
> +below::
> +
> +  echo bpf > trigger
> -- 
> 2.43.2
> 

-- 
Lee Jones [李琼斯]