On 12/10/16 11:13, Arne Schwabe wrote:
> This option was useful when Ipv6 tun support was non standard and was
> an internal/user specified flag that tracked the Ipv6 capability of
> the tun device.
>
> All supported OS support IPv6. Also tun-ipv6 is pushable by the
> remote so not putting tun-ipv6 does not forbid ipv6 addresses.
>
> This commit also clean up a bit of the ipv6 related tun.c. Changes
> for most platforms are minimal.
>
> For linux a bit more cleanup is done: - Remove compatibility defines
> that were added 2008 - Always use IFF_NO_PI for the linux tun and
> not only for IPv4 only tun setups (Android also always IFF_NO_PI
> works fine with Ipv6).
>
> This commit also remove a non ipv6 fallback for tap driver from
> OpenVPN 2.2-beta or earlier and only warns.
>
> Patch V2: Integrate Gert's comments Patch V3: Remove tun_ipv4 option.
> It only used for MTU discovery and there it was wrong since it should
> on the transport protocol if at all Patch V4: Completely remove
> support for NetBSD <= 4.0 and remove NETBSD_MULTI_AF defines
> ---
> Changes.rst | 3 ++
> src/openvpn/forward.c | 2 +-
> src/openvpn/helper.c | 2 -
> src/openvpn/init.c | 6 ---
> src/openvpn/multi.c | 8 ++-
> src/openvpn/openvpn.h | 5 --
> src/openvpn/options.c | 11 +---
> src/openvpn/options.h | 1 -
> src/openvpn/route.c | 13 ++---
> src/openvpn/tun.c | 139
> +++++++-------------------------------------------
> src/openvpn/tun.h | 2 -
> 11 files changed, 30 insertions(+), 162 deletions(-)
>
> diff --git a/Changes.rst b/Changes.rst
> index 9fcba75..2956003 100644
> --- a/Changes.rst
> +++ b/Changes.rst
> @@ -135,6 +135,9 @@ User-visible Changes
> ciphers configured in the config file. Use --ncp-disable if you don't want
> that.
>
> +- ALl tun devices on all platforms are considered always IPv6 capable. The
> --tun-ipv6
Silly typo. (I would have done this one in-flight, hadn't it been for
a few other minor things).
[...snip...]
> diff --git a/src/openvpn/options.c b/src/openvpn/options.c
> index d97dc8f..ee8d351 100644
> --- a/src/openvpn/options.c
> +++ b/src/openvpn/options.c
> @@ -4577,7 +4569,6 @@ add_option (struct options *options,
> else if (streq (p[0], "tun-ipv6") && !p[1])
> {
> VERIFY_PERMISSION (OPT_P_UP);
> - options->tun_ipv6 = true;
Should we add a "Deprecated/NO-OP option used." message?
[...snip....]
aace2af..8a3bbba 100644
> --- a/src/openvpn/route.c
> +++ b/src/openvpn/route.c
> @@ -1729,10 +1729,10 @@ add_route_ipv6 (struct route_ipv6 *r6, const struct
> tuntap *tt, unsigned int fla
> }
> #endif
>
> - if ( !tt->ipv6 )
> + if (!tt->did_ifconfig_ipv6_setup)
> {
> - msg( M_INFO, "add_route_ipv6(): not adding %s/%d, no IPv6 on if %s",
> - network, r6->netbits, device );
> + msg( M_INFO, "add_route_ipv6(): not adding %s/%d, no IPv6 ifconfig on
> if %s",
> + network, r6->netbits, device);
>
Wouldn't it be nicer we said "no IPv6 address configured on
interface %s" ? Just trying to be slightly more user friendly in the
logs.
[...snip...]
> diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
> index b7a29f7..226ff7d 100644
> --- a/src/openvpn/tun.c
> +++ b/src/openvpn/tun.c
> @@ -741,8 +741,8 @@ do_ifconfig (struct tuntap *tt,
>
> argv_init (&argv);
>
> - msg( M_INFO, "do_ifconfig, tt->ipv6=%d,
> tt->did_ifconfig_ipv6_setup=%d",
> - tt->ipv6, tt->did_ifconfig_ipv6_setup );
> + msg( M_INFO, "do_ifconfig, tt->did_ifconfig_ipv6_setup=%d",
> + tt->did_ifconfig_ipv6_setup );
While we're changing log lines ... could we make this one a bit less
technical, or at least increase the verb level if we want it to be so
technical? Again, just to be more user friendly
[...snip...]
> @@ -1526,7 +1513,7 @@ open_tun_generic (const char *dev, const char
> *dev_type, const char *dev_node,
> bool dynamic_opened = false;
>
>
> - if ( tt->ipv6 && ! ipv6_explicitly_supported )
> + if ( ! ipv6_explicitly_supported )
> msg (M_WARN, "NOTE: explicit support for IPv6 tun devices is not
> provided for this OS");
Isn't this if() block a NOOP?
$ git grep -Hni ipv6_explicitly_supported
src/openvpn/tun.c:1521: bool ipv6_explicitly_supported, bool dynamic,
src/openvpn/tun.c:1529: if ( tt->ipv6 && ! ipv6_explicitly_supported )
Could we kill this as well?
[...snip...]
> @@ -1977,53 +1940,13 @@ close_tun (struct tuntap *tt)
> int
> write_tun (struct tuntap* tt, uint8_t *buf, int len)
> {
> - if (tt->ipv6)
> - {
> - struct tun_pi pi;
> - struct iphdr *iph;
> - struct iovec vect[2];
> - int ret;
> -
> - iph = (struct iphdr *)buf;
> -
> - pi.flags = 0;
> -
> - if(iph->version == 6)
> - pi.proto = htons(OPENVPN_ETH_P_IPV6);
> - else
> - pi.proto = htons(OPENVPN_ETH_P_IPV4);
whitespace issues.
[...snip...]
> @@ -5308,11 +5208,10 @@ open_tun (const char *dev, const char *dev_type,
> const char *dev_node, struct tu
> /* usage of numeric constants is ugly, but this is really tied to
> * *this* version of the driver
> */
> - if ( tt->ipv6 && tt->type == DEV_TYPE_TUN &&
> + if (tt->type == DEV_TYPE_TUN &&
> info[0] == 9 && info[1] < 8)
> {
> - msg( M_INFO, "WARNING: Tap-Win32 driver version %d.%d does not support
> IPv6 in TUN mode. IPv6 will be disabled. Upgrade to Tap-Win32 9.8
> (2.2-beta3 release or later) or use TAP mode to get IPv6", (int) info[0],
> (int) info[1] );
> - tt->ipv6 = false;
> + msg( M_INFO, "WARNING: Tap-Win32 driver version %d.%d does not support
> IPv6 in TUN mode. IPv6 will not work. Upgrade to Tap-Win32 9.8 (2.2-beta3
> release or later) or use TAP mode to get IPv6", (int) info[0], (int) info[1]
> );
Maybe we should review the contents of this log line as well? Can at
least point at the latest 2.3 release instead of a beta release.
I see that a few of my comments are not directly related to removing
--tun-ipv6. But I'm not convinced there's any benefit in having them
in a separate patch. I'm okay with either approach.
--
kind regards,
David Sommerseth
------------------------------------------------------------------------------
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
