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

Reply via email to