| Message ID | 20200725235023.22441-1-arne@rfc2549.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [Openvpn-devel] Include utun device number in utun error messages | expand |
Hi, On Sat, Jul 25, 2020 at 7:51 PM Arne Schwabe <arne@rfc2549.org> wrote: > > For lack of a better API (or knowledge about a better API) we try to > open utun devices on macOS by trying utun0 to utun255 and use the > first one that works. On my Mac I have already 4 devices that > do nothing but are just there and another VPN connection resulting in a > number of error messages. This explicitly shows in the log that we > tried the devices instead of some unspecific error. > > This changes the log from: > > Opening utun (connect(AF_SYS_CONTROL)): Resource busy (errno=16) > Opening utun (connect(AF_SYS_CONTROL)): Resource busy (errno=16) > Opening utun (connect(AF_SYS_CONTROL)): Resource busy (errno=16) > Opening utun (connect(AF_SYS_CONTROL)): Resource busy (errno=16) > Opening utun (connect(AF_SYS_CONTROL)): Resource busy (errno=16) > Opened utun device utun5 > > to > > Opening utun0 failed (connect(AF_SYS_CONTROL)): Resource busy (errno=16) > Opening utun1 failed (connect(AF_SYS_CONTROL)): Resource busy (errno=16) > Opening utun2 failed (connect(AF_SYS_CONTROL)): Resource busy (errno=16) > Opening utun3 failed (connect(AF_SYS_CONTROL)): Resource busy (errno=16) > Opening utun4 failed (connect(AF_SYS_CONTROL)): Resource busy (errno=16) > Opened utun device utun5 Feature-ACK. The failure messages have concerned some Tunnelblick users. This _might_ help clarify things for them and it certainly won't hurt. I have not tested the code, but it looks fine. Note that the last half of the patch consists only of whitespace changes (starting at "@@ -5682,15 +5685,15 @@). Note also that macOS itself creates some utun devices, which is why the failures to open happen even on a clean install of macOS and only OpenVPN. The number created depends on the version of macOS and the Apple services such as iCloud Drive and Screen Sharing that are enabled. Best regards, Jon Bullard
> > Feature-ACK. The failure messages have concerned some Tunnelblick > users. This _might_ help clarify things for them and it certainly > won't hurt. > > I have not tested the code, but it looks fine. > > Note that the last half of the patch consists only of whitespace > changes (starting at "@@ -5682,15 +5685,15 @@). Yes my mistake, I did a uncrustify on the file before checking in but did not double check that it only modified the parts that I modified before. Arne
Acked-by: Gert Doering <gert@greenie.muc.de> I've left out the stray uncrustify fixes - not that I disagree with them, but they should go to their own patch. Sorry for not cleaning that up while merging the corresponding patch. Code change looks good. Not tested anything (only test compiled on MacOS). I wonder if we could just skip print the error message for this particular call (connect()) if the return is EBUSY? Or maybe "skip if utunnumber < 10 && EBUSY"? Your patch has been applied to the master branch. commit 1d86fae8740a6d025d550fdcb4aa13c755fc2e48 Author: Arne Schwabe Date: Sun Jul 26 01:50:23 2020 +0200 Include utun device number in utun error messages Signed-off-by: Arne Schwabe <arne@rfc2549.org> Acked-by: Gert Doering <gert@greenie.muc.de> Message-Id: <20200725235023.22441-1-arne@rfc2549.org> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20590.html Signed-off-by: Gert Doering <gert@greenie.muc.de> -- kind regards, Gert Doering
Am 26.07.20 um 15:40 schrieb Gert Doering: > Acked-by: Gert Doering <gert@greenie.muc.de> > > I've left out the stray uncrustify fixes - not that I disagree with > them, but they should go to their own patch. Sorry for not cleaning > that up while merging the corresponding patch. > > Code change looks good. Not tested anything (only test compiled on MacOS). > > I wonder if we could just skip print the error message for this > particular call (connect()) if the return is EBUSY? Or maybe > "skip if utunnumber < 10 && EBUSY"? > since you can also specify that you want utun9 that would break that. More realistically we should just skip the interfaces that are already present in the system. Baiscally if 'does interface utunX exist?' => skip on trying the first working one. Arne
Hi, On Sun, Jul 26, 2020 at 04:19:21PM +0200, Arne Schwabe wrote: > Am 26.07.20 um 15:40 schrieb Gert Doering: > > I wonder if we could just skip print the error message for this > > particular call (connect()) if the return is EBUSY? Or maybe > > "skip if utunnumber < 10 && EBUSY"? > > since you can also specify that you want utun9 that would break that. Only for the "loop from 0 to find the first working one", of course :) > More realistically we should just skip the interfaces that are already > present in the system. Baiscally if 'does interface utunX exist?' => > skip on trying the first working one. Yes that was the idea. gert
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index 82d96927..ed00644c 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -2950,14 +2950,16 @@ utun_open_helper(struct ctl_info ctlInfo, int utunnum) if (fd < 0) { - msg(M_INFO | M_ERRNO, "Opening utun (socket(SYSPROTO_CONTROL))"); + msg(M_INFO | M_ERRNO, "Opening utun%d failed (socket(SYSPROTO_CONTROL))", + utunnum); return -2; } if (ioctl(fd, CTLIOCGINFO, &ctlInfo) == -1) { close(fd); - msg(M_INFO | M_ERRNO, "Opening utun (ioctl(CTLIOCGINFO))"); + msg(M_INFO | M_ERRNO, "Opening utun%d failed (ioctl(CTLIOCGINFO))", + utunnum); return -2; } @@ -2975,7 +2977,8 @@ utun_open_helper(struct ctl_info ctlInfo, int utunnum) if (connect(fd, (struct sockaddr *)&sc, sizeof(sc)) < 0) { - msg(M_INFO | M_ERRNO, "Opening utun (connect(AF_SYS_CONTROL))"); + msg(M_INFO | M_ERRNO, "Opening utun%d failed (connect(AF_SYS_CONTROL))", + utunnum); close(fd); return -1; } @@ -5682,15 +5685,15 @@ write_dhcp_str(struct buffer *buf, const int type, const char *str, bool *error) * 0x1D 0x7 openvpn 0x3 net 0x00 0x0A duckduckgo 0x3 com 0x00 */ static void -write_dhcp_search_str(struct buffer *buf, const int type, const char * const *str_array, +write_dhcp_search_str(struct buffer *buf, const int type, const char *const *str_array, int array_len, bool *error) { - char tmp_buf[256]; - int i; - int len = 0; - int label_length_pos; + char tmp_buf[256]; + int i; + int len = 0; + int label_length_pos; - for (i=0; i < array_len; i++) + for (i = 0; i < array_len; i++) { const char *ptr = str_array[i]; @@ -5701,7 +5704,7 @@ write_dhcp_search_str(struct buffer *buf, const int type, const char * const *st return; } /* Loop over all subdomains separated by a dot and replace the dot - with the length of the subdomain */ + * with the length of the subdomain */ /* label_length_pos points to the byte to be replaced by the length * of the following domain label */ @@ -5709,7 +5712,7 @@ write_dhcp_search_str(struct buffer *buf, const int type, const char * const *st while (true) { - if (*ptr == '.' || *ptr == '\0' ) + if (*ptr == '.' || *ptr == '\0') { tmp_buf[label_length_pos] = (len-label_length_pos)-1; label_length_pos = len; @@ -5769,8 +5772,8 @@ build_dhcp_options_string(struct buffer *buf, const struct tuntap_options *o) if (o->domain_search_list_len > 0) { write_dhcp_search_str(buf, 119, o->domain_search_list, - o->domain_search_list_len, - &error); + o->domain_search_list_len, + &error); } /* the MS DHCP server option 'Disable Netbios-over-TCP/IP @@ -6149,9 +6152,9 @@ wintun_register_ring_buffer(struct tuntap *tt, const char *device_guid) else { msg(M_FATAL, "ERROR: Wintun requires SYSTEM privileges and therefore " - "should be used with interactive service. If you want to " - "use openvpn from command line, you need to do SYSTEM " - "elevation yourself (for example with psexec)."); + "should be used with interactive service. If you want to " + "use openvpn from command line, you need to do SYSTEM " + "elevation yourself (for example with psexec)."); } return ret;
For lack of a better API (or knowledge about a better API) we try to open utun devices on macOS by trying utun0 to utun255 and use the first one that works. On my Mac I have already 4 devices that do nothing but are just there and another VPN connection resulting in a number of error messages. This explicitly shows in the log that we tried the devices instead of some unspecific error. This changes the log from: Opening utun (connect(AF_SYS_CONTROL)): Resource busy (errno=16) Opening utun (connect(AF_SYS_CONTROL)): Resource busy (errno=16) Opening utun (connect(AF_SYS_CONTROL)): Resource busy (errno=16) Opening utun (connect(AF_SYS_CONTROL)): Resource busy (errno=16) Opening utun (connect(AF_SYS_CONTROL)): Resource busy (errno=16) Opened utun device utun5 to Opening utun0 failed (connect(AF_SYS_CONTROL)): Resource busy (errno=16) Opening utun1 failed (connect(AF_SYS_CONTROL)): Resource busy (errno=16) Opening utun2 failed (connect(AF_SYS_CONTROL)): Resource busy (errno=16) Opening utun3 failed (connect(AF_SYS_CONTROL)): Resource busy (errno=16) Opening utun4 failed (connect(AF_SYS_CONTROL)): Resource busy (errno=16) Opened utun device utun5 Signed-off-by: Arne Schwabe <arne@rfc2549.org> --- src/openvpn/tun.c | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-)