Hi,
with respect to this thread on SR-Users
http://lists.sip-router.org/pipermail/sr-users/2011-March/067696.html
asking why set_dlg_profile() doesn't work in failure_route, I figured out that the problem is related to dlg_manage() usage. Conceptually, tracking dialogs on-demand via that function and using the dialog flag should work equally well. However, the dialog creation process differs regarding the parameters passed to dlg_new_dialog() (located in dlg_handlers.c and called by both approaches) whose signature is
int dlg_new_dialog(struct sip_msg *msg, struct cell *t)
When using dlg_manage(), a NULL pointer is passed instead of an existing transaction cell. The pointer is subsequently passed to two tm module callback registrations where one for the callback type TMCB_MAX is meant as a "temporary workaround of missing dialog_ctx field" as noted by Daniel in commit f6a66b42. However, that very registration call is bound to a check that the transaction exists, i.e., parameter t is not NULL:
http://git.sip-router.org/cgi-bin/gitweb.cgi?p=sip-router;a=blob;f=modules_k...
Consequently, no callback of type TMCB_MAX will be carried out for dialogs generated from dlg_manage(). This poses to be a problem for a bunch of dialog profiling-related exported functions as well as is_known_dlg() since they rely on the get_current_dialog() function. For non-REQUEST routes, that function accesses the current dialog by means of iterating through all of the current transaction's callbacks looking for one of type TMCB_MAX that was previously registered in dlg_create_dialog(). However, as that registration never occurs in case of dlg_manage()-based tracking for reasons lined out above, the set of mentioned, exported functions cannot work. Refer to
http://git.sip-router.org/cgi-bin/gitweb.cgi?p=sip-router;a=blob;f=modules_k...
to see how get_current_dialog() does it.
So how do we fix this?
My idea is to simply remove the not-NULL-check against the transaction t prior to callback registration. If the transaction doesn't exists, the tm module makes sure that it will be created lazily. This is how the first callback registration for types TMCB_RESPONSE_READY and TMCB_RESPONSE_FWDED in dlg_create_dialog() already works, apparently with no issues. I also did a quick, simple test with the check left out that proved to work fine.
As this looks like a fix to a work-around, however, I'd like to ask for some feedback first.
Thanks and
cheers,
--Timo
Hello,
On 3/4/11 4:42 PM, Timo Reimann wrote:
[...]
So how do we fix this?
My idea is to simply remove the not-NULL-check against the transaction t prior to callback registration. If the transaction doesn't exists, the tm module makes sure that it will be created lazily. This is how the first callback registration for types TMCB_RESPONSE_READY and TMCB_RESPONSE_FWDED in dlg_create_dialog() already works, apparently with no issues. I also did a quick, simple test with the check left out that proved to work fine.
As this looks like a fix to a work-around, however, I'd like to ask for some feedback first.
go ahead and commit it on master. I will look over it as well and then we can backport.
Thanks, Daniel
Hey,
On 04.03.2011 21:21, Daniel-Constantin Mierla wrote:
So how do we fix this?
My idea is to simply remove the not-NULL-check against the transaction t prior to callback registration. If the transaction doesn't exists, the tm module makes sure that it will be created lazily. This is how the first callback registration for types TMCB_RESPONSE_READY and TMCB_RESPONSE_FWDED in dlg_create_dialog() already works, apparently with no issues. I also did a quick, simple test with the check left out that proved to work fine.
As this looks like a fix to a work-around, however, I'd like to ask for some feedback first.
go ahead and commit it on master. I will look over it as well and then we can backport.
Done committing to master.
Cheers,
--Timo
Hi Timo,
all looks safe, if you or anyone else had a chance to give it a try, then you can backport.
Thanks, Daniel
On 3/7/11 10:17 AM, Timo Reimann wrote:
Hey,
On 04.03.2011 21:21, Daniel-Constantin Mierla wrote:
So how do we fix this?
My idea is to simply remove the not-NULL-check against the transaction t prior to callback registration. If the transaction doesn't exists, the tm module makes sure that it will be created lazily. This is how the first callback registration for types TMCB_RESPONSE_READY and TMCB_RESPONSE_FWDED in dlg_create_dialog() already works, apparently with no issues. I also did a quick, simple test with the check left out that proved to work fine.
As this looks like a fix to a work-around, however, I'd like to ask for some feedback first.
go ahead and commit it on master. I will look over it as well and then we can backport.
Done committing to master.
Cheers,
--Timo
Hey Daniel,
On 07.03.2011 11:01, Daniel-Constantin Mierla wrote:
all looks safe, if you or anyone else had a chance to give it a try, then you can backport.
Did several tests, all looking well; back-ported the commit.
@侯旭光: Can you also give it a try using the latest git version of either master, 3.1, or 3.0?
Cheers,
--Timo
On 3/7/11 10:17 AM, Timo Reimann wrote:
Hey,
On 04.03.2011 21:21, Daniel-Constantin Mierla wrote:
So how do we fix this?
My idea is to simply remove the not-NULL-check against the transaction t prior to callback registration. If the transaction doesn't exists, the tm module makes sure that it will be created lazily. This is how the first callback registration for types TMCB_RESPONSE_READY and TMCB_RESPONSE_FWDED in dlg_create_dialog() already works, apparently with no issues. I also did a quick, simple test with the check left out that proved to work fine.
As this looks like a fix to a work-around, however, I'd like to ask for some feedback first.
go ahead and commit it on master. I will look over it as well and then we can backport.
Done committing to master.
Cheers,
--Timo
-- Daniel-Constantin Mierla http://www.asipto.com
-
Daniel-Constantin Mierla -
Timo Reimann