diff mbox series

[Openvpn-devel] dco: send SIGUSR1 upon ping timeout

Message ID 20230112000435.14119-1-a@unstable.cc
State Rejected
Headers
Series [Openvpn-devel] dco: send SIGUSR1 upon ping timeout |

Commit Message

Antonio Quartulli Jan. 12, 2023, 12:04 a.m. UTC 
  When a peer is removed with reason "ping expire", we should kill the
instance with SIGUSR1 and not SIGTERM

Cc: Arne Schwabe <arne@rfc2549.org>
Signed-off-by: Antonio Quartulli <a@unstable.cc>
--

Arne, I am not 100% sure why but it seems for ping-restart we always use
SIGUSR1, right? but the DCO handling code was apparently using SIGTERM.

What do you think?
---
 src/openvpn/multi.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Arne Schwabe Jan. 12, 2023, 1:52 a.m. UTC | #1 
Am 12.01.23 um 01:04 schrieb Antonio Quartulli:
> When a peer is removed with reason "ping expire", we should kill the
> instance with SIGUSR1 and not SIGTERM
> 
> Cc: Arne Schwabe <arne@rfc2549.org>
> Signed-off-by: Antonio Quartulli <a@unstable.cc>
> --
> 
> Arne, I am not 100% sure why but it seems for ping-restart we always use
> SIGUSR1, right? but the DCO handling code was apparently using SIGTERM.
> 
> What do you think?

This makes no difference in p2mp mode.

Arne
Gert Doering Jan. 12, 2023, 2:51 p.m. UTC | #2 
Hi,

On Thu, Jan 12, 2023 at 02:52:33AM +0100, Arne Schwabe wrote:
> Am 12.01.23 um 01:04 schrieb Antonio Quartulli:
> > Arne, I am not 100% sure why but it seems for ping-restart we always use
> > SIGUSR1, right? but the DCO handling code was apparently using SIGTERM.
> > 
> > What do you think?
> 
> This makes no difference in p2mp mode.

This is how I remember the code, from the "incoming explicit-exit-notify
vs. p2p/p2mp server" testing.  Logging is different for SIGUSR1 vs. SIGTERM
instances ("restarting / terminating client instance" or something like
that) but *then* the same thing happens.

You can't "restart" an incoming session ;-)

-> putting this to "Rejected" in patchwork.

gert
diff mbox series

Patch

diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 99123c39..10efffec 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -3234,10 +3234,12 @@  process_incoming_del_peer(struct multi_context *m, struct multi_instance *mi,
                           dco_context_t *dco)
 {
     const char *reason = "ovpn-dco: unknown reason";
+    int signal = SIGTERM;
     switch (dco->dco_del_peer_reason)
     {
         case OVPN_DEL_PEER_REASON_EXPIRED:
             reason = "ovpn-dco: ping expired";
+            signal = SIGUSR1;
             break;
 
         case OVPN_DEL_PEER_REASON_TRANSPORT_ERROR:
@@ -3270,7 +3272,7 @@  process_incoming_del_peer(struct multi_context *m, struct multi_instance *mi,
     mi->context.sig->signal_text = reason;
     mi->context.c2.dco_read_bytes = dco->dco_read_bytes;
     mi->context.c2.dco_write_bytes = dco->dco_write_bytes;
-    multi_signal_instance(m, mi, SIGTERM);
+    multi_signal_instance(m, mi, signal);
 }
 
 bool