<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://reviewboard.asterisk.org/r/1652/">https://reviewboard.asterisk.org/r/1652/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://reviewboard.asterisk.org/r/1652/diff/1/?file=22553#file22553line13606" style="color: black; font-weight: bold; text-decoration: underline;">branches/1.8/channels/chan_sip.c</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">static void reg_source_db(struct sip_peer *peer)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">13606</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">ast_random</span><span class="p">()</span> <span class="o">%</span> <span class="p">((</span><span class="n">peer</span><span class="o">-></span><span class="n">qualifyfreq</span><span class="p">)</span> <span class="o">?</span> <span class="n">peer</span><span class="o">-></span><span class="n">qualifyfreq</span> <span class="o">:</span> <span class="n">global_qualifyfreq</span><span class="p">)</span> <span class="o">+</span> <span class="mi">1</span><span class="p">,</span><span class="ew"> </span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Red blob</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://reviewboard.asterisk.org/r/1652/diff/1/?file=22553#file22553line29291" style="color: black; font-weight: bold; text-decoration: underline;">branches/1.8/channels/chan_sip.c</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">static void sip_poke_all_peers(void)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">29289</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="n">peer</span><span class="o">-></span><span class="n">maxms</span> <span class="o">></span> <span class="mi">0</span><span class="p">)</span> <span class="p">{</span> <span class="cm">/* dont schedule poking on a peer without qualify */</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This comment can be on its own line, preceeding the if statement</pre>
</div>
<br />
<p>- Matt</p>
<br />
<p>On January 5th, 2012, 3:14 a.m., schmidts wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers.</div>
<div>By schmidts.</div>
<p style="color: grey;"><i>Updated Jan. 5, 2012, 3:14 a.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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)</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://issues.asterisk.org/jira/browse/ASTERISK-19154">ASTERISK-19154</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>branches/1.8/channels/chan_sip.c <span style="color: grey">(349449)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1652/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>