-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
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...]
> @@ -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....]
> +++ 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.
> --- 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...0
> @@ -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); - - vect[0].iov_len = sizeof(pi); -
> vect[0].iov_base = π - vect[1].iov_len = len; -
> vect[1].iov_base = buf; - - ret = writev(tt->fd, vect, 2); -
> return(ret - sizeof(pi)); - } - else - return write (tt->fd,
> buf, len); + return write (tt->fd, buf, len);
whitespace issue.
> }
>
> int read_tun (struct tuntap* tt, uint8_t *buf, int len) { - if
> (tt->ipv6) - { - struct iovec vect[2]; - struct tun_pi
> pi; - int ret; - - vect[0].iov_len = sizeof(pi); -
> vect[0].iov_base = π - vect[1].iov_len = len; -
> vect[1].iov_base = buf; - - ret = readv(tt->fd, vect, 2); -
> return(ret - sizeof(pi)); - } - else - return read (tt->fd,
> buf, len); + return read (tt->fd, buf, len);
whitespace issue.
[....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
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)
iQIcBAEBAgAGBQJX/62wAAoJEIbPlEyWcf3yhJ8QALz7l0FSvhKlVqza+35D8jXL
t8rAGFEgmqXa2rQxUeJ5sB7KR41OyOuCMNwyfb1gB1wuzAhkWjtwM+bhtPqYc2rl
g9KsfXnEYOqUUyUPBZb+1AQtAoR5m3jly+KTkoUjkVPw0nNiXAYa8YceZaP9GFTp
bksEymcHx3wOJi9ulZY8CNSTfverfAZvoqnmL0rgqsoWPaz70brSLCOFE8V/3PUC
hDFpHGHyGiRcvqmWuPYwD3v5wUipWvESoxbItj5K72mrsc1B3tk1LkAwdyeXO95c
AOL9bVYPLxycGIEm6oUgFQ+gNJ/VAV7MORwD0HZCR9A0xrlfxsE+7ziXq8+++z1L
+nw8oIjgEAfwjBCcxU4vyyR+WhLfR/mPoP05qNFVVhTzYWceLW/OjiA51ejS8bN8
XHtwWsgHXHdeEg/cCHTUaEK3XNfJGKLVGEsNzjmZGtYymcTS3h7jLl4ljEpUJVni
ewLsR2Z/+rAj6Fub75HVky4Fwkjo2BvsXJlfHLu623G2x30x+j83zSDdIoII2Koh
c/nCMcheMPBRM4O9mLy2y9UnxipPU4MU8e058G3Cf1TWAv2GF3lBHyuiSH+JeMeR
v2I3i7xUUEinOvM692gzMdDFk5X4f3KeFvE6aKxmTc3sIFdpMRZ5F0YmpONQ/ikd
wqLFGz29sG4OVI3LtgTu
=yPA4
-----END PGP SIGNATURE-----
------------------------------------------------------------------------------
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
