Re: [BUG] nf: xt_LED: led-always-blink invisible

[Pablo Neira Ayuso <pablo@netfilter.org>]

Cc'ing LED subsystem developers.

On Thu, Jun 19, 2014 at 11:17:49AM +0200, Jiří Prchal wrote:
> Hi all,
> probably I found bug in kernel 3.14.0 in xt_LED module when set
> led-always-blink. If it is set, then between switch led OFF and ON
> is almost zero time. So blink is invisible. I did some fix, but I'm
> not sure if this way would be good. Please, help to direct me. This
> is not final patch so don't punish me for coding style.

Yes, coding style needs to be fixed.

On top of that, I need a second opinion on this since I'm not familiar
with the led subsystem.

@Richard, Bryan: Would you comment on this, please?

Thanks.

> diff --git a/net/netfilter/xt_LED.c b/net/netfilter/xt_LED.c
> index 993de2b..430584b 100644
> --- a/net/netfilter/xt_LED.c
> +++ b/net/netfilter/xt_LED.c
> @@ -54,30 +54,32 @@ static unsigned int
>  led_tg(struct sk_buff *skb, const struct xt_action_param *par)
>  {
>  	const struct xt_led_info *ledinfo = par->targinfo;
>  	struct xt_led_info_internal *ledinternal = ledinfo->internal_data;
> +        unsigned long t=50;
> 
>  	/*
>  	 * If "always blink" is enabled, and there's still some time until the
>  	 * LED will switch off, briefly switch it off now.
>  	 */
>  	if ((ledinfo->delay > 0) && ledinfo->always_blink &&
>  	    timer_pending(&ledinternal->timer))
> -		led_trigger_event(&ledinternal->netfilter_led_trigger, LED_OFF);
> -
> -	led_trigger_event(&ledinternal->netfilter_led_trigger, LED_FULL);
> +		//led_trigger_event(&ledinternal->netfilter_led_trigger, LED_OFF);
> +          led_trigger_blink_oneshot(&ledinternal->netfilter_led_trigger, &t, &t, 1);
> +        else
> +                led_trigger_event(&ledinternal->netfilter_led_trigger, LED_FULL);
> 
>  	/* If there's a positive delay, start/update the timer */
>  	if (ledinfo->delay > 0) {
>  		mod_timer(&ledinternal->timer,
>  			  jiffies + msecs_to_jiffies(ledinfo->delay));
> 
>  	/* Otherwise if there was no delay given, blink as fast as possible */
>  	} else if (ledinfo->delay == 0) {
>  		led_trigger_event(&ledinternal->netfilter_led_trigger, LED_OFF);
>  	}
> 
>  	/* else the delay is negative, which means switch on and stay on */
> 
>  	return XT_CONTINUE;
>  }
> 
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html