diff mbox series

[Openvpn-devel] Remove 1 second delay before running netsh

Message ID 20201215173950.26596-1-domagoj@pensa.hr
State Superseded
Headers
Series [Openvpn-devel] Remove 1 second delay before running netsh |

Commit Message

Domagoj Pensa Dec. 15, 2020, 6:39 a.m. UTC 
  When running various netsh commands before each 1 second sleep is added.
As more netsh commands are run, especially for Wintun adapters, that can
add to a noticable delayed connecting time.

This should be safe. No problems were found in tests and all netsh
commands executed properly with delay removed. Also, no delays are used
in a similar code in interactive service and netsh command executions
are guarded with a semaphore.

Signed-off-by: Domagoj Pensa <domagoj@pensa.hr>
---
 src/openvpn/tun.c | 1 -
 1 file changed, 1 deletion(-)

Comments

Gert Doering Dec. 20, 2020, 9:48 a.m. UTC | #1 
Hi,

On Tue, Dec 15, 2020 at 06:39:50PM +0100, Domagoj Pensa wrote:
> When running various netsh commands before each 1 second sleep is added.
> As more netsh commands are run, especially for Wintun adapters, that can
> add to a noticable delayed connecting time.
> 
> This should be safe. No problems were found in tests and all netsh
> commands executed properly with delay removed. Also, no delays are used
> in a similar code in interactive service and netsh command executions
> are guarded with a semaphore.

This is... interesting.  The offending sleep(1) was added there by
james, 15 years ago, in commit a9c802b2a (imported from SVN)

That commit basically split the "sleep(5) after each try" into
"sleep(1) before and sleep(4) after", so possibly it was needed 
before the very first netsh call...

The change itself is not commented in the commit message, so we 
can only guess

commit a9c802b2a3f77f2b906e22f582681cdec0790c32
Author: James Yonan <james@openvpn.net>
Date:   Thu Dec 22 18:09:40 2005 +0000

    --ip-win32 adaptive is now the default.
    --ip-win32 netsh (or --ip-win32 adaptive when in netsh
    mode) can now set DNS/WINS addresses on the TAP-Win32
    adapter.



Lev: do you have a particular opinion on this change?  You do more
testing with wintun...

gert
Lev Stipakov Dec. 20, 2020, 9:06 p.m. UTC | #2 
Hi,

> Lev: do you have a particular opinion on this change?  You do more
> testing with wintun...

Wintun uses iservice where we don't have those delays.

I tend to agree with Vladislav (themiron), let's use management_sleep(0) which
processes any pending actions on the management interface without any wait.

-Lev
Domagoj Pensa Dec. 24, 2020, 1:03 a.m. UTC | #3 
Hi!

I've sent a new patch that uses management_sleep(0) instead.

I've also added your explanation why management_sleep(0) is used.

Regards,
Domagoj

On Mon, Dec 21, 2020 at 10:06:57AM +0200, Lev Stipakov wrote:
> Hi,
> 
> > Lev: do you have a particular opinion on this change?  You do more
> > testing with wintun...
> 
> Wintun uses iservice where we don't have those delays.
> 
> I tend to agree with Vladislav (themiron), let's use management_sleep(0) which
> processes any pending actions on the management interface without any wait.
> 
> -Lev
diff mbox series

Patch

diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index 400a50ca..e9f1aadb 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -5207,7 +5207,6 @@  netsh_command(const struct argv *a, int n, int msglevel)
     for (i = 0; i < n; ++i)
     {
         bool status;
-        management_sleep(1);
         netcmd_semaphore_lock();
         argv_msg_prefix(M_INFO, a, "NETSH");
         status = openvpn_execve_check(a, NULL, 0, "ERROR: netsh command failed");