[PATCH] Add a network activity LED trigger

[PATCH] Add a network activity LED trigger

[Florian Fainelli]

[-- Attachment #1.1: Type: text/plain, Size: 419 bytes --]

Hi all,

This patch adds a new LED trigger, based on network activity. It gathers 
activity from net/core/dev.c and can be used as a LED trigger by 
specifying "network-activity". Further version should allow the user to 
specify the network interface to bind a LED to. This trigger is a "simple" 
trigger as defined by the LED subsystem.

Signed-off-by: Florian Fainelli <florian.fainelli@telecomint.eu>
-- 

[-- Attachment #1.2: ledtrig_network_activity.patch --]
[-- Type: text/plain, Size: 3571 bytes --]

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 87d2046..fdc5a8a 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -128,5 +128,12 @@ config LEDS_TRIGGER_HEARTBEAT
 	  load average.
 	  If unsure, say Y.
 
+config LEDS_TRIGGER_NETWORK_ACT
+	tristate "LED Network Activity Trigger"
+	depends on LEDS_TRIGGERS && NET
+	help
+	  This allow LEDs to be controlled by network activity at layer-3 networking.
+	  If unsure, say Y.
+
 endmenu
 
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index aa2c18e..bc899d3 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -21,3 +21,4 @@ obj-$(CONFIG_LEDS_COBALT)		+= leds-cobalt.o
 obj-$(CONFIG_LEDS_TRIGGER_TIMER)	+= ledtrig-timer.o
 obj-$(CONFIG_LEDS_TRIGGER_IDE_DISK)	+= ledtrig-ide-disk.o
 obj-$(CONFIG_LEDS_TRIGGER_HEARTBEAT)	+= ledtrig-heartbeat.o
+obj-$(CONFIG_LEDS_TRIGGER_NETWORK_ACT)  += ledtrig-network-activity.o
diff --git a/drivers/leds/ledtrig-network-activity.c b/drivers/leds/ledtrig-network-activity.c
new file mode 100644
index 0000000..5c2e051
--- /dev/null
+++ b/drivers/leds/ledtrig-network-activity.c
@@ -0,0 +1,63 @@
+/*
+ * LED Network Activity Trigger
+ *
+ * based on ledtrig-ide-disk by Richard Purdie
+ * 
+ * Copyright 2007 Florian Fainelli <florian@openwrt.org>
+ * 
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ * 
+ */
+
+#include <linux/module.h>
+#include <linux/jiffies.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/timer.h>
+#include <linux/leds.h>
+
+static void ledtrig_network_timerfunc(unsigned long data);
+
+DEFINE_LED_TRIGGER(ledtrig_network);
+static DEFINE_TIMER(ledtrig_network_timer, ledtrig_network_timerfunc, 0, 0);
+static int network_activity;
+static int network_lastactivity;
+
+void ledtrig_network_activity(void)
+{
+	network_activity++;
+	if (!timer_pending(&ledtrig_network_timer))
+		mod_timer(&ledtrig_network_timer, jiffies + msecs_to_jiffies(10));
+}
+EXPORT_SYMBOL(ledtrig_network_activity);
+
+static void ledtrig_network_timerfunc(unsigned long data)
+{
+	if (network_lastactivity != network_activity) {
+		network_lastactivity = network_activity;
+		led_trigger_event(ledtrig_network, LED_FULL);
+		mod_timer(&ledtrig_network_timer, jiffies + msecs_to_jiffies(10));
+	} else {
+		led_trigger_event(ledtrig_network, LED_OFF);
+	}
+}
+
+static int __init ledtrig_network_init(void)
+{
+	led_trigger_register_simple("network-activity", &ledtrig_network);
+	return 0;
+}
+
+static void __exit ledtrig_network_exit(void)
+{
+	led_trigger_unregister_simple(ledtrig_network);
+}
+
+module_init(ledtrig_network_init);
+module_exit(ledtrig_network_exit);
+
+MODULE_AUTHOR("Florian Fainelli <florian@openwrt.org>");
+MODULE_DESCRIPTION("LED Network Activity trigger");
+MODULE_LICENSE("GPL");
diff --git a/net/core/dev.c b/net/core/dev.c
index ee051bb..a3a4115 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -117,6 +117,7 @@
 #include <linux/err.h>
 #include <linux/ctype.h>
 #include <linux/if_arp.h>
+#include <linux/leds.h>
 
 /*
  *	The list of packet types we will receive (as opposed to discard)
@@ -1523,6 +1524,7 @@ gso:
 	 * stops preemption for RCU.
 	 */
 	rcu_read_lock_bh();
+	ledtrig_network_activity();
 
 	/* Updates of qdisc are serialized by queue_lock.
 	 * The struct Qdisc which is pointed to by qdisc is now a

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Add a network activity LED trigger

[Patrick McHardy]

Florian Fainelli wrote:
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 87d2046..fdc5a8a 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -128,5 +128,12 @@ config LEDS_TRIGGER_HEARTBEAT
>  	  load average.
>  	  If unsure, say Y.
>  
> +config LEDS_TRIGGER_NETWORK_ACT
> +	tristate "LED Network Activity Trigger"


Module isn't possible, you call the led trigger from net/core/dev.c.


> diff --git a/net/core/dev.c b/net/core/dev.c
> index ee051bb..a3a4115 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -117,6 +117,7 @@
>  #include <linux/err.h>
>  #include <linux/ctype.h>
>  #include <linux/if_arp.h>
> +#include <linux/leds.h>
>  
>  /*
>   *	The list of packet types we will receive (as opposed to discard)
> @@ -1523,6 +1524,7 @@ gso:
>  	 * stops preemption for RCU.
>  	 */
>  	rcu_read_lock_bh();
> +	ledtrig_network_activity();


Besides missing a declaration and not linking without the network
LED config option, its pretty ridiculous to call this for every
packet just to make a led blink.

Re: [PATCH] Add a network activity LED trigger

[Florian Fainelli]

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

Hello Patrick,

Le mercredi 18 juillet 2007, Patrick McHardy a écrit :
> Module isn't possible, you call the led trigger from net/core/dev.c.

You are right, it just occured to me.

> Besides missing a declaration and not linking without the network
> LED config option, its pretty ridiculous to call this for every
> packet just to make a led blink.

Could you suggest me a better way to do so ? The code was highly inspired from 
what is done with the IDE trigger. The declaration is done in linux/leds.h, 
which is included in dev.c for that purpose.

Thank you very much for your answer.

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Add a network activity LED trigger

[Patrick McHardy]

Florian Fainelli wrote:
>> Besides missing a declaration and not linking without the network
>> LED config option, its pretty ridiculous to call this for every
>> packet just to make a led blink.
>>     
>
> Could you suggest me a better way to do so ? The code was highly inspired from 
> what is done with the IDE trigger. The declaration is done in linux/leds.h, 
> which is included in dev.c for that purpose.
>   

Maybe just increment a variable and periodically check it or something
like that.

Re: [PATCH] Add a network activity LED trigger

[Richard Purdie]

On Wed, 2007-07-18 at 15:54 +0200, Patrick McHardy wrote:
> Florian Fainelli wrote:
> >> Besides missing a declaration and not linking without the network
> >> LED config option, its pretty ridiculous to call this for every
> >> packet just to make a led blink.
> >>     
> >
> > Could you suggest me a better way to do so ? The code was highly inspired from 
> > what is done with the IDE trigger. The declaration is done in linux/leds.h, 
> > which is included in dev.c for that purpose.
> >   
> 
> Maybe just increment a variable and periodically check it or something
> like that.

Are there not already packet counters that the LED trigger could just
look at? If it did that at say 20Hz or 10Hz, it would probably look
quite reasonable without impacting on the system too much?

Richard

Re: [PATCH] Add a network activity LED trigger

[Patrick McHardy]

Richard Purdie wrote:
> On Wed, 2007-07-18 at 15:54 +0200, Patrick McHardy wrote:
>   
>> Florian Fainelli wrote:
>>     
>>>> Besides missing a declaration and not linking without the network
>>>> LED config option, its pretty ridiculous to call this for every
>>>> packet just to make a led blink.
>>>>     
>>>>         
>>> Could you suggest me a better way to do so ? The code was highly inspired from 
>>> what is done with the IDE trigger. The declaration is done in linux/leds.h, 
>>> which is included in dev.c for that purpose.
>>>   
>>>       
>> Maybe just increment a variable and periodically check it or something
>> like that.
>>     
>
> Are there not already packet counters that the LED trigger could just
> look at? If it did that at say 20Hz or 10Hz, it would probably look
> quite reasonable without impacting on the system too much

The qdisc counters might work, I think all qdiscs properly maintain
at least byte and packet counters. Or simply check the queue length,
if > 0 there is activity.

Re: [PATCH] Add a network activity LED trigger

[David Miller]

From: Patrick McHardy <kaber@trash.net>
Date: Wed, 18 Jul 2007 15:54:56 +0200

> Florian Fainelli wrote:
> >> Besides missing a declaration and not linking without the network
> >> LED config option, its pretty ridiculous to call this for every
> >> packet just to make a led blink.
> >>     
> >
> > Could you suggest me a better way to do so ? The code was highly inspired from 
> > what is done with the IDE trigger. The declaration is done in linux/leds.h, 
> > which is included in dev.c for that purpose.
> >   
> 
> Maybe just increment a variable and periodically check it or something
> like that.

Yes please, calling this millions of times per second on a high end
router is just not smart.

Re: [PATCH] Add a network activity LED trigger

[Stephen Hemminger]

On Wed, 18 Jul 2007 15:27:38 +0200
Florian Fainelli <florian.fainelli@telecomint.eu> wrote:

> Hi all,
> 
> This patch adds a new LED trigger, based on network activity. It gathers 
> activity from net/core/dev.c and can be used as a LED trigger by 
> specifying "network-activity". Further version should allow the user to 
> specify the network interface to bind a LED to. This trigger is a "simple" 
> trigger as defined by the LED subsystem.
> 
> Signed-off-by: Florian Fainelli <florian.fainelli@telecomint.eu>
> -- 

You are slowing down the network receive path even if LED is not used.

Doing mod_timer() is expensive, on a busy system you would be adding
another lock roundtrip and list operation for each incoming packet.