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.

diff --git a/include/haproxy/mux_h2-t.h b/include/haproxy/mux_h2-t.h
index 523cdf0fc1794691cbed7c240ec329c0364c4435..2d5b1eeb723b7292634edff4222a020582bd45f1 100644 (file)
--- 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 9acca4550939b3ce982a83f3c12cd68843c11cc7..2fb8311642852c3194f9bcc8d7a6662951ea73a5 100644 (file)
--- 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)