<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/2919/">https://reviewboard.asterisk.org/r/2919/</a>
</td>
</tr>
</table>
<br />
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/static/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 Michael Young.</div>
<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-22236">ASTERISK-22236</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
Asterisk
</div>
<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;">One part of the fix committed to resolve the one-way audio when using the new auto_* NAT settings in ASTERISK-21374, created a problem when someone has Asterisk configured in a, let's say, not a wrong way but yet not the recommended way. I have to take blame for this since I didn't test this one scenario and my logic was off a little.
Issue:
The problem created is, that, the peer's SIP_NAT_FORCE_RPORT flag is never copied to the dialog when the global NAT setting is set to being off (nat=no) and the individual peer's NAT settings are turned on.
Scenario:
I say this is not the recommended setting because we recommend that the global NAT not be set to something different than what is used in the peer's settings. It is actually best to use the global setting and then turn NAT off, on a needed basis per peer vs turning it off globally and then on for all your peers in the peer's context. The new auto_* settings do a pretty decent job now as well, which is the default global setting.
So, when a dialog is created, it's flags are initially set to that of the global settings. Well with the commit that caused an issue, if auto_force_rport is off because the global setting is nat=no, the peer's SIP_NAT_FORCE_RPORT flag would not be copied to the dialog. That was just bad logic on my part since it incorrectly assumed that either auto_force_rport or force_rport would be on in the global settings when NAT is involved.
(I hope the above description of the issue is clear)
Fix:
What this patch does is remove the conditional checking for the SIP_PAGE3_NAT_AUTO_RPORT flag being set in order to determine if it should copy the peer's SIP_NAT_FORCE_RPORT flag or not.
The condition was put there with the incorrect thinking that if force_rport was already on, there is no need to copy the peer's flag because it would already be set on the dialog. In the case of auto_force_rport being used, then it would need the peer's flag to determine whether we needed to turn force_rport on or not for the current dialog.</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;">The reporter tested changing his settings around to the recommended way. He reported back that everything was working again. This helped to confirm that the bad logic described above was the culprit.
Tested this patch on production machine and didn't find any issues with it. Also, tested it on local dev box.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/branches/11/channels/chan_sip.c <span style="color: grey">(401105)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/2919/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>