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@xxxxxxxxx> 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@xxxxxxxxx>
> ---
>  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@xxxxxxxxx>
> + */
> +
> +#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?