Message ID | 20201215173004.26170-1-domagoj@pensa.hr |
---|---|
State | Accepted |
Headers | show |
Series | [Openvpn-devel] Skip DHCP renew with Wintun adapter | expand |
Hi, The patch looks fine, but I wonder how did you manage to trigger this case? dhcp_release() or dhcp_renew() are called if dhcp_masq_post is set to true, which only happens for tap-windows6. fork_dhcp_action() is no-op unless dhcp-renew/dhcp-pre-release are explicitly defined in the ovpn profile. If latter is the case, I wonder if options postprocessing would be a better place to fix it. -Lev
Hi! I've done some more investigation and I've found out that problem was introduced in commit: 09ae6280 tun.c: revise the IPv4 ifconfig flow on Windows in the following change: @@ -6420,10 +6419,7 @@ open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun struct gc_arena gc = gc_new(); /* used also for device_guid allocation */ tun_open_device(tt, dev_node, &device_guid, &gc); - if (tt->windows_driver == WINDOWS_DRIVER_TAP_WINDOWS6) - { tuntap_post_open(tt, device_guid); - } gc_free(&gc); Due to that, DHCP setup section in tuntap_set_ip_addr() is executed when Wintun is used, but it shouldn't. As dhcp_masq_post is false for Wintun, fork_dhcp_action() is executed and that outputs warning and does unnecessary fork. With my patch applied, DHCP setup is done only when TAP-Windows6 is used. In my case server pushes both dhcp-renew and dhcp-release. When I pull-filter ignore them no forking occurs and thus no warning. I hope this helps. Regards, Domagoj On Fri, Dec 18, 2020 at 12:45:43PM +0200, Lev Stipakov wrote: > Hi, > > The patch looks fine, but I wonder how did you manage to trigger this case? > > dhcp_release() or dhcp_renew() are called if dhcp_masq_post is set to true, > which only happens for tap-windows6. > > fork_dhcp_action() is no-op unless dhcp-renew/dhcp-pre-release > are explicitly defined in the ovpn profile. > > If latter is the case, I wonder if options postprocessing would be a > better place > to fix it. > > -Lev
Hi, > I've done some more investigation and I've found out that problem was > introduced in commit: > > 09ae6280 tun.c: revise the IPv4 ifconfig flow on Windows Right, we refactored code, added support for IPAPI and as a side-effect DHCP renew/release code is always executed no matter adapter type. With your explanation patch makes sense to me. Stared at the code and tested with wintun. Acked-by: Lev Stipakov <lstipakov@gmail.com>
Your patch has been applied to the master and release/2.5 branch. I've not tested it in any way, but looked at "git show -w", and this this makes it quite clear that the patch does not actually change as much code as it looks like, just moves the whole "dhcp_masq_post" code block into the "if (WINDOWS_DRIVER_TAP_WINDOWS6)" block - so, for TAP6, nothing changes, and for wintun, the code is not reached. commit e0e7625c6b15f85b81d4f11d02f3daf4f32f1200 (master) commit b8f4ca323969c13b039f35e5792a35f3e903b28c (release/2.5) Author: Domagoj Pensa Date: Tue Dec 15 18:30:04 2020 +0100 Skip DHCP renew with Wintun adapter Acked-by: Lev Stipakov <lstipakov@gmail.com> Message-Id: <20201215173004.26170-1-domagoj@pensa.hr> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21364.html Signed-off-by: Gert Doering <gert@greenie.muc.de> -- kind regards, Gert Doering
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index 400a50ca..6892fdb7 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -6155,35 +6155,35 @@ tuntap_set_ip_addr(struct tuntap *tt, status, strerror_win32(status, &gc)); } - } - /* - * If the TAP-Windows driver is masquerading as a DHCP server - * make sure the TCP/IP properties for the adapter are - * set correctly. - */ - if (dhcp_masq_post) - { - /* check dhcp enable status */ - if (dhcp_status(index) == DHCP_STATUS_DISABLED) + /* + * If the TAP-Windows driver is masquerading as a DHCP server + * make sure the TCP/IP properties for the adapter are + * set correctly. + */ + if (dhcp_masq_post) { - msg(M_WARN, "WARNING: You have selected '--ip-win32 dynamic', which will not work unless the TAP-Windows TCP/IP properties are set to 'Obtain an IP address automatically'"); - } + /* check dhcp enable status */ + if (dhcp_status(index) == DHCP_STATUS_DISABLED) + { + msg(M_WARN, "WARNING: You have selected '--ip-win32 dynamic', which will not work unless the TAP-Windows TCP/IP properties are set to 'Obtain an IP address automatically'"); + } - /* force an explicit DHCP lease renewal on TAP adapter? */ - if (tt->options.dhcp_pre_release) - { - dhcp_release(tt); + /* force an explicit DHCP lease renewal on TAP adapter? */ + if (tt->options.dhcp_pre_release) + { + dhcp_release(tt); + } + if (tt->options.dhcp_renew) + { + dhcp_renew(tt); + } } - if (tt->options.dhcp_renew) + else { - dhcp_renew(tt); + fork_dhcp_action(tt); } } - else - { - fork_dhcp_action(tt); - } if (tt->did_ifconfig_setup && tt->options.ip_win32_type == IPW32_SET_IPAPI) {