Attention is currently required from: cron2, plaisthos.

ordex has posted comments on this change by cron2. ( 
http://gerrit.openvpn.net/c/openvpn/+/1830?usp=email )

Change subject: SIGHUP-restart instance on UDP write error EADDRNOTAVAIL
......................................................................


Patch Set 2: Code-Review-2

(5 comments)

Patchset:

PS2:
!!!


File src/openvpn/forward.c:

http://gerrit.openvpn.net/c/openvpn/+/1830/comment/c67a9662_957cf4b1?usp=email :
PS2, Line 1865:         if (size < 0 && errno == EADDRNOTAVAIL)
Here you should be checking for `error_code`, not `errno`. check_status() 
mangles errno on Linux/Android, thus making this check wrong/no-op on those 
platforms.
I presume on macOS it works by accident.

What about Windows? 😄 do we need the same dance as above with 
`WSAEADDRNOTAVAIL`?


http://gerrit.openvpn.net/c/openvpn/+/1830/comment/a68ccbf3_e4b9715b?usp=email :
PS2, Line 1868:             register_signal(c->sig, SIGHUP, "link-changed");
There is no guard preventing this check to fire on a server....If that happens 
we are restarting the whole top instance 😊 I presume this can also happen on a 
per server-client-instance. In that case we'd drop the client, but it may be 
what we wanted.

To fix the server problem, maybe you could add a simple `&& c->options.mode == 
MODE_POINT_TO_POINT` to the check?


http://gerrit.openvpn.net/c/openvpn/+/1830/comment/fca69be4_0e3abe9f?usp=email :
PS2, Line 1868:             register_signal(c->sig, SIGHUP, "link-changed");
have you checked how this SIGHUP interacts with `--connect-retry-max` ? SIGHUP 
makes openvpn re-read the config and re-init its state, which translates in 
ignoring the retry-max configured option. It's a rare hit, but still this patch 
bypasses that logic.

If by any chance this error becomes persistent, the client would never exit 
instead of bailing out out the maximum retries.


http://gerrit.openvpn.net/c/openvpn/+/1830/comment/2d503b5e_02d2c130?usp=email :
PS2, Line 1868:             register_signal(c->sig, SIGHUP, "link-changed");
using SIGHUP this way makes `--config stdin` + `--remap-usr1 SIGHUP` a fatal 
usage error: SIGHUP re-reads the config, but stdin is at EOF.
This patch registers SIGHUP without checking both options...You can check by 
issuing SIGHUP on a stdin-config daemon and it will die with `Options error: 
blah blah blah`.



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1830?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: comment
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I96b5ea3542df77b1be17079874fb9e4970b539ed
Gerrit-Change-Number: 1830
Gerrit-PatchSet: 2
Gerrit-Owner: cron2 <[email protected]>
Gerrit-Reviewer: ordex <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
Gerrit-Attention: cron2 <[email protected]>
Gerrit-Comment-Date: Mon, 03 Aug 2026 10:00:52 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to