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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAADnVQ+BsBcp5osqiG46gjtLViQjHStVnPsySffHsybaz7OYEw@mail.gmail.com>
Date: Tue, 26 Mar 2024 21:53:14 -0400
From: Daniel Hodges <hodges.daniel.scott@...il.com>
To: alexei.starovoitov@...il.com,
	Daniel Hodges <hodges.daniel.scott@...il.com>,
	netfilter-devel <netfilter-devel@...r.kernel.org>,
	Network Development <netdev@...r.kernel.org>
Cc: ast@...nel.org,
	bpf@...r.kernel.org,
	daniel@...earbox.net,
	lee@...nel.org,
	linux-kernel@...r.kernel.org,
	linux-leds@...r.kernel.org,
	pavel@....cz
Subject: Re: [PATCH 1/3] leds: trigger: legtrig-bpf: Add ledtrig-bpf module

From: Alexei Starovoitov <alexei.starovoitov@...il.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