<html><head><meta http-equiv="content-type" content="text/html; charset=utf-8"></head><body style="overflow-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;">Hi,<br id="lineBreakAtBeginningOfMessage"><div><br><blockquote type="cite"><div>On 10 Apr 2024, at 10:57 AM, Vladimir Homutov <vl@inspert.ru> wrote:</div><br class="Apple-interchange-newline"><div><meta charset="UTF-8"><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;">On Tue, Apr 09, 2024 at 03:02:21PM +0400, Roman Arutyunyan wrote:</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;"><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">Hello Vladimir,<br><br>On Mon, Apr 08, 2024 at 03:03:27PM +0300, Vladimir Homutov via nginx-devel wrote:<br><blockquote type="cite">On Fri, Sep 22, 2023 at 03:36:25PM +0000, Roman Arutyunyan wrote:<br><blockquote type="cite">details: https://hg.nginx.org/nginx/rev/ad3d34ddfdcc<br>branches:<br>changeset: 9158:ad3d34ddfdcc<br>user: Roman Arutyunyan <arut@nginx.com><br>date: Wed Sep 13 17:59:37 2023 +0400<br>description:<br>QUIC: "handshake_timeout" configuration parameter.<br><br>Previously QUIC did not have such parameter and handshake duration was<br>controlled by HTTP/3. However that required creating and storing HTTP/3<br>session on first client datagram. Apparently there's no convenient way to<br>store the session object until QUIC handshake is complete. In the followup<br>patches session creation will be postponed to init() callback.<br><br></blockquote><br>[...]<br><br><blockquote type="cite">diff -r daf8f5ba23d8 -r ad3d34ddfdcc src/event/quic/ngx_event_quic.c<br>--- a/src/event/quic/ngx_event_quic.c<span class="Apple-tab-span" style="white-space: pre;"> </span>Fri Sep 01 20:31:46 2023 +0400<br>+++ b/src/event/quic/ngx_event_quic.c<span class="Apple-tab-span" style="white-space: pre;"> </span>Wed Sep 13 17:59:37 2023 +0400<br>@@ -211,6 +211,8 @@ ngx_quic_run(ngx_connection_t *c, ngx_qu<br> qc = ngx_quic_get_connection(c);<br><br> ngx_add_timer(c->read, qc->tp.max_idle_timeout);<br>+ ngx_add_timer(&qc->close, qc->conf->handshake_timeout);<br>+<br></blockquote><br>It looks like I've hit an issue with early data in such case.<br>See the attached patch with details.<br></blockquote><br>Indeed, there's an issue there.<br><br><blockquote type="cite">While there, I suggest a little debug improvement to better track<br>stream and their parent connections.<br><br><br></blockquote><br><blockquote type="cite"># HG changeset patch<br># User Vladimir Khomutov <vl@wbsrv.ru><br># Date 1712576340 -10800<br># Mon Apr 08 14:39:00 2024 +0300<br># Node ID 6e79f4ec40ed1c1ffec6a46b453051c01e556610<br># Parent 99e7050ac886f7c70a4048691e46846b930b1e28<br>QUIC: fixed close timer processing with early data.<br><br>The ngx_quic_run() function uses qc->close timer to limit the handshake<br>duration. Normally it is removed by ngx_quic_do_init_streams() which is<br>called once when we are done with initial SSL processing.<br><br>The problem happens when the client sends early data and streams are<br>initialized in the ngx_quic_run() -> ngx_quic_handle_datagram() call.<br>The order of set/remove timer calls is now reversed; the close timer is<br>set up and the timer fires when assigned, starting the unexpected connection<br>close process.<br><br>The patch moves timer cancelling right before the place where the stream<br>initialization flag is tested, thus making it work with early data.<br><br>The issue was introduced in ad3d34ddfdcc.<br><br>diff --git a/src/event/quic/ngx_event_quic_streams.c b/src/event/quic/ngx_event_quic_streams.c<br>--- a/src/event/quic/ngx_event_quic_streams.c<br>+++ b/src/event/quic/ngx_event_quic_streams.c<br>@@ -575,6 +575,10 @@ ngx_quic_init_streams(ngx_connection_t *<br><br> qc = ngx_quic_get_connection(c);<br><br>+ if (!qc->closing && qc->close.timer_set) {<br>+ ngx_del_timer(&qc->close);<br>+ }<br>+<br> if (qc->streams.initialized) {<br> return NGX_OK;<br> }<br>@@ -630,10 +634,6 @@ ngx_quic_do_init_streams(ngx_connection_<br><br> qc->streams.initialized = 1;<br><br>- if (!qc->closing && qc->close.timer_set) {<br>- ngx_del_timer(&qc->close);<br>- }<br>-<br> return NGX_OK;<br>}<br></blockquote><br>This assumes that ngx_quic_init_streams() is always called on handshake end,<br>even if not needed. This is true now, but it's not something we can to rely on.<br><br>Also, we probably don't need to limit handshake duration after streams are<br>initialized. Application level will set the required keepalive timeout for<br>this. Also, we need to include OCSP validation time in handshake timeout,<br>which your removed.<br><br>I assume a simpler solution would be not to set the timer in ngx_quic_run()<br>if streams are already initialized.<br></blockquote><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;"><span style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; float: none; display: inline !important;">Agreed, see the updated patch:</span><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;"><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;"><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;"><span id="cid:09D36878-7A30-497E-B420-2779A2F5AB00"><early_close_timer2.diff></span></div></blockquote><br></div><div>Thanks, committed!</div><br><div>
<div dir="auto" style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0); letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;"><div>----</div><div>Roman Arutyunyan</div><div>arut@nginx.com</div><div><br></div></div><br class="Apple-interchange-newline"><br class="Apple-interchange-newline">
</div>
<br></body></html>