[master] 5c1d08263 h2: Add a sess_close reason to h2 connection errors
Nils Goroll
nils.goroll at uplex.de
Wed Feb 10 13:19:06 UTC 2021
commit 5c1d08263a8d161b7318cca13c1778ac955bdb3d
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date: Mon Jan 25 12:26:47 2021 +0100
h2: Add a sess_close reason to h2 connection errors
And use that for logging purposes when a successfully opened h2 session
ends. RX_JUNK is still the default session close reason when existing
reasons aren't accurate enough.
Fixes #3393
diff --git a/bin/varnishd/http2/cache_http2.h b/bin/varnishd/http2/cache_http2.h
index 270306d15..782845209 100644
--- a/bin/varnishd/http2/cache_http2.h
+++ b/bin/varnishd/http2/cache_http2.h
@@ -44,15 +44,15 @@ struct h2_error_s {
uint32_t val;
int stream;
int connection;
+ enum sess_close reason;
};
typedef const struct h2_error_s *h2_error;
-#define H2EC0(U,v,d)
-#define H2EC1(U,v,d) extern const struct h2_error_s H2CE_##U[1];
-#define H2EC2(U,v,d) extern const struct h2_error_s H2SE_##U[1];
-#define H2EC3(U,v,d) H2EC1(U,v,d) H2EC2(U,v,d)
-#define H2_ERROR(NAME, val, sc, desc) H2EC##sc(NAME, val, desc)
+#define H2EC1(U,v,r,d) extern const struct h2_error_s H2CE_##U[1];
+#define H2EC2(U,v,r,d) extern const struct h2_error_s H2SE_##U[1];
+#define H2EC3(U,v,r,d) H2EC1(U,v,r,d) H2EC2(U,v,r,d)
+#define H2_ERROR(NAME, val, sc, reason, desc) H2EC##sc(NAME, val, reason, desc)
#include "tbl/h2_error.h"
#undef H2EC1
#undef H2EC2
diff --git a/bin/varnishd/http2/cache_http2_proto.c b/bin/varnishd/http2/cache_http2_proto.c
index 614d13781..cc894950e 100644
--- a/bin/varnishd/http2/cache_http2_proto.c
+++ b/bin/varnishd/http2/cache_http2_proto.c
@@ -45,10 +45,10 @@
#include "vtcp.h"
#include "vtim.h"
-#define H2EC1(U,v,d) const struct h2_error_s H2CE_##U[1] = {{#U,d,v,0,1}};
-#define H2EC2(U,v,d) const struct h2_error_s H2SE_##U[1] = {{#U,d,v,1,0}};
-#define H2EC3(U,v,d) H2EC1(U,v,d) H2EC2(U,v,d)
-#define H2_ERROR(NAME, val, sc, desc) H2EC##sc(NAME, val, desc)
+#define H2EC1(U,v,r,d) const struct h2_error_s H2CE_##U[1] = {{#U,d,v,0,1,r}};
+#define H2EC2(U,v,r,d) const struct h2_error_s H2SE_##U[1] = {{#U,d,v,1,0,r}};
+#define H2EC3(U,v,r,d) H2EC1(U,v,r,d) H2EC2(U,v,r,d)
+#define H2_ERROR(NAME, val, sc, reason, desc) H2EC##sc(NAME, val, reason, desc)
#include "tbl/h2_error.h"
#undef H2EC1
#undef H2EC2
@@ -59,7 +59,8 @@ static const struct h2_error_s H2NN_ERROR[1] = {{
"Unknown error number",
0xffffffff,
1,
- 1
+ 1,
+ SC_RX_JUNK
}};
enum h2frame {
@@ -86,10 +87,10 @@ h2_framename(enum h2frame h2f)
*/
static const h2_error stream_errors[] = {
-#define H2EC1(U,v,d)
-#define H2EC2(U,v,d) [v] = H2SE_##U,
-#define H2EC3(U,v,d) H2EC1(U,v,d) H2EC2(U,v,d)
-#define H2_ERROR(NAME, val, sc, desc) H2EC##sc(NAME, val, desc)
+#define H2EC1(U,v,r,d)
+#define H2EC2(U,v,r,d) [v] = H2SE_##U,
+#define H2EC3(U,v,r,d) H2EC1(U,v,r,d) H2EC2(U,v,r,d)
+#define H2_ERROR(NAME, val, sc, reason, desc) H2EC##sc(NAME, val, reason, desc)
#include "tbl/h2_error.h"
#undef H2EC1
#undef H2EC2
@@ -111,10 +112,10 @@ h2_streamerror(uint32_t u)
*/
static const h2_error conn_errors[] = {
-#define H2EC1(U,v,d) [v] = H2CE_##U,
-#define H2EC2(U,v,d)
-#define H2EC3(U,v,d) H2EC1(U,v,d) H2EC2(U,v,d)
-#define H2_ERROR(NAME, val, sc, desc) H2EC##sc(NAME, val, desc)
+#define H2EC1(U,v,r,d) [v] = H2CE_##U,
+#define H2EC2(U,v,r,d)
+#define H2EC3(U,v,r,d) H2EC1(U,v,r,d) H2EC2(U,v,r,d)
+#define H2_ERROR(NAME, val, sc, reason, desc) H2EC##sc(NAME, val, reason, desc)
#include "tbl/h2_error.h"
#undef H2EC1
#undef H2EC2
diff --git a/bin/varnishd/http2/cache_http2_session.c b/bin/varnishd/http2/cache_http2_session.c
index cc1816f54..36d4a1c0e 100644
--- a/bin/varnishd/http2/cache_http2_session.c
+++ b/bin/varnishd/http2/cache_http2_session.c
@@ -145,6 +145,7 @@ h2_del_sess(struct worker *wrk, struct h2_sess *h2, enum sess_close reason)
CHECK_OBJ_NOTNULL(h2, H2_SESS_MAGIC);
AZ(h2->refcnt);
assert(VTAILQ_EMPTY(&h2->streams));
+ AN(reason);
VHT_Fini(h2->dectbl);
AZ(pthread_cond_destroy(h2->winupd_cond));
@@ -432,8 +433,7 @@ h2_new_session(struct worker *wrk, void *arg)
h2->cond = NULL;
assert(h2->refcnt == 1);
h2_del_req(wrk, h2->req0);
- /* TODO: proper sess close reason */
- h2_del_sess(wrk, h2, SC_RX_JUNK);
+ h2_del_sess(wrk, h2, h2->error->reason);
}
struct transport H2_transport = {
diff --git a/bin/varnishtest/vtc_http2.c b/bin/varnishtest/vtc_http2.c
index f2deff037..cb1c6b5f5 100644
--- a/bin/varnishtest/vtc_http2.c
+++ b/bin/varnishtest/vtc_http2.c
@@ -52,7 +52,7 @@
#define BUF_SIZE (1024*2048)
static const char *const h2_errs[] = {
-#define H2_ERROR(n,v,sc,t) [v] = #n,
+#define H2_ERROR(n,v,sc,r,t) [v] = #n,
#include <tbl/h2_error.h>
NULL
};
@@ -1228,7 +1228,7 @@ cmd_var_resolve(const struct stream *s, const char *spec, char *buf)
else
return (NULL);
}
-#define H2_ERROR(U,v,sc,t) \
+#define H2_ERROR(U,v,sc,r,t) \
if (!strcmp(spec, #U)) { return (#v); }
#include "tbl/h2_error.h"
return (spec);
diff --git a/include/tbl/h2_error.h b/include/tbl/h2_error.h
index 4d397922c..d8b1df3ab 100644
--- a/include/tbl/h2_error.h
+++ b/include/tbl/h2_error.h
@@ -30,6 +30,7 @@
* RFC7540 section 11.4
*
* Types: conn=1|stream=2
+ * Reason: enum sess_close
*/
/*lint -save -e525 -e539 */
@@ -38,6 +39,7 @@ H2_ERROR(
/* name */ NO_ERROR,
/* val */ 0,
/* types */ 3,
+ /* reason */ SC_REM_CLOSE,
/* descr */ "Graceful shutdown"
)
@@ -45,6 +47,7 @@ H2_ERROR(
/* name */ PROTOCOL_ERROR,
/* val */ 1,
/* types */ 3,
+ /* reason */ SC_RX_JUNK,
/* descr */ "Protocol error detected"
)
@@ -52,6 +55,7 @@ H2_ERROR(
/* name */ INTERNAL_ERROR,
/* val */ 2,
/* types */ 3,
+ /* reason */ SC_VCL_FAILURE,
/* descr */ "Implementation fault"
)
@@ -59,6 +63,7 @@ H2_ERROR(
/* name */ FLOW_CONTROL_ERROR,
/* val */ 3,
/* types */ 3,
+ /* reason */ SC_OVERLOAD,
/* descr */ "Flow-control limits exceeded"
)
@@ -66,6 +71,7 @@ H2_ERROR(
/* name */ SETTINGS_TIMEOUT,
/* val */ 4,
/* types */ 1,
+ /* reason */ SC_RX_TIMEOUT,
/* descr */ "Settings not acknowledged"
)
@@ -73,6 +79,7 @@ H2_ERROR(
/* name */ STREAM_CLOSED,
/* val */ 5,
/* types */ 2,
+ /* reason */ SC_NULL,
/* descr */ "Frame received for closed stream"
)
@@ -80,6 +87,7 @@ H2_ERROR(
/* name */ FRAME_SIZE_ERROR,
/* val */ 6,
/* types */ 3,
+ /* reason */ SC_RX_JUNK,
/* descr */ "Frame size incorrect"
)
@@ -87,6 +95,7 @@ H2_ERROR(
/* name */ REFUSED_STREAM,
/* val */ 7,
/* types */ 2,
+ /* reason */ SC_NULL,
/* descr */ "Stream not processed"
)
@@ -94,6 +103,7 @@ H2_ERROR(
/* name */ CANCEL,
/* val */ 8,
/* types */ 2,
+ /* reason */ SC_NULL,
/* descr */ "Stream cancelled"
)
@@ -101,6 +111,7 @@ H2_ERROR(
/* name */ COMPRESSION_ERROR,
/* val */ 9,
/* types */ 1,
+ /* reason */ SC_RX_JUNK,
/* descr */ "Compression state not updated"
)
@@ -108,6 +119,7 @@ H2_ERROR(
/* name */ CONNECT_ERROR,
/* val */ 10,
/* types */ 2,
+ /* reason */ SC_NULL,
/* descr */ "TCP connection error for CONNECT method"
)
@@ -115,6 +127,7 @@ H2_ERROR(
/* name */ ENHANCE_YOUR_CALM,
/* val */ 11,
/* types */ 3,
+ /* reason */ SC_OVERLOAD,
/* descr */ "Processing capacity exceeded"
)
@@ -122,6 +135,7 @@ H2_ERROR(
/* name */ INADEQUATE_SECURITY,
/* val */ 12,
/* types */ 1,
+ /* reason */ SC_RX_JUNK,
/* descr */ "Negotiated TLS parameters not acceptable"
)
@@ -129,6 +143,7 @@ H2_ERROR(
/* name */ HTTP_1_1_REQUIRED,
/* val */ 13,
/* types */ 1,
+ /* reason */ SC_REQ_HTTP20,
/* descr */ "Use HTTP/1.1 for the request"
)
More information about the varnish-commit
mailing list