Skip to contentSkip to conversation

Remove warning on pushed tun-ipv6 option.

PATCH Superseded Gert Doering 3 participants 7 messages Nov 11, 2017

lore
Share & tools
lore

Gert Doering

author
#0 lore raw

PATCH 1/1 Remove warning on pushed tun-ipv6 option.

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.

Signed-off-by Gert Doering <[email protected]...>
---
 src/openvpn/options.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
src/openvpn/options.c +4 -2
index 641a26e2..bbf65613 100644
@@ -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 ( !(permission_mask & OPT_P_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])
Signature · 1 line
2.13.0
#1 lore raw

Hi,

Quoted 19 lines · 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.

Signed-off-by Gert Doering <[email protected]...>
---
 src/openvpn/options.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
src/openvpn/options.c +0 -0
index 641a26e2..bbf65613 100644

src/openvpn/options.c add_option(struct options *options, · line 5236

Quoted 6 lines
src/openvpn/options.c +1 -2
@@ -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 ( !(permission_mask & OPT_P_PULL_MODE) )

add_option() already has a shorthand for this:

    const bool pull_mode = BOOL_CAST(permission_mask & OPT_P_PULL_MODE);

src/openvpn/options.c add_option(struct options *options, · line 5242

Quoted 7 lines
src/openvpn/options.c +3 -0
+        {
+            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])

(I'll leave review of the correctness of the change to someone who actually understands what tun-ipv6 exactly does.)

-Steffan

author
#2 lore raw

Hi,

Quoted 2 lines · Steffan Karger

(I'll leave review of the correctness of the change to someone who actually understands what tun-ipv6 exactly does.)

"exactly and absolutely nothing" :-)

(It *used* to be a flag to tell openvpn "switch the tun interface into ipv6-compatible mode", but after Arne's revamping of this it's basically a no-op, which we keep around for compatibility reasons with old config files - and the warning points out that it does not do anything)

gert

Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...
author
#3 lore raw

PATCH v2 1/1 Remove warning on pushed tun-ipv6 option.

In reply to 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 <[email protected]...>
---
 src/openvpn/options.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
src/openvpn/options.c +4 -2
index 641a26e2..4caa2cd1 100644
@@ -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])
Signature · 1 line
2.13.0
Acked-by Arne on v2 1/1
#4 lore raw

Am 11.11.17 um 15:22 schrieb Gert Doering:

Quoted 24 lines

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 <[email protected]...>
---
 src/openvpn/options.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
src/openvpn/options.c +0 -0
index 641a26e2..4caa2cd1 100644
@@ -5233,8 +5233,10 @@ add_option(struct options *options,
     }
     else if (streq(p[0], "tun-ipv6") && !p[1])

src/openvpn/options.c add_option(struct options *options, · line 5242

Quoted 10 lines
src/openvpn/options.c +4 -2
     {
-        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

Reviewed-by Acked-by on v2 1/1
#5 lore raw
In reply to Gert Doering

Hi,

Quoted 21 lines · 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 <[email protected]...>
---
 src/openvpn/options.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
src/openvpn/options.c +0 -0
index 641a26e2..4caa2cd1 100644

src/openvpn/options.c add_option(struct options *options, · line 5236

Quoted 6 lines
src/openvpn/options.c +1 -2
@@ -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.

src/openvpn/options.c add_option(struct options *options, · line 5242

Quoted 7 lines
src/openvpn/options.c +3 -0
+        {
+            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 <[email protected]...>
Acked-by Steffan Karger <[email protected]...>

-Steffan

author
#6 lore raw
In reply to Gert Doering

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 <[email protected]...>
     Acked-by: Steffan Karger <[email protected]...>
     Acked-by: Arne Schwabe <[email protected]...>
     Message-Id: <[email protected]...>
     URL: https://www.mail-archive.com/search?l=mid&[email protected]...
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering