Hi,
On Fri, Aug 12, 2022 at 03:06:54PM +0200, Antonio Quartulli wrote:
> On Windows the high level API should still use the link_socket object to
> read and write packets. For this reason, even if dco_installed is true,
> we still need to rely on the classic link_socket object.
>
> Signed-off-by: Antonio Quartulli <a...@unstable.cc>
> ---
> src/openvpn/dco_win.c | 4 ++--
> src/openvpn/forward.c | 23 ++++++++++++++++++++++-
> 2 files changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/src/openvpn/dco_win.c b/src/openvpn/dco_win.c
> index f1160c7d..18ce9f3a 100644
> --- a/src/openvpn/dco_win.c
> +++ b/src/openvpn/dco_win.c
> @@ -355,14 +355,14 @@ dco_available(int msglevel)
> int
> dco_do_read(dco_context_t *dco)
> {
> - /* no-op on windows */
> + ASSERT(false);
> return 0;
> }
I think this ASSERT(0) should go into the patch that introduces
dco_win.c - this is just a few patches upstream, so introducing it
and then changing it right again sounds like a bit of needless
commit noise...
> diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
> index 650f7c59..8af41072 100644
> --- a/src/openvpn/forward.c
> +++ b/src/openvpn/forward.c
> @@ -1601,6 +1601,27 @@ process_ip_header(struct context *c, unsigned int
> flags, struct buffer *buf)
> }
> }
>
> +/* Depending on the platform, we may have to not use the DCO socket, even if
> DCO
> + * is being used for a specific link.
> + *
> + * This happens with Windows, where the standard link_socket API have to be
> used
> + * also with DCO.
> + *
> + * For this reason we must make the right decision and not always look at
> + * dco_installed. Note that on Windows the dco_installed field is still
> supposed
> + * to be true, because it is used in the lower level code to use the proper
> API
> + * (socket vs handle). This is why we need this function with some ifdef
> sauce
> + */
This comment could use a bit of rewording, like
/* Linux-like DCO implementations pass the socket to the kernel and
* disallow usage of it from userland, so (control) packets sent and
* received by OpenVPN need to go through the DCO interface.
*
* Windows DCO needs the control packets to be sent via the normal
* Socket API.
*
* Hide that complexity (... especially if more platforms show up
* in future...) in a small inline function
*/
> +static bool
> +should_use_dco_socket(struct link_socket *sock)
> +{
> +#if defined(TARGET_LINUX)
> + return sock->info.dco_installed;
> +#else
> + return false;
> +#endif
> +}
... which really should be "inline", no? ;-)
**WARNING** - this breaks FreeBSD DCO, so with FreeBSD DCO merged now,
it needs to be #if defined(TARGET_LINUX) || defined(TARGET_FREEBSD).
gert
--
"If was one thing all people took for granted, was conviction that if you
feed honest figures into a computer, honest figures come out. Never doubted
it myself till I met a computer with a sense of humor."
Robert A. Heinlein, The Moon is a Harsh Mistress
Gert Doering - Munich, Germany g...@greenie.muc.de
signature.asc
Description: PGP signature
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
