Hi,
It is the same fix I applied to my working copy when i started this thread. So, it has been running for a week now, no problems so far.
Regards,
Cesc
Unclassified
Jan Janak jan@iptel.org 05/31/05 03:53PM >>>
It should be fixed now, please update your sources and re-try.
thanks for the report, Jan.
On 24-05-2005 10:42, Cesc Santasusana wrote:
Hi,
Ok ... so it is a bug ... now how to fix it?
static inline int after_loose(struct sip_msg* _m, int preloaded) ..... if (enable_double_rr && is_2rr(&puri.params)) { .... if (res > 0) { /* No next route found */ DBG("after_loose: No next URI found\n"); return (preloaded ? NOT_RR_DRIVEN : RR_DRIVEN); or simply return (preloaded RR_DRIVEN);
Cesc
Unclassified
Jan Janak jan@iptel.org 05/23/05 07:25PM >>>
Hello, comments inline.
On 23-05-2005 18:57, Cesc Santasusana wrote:
Hi,
I found what i think is a bug in the return value of loose_route() ..... bear with me because it is a rather long email.
First, the scenario:
phone1 ----- SER1 ----- SER2 ----- phone2
p1 to s1 and p2 to s2 use UDP s1 to s2 uses tcp or tls
The bug comes into place when there is a change of protocol in ser, from UDP into TCP or TLS. We do not use a preloaded route set from the phones. What ser does is it adds 2 route headers, using the r2 parameter. Route: <route1>, <route2>
CASE A: -------------------------------------------- In this scenario, say p1 places a call to p2, going through s1 and s2, creating the route set with the record-route. Now, let's say p2 generates the BYE message. When the BYE gets to s1, the last hop, in the config we do: if( loose_route () ) { t_relay(); } But, and here is the inconsistency, loose_route (remember we are using TCP/TLS, so there are 2 routes in one header field) returns FALSE.
CASE B: -------------------------------------------- If this experiment is repeated using ALL UDP, the route headers when they get to s1 contain only one route, and the call to loose_route() returns ..... TRUE!
Unless the config file does not allow blind sip message relaying, both cases end up working ... the BYE gets to the final destination. But if the config file does not accept relaying ... the request-uri in the BYE is not taken as a local domain ... and thus the message is rejected ... (To Alex Mack: could this be the solution to your problem? the BYEs not reaching the phone? add some log output and check it ... )
================================================================ So, this is not consistent. Now, what is the solution? I think that both case A and B should return the same, but what, true or false?
Yes, they should return the same value in this case and it should be true (see below). It looks like you discovered a bug.
As I see it, the solution causing less trouble is that both return true. So, the last proxy considers this BYE as being loose_routed. For this ... change the code in: rr::loose_route.c:: static inline int after_loose(struct sip_msg* _m, int preloaded) ..... if (enable_double_rr && is_2rr(&puri.params)) { .... if (res > 0) { /* No next route found */ DBG("after_loose: No next URI found\n"); return (preloaded ? NOT_RR_DRIVEN : RR_DRIVEN); or simply return (preloaded RR_DRIVEN);
And now ... the other option is that both return false ... but this would make big changes. Correct me if i am wrong, but wouldn't this be the compliant solution? Considering that the r-uri is not part of the route set, when the last proxy processes all route headers and there are no more left, there is no loose routing done. The destination is taken from the r-uri, not the route set ... thus loose_route should return false. Or I am getting it all wrong? rfc and goal of loose_route() ?
The meaning of the return value of loose_route is as follows (or should be):
1 - Record routing was used to route the message 0 - Record routing was not (usually requests establishing a dialog or transactional requests).
So the function should return true in the mentioned case, because record routing was used although the message does not contain any Route header fields anymore.
In other words the purpose of loose_route return value is to distinguish mid-dialog requests (such as ACK or BYE) from dialog-establishing requests (such as INVITE). The reason why we have to do this is that we typically do not apply all the logic in the script to mid-dialog requests (because you should not change the request-uri of the request anymore once the dialog has been established -- changes caused by loose_route do not count).
You may also wonder why don't we use the contents of the Request-URI for this -- there should be the Contact value of phone1 in the Request-URI in mid-dialog requests and the value should not match uri==myself so all we need to do is to enclose all the processing logic within if (uri == myself) condition.
This would work in a perfect world without the need to be backwards compatible with strict routers. In our world we have to be backwards compatible with strict routers and we have to deal with broken implementations too (that strip ;lr parameters). Due to backwards compatibility with strict routers it can happen that the Request-URI will contain the URI of the proxy that is currently processing the request and subsequent if (uri==myself) would be true.
Jan.