Set a low interface metric for tap adapter when block-outside-dns is in use
PATCH 1/1 Set a low interface metric for tap adapter when block-outside-dns is in use
From: ValdikSS <[email protected]...>
Windows 10 before Creators Update used to resolve DNS using all available adapters and IP addresses in parallel. Now it still resolves addresses using all available adapters but in a round-robin way, beginning with random adapter. This behaviour introduces significant delay when block-outside-dns is in use.
Fortunately, setting low metric for the TAP interface solves this issue, making Windows always pick with TAP adapter first.
--- src/openvpn/block_dns.c | 75 +++++++++++++++++++++++++++++++++++++++++++ src/openvpn/block_dns.h | 30 +++++++++++++++++ src/openvpn/init.c | 4 +-- src/openvpn/win32.c | 29 +++++++++++++++-- src/openvpn/win32.h | 2 +- src/openvpnserv/interactive.c | 61 ++++++++++++++++++++++++++++++++--- 6 files changed, 191 insertions(+), 10 deletions(-)
src/openvpn/block_dns.c +75 -0
@@ -341,4 +341,79 @@ delete_block_dns_filters(HANDLE engine_handle) return err; } +/* +* Returns interface metric value for specified interface index. +* +* Arguments: +* index : The index of TAP adapter. +* family : Address family (AF_INET for IPv4 and AF_INET6 for IPv6). +* Returns positive metric value or zero for automatic metric on success, +* a less then zero error code on failure. +*/ + +int +get_interface_metric (const NET_IFINDEX index,const ADDRESS_FAMILY family) +{ +DWORD err =0 ; +MIB_IPINTERFACE_ROW ipiface; +InitializeIpInterfaceEntry (&ipiface); + ipiface.Family = family; + ipiface.InterfaceIndex = index; + err =GetIpInterfaceEntry (&ipiface); +if (err ==NO_ERROR ) + { +if (ipiface.UseAutomaticMetric) + { +return 0 ; + } +return ipiface.Metric; + } +return -err; +} + +/* +* Sets interface metric value for specified interface index. +* +* Arguments: +* index : The index of TAP adapter. +* family : Address family (AF_INET for IPv4 and AF_INET6 for IPv6). +* metric : Metric value. 0 for automatic metric. +* Returns 0 on success, a non-zero status code of the last failed action on failure. +*/ + +DWORD +set_interface_metric (const NET_IFINDEX index,const ADDRESS_FAMILY family, +const ULONG metric) +{ +DWORD err =0 ; +MIB_IPINTERFACE_ROW ipiface; +InitializeIpInterfaceEntry (&ipiface); + ipiface.Family = family; + ipiface.InterfaceIndex = index; + err =GetIpInterfaceEntry (&ipiface); +if (err ==NO_ERROR ) + { +if (family ==AF_INET ) + { +/* required for IPv4 as per MSDN */ + ipiface.SitePrefixLength =0 ; + } + ipiface.Metric = metric; +if (metric ==0 ) + { + ipiface.UseAutomaticMetric = TRUE; + } +else + { + ipiface.UseAutomaticMetric = FALSE; + } + err =SetIpInterfaceEntry (&ipiface); +if (err ==NO_ERROR ) + { +return 0 ; + } + } +return err; +} +#endif /* ifdef _WIN32 */
src/openvpn/block_dns.h +30 -0
@@ -27,6 +27,9 @@#ifndef OPENVPN_BLOCK_DNS_H #define OPENVPN_BLOCK_DNS_H +/* Any value less than 5 should work fine. 3 is choosen without any real reason. */ +#define BLOCK_DNS_IFACE_METRIC 3 +typedef void (*block_dns_msg_handler_t ) (DWORD err,const char *msg);DWORD
@@ -36,5 +39,32 @@ DWORD add_block_dns_filters(HANDLE *engine, int iface_index, const WCHAR *exe_path, block_dns_msg_handler_t msg_handler_callback); +/** +* Sets interface metric value for specified interface index +* +* @param index The index of TAP adapter +* @param family Address family (AF_INET for IPv4 and AF_INET6 for IPv6) +* +* @return positive metric value or zero for automatic metric on success, +* a less then zero error code on failure. +*/ + +int +get_interface_metric (const NET_IFINDEX index,const ADDRESS_FAMILY family); + +/** +* Sets interface metric value for specified interface index +* +* @param index The index of TAP adapter +* @param family Address family (AF_INET for IPv4 and AF_INET6 for IPv6) +* @param metric Metric value. 0 for automatic metric +* +* @return 0 on success, a non-zero status code of the last failed action on failure. +*/ + +DWORD +set_interface_metric (const NET_IFINDEX index,const ADDRESS_FAMILY family, +const ULONG metric); +#endif #endif
src/openvpn/init.c +2 -2
@@ -1837,7 +1837,7 @@ do_close_tun(struct context *c, bool force)#if defined(_WIN32)if (c->options.block_outside_dns) { -if (!win_wfp_uninit (c->options.msg_channel)) +if (!win_wfp_uninit (adapter_index, c->options.msg_channel)) {msg (M_FATAL ,"Uninitialising WFP failed!" ); }
@@ -1877,7 +1877,7 @@ do_close_tun(struct context *c, bool force)#if defined(_WIN32)if (c->options.block_outside_dns) { -if (!win_wfp_uninit (c->options.msg_channel)) +if (!win_wfp_uninit (adapter_index, c->options.msg_channel)) {msg (M_FATAL ,"Uninitialising WFP failed!" ); }
src/openvpn/win32.c +27 -2
@@ -61,6 +61,11 @@static HANDLE m_hEngineHandle =NULL ;/* GLOBAL */ /* +* TAP adapter original metric value +*/ +static int tap_metric =-1 ;/* GLOBAL */ + +/* * Windows internal socket API state (opaque). */ static struct WSAData wsa_state;/* GLOBAL */
@@ -1337,6 +1342,21 @@ win_wfp_block_dns(const NET_IFINDEX index, const HANDLE msg_channel) status =add_block_dns_filters (&m_hEngineHandle, index, openvpnpath, block_dns_msg_handler); +if (status ==0 ) + { + tap_metric =get_interface_metric (index,AF_INET ); +if (tap_metric <0 ) + { +/* error, should not restore metric */ + tap_metric =-1 ; + } + status =set_interface_metric (index,AF_INET ,BLOCK_DNS_IFACE_METRIC ); +if (!status) + { +set_interface_metric (index,AF_INET6 ,BLOCK_DNS_IFACE_METRIC ); + } + } + ret = (status ==0 ); out:
@@ -1345,19 +1365,24 @@ out: }bool -win_wfp_uninit (const HANDLE msg_channel) +win_wfp_uninit (const NET_IFINDEX index,const HANDLE msg_channel) {dmsg (D_LOW ,"Uninitializing WFP" );if (msg_channel) {msg (D_LOW ,"Using service to delete block dns filters" ); -win_block_dns_service (false,-1 , msg_channel); +win_block_dns_service (false, index, msg_channel); }else {delete_block_dns_filters (m_hEngineHandle); m_hEngineHandle =NULL ; +if (tap_metric >=0 ) + { +set_interface_metric (index,AF_INET , tap_metric); +set_interface_metric (index,AF_INET6 , tap_metric); + } }return true;
src/openvpn/win32.h +1 -1
@@ -293,7 +293,7 @@ WCHAR *wide_string(const char *utf8, struct gc_arena *gc);bool win_wfp_block_dns (const NET_IFINDEX index,const HANDLE msg_channel); -bool win_wfp_uninit (const HANDLE msg_channel); +bool win_wfp_uninit (const NET_IFINDEX index,const HANDLE msg_channel);#define WIN_XP 0#define WIN_VISTA 1
src/openvpnserv/interactive.c +56 -5
98 lines not on this page · raw message
Signature · 1 line
2.9.3
Hi,
Quoted 6 lines · ValdikSS
Windows 10 before Creators Update used to resolve DNS using all available adapters and IP addresses in parallel.
Now it still resolves addresses using all available adapters but in a round-robin way, beginning with random adapter.
Hmm... If it starts with a random adapter, this metric lowering is not the right fix, isn't it? Or did you mean to say "starting from lowest metric adapter"?
Quoted 4 lines
This behaviour introduces significant delay when block-outside-dns is in use.
Fortunately, setting low metric for the TAP interface solves this issue, making Windows always pick with TAP adapter first.
Well, I'm looking for to excuses to avoid the hard work of a careful review :) If reducing metric is indeed the right approach in this cat and mouse game with MS, I'll try to look through this over the weekend.
Selva
Quoted 8 lines · Selva Nair
Hi,
On Thu, May 4, 2017 at 1:36 PM, ValdikSS <[email protected]... <mailto:[email protected]...>> wrote:
Hmm... If it starts with a random adapter, this metric lowering is not the right fix, isn't it? Or did you mean to say "starting from lowest metric adapter"?
Well, I'm looking for to excuses to avoid the hard work of a careful review :) If reducing metric is indeed the right approach in this cat and mouse game with MS, I'll try to look through this over the weekend.
From what I understand, Windows tries to use DNS in a round-robin fashion if adapter's metric is set to automatic, regardless of computed metric value. If we disable automatic metric on TAP adapter and set it lower than computed metric of other adapters, it would be used first. I could be not completely correct with this, but I tested it and it works.
Quoted 15 lines
Selva
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Hi,
Please bear with me for making a few more comments. This close to final so only
a few minor issues.
Quoted 10 lines · ValdikSS
Windows 10 before Creators Update used to resolve DNS using all available adapters and IP addresses in parallel.
Now it still resolves addresses using all available adapters but in a round-robin way, beginning with random adapter.
This behaviour introduces significant delay when block-outside-dns is in use.
Fortunately, setting low metric for the TAP interface solves this issue, making Windows always pick with TAP adapter first.
Nitpicking here, but anyway a new version is required (see below), so it would be nice if you wrap lines in commit message to something like about 70 to 80 at most. Not sure whether we have a policy on this..
src/openvpn/block_dns.c +0 -0
Quoted 16 lines
index e31765e..9d49355 100644
src/openvpn/block_dns.c +6
-0@@ -341,4 +341,79 @@ delete_block_dns_filters(HANDLE engine_handle)return err; } +/* +* Returns interface metric value for specified interface index. +* +* Arguments: +* index : The index of TAP adapter. +* family : Address family (AF_INET for IPv4 and AF_INET6 for IPv6).
+ * Returns positive metric value or zero for automatic metric on success,
+ * a less then zero error code on failure.
+ */
src/openvpn/block_dns.c
Quoted 7 lines
src/openvpn/block_dns.c +7
-0+ +int +get_interface_metric (const NET_IFINDEX index,const ADDRESS_FAMILY family) +{ +DWORD err =0 ; +MIB_IPINTERFACE_ROW ipiface; +InitializeIpInterfaceEntry (&ipiface);
InitializeIpInterfaceEntry() is missing in all but very recent mingw32 versions
(their commit logs show it was added in early 2015) so we may need to declare it in block_dns.c. I use Debian jessie (8.7) -- mingw gcc 4.9.1 and its
not there. I believe Samuli's build system uses an even older version.
Note that 64 bit build still succeeds with a warning but 32 bit will fail because
of stdcall. Yes, its stdcall though MSDN doesn't mention it as such.
src/openvpn/block_dns.c DWORD · line 45
Quoted 11 lines
typedef void (
*block_dns_msg_handler_t) (DWORD err, const char *msg);DWORD
src/openvpn/block_dns.c +2
-0@@ -36,5 +39,32 @@ DWORD add_block_dns_filters(HANDLE *engine, int iface_index, const WCHAR *exe_path, block_dns_msg_handler_t msg_handler_callback); +/** +* Sets interface metric value for specified interface index
This would be "Get" or "Return" not "Set"
Quoted 13 lines
src/openvpn/block_dns.c +10
-0+* +* @param index The index of TAP adapter +* @param family Address family (AF_INET for IPv4 and AF_INET6 for IPv6) +* +* @return positive metric value or zero for automatic metric on success, +* a less then zero error code on failure. +*/ + +int +get_interface_metric(const NET_IFINDEX index, const ADDRESS_FAMILYfamily);
..
Quoted 17 lines
src/openvpn/win32.c +5
-0@@ -61,6 +61,11 @@static HANDLE m_hEngineHandle =NULL ;/* GLOBAL */ /* +* TAP adapter original metric value +*/ +static int tap_metric =-1 ;/* GLOBAL */ + +/* * Windows internal socket API state (opaque). */ static struct WSAData wsa_state;/* GLOBAL */
src/openvpn/win32.c win_wfp_block_dns(const NET_IFINDEX index, const · line 1349
Quoted 8 lines
src/openvpn/win32.c +3
-0@@ -1337,6 +1342,21 @@ win_wfp_block_dns(const NET_IFINDEX index, const HANDLE msg_channel) status =add_block_dns_filters (&m_hEngineHandle, index, openvpnpath, block_dns_msg_handler); +if (status ==0 ) + { + tap_metric =get_interface_metric (index,AF_INET );
The current metric is queried and saved only for v4 while the metric is set and restored for both v4 and v6. Isn't it necessary to also save the metric for both independently? I'm not sure whether anyone really uses different metric for v4 and v6 but Windows allows it.
Quoted 27 lines
src/openvpn/win32.c +12
-0+if (tap_metric <0 ) + { +/* error, should not restore metric */ + tap_metric =-1 ; + } + status =set_interface_metric (index,AF_INET ,BLOCK_DNS_IFACE_METRIC ); +if (!status) + { +set_interface_metric (index,AF_INET6 ,BLOCK_DNS_IFACE_METRIC ); + } + } +ret = (status == 0);out:
src/openvpn/win32.c +1
-0@@ -1325,6 +1368,7 @@ static VOID Undo(undo_lists_t *lists) {undo_type_t type; +block_dns_data_t *interface_data;for (type =0 ; type < _undo_type_max; type++) {list_item_t **pnext = &(*lists)[type];
src/openvpn/win32.c Undo(undo_lists_t *lists) · line 1400
Quoted 9 lines
src/openvpn/win32.c +3
-2@@ -1350,8 +1394,15 @@ Undo(undo_lists_t *lists)break ;case block_dns: -delete_block_dns_filters (item->data); - item->data =NULL ; + interface_data = (block_dns_data_t *)(item->data); +delete_block_dns_filters (interface_data->engine); +if (interface_data->metric >0 )
Here ">" should be ">=0", else automatic metric is not restored.
Quoted 11 lines
src/openvpn/win32.c +6
-0+ { +set_interface_metric (interface_data->index,AF_INET , + interface_data->metric); +set_interface_metric (interface_data->index,AF_INET6 , + interface_data->metric); + }break; }
Finally there are some trailing white space errors (6 places). This is a good
opportunity to fix those too.
Thanks,
Selva
Quoted 3 lines · Selva Nair
Hi,
Please bear with me for making a few more comments.
Please don't excuse. That's totally fine.
Quoted 8 lines
This close to final so only
a few minor issues.On Thu, May 4, 2017 at 1:36 PM, ValdikSS <[email protected]... <mailto:[email protected]...>> wrote:
Nitpicking here, but anyway a new version is required (see below), so it would be nice if you wrap lines in commit message to something like about 70 to 80 at most. Not sure whether we have a policy on this..
Done.
Quoted 6 lines
InitializeIpInterfaceEntry()is missing in all but very recent mingw32 versions (their commit logs show it was added in early 2015) so we may need
to declare it inblock_dns.c. I use Debian jessie (8.7) -- mingw gcc 4.9.1 and its not there. I believe Samuli's build system uses an even older version.
I don't know how to do that correctly. Can you please help me?
Quoted 6 lines
Note that 64 bit build still succeeds with a warning but 32 bit will fail because of stdcall. Yes, its stdcall though MSDN doesn't mention it as such.
This would be "Get" or "Return" not "Set"
Done.
Quoted 10 lines
..
The current metric is queried and saved only for v4 while the metric is set and restored for both v4 and v6. Isn't it necessary to also save the metric for both independently? I'm not sure whether anyone really uses different metric for v4 and v6 but Windows allows it.
Done.
Quoted 3 lines
Here ">" should be ">=0", else automatic metric is not restored.
Done.
Quoted 4 lines
Finally there are some trailing white space errors (6 places). This is a good opportunity to fix those too.
Done.
Please check
https://github.com/ValdikSS/openvpn-with-patches/commit/02058c8f043d2731011222a05686478a09e479ec
Text vesion:
https://github.com/ValdikSS/openvpn-with-patches/commit/02058c8f043d2731011222a05686478a09e479ec.patch
Quoted 15 lines
Thanks,
Selva
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Quoted 10 lines · ValdikSS
InitializeIpInterfaceEntry()is missing in all but very recent mingw32 versions
(their commit logs show it was added in early 2015) so we may need to declare it inblock_dns.c. I use Debian jessie (8.7) -- mingw gcc 4.9.1 and its
not there. I believe Samuli's build system uses an even older version.I don't know how to do that correctly. Can you please help me?
In block-dns.c where "WFP-related defines and GUIDs not in mingw32" are defined, add this
VOID NETIOAPI_API_ InitializeIpInterfaceEntry(PMIB_IPINTERFACE_ROW Row );
Do a test build for 32 bit to see now this function compiles without warnings
and links without errors. 32 bit is a more stringent test because only in this
case the linker will fail to find stdcall functions without proper declarations.
That should be it.
Selva
Added it. Can't compile for 32 bit, I tried multiple times but can't wrestle down build system to compile my version from git. It never finds OpenSSL. It successfully compiles for 64 bit.
Please check v2.
Quoted 27 lines · Selva Nair
On Wed, May 10, 2017 at 12:08 PM, ValdikSS <[email protected]... <mailto:[email protected]...>> wrote:
In
block-dns.cwhere "WFP-related defines and GUIDs not in mingw32" are defined, add thisVOID
NETIOAPI_API_InitializeIpInterfaceEntry(PMIB_IPINTERFACE_ROW Row );Do a test build for 32 bit to see now this function compiles without warnings and links without errors. 32 bit is a more stringent test because only in this case the linker will fail to find stdcall functions without proper declarations.
That should be it.
Selva
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel