fr_inv_timer restarted when restart_fr_on_each_reply = 0 and replies sequence is 100,183,180,183

[mikomarrache]

Hi,

We just set the restart_fr_on_each_reply parameter to 0 so that fr_inv_timer is not restarted when provisional replies are received but we got the following issue.

The SIP flow was:

10:56:37.203070      |-----------INVITE sip:+27137445208@196.25.241.86 SIP/2.0----------->|
10:56:37.438361      |<------------------------100 Giving a try...------------------------|
10:56:38.184987      |<------------------------183 Session Progress-----------------------|                                                                 
10:56:38.275681      |---PRACK sip:27137445208@196.25.241.86:5060;transport=udp SIP/2.0-->|
10:56:38.516773      |<-------------------------------200 OK------------------------------|
10:56:38.517898      |<----------------------------180 Ringing----------------------------|
10:56:38.601496      |---PRACK sip:27137445208@196.25.241.86:5060;transport=udp SIP/2.0-->|
10:56:38.843527      |<-------------------------------200 OK------------------------------|
10:57:37.925117      |<------------------------183 Session Progress-----------------------|
10:57:38.005561      |---PRACK sip:27137445208@196.25.241.86:5060;transport=udp SIP/2.0-->|
10:57:38.247836      |<-------------------------------200 OK------------------------------|
10:58:28.132556      |-----------CANCEL sip:+27137445208@196.25.241.86 SIP/2.0----------->|

As you can see, the replies received by OpenSIPS for the initial INVITE transaction are 100, 183, 180, 183. The transaction is cancelled by the UAC (the CANCEL is not generated by OpenSIPS) around 10:58:28.

The fr_inv_timer was set to 60 seconds, therefore I expected the call to be cancelled around 10:57:37 (i.e. 10:56:37 + 60 sec).

I looked at the t_should_relay_response function (see source file t_reply.c at line 1073), the UAC last_received field is overridden by the received status for each received provisional reply. Also, in the reply_received function (see source file t_reply.c at lines 1640-1642), the condition

(last_uac_status < msg_status) && ((msg_status >= 180) || (last_uac_status == 0))

will match for:

• the 100 reply (since it is the first reply - last_uac_status = 0)
• the first 183 reply (since 100 < 183 and 183 >= 180)
• the second 183 reply (since 180 < 183 and 183 >= 180). Note when the second 183 reply is processed, last_uac_status is 180 because of the last received 180 reply.

Therefore, it seems the fr_inv_timer may be restarted "indefinitely" is the sequence 180,183,180,183... is returned by the UAS.

Is this behavior expected?

Regards, Mickael

[jalung]

fr_inv_timer no longer exists since 1.11, it was changed to fr_inv_timeout if that is what you mean.

[bogdan-iancu]

@miko95 , the fix should prevent the trimer restart because of a 180 reply between two 183 ? (like keeping the highest reply code received on the branch and do restart only if you got something higher).

[mikomarrache]

@bogdan-iancu Yes, this is what I meant. Also, if restart_fr_on_each_reply is disabled, why the timer should be restarted if a 183 is received after a 100? Once we received the 100 reply, we assume the next hop will attempt to reach the called party (or fail to) and therefore the timer should not be restarted if a 183/180 reply is received after.

[bogdan-iancu]

@mikomarrache , please try this patch and let me know if it solves the problem: https://gist.github.com/bogdan-iancu/0d21b8a56d67a33272db

Thanks and regards, Bogdan

[mikomarrache]

@bogdan-iancu Thanks for the patch. I see where the new field (highest_received) is assigned but I don't see where it is used.

[bogdan-iancu]

@mikomarrache sorry for that - that was an intermediary version. I updated the patch, please check it again. Thanks, Bogdan

[bogdan-iancu]

@mikomarrache , any updates with the testing ?

[mikomarrache]

Hi,

Sorry for the long delay, the patch causes a new issue: OpenSIPS doesn't switch from the fr_timer to the fr_inv_timer when the first provisional reply is received.

Looking at the code without applying the patch, I see that last_uac_status is 0 when the first provisioning reply is received. Therefore, the condition last_uac_status<msg_status evaluates to true and OpenSIPS switches to the fr_inv_timer.

After applying the patch, I see that uac->highest_received is 100 when the first provisioning reply is received. So, the condition uac->highest_received<msg_status evaluates to false and OpenSIPS doesn't switch to the fr_inv_timer.

I expect uac->highest_received to be 0 as before since no reply has been received yet.

Thanks, Mickael

[mikomarrache]

Following my previous comment.

It seems that the purpose of the last_uac_status line was to save the value of uac->last_received before it is modified. This way last_uac_status would have the value 0 when the first provisioning reply is received.

I guess we need the same thing here, right?

[bogdan-iancu]

@mikomarrache , are you still looking into troubleshooting this ? I can extend the patch for some debugging.

[ankogan]

The same thing is in 3.2 Does this patch broke anything?

Index: t_reply.c
===================================================================
--- t_reply.c   (revision 12394)
+++ t_reply.c   (working copy)
@@ -1671,10 +1671,8 @@
                goto done;

        /* update FR/RETR timers on provisional replies */
-       if (msg_status < 200 && (restart_fr_on_each_reply ||
-       ((last_uac_status<msg_status) &&
-       ((msg_status >= 180) || (last_uac_status == 0)))
-       ) ) { /* provisional now */
+       if (msg_status < 200 && (restart_fr_on_each_reply || last_uac_status == 0))
+       { /* provisional now */
                if (is_invite(t)) {
                        /* invite: change FR to longer FR_INV, do not
                         * attempt to restart retransmission any more