Re: [Openvpn-devel] [PATCH v2 2/7] wintun: add --windows-driver config option
Hi, Should there not be a manpage entry in this commit too? > Indeed. Added to manpage. Why do we have to set c->c2.tuntap->wintun in both do_init_tun and > do_open_tun? Should one of them not suffice? > Just to be sure that it is assigned :) Removed unneeded second assignment. Should this --windows-driver option not be inside the #ifdef _WIN32 ? > In fact it is already in _WIN32, but the next option has #ifdef _WIN32 again, so I removed it. > > +#ifdef _WIN32 > > +bool > > +parse_windows_driver(const char *str, const int msglevel) > > I think this should be a static function. Also a short (doxygen) comment > explaining what the return value means would be nice. > Done. > > +bool wintun; > > Did you consider using an enum instead? I think it would make the code > easier to read. > Honestly I do not see to much value in converting bool to enum, it is unlikely that we'll have more tun drivers in the near future. Also changing it here would break follow-up patches. As verbally agreed, I'll look into it afterwards. v3 is on its way. -- -Lev ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v2 2/7] wintun: add --windows-driver config option
Hi,
Thanks for looking into wintun support. Definitely feature-ack.
On 07-11-2019 18:45, Lev Stipakov wrote:
> From: Lev Stipakov
>
> This allows to specify which tun driver openvpn should use,
> tap-windows6 (default) or wintun.
>
> Note than wintun support will be added in follow-up patches.
>
> Signed-off-by: Lev Stipakov
> ---
> src/openvpn/init.c| 7 +++
> src/openvpn/options.c | 37 +
> src/openvpn/options.h | 1 +
> src/openvpn/tun.h | 1 +
> 4 files changed, 46 insertions(+)
Should there not be a manpage entry in this commit too?
>
> diff --git a/src/openvpn/init.c b/src/openvpn/init.c
> index ae7bd63..c6d4953 100644
> --- a/src/openvpn/init.c
> +++ b/src/openvpn/init.c
> @@ -1733,6 +1733,10 @@ do_init_tun(struct context *c)
> c->c2.es,
> >net_ctx);
>
> +#ifdef _WIN32
> +c->c1.tuntap->wintun = c->options.wintun;
> +#endif
> +
> init_tun_post(c->c1.tuntap,
>>c2.frame,
>>options.tuntap_options);
> @@ -1775,6 +1779,9 @@ do_open_tun(struct context *c)
> /* store (hide) interactive service handle in tuntap_options */
> c->c1.tuntap->options.msg_channel = c->options.msg_channel;
> msg(D_ROUTE, "interactive service msg_channel=%u", (unsigned int)
> c->options.msg_channel);
> +
> +c->c1.tuntap->wintun = c->options.wintun;
> +
Why do we have to set c->c2.tuntap->wintun in both do_init_tun and
do_open_tun? Should one of them not suffice?
> #endif
>
> /* allocate route list structure */
> diff --git a/src/openvpn/options.c b/src/openvpn/options.c
> index 1838a69..5c5033e 100644
> --- a/src/openvpn/options.c
> +++ b/src/openvpn/options.c
> @@ -747,6 +747,9 @@ static const char usage_message[] =
> " optional parameter controls the initial state of
> ex.\n"
> "--show-net-up : Show " PACKAGE_NAME "'s view of routing table and net
> adapter list\n"
> " after TAP adapter is up and routes have been added.\n"
> +"--windows-driver : Which tun driver to use?\n"
> +" tap-windows6 (default)\n"
> +" wintun\n"
> #ifdef _WIN32
Should this --windows-driver option not be inside the #ifdef _WIN32 ?
> "--block-outside-dns : Block DNS on other network adapters to prevent
> DNS leaks\n"
> #endif
> @@ -851,6 +854,7 @@ init_options(struct options *o, const bool init_gc)
> o->tuntap_options.dhcp_masq_offset = 0; /* use network address as
> internal DHCP server address */
> o->route_method = ROUTE_METHOD_ADAPTIVE;
> o->block_outside_dns = false;
> +o->wintun = false;
> #endif
> o->vlan_accept = VLAN_ONLY_UNTAGGED_OR_PRIORITY;
> o->vlan_pvid = 1;
> @@ -2994,6 +2998,12 @@ options_postprocess_mutate_invariant(struct options
> *options)
> options->ifconfig_noexec = false;
> }
>
> +/* for wintun kernel doesn't send DHCP requests, so use ipapi to set IP
> address and netmask */
> +if (options->wintun)
> +{
> +options->tuntap_options.ip_win32_type = IPW32_SET_IPAPI;
> +}
> +
> remap_redirect_gateway_flags(options);
> #endif
>
> @@ -4039,6 +4049,26 @@ foreign_option(struct options *o, char *argv[], int
> len, struct env_set *es)
> }
> }
>
> +#ifdef _WIN32
> +bool
> +parse_windows_driver(const char *str, const int msglevel)
I think this should be a static function. Also a short (doxygen) comment
explaining what the return value means would be nice.
> +{
> +if (streq(str, "tap-windows6"))
> +{
> +return false;
> +}
> +else if (streq(str, "wintun"))
> +{
> +return true;
> +}
> +else
> +{
> +msg(msglevel, "--windows-driver must be tap-windows6 or wintun");
> +return false;
> +}
> +}
> +#endif
> +
> /*
> * parse/print topology coding
> */
> @@ -5281,6 +5311,13 @@ add_option(struct options *options,
> VERIFY_PERMISSION(OPT_P_GENERAL);
> options->dev_type = p[1];
> }
> +#ifdef _WIN32
> +else if (streq(p[0], "windows-driver") && p[1] && !p[2])
> +{
> +VERIFY_PERMISSION(OPT_P_GENERAL);
> +options->wintun = parse_windows_driver(p[1], M_FATAL);
> +}
> +#endif
> else if (streq(p[0], "dev-node") && p[1] && !p[2])
> {
> VERIFY_PERMISSION(OPT_P_GENERAL);
> diff --git a/src/openvpn/options.h b/src/openvpn/options.h
> index ff7a5bb..0a24e5e 100644
> --- a/src/openvpn/options.h
> +++ b/src/openvpn/options.h
> @@ -632,6 +632,7 @@ struct options
> bool show_net_up;
> int route_method;
> bool block_outside_dns;
> +bool wintun;
Did you consider using an enum instead? I think it would make the code
easier to read. E.g. with
typedef enum { WINDRV_TAP6, WINDRV_WINTUN } windrv_t;
in tun.h or so, we'd get
options->windrv = WINDRV_TAP6
instead of
options->wintun = false.
>
[Openvpn-devel] [PATCH v2 2/7] wintun: add --windows-driver config option
From: Lev Stipakov
This allows to specify which tun driver openvpn should use,
tap-windows6 (default) or wintun.
Note than wintun support will be added in follow-up patches.
Signed-off-by: Lev Stipakov
---
src/openvpn/init.c| 7 +++
src/openvpn/options.c | 37 +
src/openvpn/options.h | 1 +
src/openvpn/tun.h | 1 +
4 files changed, 46 insertions(+)
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index ae7bd63..c6d4953 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -1733,6 +1733,10 @@ do_init_tun(struct context *c)
c->c2.es,
>net_ctx);
+#ifdef _WIN32
+c->c1.tuntap->wintun = c->options.wintun;
+#endif
+
init_tun_post(c->c1.tuntap,
>c2.frame,
>options.tuntap_options);
@@ -1775,6 +1779,9 @@ do_open_tun(struct context *c)
/* store (hide) interactive service handle in tuntap_options */
c->c1.tuntap->options.msg_channel = c->options.msg_channel;
msg(D_ROUTE, "interactive service msg_channel=%u", (unsigned int)
c->options.msg_channel);
+
+c->c1.tuntap->wintun = c->options.wintun;
+
#endif
/* allocate route list structure */
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 1838a69..5c5033e 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -747,6 +747,9 @@ static const char usage_message[] =
" optional parameter controls the initial state of
ex.\n"
"--show-net-up : Show " PACKAGE_NAME "'s view of routing table and net
adapter list\n"
" after TAP adapter is up and routes have been added.\n"
+"--windows-driver : Which tun driver to use?\n"
+" tap-windows6 (default)\n"
+" wintun\n"
#ifdef _WIN32
"--block-outside-dns : Block DNS on other network adapters to prevent
DNS leaks\n"
#endif
@@ -851,6 +854,7 @@ init_options(struct options *o, const bool init_gc)
o->tuntap_options.dhcp_masq_offset = 0; /* use network address as
internal DHCP server address */
o->route_method = ROUTE_METHOD_ADAPTIVE;
o->block_outside_dns = false;
+o->wintun = false;
#endif
o->vlan_accept = VLAN_ONLY_UNTAGGED_OR_PRIORITY;
o->vlan_pvid = 1;
@@ -2994,6 +2998,12 @@ options_postprocess_mutate_invariant(struct options
*options)
options->ifconfig_noexec = false;
}
+/* for wintun kernel doesn't send DHCP requests, so use ipapi to set IP
address and netmask */
+if (options->wintun)
+{
+options->tuntap_options.ip_win32_type = IPW32_SET_IPAPI;
+}
+
remap_redirect_gateway_flags(options);
#endif
@@ -4039,6 +4049,26 @@ foreign_option(struct options *o, char *argv[], int len,
struct env_set *es)
}
}
+#ifdef _WIN32
+bool
+parse_windows_driver(const char *str, const int msglevel)
+{
+if (streq(str, "tap-windows6"))
+{
+return false;
+}
+else if (streq(str, "wintun"))
+{
+return true;
+}
+else
+{
+msg(msglevel, "--windows-driver must be tap-windows6 or wintun");
+return false;
+}
+}
+#endif
+
/*
* parse/print topology coding
*/
@@ -5281,6 +5311,13 @@ add_option(struct options *options,
VERIFY_PERMISSION(OPT_P_GENERAL);
options->dev_type = p[1];
}
+#ifdef _WIN32
+else if (streq(p[0], "windows-driver") && p[1] && !p[2])
+{
+VERIFY_PERMISSION(OPT_P_GENERAL);
+options->wintun = parse_windows_driver(p[1], M_FATAL);
+}
+#endif
else if (streq(p[0], "dev-node") && p[1] && !p[2])
{
VERIFY_PERMISSION(OPT_P_GENERAL);
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index ff7a5bb..0a24e5e 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -632,6 +632,7 @@ struct options
bool show_net_up;
int route_method;
bool block_outside_dns;
+bool wintun;
#endif
bool use_peer_id;
diff --git a/src/openvpn/tun.h b/src/openvpn/tun.h
index 5a0a933..df935f6 100644
--- a/src/openvpn/tun.h
+++ b/src/openvpn/tun.h
@@ -175,6 +175,7 @@ struct tuntap
* ~0 if undefined */
DWORD adapter_index;
+bool wintun; /* true if wintun is used instead of tap-windows6 */
int standby_iter;
#else /* ifdef _WIN32 */
int fd; /* file descriptor for TUN/TAP dev */
--
2.7.4
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
