Hi,

On Sat, Aug 13, 2022 at 10:42:19PM +0200, Antonio Quartulli wrote:
> At the moment dco-win doesn't support --persist-tun and --server,
> so check for these options at startup time.

This needs rebasing anyway (due to the startup change), but while at it...

> +
> +    if (options->windows_driver == WINDOWS_DRIVER_DCO)
> +    {
> +        dco_check_option_conflict(M_USAGE, options);
> +    }

... this should read "if (dco_enabled(options))", no?  Patch 3/7 changes
dco_enabled() to do exactly this "if (driver == WINDOWS_DRIVER_DCO)" and
it would make *this* function more clear.

OTOH, the first thing dco_check_options_conflict() does is "check
if DCO is enabled at all", so why check it in the caller again?


Staring at this for a while, I do wonder why we do the "linux DCO"
options check in options_postprocess_mutate(), and the "Win32 DCO"
options check in options_postprocess_verify_ce() (while it does not
look at the actual ->ce entries, and really *really* does not belong).

Can we please clean this up, and have one joint call for all DCO
platforms?  I see no good reason for having two calls to the same
function, doing the same thing, in two different call chains for
different OSes.


Now there is a material difference, if the Win32 invocation finds
a dco-incompatible option, it will exit M_USAGE, while Linux will just
disable DCO - if this is what we want, we should still keep the stuff
closer together, like

    /* 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);
#else if _WIN32
    /* we can not currently fallback to non-DCO on Windows, so exit M_USAGE
     * if an option conflict is discovered */
    dco_check_option_conflict(M_USAGE, o);
    dco_check_startup_option_conflict(M_USAGE, o);
#endif


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

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to