Use an exit-event to stop openvpn instead of killing it #1
Conversation
Currently openvpn is stopped by killing the process causing potential truncation of log messages, client not sending exit-notify to the server etc. Using an exit-event provides a way to cleanly stop the process. Signed-off-by: Selva Nair <selva.nair@gmail.com>
Fix delay in log file updates and missing log messages on exit. Also simplifies the code as the timer used to periodically trigger flushing is no more needed. Signed-off-by: Selva Nair <selva.nair@gmail.com>
|
@selvanair: I found about the lack of exit-events in openvpnserv2 when running my Powershell testsuite against the t_client servers that also setup IPv6 routes. I noticed that the first IPv6 ping tests always succeeded, but all subsequent ones failed. I did not debug the problem, but some netsh calls were failing. Possibly the default IPv6 route was left behind, and further netsh calls to add a different default route failed. This is clearly a bug that we should be fixed. Interestingly the problem manifested itself only when testing with openvpnserv2, not when running OpenVPN from cmd.exe or via openvpn-gui.exe. I was unable to break IPv4 connectivity despite brutally killing openvpn processes on the fly without notice, so the problem seems limited to IPv6. Anyways, I will have a look at the C# code today, so that we can get this into 2.4_beta1 due on Wednesday. |
|
The code changes look reasonable, but my experience with C# in general and Windows programming in particular is fairly limited. I'll send email to openvpn-devel to see if somebody with more experience would volunteer to review this PR. |
|
So far no response from any C# developers on openvpn-devel list. I would be inclined to merge this just before 2.4_beta1 if nobody has done the review by then. FYI: the related IPv6 route setup bug has now been reported here. |
|
Hi, On Wed, Nov 16, 2016 at 12:43:52AM -0800, Samuli Seppänen wrote:
I'm not sure there is any other C# developer than you... :-/ What about the original author? Maybe he could have a look? gertUSENET is not the non-clickable part of WWW! |
|
@cron2 : I can poke at xkjyeah (the original author). Chipitsine also does C#. I also believe SviMik is a Windows dev, but he has not been around that much lately. |
|
I asked xkjyeah if he could do the review. |
|
Whoa. That's highly dubious (the use of native C system calls). I cannot test a build, but I'm going to propose some changes |
|
Differences between Selva's version and mine. Syntactically they look like a 1-for-1 translation, but I'm not sure if the semantics are the same. Testers, anyone? selvanair/openvpnserv2@exit-event-v2...xkjyeah:exit-event-v2 |
|
@selvanair : this PR can be closed, right? |
|
Closing as PR #2 replaces this. |
For previous discussion on this topic see here
@mattock: Thanks for testing. The main improvement I see is that log file is not anymore incomplete on exit. Tearing down routes is nice too, but at least for ipv4 it seems routes do disappear even otherwise when the tunnel goes down. Sending exit notifies with --explicit-exit-notify also works now.
Note: The last fixup commit is squashed down for ease of review.