Hi,

Going through patchworks noticed this.

Thankfully this never got committed so here goes a retraction.

On Sun, Jan 21, 2018 at 1:45 PM Selva Nair <selva.n...@gmail.com> wrote:

> Hi,
>
> I'm on a reviewing spree (doing my penance), so here goes..
>
> Thanks for the patch
>
> On Tue, Jan 9, 2018 at 2:16 AM, Eyal Birger <eyal.bir...@gmail.com> wrote:
> > Address prefix length defaults to /64 on Windows. This change allows
> using
> > Windows clients in setups that use a different prefix length.
> >
> > Note: the ability to set the prefix length is documented in the netsh
> > 'add address' command, but works on the 'set address' command as well.
>
> Aside:
> If interactive service is in use, the ip helper API is used and setting
> prefix already works.  Ideally I would like to see openvpn on Windows
> used only with the interactive service, but we are not there yet --
> instances started by the automatic service does not use it and there
> are some users still running the GUI as admin for some inexplicable
> reasons.
>
> So we need to continue supporting these code paths.
>
> >
> > Signed-off-by: Eyal Birger <eyal.bir...@gmail.com>
> > ---
> >  src/openvpn/tun.c | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
> > index 25831ce..b2b4795 100644
> > --- a/src/openvpn/tun.c
> > +++ b/src/openvpn/tun.c
> > @@ -1561,15 +1561,16 @@ do_ifconfig(struct tuntap *tt,
> >              }
> >              else
> >              {
> > -                /* example: netsh interface ipv6 set address
> interface=42 2001:608:8003::d store=active */
> > +                /* example: netsh interface ipv6 set address
> interface=42 2001:608:8003::d/64 store=active */
> >                  char iface[64];
> >                  openvpn_snprintf(iface, sizeof(iface), "interface=%lu",
> tt->adapter_index );
> >                  argv_printf(&argv,
> > -                            "%s%sc interface ipv6 set address %s %s
> store=active",
> > +                            "%s%sc interface ipv6 set address %s %s/%d
> store=active",
> >                              get_win_sys_path(),
> >                              NETSH_PATH_SUFFIX,
> >                              iface,
> > -                            ifconfig_ipv6_local );
> > +                            ifconfig_ipv6_local,
> > +                            tt->netbits_ipv6);
> >                  netsh_command(&argv, 4, M_FATAL);
> >                  /* set ipv6 dns servers if any are specified */
> >                  netsh_set_dns6_servers(tt->options.dns6,
> tt->options.dns6_len, actual);
>
> Works as expected and the code is good.
>
> Currently, on setting the address, a default route gets set with
> prefix /64 with gateway as OnLink (does not happen when iphelper api
> is used). Although our explicit route to fe80::8 may override it, it
> looks better to set the correct prefix in the address. So:
>
> Acked-by: Selva Nair <selva.n...@gmail.com>
>

Though this works in my tests I want to retract this ACK.

Apart from possible issues due to the appearance of the onlink route in
some cases, I think the correct approach going forward is to stop using
netsh and use the IP helper API for such tasks. And do it in the same way
as done using the service.

Un-Acked-by: Selva Nair <selva.n...@gmail.com> :)

Selva
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to