Re: [Openvpn-devel] [PATCH 2/2] tun.c: revise the IPv4 ifconfig flow on Windows

2020-03-14 Thread Gert Doering 
Hi,

On Fri, Mar 13, 2020 at 02:47:39AM +, Simon Rozman wrote:
> True. I'm sure I got this in the original commit back then. Found it... When 
> rebasing onto the current master, it conflicted and I dropped that change 
> rather than fixing it. Wrong choice, sorry.

Am I reading this correctly, a v2 version of this patch is coming?

gert
-- 
"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
 Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany g...@greenie.muc.de


smime.p7s
Description: S/MIME cryptographic signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH 2/2] tun.c: revise the IPv4 ifconfig flow on Windows

2020-03-12 Thread Simon Rozman 
Hi,

> A few questions:
> 
> >   This also fixes an issue with --windows-driver wintun overriding
> >  --ip-win32 manual, the later being perfectly fine choice for Wintun
> too.
> 
> We do still have code which forces netsh for wintun:
> 
> if (options->windows_driver == WINDOWS_DRIVER_WINTUN)
> {
> options->tuntap_options.ip_win32_type = IPW32_SET_NETSH;
> }
> 
> Shouldn't we update options.c and ensure that only allowed ip_win32 for
> wintun are manual, netsh and ipapi?

True. I'm sure I got this in the original commit back then. Found it... When 
rebasing onto the current master, it conflicted and I dropped that change 
rather than fixing it. Wrong choice, sorry.

> > - open_tun() & tuntap_post_open(): unify Wintun and TAP-Windows6
> workflow.
> 
> With that change, we flush ARP cache also for wintun. Is this needed?ΒΈ

Nope, FlushIpNetTable() is definitely not needed on wintun adapters. I shall 
modify the ARP-flushing condition to include "&& tt->windows_driver == 
WINDOWS_DRIVER_TAP_WINDOWS6" and update the commit message.

> Also, while testing this I found (and fixed) a bug
> (https://patchwork.openvpn.net/patch/1039/),
> I think that fix should go first - your patch exposes bug (prints
> device_guid value) for wintun case.

I agree. Your patch should be applied first.

Regards,
Simon


smime.p7s
Description: S/MIME cryptographic signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH 2/2] tun.c: revise the IPv4 ifconfig flow on Windows

2020-03-12 Thread Lev Stipakov 
Hi,

Looks good, tested with interactive service and without (netsh, ipapi).

A few questions:

>   This also fixes an issue with --windows-driver wintun overriding
>  --ip-win32 manual, the later being perfectly fine choice for Wintun too.

We do still have code which forces netsh for wintun:

if (options->windows_driver == WINDOWS_DRIVER_WINTUN)
{
options->tuntap_options.ip_win32_type = IPW32_SET_NETSH;
}

Shouldn't we update options.c and ensure that only allowed ip_win32
for wintun are manual, netsh and ipapi?

> - open_tun() & tuntap_post_open(): unify Wintun and TAP-Windows6 workflow.

With that change, we flush ARP cache also for wintun. Is this needed?

> This allows allows --ip-win32 ipapi now.

See above - we need to get rid of forcing netsh with wintun for that to work.
To test ipapi case, I had to comment out netsh forcing.

Also, while testing this I found (and fixed) a bug
(https://patchwork.openvpn.net/patch/1039/),
I think that fix should go first - your patch exposes bug (prints
device_guid value) for wintun case.


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