24 Dec
2015
24 Dec
'15
1:32 a.m.
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
30 Dec
30 Dec
9:39 a.m.
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
31 Dec
31 Dec
5:42 a.m.
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
4 Jan
4 Jan
1:33 p.m.
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
10:33 p.m.
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(a)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(a)lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
10:23 p.m.
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
10:23 p.m.
Closed #454.
---
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/454#event-505305452
3053
days inactive
3064
days old
6 comments
4 participants
participants (4)
-
Armen Babikyan -
Armen Babikyan -
Camille Oudot
-
Daniel-Constantin Mierla