Hi, On 06/12/2018 07:40, Gert Doering wrote: > The IPv6 routing code inherited assumptions and the message > > "OpenVPN ROUTE6: OpenVPN needs a gateway parameter for a --route-ipv6 > option and no default was specified by either --route-ipv6-gateway or > --ifconfig-ipv6 options" > > from the IPv4 routing code. > > This was never really correct, as no gateway is needed for "into tun > device" IPv6 routes, and the "--route-ipv6-gateway" option it refers > to also never existed. (Routes on tap interfaces *do* need a gateway > due to neighbour discovery being involved. As do routes on Windows, > but there we fake the gateway in tun mode anyway). > > While commit d24e1b179b95 introduces support for "--route-ipv6-gateway", > the message is still falsely triggered for IPv6 routes in tun mode. > > Change the code to generally accept IPv6 routes with no gateway > specification (so "--block-ipv6 --redirect-gateway ipv6" can work > without additional config). When installing IPv6 routes, check > if a gateway is needed (tap mode) but missing, and if yes, print > correct message. >
Thanks for this commit message. It was very clear as of what the patch
is addressing, how and why,
> Trac: #1143
>
> Signed-off-by: Gert Doering <g...@greenie.muc.de>
> ---
> src/openvpn/route.c | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/src/openvpn/route.c b/src/openvpn/route.c
> index d97e8dba..ac38bf15 100644
> --- a/src/openvpn/route.c
> +++ b/src/openvpn/route.c
> @@ -448,11 +448,6 @@ init_route_ipv6(struct route_ipv6 *r6,
> {
> r6->gateway = rl6->remote_endpoint_ipv6;
> }
> - else
> - {
> - msg(M_WARN, PACKAGE_NAME " ROUTE6: " PACKAGE_NAME " needs a gateway
> parameter for a --route-ipv6 option and no default was specified by either
> --route-ipv6-gateway or --ifconfig-ipv6 options");
> - goto fail;
> - }
>
> /* metric */
>
> @@ -1917,6 +1912,16 @@ add_route_ipv6(struct route_ipv6 *r6, const struct
> tuntap *tt, unsigned int flag
> gateway_needed = true;
> }
>
> + if (gateway_needed && IN6_IS_ADDR_UNSPECIFIED(&r6->gateway) )
please remove the space before the last ')'
> + {
> + msg(M_WARN, "ROUTE6 WARNING: " PACKAGE_NAME " needs a gateway "
> + "parameter for a --route-ipv6 option and no default was set via "
> + "--ifconfig-ipv6 or --route-ipv6-gateway option. Not installing
> "
> + "IPv6 route to %s/%d.", network, r6->netbits );
please remove the space before the last ')'
As quickly discussed during the meeting, it might be better to have the
string all on one line to make it easier to grep. You could go on a new
line after M_WARN, and then right after the long string.
One dis-advantage I have seen while testing this patch is that we now
get one message for each route we try to install.
However, I think it is ok because the message explicitly mentions each
failing route.
> + status = false;
> + goto done;
> + }
> +
> #if defined(TARGET_LINUX)
> #ifdef ENABLE_IPROUTE
> argv_printf(&argv, "%s -6 route add %s/%d dev %s",
> @@ -2114,6 +2119,7 @@ add_route_ipv6(struct route_ipv6 *r6, const struct
> tuntap *tt, unsigned int flag
> msg(M_FATAL, "Sorry, but I don't know how to do 'route ipv6' commands on
> this operating system. Try putting your routes in a --route-up script");
> #endif /* if defined(TARGET_LINUX) */
>
> +done:
> if (status)
> {
> r6->flags |= RT_ADDED;
>
Other than those comments above, the patch looks good and my tests did
not reveal any issue.
Acked-by: Antonio Quartulli <anto...@openvpn.net>
Regards,
--
Antonio Quartulli
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
