Comments are welcome You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/454
-- Commit Summary --
* tcpops: implement event_route for unexpectedly closed tcp connections
-- File Changes --
M events.c (13) M events.h (2) M modules/tcpops/tcpops.c (44) M modules/tcpops/tcpops.h (2) M modules/tcpops/tcpops_mod.c (6) M tcp_read.c (24)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/454.patch https://github.com/kamailio/kamailio/pull/454.diff
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/454
Many thanks for this contribution, it was on a to-do list for quite some time. I will look over the patch and merge it if all ok.
One question for now -- given your details that the event route is executed on tcp connection kill or time out, I guess those were some examples and the event route is also executed if the client closes the connection (e.g., it goes offline/closing the sip application), isn't it?
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/454#issuecomment-167967599
Hi Daniel,
I just repushed this branch. Originally, I only considered tcp connection kill via TCP RST (which results in read() == -1 && errno == ECONNRESET) and timeout events (read() == -1 && errno == ETIMEDOUT). I have revised my branch to also execute the event_route on gracefully-closed TCP connections (i.e. via TCP FIN), which is delivered to the program as read() == 0 (i.e. EOF).
Additionally, I have restricted the execution of this event_route that have a $conid variable that does not evaluate to "<null>" in the script. This is determined via connections that have TCP_REQ_COMPLETE(req) set to true (which is when con->id is copied to con->proto_reserved1). Do let me know if this is not the correct strategy here.
Thanks!
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/454#issuecomment-168128187
Hi Armen, thanks for your patch, I think this feature will be very useful. I have one comment: if the tcpops module is loaded but no `tcp:closed` route is defined, a debug log line will complain about it. I think it would make sense to add a module parameter to globally enable or disable the event route lookup and execution. Besides, as many of the tcpops function can act on a per-socket basis, it would be nice to have this event route called or not called depending on the socket (e.g. using a `tcp_connection` flag and defining a new tcpops function to set/reset this flag). What do you think?
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/454#issuecomment-168677156
Camille, thanks for the feedback. Those are great points, and I'll try to complete work on those configuration features soon.
On Mon, Jan 4, 2016 at 5:33 AM, Camille Oudot notifications@github.com wrote:
Hi Armen, thanks for your patch, I think this feature will be very useful. I have one comment: if the tcpops module is loaded but no tcp:closed route is defined, a debug log line will complain about it. I think it would make sense to add a module parameter to globally enable or disable the event route lookup and execution. Besides, as many of the tcpops function can act on a per-socket basis, it would be nice to have this event route called or not called depending on the socket (e.g. using a tcp_connection flag and defining a new tcpops function to set/reset this flag). What do you think?
— Reply to this email directly or view it on GitHub https://github.com/kamailio/kamailio/pull/454#issuecomment-168677156.
sr-dev mailing list sr-dev@lists.sip-router.org http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
I merged the patch manually, to split it in commits per component, willing to make it easier to track changes on those parts.
I also think that the suggestions by @camilleoudot are useful - a parameter for a global enable/disable of event route execution, plus the flexibility to do it per connection with a flag.
As the initial patch was merged, anyone can add those proposed enhancements.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/454#issuecomment-168829849
Closed #454.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/454#event-505305452
-
Armen Babikyan -
Armen Babikyan -
Camille Oudot
-
Daniel-Constantin Mierla