[Openvpn-devel] [S] Change in openvpn[master]: SIGHUP-restart instance on UDP write error EADDRNOTAVAIL
"ordex \(Code Review\) via Openvpn-devel" <[email protected]> Mon, 3 Aug 2026 10:00:52 +0000
| Newsgroups | net.sourceforge.lists.openvpn-devel |
|---|---|
| Message-ID | <75dcf8ff794a08aac1d81038bfa48b2d307e358e-EmailReviewComments-HTML@gerrit.openvpn.net> |
--===============6029125125371713725==
Content-Transfer-Encoding: 8bit
Content-Disposition: inline
Content-Type: multipart/alternative; boundary="Urqlc+pU4CE="; charset=UTF-8
--Urqlc+pU4CE=
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Attention is currently required from: cron2, plaisthos=2E
ordex has posted=
comments on this change by cron2=2E ( http://gerrit=2Eopenvpn=2Enet/c/open=
vpn/+/1830?usp=3Demail )
Change subject: SIGHUP-restart instance on UDP wr=
ite error EADDRNOTAVAIL
=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=
=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=
=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=
=2E=2E=2E
Patch Set 2: Code-Review-2
(5 comments)
Patchset:
PS2:
!!!
=
File src/openvpn/forward=2Ec:
http://gerrit=2Eopenvpn=2Enet/c/openvpn/+/=
1830/comment/c67a9662_957cf4b1?usp=3Demail :
PS2, Line 1865: if (si=
ze < 0 && errno =3D=3D EADDRNOTAVAIL)
Here you should be checking for `erro=
r_code`, not `errno`=2E check_status() mangles errno on Linux/Android, thus=
making this check wrong/no-op on those platforms=2E
I presume on macOS it =
works by accident=2E
What about Windows? =F0=9F=98=84 do we need the same =
dance as above with `WSAEADDRNOTAVAIL`?
http://gerrit=2Eopenvpn=2Enet/c/o=
penvpn/+/1830/comment/a68ccbf3_e4b9715b?usp=3Demail :
PS2, Line 1868: =
register_signal(c->sig, SIGHUP, "link-changed");
There is no guard p=
reventing this check to fire on a server=2E=2E=2E=2EIf that happens we are =
restarting the whole top instance =F0=9F=98=8A I presume this can also happ=
en on a per server-client-instance=2E In that case we'd drop the client, bu=
t it may be what we wanted=2E
To fix the server problem, maybe you could a=
dd a simple `&& c->options=2Emode =3D=3D MODE_POINT_TO_POINT` to the check?=
http://gerrit=2Eopenvpn=2Enet/c/openvpn/+/1830/comment/fca69be4_0e3abe9f=
?usp=3Demail :
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=2E It's a ra=
re hit, but still this patch bypasses that logic=2E
If by any chance this =
error becomes persistent, the client would never exit instead of bailing ou=
t out the maximum retries=2E
http://gerrit=2Eopenvpn=2Enet/c/openvpn/+/18=
30/comment/2d503b5e_02d2c130?usp=3Demail :
PS2, Line 1868: regi=
ster_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=2E
This patch registers SIGHUP without che=
cking both options=2E=2E=2EYou can check by issuing SIGHUP on a stdin-confi=
g daemon and it will die with `Options error: blah blah blah`=2E
--
To =
view, visit http://gerrit=2Eopenvpn=2Enet/c/openvpn/+/1830?usp=3Demail
To u=
nsubscribe, or for help writing mail filters, visit http://gerrit=2Eopenvpn=
=2Enet/settings?usp=3Demail
Gerrit-MessageType: comment
Gerrit-Project: op=
envpn
Gerrit-Branch: master
Gerrit-Change-Id: I96b5ea3542df77b1be17079874fb=
9e4970b539ed
Gerrit-Change-Number: 1830
Gerrit-PatchSet: 2
Gerrit-Owner: cr=
on2 <gert@greenie=2Emuc=2Ede>
Gerrit-Reviewer: ordex <antonio@mandelbit=2Ec=
om>
Gerrit-Reviewer: plaisthos <arne-openvpn@rfc2549=2Eorg>
Gerrit-CC: open=
vpn-devel <openvpn-devel@lists=2Esourceforge=2Enet>
Gerrit-Attention: plais=
thos <arne-openvpn@rfc2549=2Eorg>
Gerrit-Attention: cron2 <gert@greenie=2Em=
uc=2Ede>
Gerrit-Comment-Date: Mon, 03 Aug 2026 10:00:52 +0000
Gerrit-HasCom=
ments: Yes
Gerrit-Has-Labels: Yes
--Urqlc+pU4CE=
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
<!DOCTYPE html><html><head><style></style></head><body><p> Attention is cur=
rently required from: cron2, plaisthos=2E </p>
<p>Patch set 2:<span style=
=3D"border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;=
background-color: #ffd4d4;">Code-Review -2</span></p><p><a href=3D"http://g=
errit=2Eopenvpn=2Enet/c/openvpn/+/1830?usp=3Demail">View Change</a></p><p>5=
comments:</p><ul style=3D"list-style: none; padding: 0;"><li style=3D"marg=
in: 0; padding: 0;"><p>Patchset:</p><ul style=3D"list-style: none; padding:=
0;"><li style=3D"margin: 0; padding: 0 0 0 16px;"><p style=3D"margin-botto=
m: 4px;"><a href=3D"http://gerrit=2Eopenvpn=2Enet/c/openvpn/+/1830?tab=3Dco=
mments&usp=3Demail">Patch Set #2:</a> </p><p class=3D"blocks" style=3D"=
white-space: pre-wrap; word-wrap: break-word;">!!!</p></li></ul></li><li st=
yle=3D"margin: 0; padding: 0;"><p>File src/openvpn/forward=2Ec:</p><ul styl=
e=3D"list-style: none; padding: 0;"><li style=3D"margin: 0; padding: 0 0 0 =
16px;"><p style=3D"margin-bottom: 4px;"><a href=3D"http://gerrit=2Eopenvpn=
=2Enet/c/openvpn/+/1830/comment/c67a9662_957cf4b1?usp=3Demail">Patch Set #2=
, Line 1865:</a> <code style=3D"font-family:monospace,monospace"> if=
(size < 0 && errno =3D=3D EADDRNOTAVAIL)</code></p><p class=3D"=
blocks" style=3D"white-space: pre-wrap; word-wrap: break-word;">Here you sh=
ould be checking for `error_code`, not `errno`=2E check_status() mangles er=
rno on Linux/Android, thus making this check wrong/no-op on those platforms=
=2E<br>I presume on macOS it works by accident=2E</p><p class=3D"blocks" st=
yle=3D"white-space: pre-wrap; word-wrap: break-word;">What about Windows? =
=F0=9F=98=84 do we need the same dance as above with `WSAEADDRNOTAVAIL`?</p=
></li><li style=3D"margin: 0; padding: 0 0 0 16px;"><p style=3D"margin-bott=
om: 4px;"><a href=3D"http://gerrit=2Eopenvpn=2Enet/c/openvpn/+/1830/comment=
/a68ccbf3_e4b9715b?usp=3Demail">Patch Set #2, Line 1868:</a> <code style=3D=
"font-family:monospace,monospace"> register_signal(c->sig, SI=
GHUP, "link-changed");</code></p><p class=3D"blocks" style=3D"whi=
te-space: pre-wrap; word-wrap: break-word;">There is no guard preventing th=
is check to fire on a server=2E=2E=2E=2EIf that happens we are restarting t=
he whole top instance =F0=9F=98=8A I presume this can also happen on a per =
server-client-instance=2E In that case we'd drop the client, but it may=
be what we wanted=2E</p><p class=3D"blocks" style=3D"white-space: pre-wrap=
; word-wrap: break-word;">To fix the server problem, maybe you could add a =
simple `&& c->options=2Emode =3D=3D MODE_POINT_TO_POINT` to the =
check?</p></li><li style=3D"margin: 0; padding: 0 0 0 16px;"><p style=3D"ma=
rgin-bottom: 4px;"><a href=3D"http://gerrit=2Eopenvpn=2Enet/c/openvpn/+/183=
0/comment/fca69be4_0e3abe9f?usp=3Demail">Patch Set #2, Line 1868:</a> <code=
style=3D"font-family:monospace,monospace"> register_signal(c-&g=
t;sig, SIGHUP, "link-changed");</code></p><p class=3D"blocks" sty=
le=3D"white-space: pre-wrap; word-wrap: break-word;">have you checked how t=
his SIGHUP interacts with `--connect-retry-max` ? SIGHUP makes openvpn re-r=
ead the config and re-init its state, which translates in ignoring the retr=
y-max configured option=2E It's a rare hit, but still this patch bypass=
es that logic=2E</p><p class=3D"blocks" style=3D"white-space: pre-wrap; wor=
d-wrap: break-word;">If by any chance this error becomes persistent, the cl=
ient would never exit instead of bailing out out the maximum retries=2E</p>=
</li><li style=3D"margin: 0; padding: 0 0 0 16px;"><p style=3D"margin-botto=
m: 4px;"><a href=3D"http://gerrit=2Eopenvpn=2Enet/c/openvpn/+/1830/comment/=
2d503b5e_02d2c130?usp=3Demail">Patch Set #2, Line 1868:</a> <code style=3D"=
font-family:monospace,monospace"> register_signal(c->sig, SIG=
HUP, "link-changed");</code></p><p class=3D"blocks" style=3D"whit=
e-space: pre-wrap; word-wrap: break-word;">using SIGHUP this way makes `--c=
onfig stdin` + `--remap-usr1 SIGHUP` a fatal usage error: SIGHUP re-reads t=
he config, but stdin is at EOF=2E<br>This patch registers SIGHUP without ch=
ecking both options=2E=2E=2EYou can check by issuing SIGHUP on a stdin-conf=
ig daemon and it will die with `Options error: blah blah blah`=2E</p></li><=
/ul></li></ul><p>To view, visit <a href=3D"http://gerrit=2Eopenvpn=2Enet/c/=
openvpn/+/1830?usp=3Demail">change 1830</a>=2E To unsubscribe, or for help =
writing mail filters, visit <a href=3D"http://gerrit=2Eopenvpn=2Enet/settin=
gs?usp=3Demail">settings</a>=2E</p><div itemscope itemtype=3D"http://schema=
=2Eorg/EmailMessage"><div itemscope itemprop=3D"action" itemtype=3D"http://=
schema=2Eorg/ViewAction"><link itemprop=3D"url" href=3D"http://gerrit=2Eope=
nvpn=2Enet/c/openvpn/+/1830?usp=3Demail"/><meta itemprop=3D"name" content=
=3D"View Change"/></div></div>
<div style=3D"display:none"> Gerrit-Message=
Type: comment </div>
<div style=3D"display:none"> Gerrit-Project: openvpn <=
/div>
<div style=3D"display:none"> Gerrit-Branch: master </div>
<div style=
=3D"display:none"> Gerrit-Change-Id: I96b5ea3542df77b1be17079874fb9e4970b53=
9ed </div>
<div style=3D"display:none"> Gerrit-Change-Number: 1830 </div>
<=
div style=3D"display:none"> Gerrit-PatchSet: 2 </div>
<div style=3D"display=
:none"> Gerrit-Owner: cron2 <gert@greenie=2Emuc=2Ede> </div>
<div sty=
le=3D"display:none"> Gerrit-Reviewer: ordex <antonio@mandelbit=2Ecom>=
</div>
<div style=3D"display:none"> Gerrit-Reviewer: plaisthos <arne-op=
envpn@rfc2549=2Eorg> </div>
<div style=3D"display:none"> Gerrit-CC: open=
vpn-devel <openvpn-devel@lists=2Esourceforge=2Enet> </div>
<div style=
=3D"display:none"> Gerrit-Attention: plaisthos <arne-openvpn@rfc2549=2Eo=
rg> </div>
<div style=3D"display:none"> Gerrit-Attention: cron2 <gert=
@greenie=2Emuc=2Ede> </div>
<div style=3D"display:none"> Gerrit-Comment-=
Date: Mon, 03 Aug 2026 10:00:52 +0000 </div>
<div style=3D"display:none"> G=
errit-HasComments: Yes </div>
<div style=3D"display:none"> Gerrit-Has-Label=
s: Yes </div>
</body></html>
--Urqlc+pU4CE=--
--===============6029125125371713725==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
--===============6029125125371713725==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
--===============6029125125371713725==--