Skip to content

Commit

Permalink
BUG/MINOR: mux-h2: update tracked counters with req cnt/req err
Browse files Browse the repository at this point in the history
Originally H2 would transfer everything to H1 and parsing errors were
handled there, so that if there was a track-sc rule in effect, the
counters would be updated as well. As we started to add more and more
HTTP-compliance checks at the H2 layer, then switched to HTX, we
progressively lost this ability. It's a bit annoying because it means
we will not maintain accurate error counters for a given source, for
example.

This patch adds the calls to session_inc_http_req_ctr() and
session_inc_http_err_ctr() when needed (i.e. when failing to parse
an HTTP request since all other cases are handled by the stream),
just like mux-h1 does. The same should be done for mux-h3 by the
way.

This can be backported to recent stable versions. It's not exactly a
bug, rather a missing feature in that we had never updated this counter
for H2 till now, but it does make sense to do it especially based on
what the doc says about its usage.

(cherry picked from commit 380f115)
Signed-off-by: Amaury Denoyelle <[email protected]>
(cherry picked from commit d6e5cde)
Signed-off-by: Amaury Denoyelle <[email protected]>
(cherry picked from commit d16ae5d)
Signed-off-by: Amaury Denoyelle <[email protected]>
  • Loading branch information
wtarreau authored and a-denoyelle committed Oct 27, 2023
1 parent db29163 commit 07fe800
Showing 1 changed file with 9 additions and 0 deletions.
9 changes: 9 additions & 0 deletions src/mux_h2.c
Original file line number Diff line number Diff line change
Expand Up @@ -1702,6 +1702,8 @@ static struct h2s *h2c_frt_stream_new(struct h2c *h2c, int id, struct buffer *in

if (h2c->nb_streams >= h2_settings_max_concurrent_streams) {
TRACE_ERROR("HEADERS frame causing MAX_CONCURRENT_STREAMS to be exceeded", H2_EV_H2S_NEW|H2_EV_RX_FRAME|H2_EV_RX_HDR, h2c->conn);
session_inc_http_req_ctr(sess);
session_inc_http_err_ctr(sess);
goto out;
}

Expand Down Expand Up @@ -2930,6 +2932,8 @@ static struct h2s *h2c_frt_handle_headers(struct h2c *h2c, struct h2s *h2s)
TRACE_ERROR("HEADERS on invalid stream ID", H2_EV_RX_FRAME|H2_EV_RX_HDR, h2c->conn);
HA_ATOMIC_INC(&h2c->px_counters->conn_proto_err);
sess_log(h2c->conn->owner);
session_inc_http_req_ctr(h2c->conn->owner);
session_inc_http_err_ctr(h2c->conn->owner);
goto conn_err;
}
else if (h2c->flags & H2_CF_DEM_TOOMANY)
Expand All @@ -2941,6 +2945,8 @@ static struct h2s *h2c_frt_handle_headers(struct h2c *h2c, struct h2s *h2s)
if (h2c->st0 >= H2_CS_ERROR) {
TRACE_USER("Unrecoverable error decoding H2 request", H2_EV_RX_FRAME|H2_EV_RX_HDR|H2_EV_STRM_NEW|H2_EV_STRM_END, h2c->conn, 0, &rxbuf);
sess_log(h2c->conn->owner);
session_inc_http_req_ctr(h2c->conn->owner);
session_inc_http_err_ctr(h2c->conn->owner);
goto out;
}

Expand All @@ -2956,6 +2962,9 @@ static struct h2s *h2c_frt_handle_headers(struct h2c *h2c, struct h2s *h2s)
* but the HPACK decompressor is still synchronized.
*/
sess_log(h2c->conn->owner);
session_inc_http_req_ctr(h2c->conn->owner);
session_inc_http_err_ctr(h2c->conn->owner);

h2s = (struct h2s*)h2_error_stream;

/* This stream ID is now opened anyway until we send the RST on
Expand Down

0 comments on commit 07fe800

Please sign in to comment.