BUG/MINOR: mux-h2: always check the stream ID limit in h2_avail_streams()
This function is used to decide whether to put an idle connection back
into the idle pool. While it considers the limit in number of concurrent
requests, it does not consider the limit in number of streams, so if a
server announces a low limit in a GOAWAY frame, it will be ignored.
However there is a caveat : since we assign the stream IDs when sending
them, we have a number of allocated streams which max_id doesn't take
care of. This can be addressed by adding a new nb_reserved count on each
connection to keep track of the ID-less streams.
This patch makes sure we take care of the remaining number of streams
if such a limit was announced, or of the number of streams before the
highest ID. Now it is possible to accurately know how many streams
can be allocated, and the number of failed outgoing streams has dropped
in half.
This must be backported to 1.9.
diff --git a/src/mux_h2.c b/src/mux_h2.c
index be30183..57bf469 100644
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -118,6 +118,8 @@
int shut_timeout; /* idle timeout duration in ticks after GOAWAY was sent */
unsigned int nb_streams; /* number of streams in the tree */
unsigned int nb_cs; /* number of attached conn_streams */
+ unsigned int nb_reserved; /* number of reserved streams */
+ /* 32 bit hole here */
struct proxy *proxy; /* the proxy this connection was created for */
struct task *task; /* timeout management task */
struct eb_root streams_by_id; /* all active streams by their ID */
@@ -387,12 +389,36 @@
}
}
+/* returns the number of allocatable outgoing streams for the connection taking
+ * the last_sid and the reserved ones into account.
+ */
+static inline int h2_streams_left(const struct h2c *h2c)
+{
+ int ret;
+
+ /* consider the number of outgoing streams we're allowed to create before
+ * reaching the last GOAWAY frame seen. max_id is the last assigned id,
+ * nb_reserved is the number of streams which don't yet have an ID.
+ */
+ ret = (h2c->last_sid >= 0) ? h2c->last_sid : 0x7FFFFFFF;
+ ret = (unsigned int)(ret - h2c->max_id) / 2 - h2c->nb_reserved - 1;
+ if (ret < 0)
+ ret = 0;
+ return ret;
+}
+
+/* returns the number of concurrent streams available on the connection */
static int h2_avail_streams(struct connection *conn)
{
struct h2c *h2c = conn->ctx;
+ int ret1, ret2;
/* XXX Should use the negociated max concurrent stream nb instead of the conf value */
- return (h2_settings_max_concurrent_streams - h2c->nb_streams);
+ ret1 = h2_settings_max_concurrent_streams - h2c->nb_streams;
+
+ /* we must also consider the limit imposed by stream IDs */
+ ret2 = h2_streams_left(h2c);
+ return MIN(ret1, ret2);
}
static int h2_max_streams(struct connection *conn)
@@ -465,6 +491,7 @@
h2c->rcvd_s = 0;
h2c->nb_streams = 0;
h2c->nb_cs = 0;
+ h2c->nb_reserved = 0;
h2c->dbuf = BUF_NULL;
h2c->dsi = -1;
@@ -771,8 +798,11 @@
*/
static inline void h2s_close(struct h2s *h2s)
{
- if (h2s->st != H2_SS_CLOSED)
+ if (h2s->st != H2_SS_CLOSED) {
h2s->h2c->nb_streams--;
+ if (!h2s->id)
+ h2s->h2c->nb_reserved--;
+ }
h2s->st = H2_SS_CLOSED;
}
@@ -847,6 +877,8 @@
h2s->by_id.key = h2s->id = id;
if (id > 0)
h2c->max_id = id;
+ else
+ h2c->nb_reserved++;
eb32_insert(&h2c->streams_by_id, &h2s->by_id);
h2c->nb_streams++;
@@ -5156,6 +5188,7 @@
eb32_delete(&h2s->by_id);
h2s->by_id.key = h2s->id = id;
h2s->h2c->max_id = id;
+ h2s->h2c->nb_reserved--;
eb32_insert(&h2s->h2c->streams_by_id, &h2s->by_id);
}