Hi, On Wed, Dec 28, 2016 at 08:37:48PM +0100, Christian Hesse wrote: > > We definitely need a better approach than "litter ENABLE_SYSTEMD all > > over the code". > > Well, openvpn supports a number of modes of operation... Some of these have > other requirements than others.
This I understand quite well :-) (look at the discussions surrounding
NCP and poor man's NCP, and why that ended up patch version 7 or so)
[..]
> IMHO we should go with option three. Given the number of #ifdefs all over the
> code - is this really an issue? My patch increases the number of sd_notify()
> calls from four to five.
Especially given the high amount of #ifdef we already have, every additional
one is "one too many" - doubly so for platform-specific stuff in otherwise
platform-independent code like init.c
While technically not "platform", nothing else but a subset of Linux
distributions use systemd today, so I consider this to be in the same
league as #ifdef TARGET_FREEBSD or #if _WIN32, which both should not
appear in generic high-level code. tun.c, route.c, platform.c are
unavoidably bad... but this should serve as a bad example, not as
"match this!" target :-)
[..]
> For informational purpose we could add even more calls. That would allow to
> set intermediate status message, something like: "Up and running, currently
> serving 25 client connections."
True, and I think that is a useful feature - but it needs an abstraction
layer somewhere in between. Adding #ifdef ENABLE_SYSTEMD all across the
code is something that is not a good approach for long-term maintainability,
and to support *other* interfaces to status reporting - like dbus, or
whatever interesting APIs other operating operating systems come up with
one day .
We already update our status file regularily, so maybe this is a reasonable
point in the code to abstract out something like "platform_notify_status()"
(or whatever good name you can come up with), which could then go to,
like, "platform-notify.c", and have all the #ifdef ENABLE_SYSTEMD that
are needed.
[..]
> > (Also, this is the wrong check anyway. p2p mode can go along with
> > TLS just fine - what you need to check for is --server or --client,
> > which is something else than --tls-server / --tls-client)
>
> No, the check is correct.
>
> In server move openvpn reports ready when it finally is ready to handle
> incoming connections. That is fine.
"server" or "TLS server"? These are not the same :-) - you can have
p2p mode with --tls-server.
But I'll accept that you're likely right there, and --tls-server reports
"ready" when it's set up and ready to go, not having to wait for a packet
from the tls-client - this stuff has bitten us in t_client.rc recently
(p2p startup not reporting "ready!" and t_client.rc then aborting the
test run).
gert
--
USENET is *not* the non-clickable part of WWW!
//www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025 g...@net.informatik.tu-muenchen.de
signature.asc
Description: PGP signature
------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
