[asterisk-dev] [Code Review]: dont poke registered sip peers immediately after sip reload to avoid packet storm

[Matt Jordan]

> On Jan. 10, 2012, 8:44 a.m., Matt Jordan wrote:
> > The issue I have with this patch as it currently exists is that it changes expected behavior in 1.8 unilaterally, for something that is arguably not a bug (at least for a small to medium number of peers).  There may be people who, because they reload chan_sip often, depend on the qualify going out immediately.  However, I can see the case for this going into 1.8 as, for a large number of peers, the behavior may be unacceptable.
> > 
> > I'd prefer to see a global option in sip.conf that would control this behavior.  The options would be to:
> >  (a) schedule the qualify immediately on reload using the previous logic, and use the range of times to schedule in the case of initial startup (keeping the qualifyfreq as the maximum range however, which - in my opinion - does make more sense the hardcoded 5 seconds)
> >  (b) use the range of times for all cases
> > 
> > Default option would be (a), but would allow people with a large number of defined peers to throttle back the pokes.
> > 
> > Either way, changes in the behavior of chan_sip should also be noted in the CHANGES file.
> 
> wdoekes wrote:
>     Noting the change in the CHANGES is good, but adding more options feels like overkill to me.
>     
>     Can't we have it "guess" whether we want behaviour (a) or (b):
>     e.g. count(peers) >= 100, use (b)

Meh.  After stumbling across this review, I think my previous objection was a bit too conservative.

If you reload chan_sip, the previous status of the peers isn't lost.  This just prevents packet storm.  As the original issue reporter pointed out, this is a detrimental situation, given a sufficient number of peers.

Shipping it.


- Matt


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1652/#review5151
-----------------------------------------------------------


On Jan. 5, 2012, 3:14 a.m., schmidts wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1652/
> -----------------------------------------------------------
> 
> (Updated Jan. 5, 2012, 3:14 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> with a big amount of sip peers registered doing a sip reload causes a packet flood of sip OPTIONS messages cause they are immediately sent out when the peer is loaded back from the astdb.
> This fix prevents this packet storm and just schedule a poke in a random value between 1 ms and peers qualifyfreq or the global qualifyfreq. 
> Another very small fix is to schedule a poke on peers with qualify disabled. This doesnt cost much but its just unnecessary to do this.
> 
> maybe it would be even better to dont call the poke function from source_db on a reload cause poking all peers is done after this again.
> 
> 
> This addresses bug ASTERISK-19154.
>     https://issues.asterisk.org/jira/browse/ASTERISK-19154
> 
> 
> Diffs
> -----
> 
>   branches/1.8/channels/chan_sip.c 349449 
> 
> Diff: https://reviewboard.asterisk.org/r/1652/diff
> 
> 
> Testing
> -------
> 
> tested with 4500 peers with qualify=no and no poke is scheduled after a sip reload.
> tested with 2500 peers with qualify=yes and pokes are scheduled in the right time (1 to 60000 ms)
> 
> 
> Thanks,
> 
> schmidts
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120731/ba711b72/attachment.htm>