Don't send an ACK to a 500 until *after* the callbacks

[jes]

Summary

We had an issue where HEP traces would show the ACK to a 500 being sent out before the 500 was received.

Details

This is confusing because it makes it look like OpenSIPS is preemptively ACK'ing a 500 it hasn't yet received.

Solution

This PR delays the sending of the ACK until after the callbacks for the 500 have run, which allows the HEP for the 500 response to be transmitted before the ACK and its HEP are transmitted.

Compatibility

I don't imagine this breaks any other scenarios, but it is conceivable that it does. I don't know enough.

Closing issues

[github-actions]

Any updates here? No progress has been made in the last 30 days, marking as stale.

[jes]

No updates here.

[github-actions]

Any updates here? No progress has been made in the last 30 days, marking as stale.

[jes]

No updates here.

[bogdan-iancu]

Hi @jes , there is a very important comment, just on top of the code you moved:

/* acknowledge negative INVITE replies (do it before detailed
	 * on_reply processing, which may take very long, like if it
	 * is attempted to establish a TCP connection to a fail-over dst */

The idea here is to be able to send back the ACK (as confirmation for the negative reply) and stop any potential retransmission from the callee side - any extra processing here (like reply related callbacks) may delay the sending of the ACK and increase the chances of useless negative reply retransmissions.

Still, I agree with you on the bogus tracing behavior (like tracing first the ACK and then the reply) - it is a know issue and there is an outstanding bug report on that. Nevertheless, we do not have a solution here to make everybody happy :)

[liviuchircu]

Some additional context: the issue with the swapped "negative-reply" and ACK HEP packets is a side-effect of commit aaa6b68 which, as @bogdan-iancu mentioned, improves the SIP server's responsiveness and helps reduce UDP retransmission traffic.

The issue is easily reproducible, which is helpful. And as a solution to make both ends meet (fix the HEP without compromising on performance), I pushed a patch to master which delays running the negative ACK callbacks until after the 200 OK callbacks are run, effectively reverting to the same callback ordering prior to aaa6b68, while retaining the performance boost.

If you could give some feedback on the patch as well before we backport it, @jes, that would be great! Thanks in advance!

[liviuchircu]

Enough time has passed here for testing the fix on 3.5 beta - now also backported to 3.4. Closing as "Fixed" until further notice.

[jes]

Thanks!