Module: sip-router Branch: master Commit: e973bbe5e7310861f77b17ce0afaf1cca35fe48a URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=e973bbe5...
Author: Juha Heinanen jh@tutpro.com Committer: Juha Heinanen jh@tutpro.com Date: Fri May 28 15:38:41 2010 +0300
modules/tm: new implementation of load_contacts()/next_contacts()
- Simpler implementation of load_contacts()/next_contacts() functions based on recent changes related to Request-URI handling. - Function next_contacts() does not anymore set any timers. Check contact_avp and call t_set_fr() before calling t_relay() instead. - Removed fr_timer_next module parameter, because it is not needed anymore.
---
modules/tm/README | 255 ++++++++++++++------------------------ modules/tm/doc/functions.xml | 51 +++----- modules/tm/doc/params.xml | 73 ----------- modules/tm/doc/tm.xml | 15 +-- modules/tm/t_serial.c | 279 ++++++++++++------------------------------ modules/tm/tm.c | 1 - 6 files changed, 190 insertions(+), 484 deletions(-)
Diff: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commitdiff;h=e973...
Juha,
this change broke support for calling t_load_contacts from failure_route. When used from failure_route, the current ruri is added again as new branch. Can you elaborate on why the relevant code was removed?
Attached patch restores the functionality. Is there anything wrong with this fix or shall i commit it?
Alex.
On Friday 28 May 2010 14:43:32 Juha Heinanen wrote:
Module: sip-router Branch: master Commit: e973bbe5e7310861f77b17ce0afaf1cca35fe48a URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=e973bb e5e7310861f77b17ce0afaf1cca35fe48a
Author: Juha Heinanen jh@tutpro.com Committer: Juha Heinanen jh@tutpro.com Date: Fri May 28 15:38:41 2010 +0300
modules/tm: new implementation of load_contacts()/next_contacts()
- Simpler implementation of load_contacts()/next_contacts() functions based on recent changes related to Request-URI handling.
- Function next_contacts() does not anymore set any timers. Check contact_avp and call t_set_fr() before calling t_relay() instead.
- Removed fr_timer_next module parameter, because it is not needed anymore.
modules/tm/README | 255 ++++++++++++++------------------------ modules/tm/doc/functions.xml | 51 +++----- modules/tm/doc/params.xml | 73 ----------- modules/tm/doc/tm.xml | 15 +-- modules/tm/t_serial.c | 279 ++++++++++++------------------------------ modules/tm/tm.c | 1 - 6 files changed, 190 insertions(+), 484 deletions(-)
Diff: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commitdiff;h=e9 73bbe5e7310861f77b17ce0afaf1cca35fe48a
sr-dev mailing list sr-dev@lists.sip-router.org http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
Alex Hermann writes:
this change broke support for calling t_load_contacts from failure_route. When used from failure_route, the current ruri is added again as new branch. Can you elaborate on why the relevant code was removed?
if i remember correctly, the change in ruri handling was such that ruri can be re-set in failure/reply routes instead of calling append_branch, and when t_relay is called again, it uses both ruri and branches (if added).
i'm not currently in situation where i could do tests on this, but i'm pretty sure that the code used to work. also jan did some work on it.
-- juha
On Tuesday 02 August 2011 08:22:39 Juha Heinanen wrote:
Alex Hermann writes:
this change broke support for calling t_load_contacts from failure_route. When used from failure_route, the current ruri is added again as new branch. Can you elaborate on why the relevant code was removed?
if i remember correctly, the change in ruri handling was such that ruri can be re-set in failure/reply routes instead of calling append_branch, and when t_relay is called again, it uses both ruri and branches (if added).
I went over tm documentation again, but couldn't find any setting or function to reset/ignore ruri.
Maybe there is some internal state keeping track of the ruri changes. In that case, i'll adjust the patch to use ruri only depending on that state. Somebody will have to tell me where that state is kept though.
i'm not currently in situation where i could do tests on this, but i'm pretty sure that the code used to work. also jan did some work on it.
Is my testcase wrong?
Alex Hermann writes:
I went over tm documentation again, but couldn't find any setting or function to reset/ignore ruri.
Maybe there is some internal state keeping track of the ruri changes. In that case, i'll adjust the patch to use ruri only depending on that state. Somebody will have to tell me where that state is kept though.
every time t_next_contacts() is called, it rewrites request uri with first remaining contact and if there are other contacts with the same q value, it appends then as branches.
what is wrong with that?
-- juha
On Friday 05 August 2011, Juha Heinanen wrote:
Alex Hermann writes:
I went over tm documentation again, but couldn't find any setting or function to reset/ignore ruri.
Maybe there is some internal state keeping track of the ruri changes. In that case, i'll adjust the patch to use ruri only depending on that state. Somebody will have to tell me where that state is kept though.
every time t_next_contacts() is called, it rewrites request uri with first remaining contact and if there are other contacts with the same q value, it appends then as branches.
The issue is not with t_next_contacts, but with t_load_contacts. It is not discarding the ruri of the request that made the request end up in failure_route. Instead it is added again to the dset, where it obviously doesn't belong.
Alex Hermann writes:
The issue is not with t_next_contacts, but with t_load_contacts. It is not discarding the ruri of the request that made the request end up in failure_route. Instead it is added again to the dset, where it obviously doesn't belong.
after you have called t_load_contacts(), you should call t_next_contacts(), which does what i explained earlier. after that ruri contains a contact with highest q value. calling t_load_contacts() alone does not make sense.
-- juha
On Friday 05 August 2011, Juha Heinanen wrote:
Alex Hermann writes:
The issue is not with t_next_contacts, but with t_load_contacts. It is not discarding the ruri of the request that made the request end up in failure_route. Instead it is added again to the dset, where it obviously doesn't belong.
after you have called t_load_contacts(), you should call t_next_contacts(), which does what i explained earlier. after that ruri contains a contact with highest q value.
Of course, but is that happens to be the old ruri, it is used again directly.
If it is not the highest q, it will be added to the contacts avp and it will be used again if t_next_contacts is called again.
calling t_load_contacts() alone does not make sense.
I did not say that, i just said the issue is with t_load_contacts. t_next_contacts does function correctly, but as the old request uri is added to the contacts avp, some invocation (depending on ruri's q value) of t_next_contacts will show the failure of t_load_contacts.
Alex Hermann writes:
after you have called t_load_contacts(), you should call t_next_contacts(), which does what i explained earlier. after that ruri contains a contact with highest q value.
Of course, but is that happens to be the old ruri, it is used again
directly.
ruti is not "old" to load_contacts() function if you call it first time in failure route.
load_contacts() works on current destination set (ruri + branches). before calling t_load_contacts(), you need to arrange so that destination set contains whatever contacts are correct for you.
-- juha
On Friday 05 August 2011 16:24:41 you wrote:
load_contacts() works on current destination set (ruri + branches). before calling t_load_contacts(), you need to arrange so that destination set contains whatever contacts are correct for you.
So, how would i actually remove the 'used' ruri then?
Alex Hermann writes:
So, how would i actually remove the 'used' ruri then?
perhaps there are script functions to manipulate destination set.
how did you get your destination set in the first place?
-- juha
On Saturday 06 August 2011, you wrote:
Alex Hermann writes:
So, how would i actually remove the 'used' ruri then?
perhaps there are script functions to manipulate destination set.
how did you get your destination set in the first place?
Set $ru, t_relay(), receive a 302, in failure_route call uac_redirect(), then t_load_contacts() and t_next_contacts().
This used to work ok in 1.x, but not so in current master.
Alex Hermann writes:
Set $ru, t_relay(), receive a 302, in failure_route call uac_redirect(), then t_load_contacts() and t_next_contacts().
This used to work ok in 1.x, but not so in current master.
if you call t_relay() after calling uac_redirect(), does t_relay() try to re-use ruri?
since in current sr release ruri can be re-set in failure route, i.e., there is no need to append new ruri as branch, in my opinion uac_redirect() implementation should be changed so that it rewrites ruri with one 302 contact and appends the remaining 302 contacts as branches.
-- juha
if it is possible to somehow determine that ruri of ds has already been used by t_relay, it would be possible to patch load contacts so that it ignores such a used ruri.
-- juha
On Saturday 06 August 2011 19:44:59 Juha Heinanen wrote:
if it is possible to somehow determine that ruri of ds has already been used by t_relay, it would be possible to patch load contacts so that it ignores such a used ruri.
That's what i suggested in my second email in this thread. :)
Anyway, i found the responsible variable and respun the patch, see below.
If this is acceptable, i'll commit this version.
alex,
thanks for the patch. i applied it to master and 3.1 branch.
-- juha
alex,
even in failure route, script writer could have called enum_query(), uac_redict(), etc., that establish new destination set, and then call t_load_contacts(). that is why t_load_contacts() behavior is independent on where it is called. it cannot assume that in failure route ruri has already been used.
-- juha
-
Alex Hermann -
Juha Heinanen