<html><head><meta http-equiv="Content-Type" content="text/html charset=iso-8859-1"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><br><div><div>On 18 Nov 2013, at 16:46, Mark Michelson <<a href="mailto:mmichelson@digium.com">mmichelson@digium.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">
<meta content="text/html; charset=ISO-8859-1" http-equiv="Content-Type">
<div bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">On 11/18/2013 08:39 AM, Olle E.
Johansson wrote:<br>
</div>
<blockquote cite="mid:5C37F33E-0885-43AE-B50F-0873F1015A96@edvina.net" type="cite">
<pre wrap="">Hi!
Someone (I can't track the change down in svn here) changed the to-tag handling in chan_sip to this:
/* Get their tag if we haven't already */
if (ast_strlen_zero(p->theirtag) || (resp >= 200)) {
char tag[128];
gettag(req, "To", tag, sizeof(tag));
ast_string_field_set(p, theirtag, tag);
} else {
/* Store theirtag to track for changes when 200 responses to invites are received without SDP */
ast_string_field_set(p, theirprovtag, p->theirtag);
}
This change broke a lot of installations with forking proxys and PRACK. Before an established dialog, many to-tags can come in from various phones and servers ringing, but you lock when the dialog is established.
This code makes our responses use the first tag and not use the tag from the request we're responding to, which breaks communication.
In order for me to fix this, I would need help in finding the commit and the reason for this change. Maybe we could change like this:
if (!ast_test_flag(&p->flags[1], SIP_PAGE2_DIALOG_ESTABLISHED)) {
Thanks for your help!
/O
</pre>
</blockquote>
Here's the review that resulted in that change:<br>
<br>
<meta http-equiv="content-type" content="text/html;
charset=ISO-8859-1">
<a href="https://reviewboard.asterisk.org/r/2827/">https://reviewboard.asterisk.org/r/2827/</a><br>
<br>
It was committed to 1.8 in revision 399989.<br></div></blockquote>Thanks. Always interesting to work on networks with firewalls that do strange stuff...</div><div><br><blockquote type="cite"><div bgcolor="#FFFFFF" text="#000000">
<br>
You and I both shipped it. :)<br></div></blockquote>(oh dear).</div><div><br><blockquote type="cite"><div bgcolor="#FFFFFF" text="#000000">
<br>
To sum up the change, the idea was that if we get a 200 OK with no
SDP in response to on an outgoing initial INVITE, then we should
still be able to accept the 200 OK if a provisional response was
received that had an SDP. In order to make sure that the 200 OK was
received from the same destination that sent the provisional
response with SDP, we save the to-tag from the provisional response.<br>
<br>
The problem, as you have noted, is that we can receive provisional
responses from many destinations if the call is forked. I made the
following note on the review:<br>
<br>
"
<meta http-equiv="content-type" content="text/html;
charset=ISO-8859-1">
Our INVITE could be forked by a proxy to multiple UASs. If multiple
destinations send 183 responses with SDP answers, then it becomes
important not only that we negotiated an SDP but that we received an
SDP on the particular branch that the 200 OK is received on. And as
you know, chan_sip's transaction and forking awareness is pretty
dismal."<br>
<br>
The to-tag problem can be worked around (likely with the suggested
patch in your e-mail). I just fear that in a forking scenario that
there's more that can go wrong. To fix it correctly, we need proper
transaction and forking support in chan_sip. Let's say we send an
INVITE, it gets forked, and we receive 183 responses from
destinations A, B, and C with SDPs a, b, and c respectively. Now
destination B sends a 200 OK with no SDP. We now have to not only
use the appropriate to-tag to match the incoming 200 OK, but we also
have to apply SDP b to the outgoing call as well, even though the
latest SDP we received may have been SDP a or c. This seems like a
fairly reasonable scenario given the setup you have described.<br></div></blockquote><div><br></div><div>Well, normally we don't answer a provisional response, but with PRACK we do, so the to-tag becomes more</div><div>important. I'll include this change in my PRACK branch. I am not sure that it will affect anything else in 1.8.</div><div>For later releases we might consider fixing this.</div><div><br></div><div>Again, thank you for your quick answer.</div><div><br></div><div>/O</div><br><blockquote type="cite"><div bgcolor="#FFFFFF" text="#000000">
<br>
Mark Michelson<br>
</div>
-- <br>_____________________________________________________________________<br>-- Bandwidth and Colocation Provided by <a href="http://www.api-digital.com">http://www.api-digital.com</a> --<br><br>asterisk-dev mailing list<br>To UNSUBSCRIBE or update options visit:<br> <a href="http://lists.digium.com/mailman/listinfo/asterisk-dev">http://lists.digium.com/mailman/listinfo/asterisk-dev</a></blockquote></div><br></body></html>