<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/1698/">https://reviewboard.asterisk.org/r/1698/</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;">I don't think that this patch as written is adequate; it turns on *both* force_rport and comedia when NAT is detected, and there could definitely be situations where that results in broken calls. If we're going to provide an 'auto' mode, we should also provide 'force_rport_auto' and 'comedia_auto', so that those can be individually automatically enabled in cases where the UA (or mid-path network element) in use doesn't work with both of them enabled.
However, I think there is a potentially larger problem here: whether 'force_rport' and/or 'comedia' will work properly for any given path that the remote UA is using is dependent on *both* the UA's behavior and the behavior of the network elements (NATs, SBCs, proxies, etc.) in between that UA and Asterisk. To go back to the reporter's statements in JIRA... what if the 3G operator's edge device (presumably an SBC) doesn't hide itself (so Asterisk detects that NAT is in use) but still doesn't support symmetric RTP (comedia)? The risk here is that users will just expect 'auto' mode to make everything work smoothly all the time, and that won't always be the case. To some extent, the current situation of requiring the Asterisk admin to take some sort of action (learning what is required for that configuration) is potentially better, because the admin will have to learn how to deal with these network complexities. Having an 'automatic' mode leads to users believing that they don't need to understand or care about these topics, and then we have to help them later when their calls "don't work".</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/1698/diff/2/?file=23678#file23678line16218" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/channels/chan_sip.c</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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 check_via(struct sip_pvt *p, struct sip_request *req)</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">16218</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="cm">/* Check and see if we are likely to be behind a NAT. If we are, set the</span></pre></td>
</tr>
<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">16219</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> * natdetected flag so that later, peers with nat=auto can use the value.</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;">If 'we' are behind a NAT? I don't this feature is related to *Asterisk* being behind a NAT at all. Instead of using personal pronouns, it would be best to refer to the entities involved by their proper designations, since that avoids confusion.</pre>
</div>
<br />
<p>- Kevin</p>
<br />
<p>On January 26th, 2012, 8:28 p.m., Terry Wilson 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 Terry Wilson.</div>
<p style="color: grey;"><i>Updated Jan. 26, 2012, 8:28 p.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;">First, this patch as originally posted is from JIRA user pedro-garcia. It has been in JIRA for a long while, and has finally come up to be considered. There were some changes that I wanted to made to the original patch, so I tried contacting the author to get them to put the patch on reviewboard, but after a week I got no response. So, I'm putting the original patch up, then will immediately add a review with my changes. Many thanks to pedro-garcia for his contribution.