| Message ID | 20220818100953.191328-1-a@unstable.cc |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | None | expand |
Hi, On Thu, Aug 18, 2022 at 12:09:53PM +0200, Antonio Quartulli wrote: > At the moment dco-win doesn't support --persist-tun and --server, > so check for these options at startup time. > > Signed-off-by: Antonio Quartulli <a@unstable.cc> > Signed-off-by: Lev Stipakov <lev@openvpn.net> I was about to merge this (and thank you for bearing with a grumpy old man's preferences ;-) - but I really think that the difference in these platforms need to stand out right there...). Butbut... This breaks Linux/Non-DCO builds with --dev tun30, and I can't see an obvious reason *why* $ src/openvpn/openvpn --version OpenVPN 2.6_git [git:vw/master/f10c870c140bd3c6] x86_64-pc-linux-gnu [SSL (mbed TLS)] [LZO] [LZ4] [EPOLL] [MH/PKTINFO] [AEAD] built on Aug 18 2022 (no DCO here, this is "master + 2/7 v103", on Linux) $ openvpn ... --dev tun30 --proto udp ... ... 2022-08-18 21:43:52 OPTIONS IMPORT: peer-id set 2022-08-18 21:43:52 OPTIONS IMPORT: data channel crypto options modified 2022-08-18 21:43:52 net_route_v4_best_gw query: dst 0.0.0.0 2022-08-18 21:43:52 net_route_v4_best_gw result: via 195.30.8.94 dev ens160 2022-08-18 21:43:52 GDG6: remote_host_ipv6=n/a 2022-08-18 21:43:52 net_route_v6_best_gw query: dst :: 2022-08-18 21:43:52 net_route_v6_best_gw result: via 2001:608:1:995a::ffff dev ens160 2022-08-18 21:43:52 DCO device tun30 opened 2022-08-18 21:43:52 net_iface_mtu_set: rtnl: cannot get ifindex for tun30: No such device (errno=19) 2022-08-18 21:43:52 Linux can't set mtu (1500) on tun30 2022-08-18 21:43:52 Exiting due to fatal error why is it talking about DCO here? Why would it even *know* about DCO in a non-DCO build? Not sure why any of the hunks in this patch have an effect, but if I go back to the current master, --dev tun30 works, both with and without --enable-dco. *scratch head* Ow. +#if defined(TARGET_LINUX) && defined(TARGET_FREEBSD) /* check if any option should force disabling DCO */ -#if defined(TARGET_LINUX) || defined(TARGET_FREEBSD) this is not "just moving" these lines, but requiring "LEENUX AND FREEBSD" now... You owe me a beer :-) (Changing the && to || makes the --dev tun30 test pass again - I'm still not sure *what* happened here... but maybe we should reconsider the decision to have disable_dco default to "false" even on non-DCO builds...) gert
diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c index 757ac19b..0aeecc54 100644 --- a/src/openvpn/dco.c +++ b/src/openvpn/dco.c @@ -225,7 +225,20 @@ dco_update_keys(dco_context_t *dco, struct tls_multi *multi) bool dco_check_startup_option_conflict(int msglevel, const struct options *o) { -#if defined(TARGET_LINUX) +#if defined(_WIN32) + if (o->mode == MODE_SERVER) + { + msg(msglevel, "Only client and p2p data channel offload is supported " + "with ovpn-dco-win."); + return false; + } + + if (o->persist_tun) + { + msg(msglevel, "--persist-tun is not supported with ovpn-dco-win."); + return false; + } +#elif defined(TARGET_LINUX) /* if the device name is fixed, we need to check if an interface with this * name already exists. IF it does, it must be a DCO interface, otherwise * DCO has to be disabled in order to continue. @@ -250,7 +263,7 @@ dco_check_startup_option_conflict(int msglevel, const struct options *o) strerror(-ret), ret); } } -#endif /* if defined(TARGET_LINUX) */ +#endif /* if defined(_WIN32) */ #if defined(HAVE_LIBCAPNG) /* DCO can't operate without CAP_NET_ADMIN. To retain it when switching user diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 2415c1a8..272f8741 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -3669,10 +3669,16 @@ options_postprocess_mutate(struct options *o, struct env_set *es) "incompatible with each other."); } +#if defined(TARGET_LINUX) && defined(TARGET_FREEBSD) /* check if any option should force disabling DCO */ -#if defined(TARGET_LINUX) || defined(TARGET_FREEBSD) o->tuntap_options.disable_dco = !dco_check_option_conflict(D_DCO, o) || !dco_check_startup_option_conflict(D_DCO, o); +#elif defined(_WIN32) + /* in Windows we have no 'fallback to non-DCO' strategy, so if a conflicting + * option is found, we simply bail out by means of M_USAGE + */ + dco_check_option_conflict(M_USAGE, o); + dco_check_startup_option_conflict(M_USAGE, o); #endif if (dco_enabled(o) && o->dev_node)