diff mbox series

[Openvpn-devel] dco_p2p_add_new_peer: do not warn about missing default gw

Message ID 20220810094605.123072-1-frank@lichtenheld.com
State Superseded
Headers
Series [Openvpn-devel] dco_p2p_add_new_peer: do not warn about missing default gw |

Commit Message

Frank Lichtenheld Aug. 9, 2022, 11:46 p.m. UTC 
  Currently we issue a message when using --ifconfig but
not specifying a default gateway. This seems to be misleading,
the setup will still work fine since in P2P we now send
all traffic to the peer anyway. Or it might be irrelevant
if all route specifications include a gateway anyway.

Since it is probably better to have no message than a
misleading one, remove it.

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
---
 src/openvpn/dco.c | 4 ----
 1 file changed, 4 deletions(-)

Comments

Antonio Quartulli Aug. 10, 2022, 2:54 a.m. UTC | #1 
Hi,

On 10/08/2022 11:46, Frank Lichtenheld wrote:
> Currently we issue a message when using --ifconfig but
> not specifying a default gateway. This seems to be misleading,
> the setup will still work fine since in P2P we now send
> all traffic to the peer anyway. Or it might be irrelevant
> if all route specifications include a gateway anyway.
> 
> Since it is probably better to have no message than a
> misleading one, remove it.
> 
> Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>

Acked-by: Antonio Quartulli <a@unstable.cc>

Regards,

> ---
>   src/openvpn/dco.c | 4 ----
>   1 file changed, 4 deletions(-)
> 
> diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c
> index 4f40255e..8aa7e896 100644
> --- a/src/openvpn/dco.c
> +++ b/src/openvpn/dco.c
> @@ -438,10 +438,6 @@ dco_p2p_add_new_peer(struct context *c)
>           }
>           remote_addr4 = &remote_ip4;
>       }
> -    else if (c->options.ifconfig_local)
> -    {
> -        msg(M_INFO, "DCO peer init: Need a peer VPN addresss to setup IPv4 (set --route-gateway)");
> -    }
>   
>       struct sockaddr *remoteaddr = &ls->info.lsa->actual.dest.addr.sa;
>
Gert Doering Aug. 10, 2022, 9:12 a.m. UTC | #2 
Hi,

On Wed, Aug 10, 2022 at 11:46:05AM +0200, Frank Lichtenheld wrote:
> diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c
> index 4f40255e..8aa7e896 100644
> --- a/src/openvpn/dco.c
> +++ b/src/openvpn/dco.c
> @@ -438,10 +438,6 @@ dco_p2p_add_new_peer(struct context *c)
>          }
>          remote_addr4 = &remote_ip4;
>      }
> -    else if (c->options.ifconfig_local)
> -    {
> -        msg(M_INFO, "DCO peer init: Need a peer VPN addresss to setup IPv4 (set --route-gateway)");
> -    }

Looking more closely, this is a really nice gem you found here :-)

The kernel should not care at all for remote_addr4 or remote_addr6 in
p2p mode ("it is never used for anything") *but* it does - so, if you
twist openvpn into setting up an ipv4-only tunnel, with topology subnet,
and no --route-gateway in the config (which would trigger the message
above), DCO actually refuses to bring up the interface:

2022-08-10 21:09:31 dco_new_peer: netlink reports error (-7): Invalid input data or parameter
2022-08-10 21:09:31 dco_new_peer: failed to send netlink message: Invalid argument (-22)
2022-08-10 21:09:31 Cannot add peer to DCO: Invalid argument (-22)

... we did not see this in the t_client tests, as this needs *v4-only*,
and I never tested this ("3a" does now), and the kernel was happy with
"either v4 or v6, as long as I can ignore one of them"...


So, I think that for p2p, the whole remote_addr4/remote_addr6 thing 
can be removed - more than just the message :-) - but first the kernel
needs to understand this as well.

FreeBSD DCO is happy with v4 and no route-gateway...

gert


PS:

# Test 3a: UDP / p2mp tun, topology subnet, IPv4-only inside, over IPv6
# (no route-gateway, explicit gateway on routes)
#
# triggers (Linux) DCO mishap "needs v4 or v6 remote VPN address even on p2p"
#
RUN_TITLE_3a="udp / p2pm / top subnet / IPv4-only, no 'route-gateway'"
OPENVPN_CONF_3a="$OPENVPN_BASE_P2MP --dev tun --proto udp --remote $REMOTE --port 51195 --pull-filter ignore ifconfig-ipv6 --pull-filter ignore route-ipv6 --pull-filter ignore route-gateway --route 10.194.0.0 255.255.0.0 10.194.3.1"
EXPECT_IFCONFIG4_3a=$EXPECT_IFCONFIG4_3
EXPECT_IFCONFIG6_3a=-
PING4_HOSTS_3a="10.194.3.1 10.194.0.1"
PING6_HOSTS_3a=
Gert Doering Aug. 17, 2022, 5:24 a.m. UTC | #3 
Hi,

On Wed, Aug 10, 2022 at 11:46:05AM +0200, Frank Lichtenheld wrote:
> Currently we issue a message when using --ifconfig but
> not specifying a default gateway. This seems to be misleading,
> the setup will still work fine since in P2P we now send
> all traffic to the peer anyway. Or it might be irrelevant
> if all route specifications include a gateway anyway.
> 
> Since it is probably better to have no message than a
> misleading one, remove it.

For the record, this is has been obsoleted by 

  https://patchwork.openvpn.net/project/openvpn2/patch/20220815223941.26839-1-a@unstable.cc/

which rips out the whole section of code, more than "just the warning".

much better :-)

Setting this one to "superseded" in patchwork.

gert
diff mbox series

Patch

diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c
index 4f40255e..8aa7e896 100644
--- a/src/openvpn/dco.c
+++ b/src/openvpn/dco.c
@@ -438,10 +438,6 @@  dco_p2p_add_new_peer(struct context *c)
         }
         remote_addr4 = &remote_ip4;
     }
-    else if (c->options.ifconfig_local)
-    {
-        msg(M_INFO, "DCO peer init: Need a peer VPN addresss to setup IPv4 (set --route-gateway)");
-    }
 
     struct sockaddr *remoteaddr = &ls->info.lsa->actual.dest.addr.sa;