Hi,

On Tue, Oct 11, 2022 at 01:01:26PM +0200, Arne Schwabe wrote:
> From the implemention and the fact that it is a an OCC message
> (basically the rudimentary predecessor to control channel),
> this message is very old.
> 
> I think in the past this feature fit nicely to the weird inetd + openvpn mode
> that seems to have far to many hacks still left in our code. With inetd,
> it made sense that the server instance quits if you press C-c on the client.
> 
> In our current state where inetd is no longer supported, this behaviour
> to exit makes little sense and this patch changes the behaviour to SIGUSR1.

I am very confused about v2 of this patch - and would be happy to ACK v1.

I have applied v1, and tested

 - p2p without the patch -> client sends EEN -> server exits
    2022-10-14 16:56:56 us=334517 Exit message received by peer
    2022-10-14 16:56:56 us=334753 TCP/UDP: Closing socket
    2022-10-14 16:56:56 us=334814 Closing TUN/TAP interface
    2022-10-14 16:56:56 us=334836 net_addr_v4_del: 10.204.11.1 dev tun11
    2022-10-14 16:56:56 us=334987 net_addr_v6_del: fd00:abcd:204:11::1/64 dev 
tun11
    2022-10-14 16:56:56 us=342133 SIGTERM[soft,remote-exit] received, process 
exiting

 - p2p with the v1 patch -> client sends EEN -> server logs, and restarts

    2022-10-14 16:57:30 us=151079 Exit message received by peer
    2022-10-14 16:57:30 us=151254 TCP/UDP: Closing socket
    2022-10-14 16:57:30 us=151316 Closing TUN/TAP interface
    2022-10-14 16:57:30 us=151343 net_addr_v4_del: 10.204.11.1 dev tun11
    2022-10-14 16:57:30 us=151465 net_addr_v6_del: fd00:abcd:204:11::1/64 dev 
tun11
    2022-10-14 16:57:30 us=155086 SIGUSR1[soft,remote-exit] received, process 
restarting
    2022-10-14 16:57:30 us=155149 Restart pause, 5 second(s)

    (and then it rearms itself and is ready to server another p2p client)


 - p2mp server without patch -> client sends EEN -> server logs, and 
   terminates the instance

    2022-10-14 16:58:26 us=544309 cron2-freebsd-tc-amd64/194.97.140.21:17570 
Exit message received by peer
    2022-10-14 16:58:26 us=544365 cron2-freebsd-tc-amd64/194.97.140.21:17570 
SIGTERM[soft,remote-exit] received, client-instance exiting


 - p2mp server with the v1 patch -> client sends EEN -> server logs, and
   says it will "restart" the instance

    2022-10-14 16:59:02 us=113506 cron2-freebsd-tc-amd64/194.97.140.21:41691 
Exit message received by peer
    2022-10-14 16:59:02 us=113573 cron2-freebsd-tc-amd64/194.97.140.21:41691 
SIGUSR1[soft,remote-exit] received, client-instance restarting

 - with a 2.4 client, it has no "Exit message recived by peer" (because that
   is the new control channel thing), and just logs

    2022-10-14 17:03:47 us=455929 cron2-freebsd-tc-amd64/194.97.140.21:13152 
SIGUSR1[soft,remote-exit] received, client-instance restarting


Trying to understand the code is an interesting maze... that message
above comes from sig.c::print_signal(), which is called from
"multi::multi_close_instance_on_signal()".

*That* one is called (mostly) from multi_process_post(), which says

" * Also close context on signal."

and basically does

    if (IS_SIG(&mi->context))
    {
        if (flags & MPP_CLOSE_ON_SIGNAL)
        {
            multi_close_instance_on_signal(m, mi);
            ret = false;
        }
    }

... so, the nature of the (soft) signal raised does not seem to play
any role to the instance - "if signal, then close instance".  Just the
logging is a bit off now ("restarting" vs. "exiting").


If we care about logging, I think we should do the signal remap in
multi_close_instance_on_signal()  (... which already remaps SIGUSR1
by means of remap_signal(), but only if c->options.remap_sigusr1 is
set).

OTOH, maybe it should just *not* call remap_signal(), as the per-instance
signals and the global signals do different things, and always remap
SIGUSR1-in-instance to SIGTERM...


What I do not think will make us happy in future is the extra 
conditionals in v2...

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