On Mon, Jun 30, 2014 at 3:54 AM, Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
> 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);
For this kind of package activity trigger. led_trigger_blink_oneshot()
is the right API to use.
And what's always_blink for? I think for each package event we detects
here we blink once LED.
Thanks,
-Bryan
>> + 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
