[Openvpn-devel,v2] Remove warning on pushed tun-ipv6 option.

Message ID 20171111142230.3288-1-gert@greenie.muc.de
State Accepted
Headers show
Series
  • [Openvpn-devel,v2] Remove warning on pushed tun-ipv6 option.
Related show

Commit Message

Gert Doering Nov. 11, 2017, 2:22 p.m.
tun-ipv6 is a no-op nowadays, and we print a warning to let users know -
which is not helpful for server-pushed tun-ipv6 (which might be the
result of --server-ipv6 automatically pushing this).  So, remove the
warning if parsing pushed options.

Also, remove the VERIFY_PERMISSION() call here which has side effects
on the "which class of options got pushed, do we need to act on them
later on?" flag set.

v2: use existing pull_mode flag

Signed-off-by: Gert Doering <gert@greenie.muc.de>
---
 src/openvpn/options.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Arne Schwabe Nov. 11, 2017, 2:24 p.m. | #1
Am 11.11.17 um 15:22 schrieb Gert Doering:
> tun-ipv6 is a no-op nowadays, and we print a warning to let users know -
> which is not helpful for server-pushed tun-ipv6 (which might be the
> result of --server-ipv6 automatically pushing this).  So, remove the
> warning if parsing pushed options.
> 
> Also, remove the VERIFY_PERMISSION() call here which has side effects
> on the "which class of options got pushed, do we need to act on them
> later on?" flag set.
> 
> v2: use existing pull_mode flag
> 
> Signed-off-by: Gert Doering <gert@greenie.muc.de>
> ---
>  src/openvpn/options.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/src/openvpn/options.c b/src/openvpn/options.c
> index 641a26e2..4caa2cd1 100644
> --- a/src/openvpn/options.c
> +++ b/src/openvpn/options.c
> @@ -5233,8 +5233,10 @@ add_option(struct options *options,
>      }
>      else if (streq(p[0], "tun-ipv6") && !p[1])
>      {
> -        VERIFY_PERMISSION(OPT_P_UP);
> -        msg(M_WARN, "Note: option tun-ipv6 is ignored because modern operating systems do not need special IPv6 tun handling anymore.");
> +        if ( !pull_mode )
> +        {
> +            msg(M_WARN, "Note: option tun-ipv6 is ignored because modern operating systems do not need special IPv6 tun handling anymore.");
> +        }
>      }
>  #ifdef ENABLE_IPROUTE
>      else if (streq(p[0], "iproute") && p[1] && !p[2])
> 

Acked-by: Arne


The code looks right but I have not compile tested it.

Arne

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Steffan Karger Nov. 11, 2017, 2:29 p.m. | #2
Hi,

On 11-11-17 15:22, Gert Doering wrote:
> tun-ipv6 is a no-op nowadays, and we print a warning to let users know -
> which is not helpful for server-pushed tun-ipv6 (which might be the
> result of --server-ipv6 automatically pushing this).  So, remove the
> warning if parsing pushed options.
> 
> Also, remove the VERIFY_PERMISSION() call here which has side effects
> on the "which class of options got pushed, do we need to act on them
> later on?" flag set.
> 
> v2: use existing pull_mode flag
> 
> Signed-off-by: Gert Doering <gert@greenie.muc.de>
> ---
>  src/openvpn/options.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/src/openvpn/options.c b/src/openvpn/options.c
> index 641a26e2..4caa2cd1 100644
> --- a/src/openvpn/options.c
> +++ b/src/openvpn/options.c
> @@ -5233,8 +5233,10 @@ add_option(struct options *options,
>      }
>      else if (streq(p[0], "tun-ipv6") && !p[1])
>      {
> -        VERIFY_PERMISSION(OPT_P_UP);
> -        msg(M_WARN, "Note: option tun-ipv6 is ignored because modern operating systems do not need special IPv6 tun handling anymore.");
> +        if ( !pull_mode )

The code style suggests this should be spelled as "if (!pull_mode)".
I'll leave it up to you to decide to change that or not.

> +        {
> +            msg(M_WARN, "Note: option tun-ipv6 is ignored because modern operating systems do not need special IPv6 tun handling anymore.");
> +        }
>      }
>  #ifdef ENABLE_IPROUTE
>      else if (streq(p[0], "iproute") && p[1] && !p[2])
> 

Otherwise this looks good to me.

Reviewed-by: Steffan Karger <steffan@karger.me>
Acked-by: Steffan Karger <steffan@karger.me>

-Steffan

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Gert Doering Nov. 11, 2017, 2:41 p.m. | #3
Patch has been applied to the master and release/2.4 branch (with
the whitespace change as suggested by our styleguide enforcer :) ).

commit 7a216d9dba558281d4b6a04124912081a79fcb88 (master)
commit 81b6882116a55299d5da5542d757d65384829efe (release/2.4)
Author: Gert Doering
Date:   Sat Nov 11 15:22:30 2017 +0100

     Remove warning on pushed tun-ipv6 option.

     Signed-off-by: Gert Doering <gert@greenie.muc.de>
     Acked-by: Steffan Karger <steffan.karger@fox-it.com>
     Acked-by: Arne Schwabe <arne@rfc2549.org>
     Message-Id: <20171111142230.3288-1-gert@greenie.muc.de>
     URL: https://www.mail-archive.com/search?l=mid&q=20171111142230.3288-1-gert@greenie.muc.de
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Patch

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 641a26e2..4caa2cd1 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -5233,8 +5233,10 @@  add_option(struct options *options,
     }
     else if (streq(p[0], "tun-ipv6") && !p[1])
     {
-        VERIFY_PERMISSION(OPT_P_UP);
-        msg(M_WARN, "Note: option tun-ipv6 is ignored because modern operating systems do not need special IPv6 tun handling anymore.");
+        if ( !pull_mode )
+        {
+            msg(M_WARN, "Note: option tun-ipv6 is ignored because modern operating systems do not need special IPv6 tun handling anymore.");
+        }
     }
 #ifdef ENABLE_IPROUTE
     else if (streq(p[0], "iproute") && p[1] && !p[2])