On Tue, Jan 3, 2017 at 12:55 PM, Selva Nair <selva.n...@gmail.com> wrote:
> Hi,
>
> On Tue, Jan 3, 2017 at 12:17 PM, Gert Doering <g...@greenie.muc.de> wrote:
>
>> On Fri, Dec 30, 2016 at 09:45:07PM -0500, selva.n...@gmail.com wrote:
>> > From: Selva Nair <selva.n...@gmail.com>
>> >
>> > Also make sure --dhcp-pre-release results in not just dhcp_release()
>> > in open_tun() but a subsequent dhcp_renew() as well. Else dhcp
>> transaction
>> > gets aborted as this call to release() happens after the adapter status
>> > is changed to connected.
>> >
>> > Alternatively, the undocumented --dhcp-pre-release may be removed.
>> >
>> > Fixes Trac #807 (but can't say the same for Trac #665 without knowing
>> > how to reproduce it)
>>
>> After thinking about this for a while, I agree we should do this, so
>> ACK (even if Debbie10t already has ACKed it).
>>
>> I'd ask for a few changes, though :-) - if I'm reading this correctly,
>> tt->options.dhcp_release is now unused, so I think we should do it
>> all the way - get rid of that flag variable, print a message
>> ("obsolete option detected, already on" or so) if --dhcp-release
>> is configured, and update the man page ("this option is a no-op now,
>> as the functionality is enabled by default starting in 2.4.1").
>>
>
> Agreed. Will send a v2 later today.
>
There is one corner case that I realized only now: what
happens when a real dhcp server is in use (say in bridge mode).
Current dhcp_release() function is a no-op unless the
masquerading dhcp server in the tap driver is active (it checks
ltt->options.ip_win32_type == IPW32_SET_DHCP_MASQ)
Simply removing that restriction may be risky as its unclear
to me whether IpReleaseAddress() is a synchronous or async
call (MSDN doesn't say). If it blocks until the address is
released (like 'ipconfig /renew' appears to do) then it won't work with
an external dhcp server as our process is also the router for
packets through the tunnel. Internal one should be ok, as those
packets are handled in the tap driver.
Anyway this is beyond the current patch as it just uses
dhcp_release() as coded (which looks safe). So I suppose we just
go ahead and handle other cases as and when they show up..
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
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
