[SEMS] Missing to tag in outgoing PRACK (181)

[denyspozniak]

Hello!

I would like to ask for help in explaining why this commit does not take into account the response code 181?

if (((reply.code == 100) || (reply.code == 181) || (reply.code == 182))

96ea442#diff-662a586075af062e252bef12fdb1f16a7b2fd6b40eba3083d43aa19263b94e5cR370

I stumbled upon this problem again: [SEMS] Missing to tag in outgoing PRACK #100:

[dmitry-sinina]

All these PRACK/to-tag problems caused by f931ab7 that broke state machine. It is not clear why exemption for some responses was added there.

[juha-h]

So we need to revert f931ab7. OK?

[dmitry-sinina]

@juha-h looks like you tried to fix something by this commit(at least from comment "Do not go to Early state if To tag can still change. // Otherwise reply check will fail") so may be this something should be fixed in other way. Could you remember why this commit was added?

[juha-h]

Dmitry Sinina writes:

@juha-h looks like you tried to fix something by this commit(at least from comment **"Do not go to Early state if To tag can still change. // Otherwise reply check will fail"**) so may be this something should be fixed in other way. Could you remember why this commit was added?

I can't remember, it was too long time ago (2017). Regarding 181, it is sent by the server, not by the UAS and thus to tag can change. Was there to tag in the example call shown above? Is it according to the standards, to send PRACK before final response has arrived?

[denyspozniak]

@juha-h

Was there to tag in the example call shown above?

[dmitry-sinina]

Regarding 181, it is sent by the server, not by the UAS and thus to tag
can change. Was there to tag in the example call shown above?

There is no difference between UAS and server from SIP protocol point of view. Actually UAS and UAC are terms related to initiator of SIP transaction and responder so server is same UAS + UAC as end-user SIP phone - because both able to initiate transactions and respond on them

Is it according to the standards, to send PRACK before final response
has arrived?

This is why PRACK exists, to acknowledge receiving of provisioning replies.

[juha-h]

Denys Pozniak writes:

>Was there to tag in the example call shown above? 

OK, so the question is: if sems goes to early state when it receives 181 with to-tag, is it able to handle the possible change of to-tag in the final response?

[dmitry-sinina]

could you explain this "possible change of to-tag in the final response" case? Do you mean case with call forking when multiple early-dialogs are possible?

[juha-h]

Dmitry Sinina writes:

could you explain this "possible change of to-tag in the final response" case?

In case 181 was sent by proxy that is doing the forwarding, then there will be some other to-tag in the final response from the uas. An element that can act as a proxy can also send reliable provisional responses. In this case, it acts as a UAS for purposes of that transaction.

[juha-h]

juha-h writes:

In case 181 was sent by proxy that is doing the forwarding, then there will be some other to-tag in the final response from the uas. An element that can act as a proxy can also send reliable provisional responses. In this case, it acts as a UAS for purposes of that transaction.

Same case when proxy parallel forks invite to several uas. They all reply with 181 ringing.

[dmitry-sinina]

In this case SEMS have to handle multiple early-dialogs for call leg in same time(at least to be able support 100rel), I can't say exactly right now, but I think there is no such feature, so it should be fixed by implementing multiple early-dialogs support.

[juha-h]

juha-h writes:

Same case when proxy parallel forks invite to several uas. They all reply with 181 ringing.

Sorry, with 180 ringing.

[juha-h]

This issue #64 led to the commit f931ab7. So whatever the fix is, it would need to be tested to work in both cases.

[juha-h]

Perhaps it would be possible to check if response has Rseq header and if so, go to Early state.

[juha-h]

Try with this patch:

*** /usr/src/orig/sems/core/AmSipDialog.cpp	2021-11-14 13:22:22.183742473 +0200
--- AmSipDialog.cpp	2022-04-25 14:33:40.470191108 +0300
***************
*** 371,377 ****
      case Proceeding:
        if(reply.code < 200){
  	// Do not go to Early state if reply did not come from an UAS
! 	if (((reply.code == 100) || (reply.code == 181) || (reply.code == 182))
  	    || reply.to_tag.empty())
  	  setStatus(Proceeding);
  	else {
--- 371,378 ----
      case Proceeding:
        if(reply.code < 200){
  	// Do not go to Early state if reply did not come from an UAS
! 	if (((reply.code == 100 || reply.code == 181 || reply.code == 182)
! 	     && reply.rseq == 0)
  	    || reply.to_tag.empty())
  	  setStatus(Proceeding);
  	else {

[dmitry-sinina]

There are lot other problems were introduced by f931ab7 - for example UPDATE request construction, routeset changes will not be applied. Right way - rollback it and investigate why second instance of AmSipDialog is not created when response with other to-tag received.

[juha-h]

Rolled back commit f931ab7 due to several issues (including this one). Test and close if prack now has to tag.

[AlexAT]

@dmitry-sinina Thanks for noticing it in #183
Reverting this one actually may help, I'll try building with this commit reverted and tell.
Thanks!

[dmitry-sinina]

I think problem from #176 also related to f931ab7

[juha-h]

I think problem from #176 also related to f931ab7

Please test if that is the case.

[denyspozniak]

Made some basic tests with the latest build from the master, and the problem with the absence of the to tag in the outgoing PRACK is gone. I need some more time to test the rest of the functionality.

[juha-h]

Denys Pozniak writes:

Made some basic tests with the latest build from the master, and the problem with the absence of the to tag in the outgoing PRACK is gone. I need some more time to test the rest of the functionality.

Good to learn that.

[AlexAT]

I can as well confirm this reversal fixes PRACK for 181 responses.
Continuing runtime testing as well to make sure it doesn't break anything visible :)

[juha-h]

Good. Should we create a new branch 1.6.0? Latest commit to 1.5.0 was done in 2014.

[juha-h]

There was lots of stale branches, which I deleted. Now only master is remaining. Since rpm has version 1.7.0 and debian has 1.7.0-dev, I'll create new branch 1,7.0. Bug fixes should go to that and new features to master.

[juha-h]

Re-open if new problems occur.