| Message ID | 20211108135314.148-1-lstipakov@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [Openvpn-devel] tun: improve DHCP server address calculation for small subnets | expand |
This can be considered as a bugfix ("Cannot use openvpn2 as OpenVPN
Cloud connector") so could go into release/2.5 too.
ke 10. marrask. 2021 klo 12.59 Lev Stipakov (lstipakov@gmail.com) kirjoitti:
>
> From: Lev Stipakov <lev@openvpn.net>
>
> When /30 subnet is pushed (like in the case of OpenVPN Cloud),
> DHCP server address is calculated to be the same as local address,
> which causes collision and therefore connection is not established.
>
> To fix that, use openvpn3 approach, which sets DHCP server
> address to a network address for small subnets
>
> Signed-off-by: Lev Stipakov <lev@openvpn.net>
> ---
> src/openvpn/tun.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
> index 28f803ec..994e3751 100644
> --- a/src/openvpn/tun.c
> +++ b/src/openvpn/tun.c
> @@ -6363,7 +6363,9 @@ tuntap_dhcp_mask(const struct tuntap *tt, const char *device_guid)
> }
> else
> {
> - ep[2] = dhcp_masq_addr(tt->local, tt->remote_netmask, -1);
> + int prefix_len = netmask_to_netbits2(tt->adapter_netmask);
> + /* use network address as DHCP server for small subnets, otherwise last address before broadcast */
> + ep[2] = dhcp_masq_addr(tt->local, tt->remote_netmask, prefix_len < 28 ? -1 : 0);
> }
> }
> else
> --
> 2.23.0.windows.1
>
Hi, On Wed, Nov 10, 2021 at 6:01 AM Lev Stipakov <lstipakov@gmail.com> wrote: > From: Lev Stipakov <lev@openvpn.net> > > When /30 subnet is pushed (like in the case of OpenVPN Cloud), > DHCP server address is calculated to be the same as local address, > which causes collision and therefore connection is not established. > > To fix that, use openvpn3 approach, which sets DHCP server > address to a network address for small subnets > > Signed-off-by: Lev Stipakov <lev@openvpn.net> > --- > src/openvpn/tun.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c > index 28f803ec..994e3751 100644 > --- a/src/openvpn/tun.c > +++ b/src/openvpn/tun.c > @@ -6363,7 +6363,9 @@ tuntap_dhcp_mask(const struct tuntap *tt, const char > *device_guid) > } > else > { > - ep[2] = dhcp_masq_addr(tt->local, tt->remote_netmask, -1); > + int prefix_len = netmask_to_netbits2(tt->adapter_netmask); > + /* use network address as DHCP server for small subnets, > otherwise last address before broadcast */ > + ep[2] = dhcp_masq_addr(tt->local, tt->remote_netmask, > prefix_len < 28 ? -1 : 0); > } > } > else > -- Why not just use 0 offset always? Perpetuating this dance of 0 offset in some cases, -1 otherwise is not a way forward. Also see my patch from 2015 that never got any traction. I have lost touch with the context, so, I'm not sure whether this addresses the same (apart from code changes) or something related. https://sourceforge.net/p/openvpn/mailman/openvpn-devel/thread/1449454660-20050-1-git-send-email-selva.nair%40gmail.com/#msg34674818 Also: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13413.html and other messages in that thread. Selva <div dir="ltr"><div dir="ltr">Hi,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Nov 10, 2021 at 6:01 AM Lev Stipakov <<a href="mailto:lstipakov@gmail.com" target="_blank">lstipakov@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">From: Lev Stipakov <<a href="mailto:lev@openvpn.net" target="_blank">lev@openvpn.net</a>><br> <br> When /30 subnet is pushed (like in the case of OpenVPN Cloud),<br> DHCP server address is calculated to be the same as local address,<br> which causes collision and therefore connection is not established.<br> <br> To fix that, use openvpn3 approach, which sets DHCP server<br> address to a network address for small subnets<br> <br> Signed-off-by: Lev Stipakov <<a href="mailto:lev@openvpn.net" target="_blank">lev@openvpn.net</a>><br> ---<br> src/openvpn/tun.c | 4 +++-<br> 1 file changed, 3 insertions(+), 1 deletion(-)<br> <br> diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c<br> index 28f803ec..994e3751 100644<br> --- a/src/openvpn/tun.c<br> +++ b/src/openvpn/tun.c<br> @@ -6363,7 +6363,9 @@ tuntap_dhcp_mask(const struct tuntap *tt, const char *device_guid)<br> }<br> else<br> {<br> - ep[2] = dhcp_masq_addr(tt->local, tt->remote_netmask, -1);<br> + int prefix_len = netmask_to_netbits2(tt->adapter_netmask);<br> + /* use network address as DHCP server for small subnets, otherwise last address before broadcast */<br> + ep[2] = dhcp_masq_addr(tt->local, tt->remote_netmask, prefix_len < 28 ? -1 : 0);<br> }<br> }<br> else<br> --</blockquote><div><br></div><div>Why not just use 0 offset always? Perpetuating this dance of 0 offset in some cases, -1 otherwise is not a way forward. Also see my patch from 2015 that never got any traction. I have lost touch with the context, so, I'm not sure whether this addresses the same (apart from code changes) or something related.</div><div><br></div><div><a href="https://sourceforge.net/p/openvpn/mailman/openvpn-devel/thread/1449454660-20050-1-git-send-email-selva.nair%40gmail.com/#msg34674818">https://sourceforge.net/p/openvpn/mailman/openvpn-devel/thread/1449454660-20050-1-git-send-email-selva.nair%40gmail.com/#msg34674818</a><br></div><div><br></div><div>Also: <a href="https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13413.html">https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13413.html</a> and other messages in that thread.</div><div><br></div><div>Selva</div></div></div>
Hi, > Why not just use 0 offset always? Perpetuating this dance of 0 offset in some cases, -1 otherwise is not a way forward. Also see my patch from 2015 that never got any traction. I have lost touch with the context, so, I'm not sure whether this addresses the same (apart from code changes) or something related. > > https://sourceforge.net/p/openvpn/mailman/openvpn-devel/thread/1449454660-20050-1-git-send-email-selva.nair%40gmail.com/#msg34674818 > > Also: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg13413.html and other messages in that thread. Thanks for pointing me out to this, obviously I missed your original patch and that discussion. I don't know why there is an offset in the first place for both openvpn2 and openvpn3, there is probably some reason for it? I wanted to be on the safe side and use a network address for some cases only (small subnet). If you think it is safe to always use 0, let's do that - I can send V2. I tried to apply your patch but it doesn't apply anymore - tun.c has changed a lot since 2015. What comes to discussion about getting rid of the offset option - since the syntax is like this: --ip-win32 dynamic [offset] [lease-time] we cannot just remove [offset] since we might break configurations which set [lease-time].
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index 28f803ec..994e3751 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -6363,7 +6363,9 @@ tuntap_dhcp_mask(const struct tuntap *tt, const char *device_guid) } else { - ep[2] = dhcp_masq_addr(tt->local, tt->remote_netmask, -1); + int prefix_len = netmask_to_netbits2(tt->adapter_netmask); + /* use network address as DHCP server for small subnets, otherwise last address before broadcast */ + ep[2] = dhcp_masq_addr(tt->local, tt->remote_netmask, prefix_len < 28 ? -1 : 0); } } else