#### Pre-Submission Checklist <!-- Go over all points below, and after creating the PR, tick all the checkboxes that apply --> <!-- All points should be verified, otherwise, read the CONTRIBUTING guidelines from above--> <!-- If you're unsure about any of these, don't hesitate to ask on sr-dev mailing list --> - [*] Commit message has the format required by CONTRIBUTING guide - [*] Commits are split per component (core, individual modules, libs, utils, ...) - [*] Each component has a single commit (if not, squash them into one commit) - [*] No commits to README files for modules (changes must be done to docbook files in `doc/` subfolder, the README file is autogenerated)
#### Type Of Change - [ ] Small bug fix (non-breaking change which fixes an issue) - [*] New feature (non-breaking change which adds new functionality) - [ ] Breaking change (fix or feature that would change existing functionality)
#### Checklist: <!-- Go over all points below, and after creating the PR, tick the checkboxes that apply --> - [*] PR should be backported to stable branches - [*] Tested changes locally - [*] Related to issue #4249
#### Description When sending an INVITE using t_uac_send, the transaction layer automatically generates ACK on final response. However this ack doesn't invoke the tm:local-request callback. This patch adds this functionality You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/4250
-- Commit Summary --
* tm: invoke tm:local-request on generated ACK messages
-- File Changes --
M src/modules/tm/uac.c (127)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/4250.patch https://github.com/kamailio/kamailio/pull/4250.diff
miconda left a comment (kamailio/kamailio#4250)
I wonder if we should add a modparam to be able to enable this, the existing configs will get get "unexpectedly" the local ACKs in the event route starting with the next major release.
henningw left a comment (kamailio/kamailio#4250)
@miconda Its more a bug fix, similar as we have done e.g. in faca8e7a20255d90 and ccc1069228
The tm module documentation clearly states that it should work for all locally generated requests: "Executed when the tm module generates a request, in other words, the request originates from Kamailio and a transaction is created for it. The source of the requests can be tm itself or other modules that use tm internally (e.g., msilo, uac)."
I don't think we need to add a module parameter for it, documenting it clearly in the migration guide should be fine.
tsearle left a comment (kamailio/kamailio#4250)
Is there something I need to do ref updating the release notes?
miconda left a comment (kamailio/kamailio#4250)
I started travelling and this one got out of my sight. I wanted to check if the event route is executed only for transactions started locally, it is also for hop-by-hop ACKs generated on failed received+forwarded INVITEs.
Same question would be also for the CANCEL requests (https://github.com/kamailio/kamailio/commit/faca8e7a20255d90a4786fd4043005ea... and https://github.com/kamailio/kamailio/commit/ccc106922802213253f03f181d10fe83...), is the event route executed only for CANCELs of locally initiated INVITEs, or for all hop-by-hop CANCELs?
If the event route is executed only for ACK/CALCEL of locally initiated INVITEs, then I am fine with no new modparam, but if it also executed for cases of the hop-by-hop ACK/CALCEL of received+forwarded INVITEs, then I consider that the impact can be significant for instances with high SIP traffic and I would prefer to be able to control via a modparam.
henningw left a comment (kamailio/kamailio#4250)
Thanks, I think the same. I would expect that the new behaviour only applies to locally generated requests. @xkaraman please have a look to the code regarding this PR and the referenced commits above in this matter. It might be easier to just do a bunch of tests on one of our test systems.
-
Daniel-Constantin Mierla -
Henning Westerholt
-
tsearle