Subject: Re: [PATCH 1/8] isdn/gigaset: ratelimit CAPI message dumps

From: Karsten Keil <[email-masked]>
Date: Sat, 28 Apr 2012 11:29:32 +0200

Am 27.04.2012 12:29, schrieb Tilman Schmidt:
> Am 26.04.2012 08:39, schrieb Karsten Keil:
>> Am 26.04.2012 01:02, schrieb Tilman Schmidt:
>>> Introduce a global ratelimit for CAPI message dumps to protect
>>>  against possible log flood. Drop the ratelimit for ignored 
>>> messages which is now covered by the global one.
>> 
>> Hmm, I think the only CAPI messages which would need a ratelimit 
>> are related to the DATA_B3 messages. If you need CAPI debug 
>> messages in most cases you do not need all of the DATA_B3, but 
>> you do not want to miss any other message related to the call 
>> control. With a general rate limit you do not have the control, 
>> which messages are logged and which are not.
> 
> The ratelimit introduced by this patch only applies to messages 
> other than DATA_B3. Logging DATA_B3 messages is not done via 
> dump_cmsg().
> 

Thanks for the clarification, forget about my objection.
I ack this patch now.

> I'd like to ratelimit specifically non-DATA_B3 messages because I 
> saw a (possibly buggy) CAPI application flooding the log with 
> FACILITY messages. Equally important, I'd like to make the 
> ratelimit in do_nothing() / do_unsupported() bursty because I had
> a case where I needed to see several ignored/unhandled CAPI
> messages in quick succession. So this patch is killing two birds
> with one stone for me.
> 
> The burst limit of 20 messages in 20 seconds is chosen to allow a 
> complete call setup sequence to be logged, while limiting to one 
> message per second in the long run.
> 
>> And here maybe some cases, when even the DATA_B3 are important 
>> (e.g. searching bugs in flow control), so I would make it still 
>> conditional to allow to print all messages.
> 
> DATA_B3 dumps produce an enormous amount of log data and are 
> therefore controlled separately by the DEBUG_MCMD flag. Someone
> who enables that should know what she or he does. But if you need
> them, you need them all. A ratelimit doesn't make sense there in
> my experience.
> 
>> And I'm not sure, if this is really something for stable.
> 
> It's pretty simple and localized, a net simplification, and only 
> affects generation of debugging messages, so I think it's safe.
> But if you see a problem there I can drop the "CC: stable" line.
> 

I let the decision about it to you and the stable maintainers.

- -- 
Karsten Keil
Linux Kernel Development
Tel: +49 175 7249132
Mail: [email-masked]

B1 Systems GmbH
Osterfeldstraße 7 / 85088 Vohburg / http://www.b1-systems.de
GF: Ralph Dehner / Unternehmenssitz: Vohburg / AG: Ingolstadt,HRB 3537