Skip to content

Commit

Permalink
BUG/MINOR: mux-h2: fix http-request and http-keep-alive timeouts again
Browse files Browse the repository at this point in the history
Stefan Behte reported that since commit f279a2f ("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 3e448b9 ("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.
  • Loading branch information
wtarreau committed Oct 18, 2023
1 parent 91ed529 commit 3dd963b
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 1 deletion.
1 change: 1 addition & 0 deletions include/haproxy/mux_h2-t.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 8 additions & 1 deletion src/mux_h2.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 3dd963b

Please sign in to comment.