Skip to contentSkip to conversation

Do not restart dns client service as a part of --register-dns processing

PATCH In review Selva Nair 2 participants 5 messages Nov 30, 2016 – Dec 2, 2016

lore
Share & tools
lore

Selva Nair

author
#0 lore raw

PATCH 1/1 Do not restart dns client service as a part of --register-dns processing

As reported and discussed on Trac #775, restarting dns service has unwanted side effects when there are dependent services. And it appears unnecessary to restart this service to get DNS registered on Windows.

Resolve by removing two actions from --register-dns:
'net stop dnscache' and 'net start dnscache' run through the service or directly.

Trac: #775

---
 src/openvpn/tun.c             | 18 ++----------------
 src/openvpnserv/interactive.c |  8 +-------
 2 files changed, 3 insertions(+), 23 deletions(-)
src/openvpn/tun.c +2 -16
index 560b1a8..572e168 100644
@@ -4597,23 +4597,9 @@ ipconfig_register_dns (const struct env_set *es)
   bool status;
   const char err[] = "ERROR: Windows ipconfig command failed";
 
-  msg (D_TUNTAP_INFO, "Start net commands...");
+  msg (D_TUNTAP_INFO, "Start ipconfig commands for register-dns...");
   netcmd_semaphore_lock ();
 
-  argv_printf (&argv, "%s%sc stop dnscache",
-	       get_win_sys_path(),
-	       WIN_NET_PATH_SUFFIX);
-  argv_msg (D_TUNTAP_INFO, &argv);
-  status = openvpn_execve_check (&argv, es, 0, err);
-  argv_reset(&argv);
-
-  argv_printf (&argv, "%s%sc start dnscache",
-	       get_win_sys_path(),
-	       WIN_NET_PATH_SUFFIX);
-  argv_msg (D_TUNTAP_INFO, &argv);
-  status = openvpn_execve_check (&argv, es, 0, err);
-  argv_reset(&argv);
-
   argv_printf (&argv, "%s%sc /flushdns",
 	       get_win_sys_path(),
 	       WIN_IPCONFIG_PATH_SUFFIX);
@@ -4629,7 +4615,7 @@ ipconfig_register_dns (const struct env_set *es)
   argv_reset(&argv);
 
   netcmd_semaphore_release ();
-  msg (D_TUNTAP_INFO, "End net commands...");
+  msg (D_TUNTAP_INFO, "End ipconfig commands for register-dns...");
 }
 
 void
src/openvpnserv/interactive.c +1 -7
index 6c6772d..df1c214 100644
@@ -893,8 +893,7 @@ RegisterDNS (LPVOID unused)
   WCHAR sys_path[MAX_PATH];
   DWORD timeout = RDNS_TIMEOUT * 1000; /* in milliseconds */
 
-  /* default paths of net and ipconfig commands */
-  WCHAR net[MAX_PATH]   = L"C:\\Windows\\system32\\net.exe";
+  /* default path of ipconfig command */
   WCHAR ipcfg[MAX_PATH] = L"C:\\Windows\\system32\\ipconfig.exe";
 
   struct
@@ -903,8 +902,6 @@ RegisterDNS (LPVOID unused)
       WCHAR *cmdline;
       DWORD timeout;
     } cmds [] = {
-                  { net,   L"net stop dnscache",     timeout },
-                  { net,   L"net start dnscache",    timeout },
                   { ipcfg, L"ipconfig /flushdns",    timeout },
                   { ipcfg, L"ipconfig /registerdns", timeout },
                 };
@@ -914,9 +911,6 @@ RegisterDNS (LPVOID unused)
 
   if(GetSystemDirectory(sys_path, MAX_PATH))
     {
-      _snwprintf (net, MAX_PATH, L"%s\\%s", sys_path, L"net.exe");
-      net[MAX_PATH-1] = L'\0';
-
       _snwprintf (ipcfg, MAX_PATH, L"%s\\%s", sys_path, L"ipconfig.exe");
       ipcfg[MAX_PATH-1] = L'\0';
     }
Signature · 1 line
2.1.4
#1 lore raw

ACK, thanks.

Code looks good, and test compiles. Have not test-run yet but do not expect any surprises there.

I have added a Changes.rst entry (someone *might* notice and wonder if this was intentional :) ) and updated doc/openvpn.8 and the help text in options.c which both used to list all the commands run...

Your patch has been applied to the master branch.

commit fb56058a98dcc81b34cffbdc46417d672b8926e1

Author: Selva Nair
Date:   Wed Nov 30 16:51:36 2016 -0500
     Do not restart dns client service as a part of --register-dns processing

     Signed-off-by: Selva Nair <[email protected]...>
     Acked-by: Gert Doering <[email protected]...>
     Message-Id: <[email protected]...>
     URL: https://www.mail-archive.com/[email protected]/msg13331.html
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering

author
#2 lore raw

Hi,

Quoted 11 lines · Gert Doering

ACK, thanks.

Code looks good, and test compiles. Have not test-run yet but do not expect any surprises there.

I have added a Changes.rst entry (someone *might* notice and wonder if this was intentional :) ) and updated doc/openvpn.8 and the help text in options.c which both used to list all the commands run...

Your patch has been applied to the master branch.

Do we want this in 2.3.14 as well ? If so I'll send a patch with service part removed (note to self: remember to include docs and Changes.rst edits)

Selva

#3 lore raw

Hi,

Quoted 2 lines · Selva Nair

Do we want this in 2.3.14 as well ? If so I'll send a patch with service part removed (note to self: remember to include docs and Changes.rst edits)

My gut feeling is "leave 2.3 as it is, this could actually be *necessary* on XP".

We're so close to 2.4 release that telling windows users that run into this issue to "just upgrade to 2.4, it will no longer restart dnscache *and* brings lots of more goodness anyway!" can be done with good faith.

It's not like we've had tons of bug reports about that so far...

gert

Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...
author
#4 lore raw
Quoted 11 lines · Gert Doering
Quoted 2 lines · Selva Nair

Do we want this in 2.3.14 as well ? If so I'll send a patch with service part removed (note to self: remember to include docs and Changes.rst

edits)

My gut feeling is "leave 2.3 as it is, this could actually be *necessary* on XP".

We're so close to 2.4 release that telling windows users that run into this issue to "just upgrade to 2.4, it will no longer restart dnscache *and* brings lots of more goodness anyway!" can be done with good faith.

Agreed. Also, on further thought, 2.3 needs to run openvpn as admin, so replacing --register-dns with a script is a viable option for those who do not want dns service restart.

Selva