[asterisk-dev] dialog matching
Raj Jain
rj2807 at gmail.com
Sat Dec 22 18:16:43 CST 2007
Hi Steve,
> So, TELL ME, please, that if totag is empty, that You Never Will See more
than one dialog with the same callid,
> and mayhaps with different tag field values, or better yet, with different
theirtag values.
I'd not get into the fundamental root of the problem (which is that we
currently don't have a notion of sessions, dialogs and transactions as
separate state machines), but to answer your question -- you could have a
situation where a proxy forks an INVITE from Asterisk to several UASs some
of which are RFC 2543 based. These UASs may not insert a To tag in their
responses (To/From tags were optional in RFC 2543).
I'm not sure I fully understand what all goes into computing your hash key,
but To tag should not be a part of it (simply because it could be absent).
I've personally dealt with this problem (in a different implementation) by
only hashing on the Call-ID and then resolving the collision by checking
something to the effect of the following:
(!totag[0] && !p->tag[0]) || (p->tag[0] && match(p->tag,totag))
Thanks,
Raj
On Dec 20, 2007 4:57 PM, Steve Murphy <murf at parsetree.com> wrote:
> On Wed, 2007-12-19 at 17:06 -0700, Steve Murphy wrote:
>
> Olle--
>
> I thought to double check with you... the price of missing something is
> pretty high.
>
> Put on your thinking cap, pull out some paper and a pencil, turn off the
> cell phone... this is going to require
> some thought!
>
>
> For reference, here's the search loop in find_call:
>
>
> dialoglist_lock();
> for (p = dialoglist; p; p = p->next) {
> /* In pedantic, we do not want packets with bad syntax to
> be connected to a PVT */
> int found = FALSE;
> if (ast_strlen_zero(p->callid))
> continue;
> if (req->method == SIP_REGISTER)
> found = (!strcmp(p->callid, callid));
> else
> found = (!strcmp(p->callid, callid) &&
> (!pedanticsipchecking || !tag ||ast_strlen_zero(p->theirtag) || !strcmp(p->theirtag, tag))) ;
>
> ast_debug(5, "= %s Their Call ID: %s Their Tag %s Our tag:
> %s\n",
> found ? "Found" : "No
> match", p->callid, p->theirtag, p->tag);
>
> /* If we get a new request within an existing to-tag -
> check the to tag as well */
> if (pedanticsipchecking && found && req->method !=
> SIP_RESPONSE) { /* SIP Request */
> if (p->tag[0] == '\0' && totag[0]) {
> /* We have no to tag, but they have. Wrong
> dialog */
> found = FALSE;
> } else if (totag[0]) { /* Both
> have tags, compare them */
> if (strcmp(totag, p->tag)) {
> found = FALSE; /* This is
> not our packet */
> }
> }
> if (!found)
> ast_debug(5, "= Being pedantic: This ...
> request: Call ID: %s Ourtag <null> Totag %s Method %s\n",
> p->callid, totag,
> sip_methods[req->method].text);
> }
>
>
> if (found) {
> /* Found the call */
> sip_pvt_lock(p);
> dialoglist_unlock();
> return p;
> }
> }
> dialoglist_unlock();
>
> The object: remove the loop, and replace it with astobj2 hash table
> lookups.
>
> I checked Luigi's astobj2 branch, where he put dialogs in a table, but he
> implements the above as a linear traversal, which is not my goal. I want to
> improve performance, not just convert the code to be astobj2 based...
>
> There seem to be two main modes: with pedanticsipchecking ON, and
> pedanticsipchecking OFF.
> ------------------------------
>
>
>
>
> Things are simple in OFF mode:
>
> fundamentally, whether req->method is SIP_REGISTER or not, found is true
> when a dialog's callid matches.
> No code after setting found could disqualify this choice.
>
> ------------------------------
>
>
>
>
> Things get more complex in pedanticsipchecking ON mode:
>
> It seems split into whether req->method is SIP_REGISTER/SIP_RESPONSE or
> not:
>
> If req->method is SIP_REGISTER:
>
> if match(p->callid,callid) AND (!totag[0] || (p->tag[0] &&
> match(p->tag,totag))
>
> else
>
> If req->method is SIP_RESPONSE:
>
> If callid matches AND ( empty(p->theirtag) ||
> match(p->theirtag,tag))
>
> else
>
> If callid matches AND (!totag[0] || (p->tag[0] &&
> match(p->tag,totag))
> AND ( empty(p->theirtag) ||
> match(p->theirtag,tag))
>
> (I notice in the code that there is : ... || !tag || ... which
> will never be true, and can be dropped)
>
> ------------------------------
>
>
>
> OK, terms like: empty(p->theirtag) ||
> match(p->theirtag,tag) ,
>
> which means: "if p->theirtag is null/empty, then it doesn't affect the
> match, otherwise, it must match the tag.
>
> This I can handle just fine, most likely by trying to ao2_find with
> theirtag set, and if no match, look for a null theirtag match.
>
>
> But: (!totag[0] || (p->tag[0] && match(p->tag,totag))
>
> means that if the totag is empty, then it matches whatever p->tag might be
> in the db. This is impossible to replicate in hashtable searches, because
> the search term is blank, and will not match any entries where the field is
> not blank.
> Nor can I define another table, or even just enter another row with the
> tag field blank, BECAUSE THERE MIGHT BE A COLLISION.
>
> I have come to the conclusion that this is not necessarily a weakness in
> the trying to use hash tables. but could be a weakness in the linear search
> algorithm itself. If totag is empty, then you could conceivably fetch the
> wrong dialog from the db, which would probably just be the first one you hit
> in your traversal.
>
> So, TELL ME, please, that if totag is empty, that You Never Will See more
> than one dialog with the same callid, and mayhaps with different tag field
> values, or better yet, with different theirtag values.
>
> --Before the search starts, I see this code:
>
> if (ast_strlen_zero(totag) && (req->method == SIP_ACK
> ||
> req->method == SIP_BYE
> ||
> req->method == SIP_INFO )) {
> ast_debug(5, "%s must have a to tag. dropping
> callid: %s from: %s\n",
>
> sip_methods[req->method].text , callid, from );
> return NULL;
> }
>
> This doesn't protect the SIP_REGISTER case; and the other non-SIP_RESPONSE
> cases:
> SIP_UNKNOWN, /*!< Unknown response */
> SIP_OPTIONS, /*!< Check capabilities of a device, used
> for "ping" too */
> SIP_NOTIFY, /*!< Status update, Part of the event
> package standard, result of a SUBSCRIBE or a REFER */
> SIP_INVITE, /*!< Set up a session */
> SIP_PRACK, /*!< Reliable pre-call signalling. Not
> supported in Asterisk. */
> SIP_REFER, /*!< Refer to another URI (transfer) */
> SIP_SUBSCRIBE, /*!< Subscribe for updates (voicemail,
> session status, device status, presence) */
> SIP_MESSAGE, /*!< Text messaging */
> SIP_UPDATE, /*!< Update a dialog. We can send UPDATE;
> but not accept it */
> SIP_CANCEL, /*!< Cancel an INVITE */
> SIP_PUBLISH, /*!< Not supported in Asterisk */
> SIP_PING, /*!< Not supported at all, no standard but
> still implemented out there */
>
> So, is there a problem here or not?
>
> murf
>
>
>
> _______________________________________________
> --Bandwidth and Colocation Provided by http://www.api-digital.com--
>
> asterisk-dev mailing list
> To UNSUBSCRIBE or update options visit:
> http://lists.digium.com/mailman/listinfo/asterisk-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.digium.com/pipermail/asterisk-dev/attachments/20071222/e3e4a693/attachment-0001.htm
More information about the asterisk-dev
mailing list