BUG/MINOR: h2/rhttp: fix HTTP2 conn counters on reverse

Dedicated HTTP/2 stats proxy counters are available for current and
total number of HTTP/2 connection on both frontend and backend sides.
Both counters are simply incremented into h2_init().

This causes issues when using reverse HTTP. First, increment is not
performed on the expected side, as it is triggered before
h2_conn_reverse() which switches a connection from frontend to backend
or vice versa. For example on active revers side, h2_total_connections
is incremented on the backend only even after connection is reversed and
attached to a listener for the remainder of its lifetime.

h2_open_connections suffers from a similar but arguably worst behavior
as it is also decremented. If increment and decrement operations are not
performed on the same proxy side, which happens for every connection
which has been successfully reversed, it causes an invalid counter
value, possibly with an integer overflow.

To fix this, delay increment operations on reverse HTTP from h2_init()
to h2_conn_reverse(). Both counters are updated only after reverse has
completed, thus using the expected frontend or backend side.

To prevent overflow on h2_open_connections, ensure h2_release()
decrement is not performed if a connection is freed before achieving its
reversal, as in this case it would not have been accounted by H2
counters.

This should be backported up to 2.9.

This should fix github issue #2821.

(cherry picked from commit 8633446337cd322f010ce7b3ca9c257d1ce7848c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit aedc04be5a1fbf72392e68037c4e23b9aa034dc0)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

diff --git a/src/mux_h2.c b/src/mux_h2.c
index 170d34a..bea8855 100644 (file)
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -1134,8 +1134,12 @@ static int h2_init(struct connection *conn, struct proxy *prx, struct session *s
 
        if (sess)
                proxy_inc_fe_cum_sess_ver_ctr(sess->listener, prx, 2);
-       HA_ATOMIC_INC(&h2c->px_counters->open_conns);
-       HA_ATOMIC_INC(&h2c->px_counters->total_conns);
+
+       /* Rhttp connections are only accounted after reverse completion. */
+       if (!conn_is_reverse(conn)) {
+               HA_ATOMIC_INC(&h2c->px_counters->open_conns);
+               HA_ATOMIC_INC(&h2c->px_counters->total_conns);
+       }
 
        /* prepare to read something */
        h2c_restart_reading(h2c, 1);
@@ -1211,7 +1215,9 @@ static void h2_release(struct h2c *h2c)
                conn->xprt->unsubscribe(conn, conn->xprt_ctx, h2c->wait_event.events,
                                        &h2c->wait_event);
 
-       HA_ATOMIC_DEC(&h2c->px_counters->open_conns);
+       /* Rhttp connections are not accounted prior to their reverse. */
+       if (!conn || !conn_is_reverse(conn))
+               HA_ATOMIC_DEC(&h2c->px_counters->open_conns);
 
        pool_free(pool_head_h2c, h2c);
 
@@ -3510,6 +3516,9 @@ static int h2_conn_reverse(struct h2c *h2c)
                            &h2c->conn->stopping_list);
        }
 
+       HA_ATOMIC_INC(&h2c->px_counters->open_conns);
+       HA_ATOMIC_INC(&h2c->px_counters->total_conns);
+
        /* Check if stream creation is initially forbidden. This is the case
         * for active preconnect until reversal is done.
         */