| Message ID | 1509930876-30728-1-git-send-email-selva.nair@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Gert Doering |
| Headers | show |
| Series | [Openvpn-devel,v2] Use lowest metric interface when multiple interfaces match a route | expand |
2017-11-06 6:14 GMT+05:00 <selva.nair@gmail.com>: > From: Selva Nair <selva.nair@gmail.com> > > Currently a route addition using IPAPI or service is skipped if the > route gateway is reachable by multiple interfaces. This changes that > to use the interface with lowest metric. Implemented by > > (i) Do not over-write the return value with TUN_ADAPTER_INDEX_INVALID in > windows_route_find_if_index() if multiple interfaces match a route. > (ii) Select the interface with lowest metric in adapter_index_of_ip() > instead of the first one found when multiple interfaces match. > > Reported by Jan Just Keijser <janjust@nikhef.nl> > > v2: - A private get_interface_metric() method and better error reporting > - Revert an unintented edit of route.c (a_index = ...) > - Improve the commit message > > Signed-off-by: Selva Nair <selva.nair@gmail.com> > --- > src/openvpn/route.c | 1 - > src/openvpn/tun.c | 59 ++++++++++++++++++++++++++++++ > +++++++++++++++++++++-- > 2 files changed, 57 insertions(+), 3 deletions(-) > > diff --git a/src/openvpn/route.c b/src/openvpn/route.c > index 8c71e6e..66a8ae3 100644 > --- a/src/openvpn/route.c > +++ b/src/openvpn/route.c > @@ -2780,7 +2780,6 @@ windows_route_find_if_index(const struct route_ipv4 > *r, const struct tuntap *tt) > msg(M_WARN, "Warning: route gateway is ambiguous: %s (%d > matches)", > print_in_addr_t(r->gateway, 0, &gc), > count); > - ret = TUN_ADAPTER_INDEX_INVALID; > } > > dmsg(D_ROUTE_DEBUG, "DEBUG: route find if: on_tun=%d count=%d > index=%d", > diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c > index 3639718..7603133 100644 > --- a/src/openvpn/tun.c > +++ b/src/openvpn/tun.c > @@ -4474,6 +4474,49 @@ is_ip_in_adapter_subnet(const IP_ADAPTER_INFO *ai, > const in_addr_t ip, in_addr_t > return ret; > } > > +/** > + * Given an interface index return the interface metric. > + * > + * Arguments: > + * index : The index of the interface > + * family : AF_INET for IPv4 or AF_INET6 for IPv6 > + * On error returns -1 > + */ > + > +/* function signature missing in mingw iphlpapi.h */ > that definition apparently present in mingw since 2015 https://sourceforge.net/p/mingw-w64/mingw-w64/ci/ea95d55e3387353e453d6ae8fc5cb8f7503947c2/tree/mingw-w64-headers/include/netioapi.h > +VOID NETIOAPI_API_ > +InitializeIpInterfaceEntry(PMIB_IPINTERFACE_ROW Row); > + > +static int > +get_interface_metric(NET_IFINDEX index, ADDRESS_FAMILY family) > +{ > + DWORD err; > + int msglevel = D_ROUTE|M_WARN; > + MIB_IPINTERFACE_ROW ipiface; > + > + InitializeIpInterfaceEntry(&ipiface); > + ipiface.Family = family; > + ipiface.InterfaceIndex = index; > + > + err = GetIpInterfaceEntry(&ipiface); > + if (err == NO_ERROR) > + { > + return ipiface.Metric; > + } > + else if (err == ERROR_NOT_FOUND) > + { > + /* > + * This happens if the address family is not enabled for the > + * interface, which is benign -- display only at a debug level > + */ > + msglevel = D_ROUTE_DEBUG; > + } > + msg(msglevel, "Note: failed to determine metric of interface " > + "<%lu> for %s : (error code = %lu)", > + index, (family == AF_INET)? "ipv4" : "ipv6", err); > + return -1; > +} > + > DWORD > adapter_index_of_ip(const IP_ADAPTER_INFO *list, > const in_addr_t ip, > @@ -4483,6 +4526,7 @@ adapter_index_of_ip(const IP_ADAPTER_INFO *list, > struct gc_arena gc = gc_new(); > DWORD ret = TUN_ADAPTER_INDEX_INVALID; > in_addr_t highest_netmask = 0; > + int lowest_metric = INT_MAX; > bool first = true; > > if (count) > @@ -4496,9 +4540,14 @@ adapter_index_of_ip(const IP_ADAPTER_INFO *list, > > if (is_ip_in_adapter_subnet(list, ip, &hn)) > { > + int metric = get_interface_metric(list->Index, AF_INET); > if (first || hn > highest_netmask) > { > highest_netmask = hn; > + if (metric >= 0) > + { > + lowest_metric = metric; > + } > if (count) > { > *count = 1; > @@ -4512,16 +4561,22 @@ adapter_index_of_ip(const IP_ADAPTER_INFO *list, > { > ++*count; > } > + if (metric >= 0 && metric < lowest_metric) > + { > + ret = list->Index; > + lowest_metric = metric; > + } > } > } > list = list->Next; > } > > - dmsg(D_ROUTE_DEBUG, "DEBUG: IP Locate: ip=%s nm=%s index=%d count=%d", > + dmsg(D_ROUTE_DEBUG, "DEBUG: IP Locate: ip=%s nm=%s index=%d count=%d > metric=%d", > print_in_addr_t(ip, 0, &gc), > print_in_addr_t(highest_netmask, 0, &gc), > (int)ret, > - count ? *count : -1); > + count ? *count : -1, > + lowest_metric); > > if (ret == TUN_ADAPTER_INDEX_INVALID && count) > { > -- > 2.1.4 > > > ------------------------------------------------------------ > ------------------ > 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 > Openvpn-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/openvpn-devel > <div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">2017-11-06 6:14 GMT+05:00 <span dir="ltr"><<a href="mailto:selva.nair@gmail.com" target="_blank">selva.nair@gmail.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="gmail-">From: Selva Nair <<a href="mailto:selva.nair@gmail.com">selva.nair@gmail.com</a>><br> <br> Currently a route addition using IPAPI or service is skipped if the<br> route gateway is reachable by multiple interfaces. This changes that<br> </span>to use the interface with lowest metric. Implemented by<br> <br> (i) Do not over-write the return value with TUN_ADAPTER_INDEX_INVALID in<br> windows_route_find_if_index() if multiple interfaces match a route.<br> (ii) Select the interface with lowest metric in adapter_index_of_ip()<br> instead of the first one found when multiple interfaces match.<br> <span class="gmail-"><br> Reported by Jan Just Keijser <<a href="mailto:janjust@nikhef.nl">janjust@nikhef.nl</a>><br> <br> </span>v2: - A private get_interface_metric() method and better error reporting<br> - Revert an unintented edit of route.c (a_index = ...)<br> - Improve the commit message<br> <span class="gmail-"><br> Signed-off-by: Selva Nair <<a href="mailto:selva.nair@gmail.com">selva.nair@gmail.com</a>><br> ---<br> </span> src/openvpn/route.c | 1 -<br> src/openvpn/tun.c | 59 ++++++++++++++++++++++++++++++<wbr>+++++++++++++++++++++--<br> 2 files changed, 57 insertions(+), 3 deletions(-)<br> <br> diff --git a/src/openvpn/route.c b/src/openvpn/route.c<br> index 8c71e6e..66a8ae3 100644<br> --- a/src/openvpn/route.c<br> +++ b/src/openvpn/route.c<br> <span class="gmail-">@@ -2780,7 +2780,6 @@ windows_route_find_if_index(<wbr>const struct route_ipv4 *r, const struct tuntap *tt)<br> msg(M_WARN, "Warning: route gateway is ambiguous: %s (%d matches)",<br> print_in_addr_t(r->gateway, 0, &gc),<br> count);<br> - ret = TUN_ADAPTER_INDEX_INVALID;<br> }<br> <br> dmsg(D_ROUTE_DEBUG, "DEBUG: route find if: on_tun=%d count=%d index=%d",<br> diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c<br> </span>index 3639718..7603133 100644<br> --- a/src/openvpn/tun.c<br> +++ b/src/openvpn/tun.c<br> @@ -4474,6 +4474,49 @@ is_ip_in_adapter_subnet(const IP_ADAPTER_INFO *ai, const in_addr_t ip, in_addr_t<br> return ret;<br> }<br> <br> +/**<br> + * Given an interface index return the interface metric.<br> + *<br> + * Arguments:<br> + * index : The index of the interface<br> + * family : AF_INET for IPv4 or AF_INET6 for IPv6<br> + * On error returns -1<br> + */<br> +<br> +/* function signature missing in mingw iphlpapi.h */<br></blockquote><div><br></div><div>that definition apparently present in mingw since 2015<br><br><a href="https://sourceforge.net/p/mingw-w64/mingw-w64/ci/ea95d55e3387353e453d6ae8fc5cb8f7503947c2/tree/mingw-w64-headers/include/netioapi.h">https://sourceforge.net/p/mingw-w64/mingw-w64/ci/ea95d55e3387353e453d6ae8fc5cb8f7503947c2/tree/mingw-w64-headers/include/netioapi.h</a><br></div><div><br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> +VOID NETIOAPI_API_<br> +InitializeIpInterfaceEntry(<wbr>PMIB_IPINTERFACE_ROW Row);<br> +<br> +static int<br> +get_interface_metric(NET_<wbr>IFINDEX index, ADDRESS_FAMILY family)<br> +{<br> + DWORD err;<br> + int msglevel = D_ROUTE|M_WARN;<br> + MIB_IPINTERFACE_ROW ipiface;<br> +<br> + InitializeIpInterfaceEntry(&<wbr>ipiface);<br> + ipiface.Family = family;<br> + ipiface.InterfaceIndex = index;<br> +<br> + err = GetIpInterfaceEntry(&ipiface);<br> + if (err == NO_ERROR)<br> + {<br> + return ipiface.Metric;<br> + }<br> + else if (err == ERROR_NOT_FOUND)<br> + {<br> + /*<br> + * This happens if the address family is not enabled for the<br> + * interface, which is benign -- display only at a debug level<br> + */<br> + msglevel = D_ROUTE_DEBUG;<br> + }<br> + msg(msglevel, "Note: failed to determine metric of interface "<br> + "<%lu> for %s : (error code = %lu)",<br> + index, (family == AF_INET)? "ipv4" : "ipv6", err);<br> + return -1;<br> +}<br> +<br> DWORD<br> adapter_index_of_ip(const IP_ADAPTER_INFO *list,<br> const in_addr_t ip,<br> @@ -4483,6 +4526,7 @@ adapter_index_of_ip(const IP_ADAPTER_INFO *list,<br> <span class="gmail-"> struct gc_arena gc = gc_new();<br> DWORD ret = TUN_ADAPTER_INDEX_INVALID;<br> in_addr_t highest_netmask = 0;<br> + int lowest_metric = INT_MAX;<br> bool first = true;<br> <br> if (count)<br> </span>@@ -4496,9 +4540,14 @@ adapter_index_of_ip(const IP_ADAPTER_INFO *list,<br> <span class="gmail-"><br> if (is_ip_in_adapter_subnet(list, ip, &hn))<br> {<br> + int metric = get_interface_metric(list-><wbr>Index, AF_INET);<br> if (first || hn > highest_netmask)<br> {<br> highest_netmask = hn;<br> </span>+ if (metric >= 0)<br> + {<br> <span class="gmail-">+ lowest_metric = metric;<br> + }<br> </span><span class="gmail-"> if (count)<br> {<br> *count = 1;<br> </span>@@ -4512,16 +4561,22 @@ adapter_index_of_ip(const IP_ADAPTER_INFO *list,<br> {<br> ++*count;<br> }<br> + if (metric >= 0 && metric < lowest_metric)<br> <span class="gmail-">+ {<br> + ret = list->Index;<br> + lowest_metric = metric;<br> + }<br> }<br> }<br> list = list->Next;<br> }<br> <br> - dmsg(D_ROUTE_DEBUG, "DEBUG: IP Locate: ip=%s nm=%s index=%d count=%d",<br> + dmsg(D_ROUTE_DEBUG, "DEBUG: IP Locate: ip=%s nm=%s index=%d count=%d metric=%d",<br> print_in_addr_t(ip, 0, &gc),<br> print_in_addr_t(highest_<wbr>netmask, 0, &gc),<br> (int)ret,<br> - count ? *count : -1);<br> + count ? *count : -1,<br> </span>+ lowest_metric);<br> <div class="gmail-HOEnZb"><div class="gmail-h5"><br> if (ret == TUN_ADAPTER_INDEX_INVALID && count)<br> {<br> --<br> 2.1.4<br> <br> <br> ------------------------------<wbr>------------------------------<wbr>------------------<br> Check out the vibrant tech community on one of the world's most<br> engaging tech sites, Slashdot.org! <a href="http://sdm.link/slashdot" rel="noreferrer" target="_blank">http://sdm.link/slashdot</a><br> ______________________________<wbr>_________________<br> Openvpn-devel mailing list<br> <a href="mailto:Openvpn-devel@lists.sourceforge.net">Openvpn-devel@lists.<wbr>sourceforge.net</a><br> <a href="https://lists.sourceforge.net/lists/listinfo/openvpn-devel" rel="noreferrer" target="_blank">https://lists.sourceforge.net/<wbr>lists/listinfo/openvpn-devel</a><br> </div></div></blockquote></div><br></div></div> ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
On Wed, Dec 6, 2017 at 8:28 AM, Илья Шипицин <chipitsine@gmail.com> wrote: > > > 2017-11-06 6:14 GMT+05:00 <selva.nair@gmail.com>: >> >> .. >> +/** >> + * Given an interface index return the interface metric. >> + * >> + * Arguments: >> + * index : The index of the interface >> + * family : AF_INET for IPv4 or AF_INET6 for IPv6 >> + * On error returns -1 >> + */ >> + >> +/* function signature missing in mingw iphlpapi.h */ > > > that definition apparently present in mingw since 2015 > > https://sourceforge.net/p/mingw-w64/mingw-w64/ci/ea95d55e3387353e453d6ae8fc5cb8f7503947c2/tree/mingw-w64-headers/include/netioapi.h > > >> >> +VOID NETIOAPI_API_ >> +InitializeIpInterfaceEntry(PMIB_IPINTERFACE_ROW Row); >> 2015 may sound old, but we are targeting ubuntu 16.04 which has mingw 3.1.0 from mid 2014. Selva ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
diff --git a/src/openvpn/route.c b/src/openvpn/route.c index 8c71e6e..66a8ae3 100644 --- a/src/openvpn/route.c +++ b/src/openvpn/route.c @@ -2780,7 +2780,6 @@ windows_route_find_if_index(const struct route_ipv4 *r, const struct tuntap *tt) msg(M_WARN, "Warning: route gateway is ambiguous: %s (%d matches)", print_in_addr_t(r->gateway, 0, &gc), count); - ret = TUN_ADAPTER_INDEX_INVALID; } dmsg(D_ROUTE_DEBUG, "DEBUG: route find if: on_tun=%d count=%d index=%d", diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index 3639718..7603133 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -4474,6 +4474,49 @@ is_ip_in_adapter_subnet(const IP_ADAPTER_INFO *ai, const in_addr_t ip, in_addr_t return ret; } +/** + * Given an interface index return the interface metric. + * + * Arguments: + * index : The index of the interface + * family : AF_INET for IPv4 or AF_INET6 for IPv6 + * On error returns -1 + */ + +/* function signature missing in mingw iphlpapi.h */ +VOID NETIOAPI_API_ +InitializeIpInterfaceEntry(PMIB_IPINTERFACE_ROW Row); + +static int +get_interface_metric(NET_IFINDEX index, ADDRESS_FAMILY family) +{ + DWORD err; + int msglevel = D_ROUTE|M_WARN; + MIB_IPINTERFACE_ROW ipiface; + + InitializeIpInterfaceEntry(&ipiface); + ipiface.Family = family; + ipiface.InterfaceIndex = index; + + err = GetIpInterfaceEntry(&ipiface); + if (err == NO_ERROR) + { + return ipiface.Metric; + } + else if (err == ERROR_NOT_FOUND) + { + /* + * This happens if the address family is not enabled for the + * interface, which is benign -- display only at a debug level + */ + msglevel = D_ROUTE_DEBUG; + } + msg(msglevel, "Note: failed to determine metric of interface " + "<%lu> for %s : (error code = %lu)", + index, (family == AF_INET)? "ipv4" : "ipv6", err); + return -1; +} + DWORD adapter_index_of_ip(const IP_ADAPTER_INFO *list, const in_addr_t ip, @@ -4483,6 +4526,7 @@ adapter_index_of_ip(const IP_ADAPTER_INFO *list, struct gc_arena gc = gc_new(); DWORD ret = TUN_ADAPTER_INDEX_INVALID; in_addr_t highest_netmask = 0; + int lowest_metric = INT_MAX; bool first = true; if (count) @@ -4496,9 +4540,14 @@ adapter_index_of_ip(const IP_ADAPTER_INFO *list, if (is_ip_in_adapter_subnet(list, ip, &hn)) { + int metric = get_interface_metric(list->Index, AF_INET); if (first || hn > highest_netmask) { highest_netmask = hn; + if (metric >= 0) + { + lowest_metric = metric; + } if (count) { *count = 1; @@ -4512,16 +4561,22 @@ adapter_index_of_ip(const IP_ADAPTER_INFO *list, { ++*count; } + if (metric >= 0 && metric < lowest_metric) + { + ret = list->Index; + lowest_metric = metric; + } } } list = list->Next; } - dmsg(D_ROUTE_DEBUG, "DEBUG: IP Locate: ip=%s nm=%s index=%d count=%d", + dmsg(D_ROUTE_DEBUG, "DEBUG: IP Locate: ip=%s nm=%s index=%d count=%d metric=%d", print_in_addr_t(ip, 0, &gc), print_in_addr_t(highest_netmask, 0, &gc), (int)ret, - count ? *count : -1); + count ? *count : -1, + lowest_metric); if (ret == TUN_ADAPTER_INDEX_INVALID && count) {