From 3dd963b35fccf5eb12aee01a61360e813f82477f Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Wed, 18 Oct 2023 11:39:43 +0200 Subject: [PATCH] BUG/MINOR: mux-h2: fix http-request and http-keep-alive timeouts again Stefan Behte reported that since commit f279a2f14 ("BUG/MINOR: mux-h2: refresh the idle_timer when the mux is empty"), the http-request and http-keep-alive timeouts don't work anymore on H2. Before this patch, and since 3e448b9b64 ("BUG/MEDIUM: mux-h2: make sure control frames do not refresh the idle timeout"), they would only be refreshed after stream frames were sent (HEADERS or DATA) but the patch above that adds more refresh points broke these so they don't expire anymore as long as there's some activity. We cannot just revert the fix since it also addressed an isse by which sometimes the timeout would trigger too early and provoque truncated responses. The right approach here is in fact to only use refresh the idle timer when the mux buffer was flushed from any such stream frames. In order to achieve this, we're now setting a flag on the connection whenever we write a stream frame, and we consider that flag when deciding to refresh the buffer after it's emptied. This way we'll only clear that flag once the buffer is empty and there were stream data in it, not if there were no such stream data. In theory it remains possible to leave the flag on if some control data is appended after the buffer and it's never cleared, but in practice it's not a problem as a buffer will always get sent in large blocks when the window opens. Even a large buffer should be emptied once in a while as control frames will not fill it as much as data frames could. Given the patch above was backported as far as 2.6, this patch should also be backported as far as 2.6. --- include/haproxy/mux_h2-t.h | 1 + src/mux_h2.c | 9 ++++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/include/haproxy/mux_h2-t.h b/include/haproxy/mux_h2-t.h index 523cdf0fc..2d5b1eeb7 100644 --- a/include/haproxy/mux_h2-t.h +++ b/include/haproxy/mux_h2-t.h @@ -54,6 +54,7 @@ #define H2_CF_DEM_IN_PROGRESS 0x00000400 // demux in progress (dsi,dfl,dft are valid) /* other flags */ +#define H2_CF_MBUF_HAS_DATA 0x00000800 // some stream data (data, headers) still in mbuf #define H2_CF_GOAWAY_SENT 0x00001000 // a GOAWAY frame was successfully sent #define H2_CF_GOAWAY_FAILED 0x00002000 // a GOAWAY frame failed to be sent #define H2_CF_WAIT_FOR_HS 0x00004000 // We did check that at least a stream was waiting for handshake diff --git a/src/mux_h2.c b/src/mux_h2.c index 9acca4550..2fb831164 100644 --- a/src/mux_h2.c +++ b/src/mux_h2.c @@ -4084,8 +4084,10 @@ static int h2_send(struct h2c *h2c) /* We're done, no more to send */ if (!(conn->flags & CO_FL_WAIT_XPRT) && !br_data(h2c->mbuf)) { TRACE_DEVEL("leaving with everything sent", H2_EV_H2C_SEND, h2c->conn); - if (!h2c->nb_sc) + if (h2c->flags & H2_CF_MBUF_HAS_DATA && !h2c->nb_sc) { + h2c->flags &= ~H2_CF_MBUF_HAS_DATA; h2c->idle_start = now_ms; + } goto end; } @@ -5537,6 +5539,7 @@ static size_t h2s_snd_fhdrs(struct h2s *h2s, struct htx *htx) /* commit the H2 response */ b_add(mbuf, outbuf.data); + h2c->flags |= H2_CF_MBUF_HAS_DATA; /* indicates the HEADERS frame was sent, except for 1xx responses. For * 1xx responses, another HEADERS frame is expected. @@ -5957,6 +5960,7 @@ static size_t h2s_snd_bhdrs(struct h2s *h2s, struct htx *htx) /* commit the H2 response */ b_add(mbuf, outbuf.data); + h2c->flags |= H2_CF_MBUF_HAS_DATA; h2s->flags |= H2_SF_HEADERS_SENT; h2s->st = H2_SS_OPEN; @@ -6134,6 +6138,7 @@ static size_t h2s_make_data(struct h2s *h2s, struct buffer *buf, size_t count) buf->data = buf->head = 0; total += fsize; fsize = 0; + h2c->flags |= H2_CF_MBUF_HAS_DATA; TRACE_PROTO("sent H2 DATA frame (zero-copy)", H2_EV_TX_FRAME|H2_EV_TX_DATA, h2c->conn, h2s); goto out; @@ -6258,6 +6263,7 @@ static size_t h2s_make_data(struct h2s *h2s, struct buffer *buf, size_t count) /* commit the H2 response */ b_add(mbuf, fsize + 9); + h2c->flags |= H2_CF_MBUF_HAS_DATA; out: if (es_now) { @@ -6487,6 +6493,7 @@ static size_t h2s_make_trailers(struct h2s *h2s, struct htx *htx) /* commit the H2 response */ TRACE_PROTO("sent H2 trailers HEADERS frame", H2_EV_TX_FRAME|H2_EV_TX_HDR|H2_EV_TX_EOI, h2c->conn, h2s); b_add(mbuf, outbuf.data); + h2c->flags |= H2_CF_MBUF_HAS_DATA; h2s->flags |= H2_SF_ES_SENT; if (h2s->st == H2_SS_OPEN)