netdev - Re: [PATCH leds v1 4/5] leds: trigger: netdev: support HW offloading

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]

Date:   Fri, 28 May 2021 08:45:56 +0200
From:   Marek Behún <marek.behun@....cz>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Marek Behún <kabel@...nel.org>,
        linux-leds@...r.kernel.org, netdev@...r.kernel.org,
        Pavel Machek <pavel@....cz>, Dan Murphy <dmurphy@...com>,
        Russell King <linux@...linux.org.uk>,
        Matthias Schiffer <matthias.schiffer@...tq-group.com>,
        Jacek Anaszewski <jacek.anaszewski@...il.com>,
        Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
Subject: Re: [PATCH leds v1 4/5] leds: trigger: netdev: support HW
 offloading

On Thu, 27 May 2021 18:57:17 +0200
Andrew Lunn <andrew@...n.ch> wrote:

> On Wed, May 26, 2021 at 08:00:19PM +0200, Marek Behún wrote:
> > Add support for HW offloading of the netdev trigger.
> > 
> > We need to export the netdev_led_trigger variable so that drivers
> > may check whether the LED is set to this trigger.  
> 
> Without seeing the driver side, it is not obvious to me why this is
> needed. Please add the driver changes to this patchset, so we can
> fully see how the API works.

OK, I will send an implementation for leds-turris-omnia with v2.

The idea is that the trigger_offload() method should check which
trigger it should offload. A potential LED controller may be configured
to link the LED on net activity, or on SATA activity. So the method
should do something like this:

  static int my_trigger_offload(struct led_classdev *cdev, bool enable)
  {
    if (!enable)
      return my_disable_hw_triggering(cdev);
	
    if (cdev->trigger == &netdev_led_trigger)
      return my_offload_netdev_triggering(cdev);
    else if (cdev->trigger == &blkdev_led_trigger)
      return my_offload_blkdev_triggering(cdev);
    else
      return -EOPNOTSUPP;
  }

> > -static struct led_trigger netdev_led_trigger = {
> > +struct led_trigger netdev_led_trigger = {
> >  	.name = "netdev",
> >  	.activate = netdev_trig_activate,
> >  	.deactivate = netdev_trig_deactivate,
> >  	.groups = netdev_trig_groups,
> >  };
> > +EXPORT_SYMBOL_GPL(netdev_led_trigger);  
> 
> If these are going to be exported, maybe they should be made const to
> protect them a bit?

The trigger structure must be defined writable, for the code holds
a list of LEDs that have this trigger activated in the structure, among
other data. I don't think if it can be declared as const and then
defined non-const.

Marek