[master] e1e2c72 Overhaul H2 Settings handling
Poul-Henning Kamp
phk at FreeBSD.org
Mon Mar 6 16:07:06 CET 2017
commit e1e2c723885d06ad98509a22fccb7d1fe1e2eb5f
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date: Mon Mar 6 15:01:18 2017 +0000
Overhaul H2 Settings handling
I guess if you can't say anything else good about it, H2 is forcing
me to invent new ways to implement table-driven object-oriented programming.
diff --git a/bin/varnishd/http2/cache_http2.h b/bin/varnishd/http2/cache_http2.h
index 041bced..d1de3a8 100644
--- a/bin/varnishd/http2/cache_http2.h
+++ b/bin/varnishd/http2/cache_http2.h
@@ -34,6 +34,8 @@ struct h2_frame_s;
#include "hpack/vhp.h"
+/**********************************************************************/
+
struct h2_error_s {
const char *name;
const char *txt;
@@ -44,6 +46,18 @@ struct h2_error_s {
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)
+#include "tbl/h2_error.h"
+#undef H2EC1
+#undef H2EC2
+#undef H2EC3
+
+/**********************************************************************/
+
typedef h2_error h2_rxframe_f(struct worker *, struct h2_sess *,
struct h2_req *);
@@ -62,19 +76,32 @@ struct h2_frame_s {
uint8_t final_flags;
};
-
#define H2_FRAME(l,U,...) extern const struct h2_frame_s H2_F_##U[1];
#include "tbl/h2_frames.h"
-#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)
-#include "tbl/h2_error.h"
-#undef H2EC1
-#undef H2EC2
-#undef H2EC3
+/**********************************************************************/
+
+struct h2_settings {
+#define H2_SETTING(U,l,...) uint32_t l;
+#include "tbl/h2_settings.h"
+};
+
+typedef void h2_setsetting_f(struct h2_settings*, uint32_t);
+
+struct h2_setting_s {
+ const char *name;
+ h2_setsetting_f *setfunc;
+ uint16_t ident;
+ uint32_t defval;
+ uint32_t minval;
+ uint32_t maxval;
+ h2_error range_error;
+};
+
+#define H2_SETTING(U,...) extern const struct h2_setting_s H2_SET_##U[1];
+#include "tbl/h2_settings.h"
+
+/**********************************************************************/
enum h2_stream_e {
H2_STREAM__DUMMY = -1,
@@ -85,14 +112,6 @@ enum h2_stream_e {
#define H2_FRAME_FLAGS(l,u,v) extern const uint8_t H2FF_##u;
#include "tbl/h2_frames.h"
-enum h2setting {
- H2_SETTINGS__DUMMY = -1,
-#define H2_SETTINGS(n,v,d) H2S_##n = v,
-#include "tbl/h2_settings.h"
-#undef H2_SETTINGS
- H2_SETTINGS_N
-};
-
struct h2_req {
unsigned magic;
#define H2_REQ_MAGIC 0x03411584
@@ -133,8 +152,8 @@ struct h2_sess {
unsigned rxf_stream;
uint8_t *rxf_data;
- uint32_t their_settings[H2_SETTINGS_N];
- uint32_t our_settings[H2_SETTINGS_N];
+ struct h2_settings remote_settings;
+ struct h2_settings local_settings;
struct req *new_req;
int go_away;
@@ -184,7 +203,7 @@ struct h2_req * h2_new_req(const struct worker *, struct h2_sess *,
unsigned stream, struct req *);
void h2_del_req(struct worker *, struct h2_req *);
int h2_rxframe(struct worker *, struct h2_sess *);
-void h2_setting(struct h2_sess *, const uint8_t *);
+h2_error h2_set_setting(struct h2_sess *, const uint8_t *);
void h2_req_body(struct req*);
diff --git a/bin/varnishd/http2/cache_http2_proto.c b/bin/varnishd/http2/cache_http2_proto.c
index 2f271ff..9dbef5b 100644
--- a/bin/varnishd/http2/cache_http2_proto.c
+++ b/bin/varnishd/http2/cache_http2_proto.c
@@ -69,19 +69,6 @@ h2_framename(enum h2frame h2f)
}
}
-static const char *
-h2_settingname(enum h2setting h2f)
-{
-
- switch(h2f) {
-#define H2_SETTINGS(n,v,d) case H2S_##n: return #n;
-#include "tbl/h2_settings.h"
-#undef H2_SETTINGS
- default:
- return (NULL);
- }
-}
-
#define H2_FRAME_FLAGS(l,u,v) const uint8_t H2FF_##u = v;
#include "tbl/h2_frames.h"
@@ -294,49 +281,85 @@ h2_rx_priority(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2)
* Incoming SETTINGS, possibly an ACK of one we sent.
*/
-void
-h2_setting(struct h2_sess *h2, const uint8_t *d)
+#define H2_SETTING(U,l, ...) \
+static void __match_proto__(h2_setsetting_f) \
+h2_setting_##l(struct h2_settings* s, uint32_t v) \
+{ \
+ s -> l = v; \
+}
+#include <tbl/h2_settings.h>
+
+#define H2_SETTING(U, l, ...) \
+const struct h2_setting_s H2_SET_##U[1] = {{ \
+ #l, \
+ h2_setting_##l, \
+ __VA_ARGS__ \
+}};
+#include <tbl/h2_settings.h>
+
+static const struct h2_setting_s * const h2_setting_tbl[] = {
+#define H2_SETTING(U,l,v, ...) [v] = H2_SET_##U,
+#include <tbl/h2_settings.h>
+};
+
+#define H2_SETTING_TBL_LEN (sizeof(h2_setting_tbl)/sizeof(h2_setting_tbl[0]))
+
+h2_error
+h2_set_setting(struct h2_sess *h2, const uint8_t *d)
{
+ const struct h2_setting_s *s;
uint16_t x;
uint32_t y;
- const char *n;
- char nb[8];
x = vbe16dec(d);
y = vbe32dec(d + 2);
- n = h2_settingname((enum h2setting)x);
- if (n == NULL) {
- bprintf(nb, "0x%04x", x);
- n = nb;
+ if (x >= H2_SETTING_TBL_LEN || h2_setting_tbl[x] == NULL) {
+ // rfc7540,l,2181,2182
+ VSLb(h2->vsl, SLT_Debug,
+ "H2SETTING unknown setting 0x%04x=%08x (ignored)", x, y);
+ return (0);
+ }
+ s = h2_setting_tbl[x];
+ AN(s);
+ if (y < s->minval || y > s->maxval) {
+ VSLb(h2->vsl, SLT_Debug, "H2SETTING invalid %s=0x%08x",
+ s->name, y);
+ AN(s->range_error);
+ return (s->range_error);
}
- VSLb(h2->vsl, SLT_Debug, "H2SETTING %s 0x%08x", n, y);
- if (x > 0 && x < H2_SETTINGS_N)
- h2->their_settings[x] = y;
+ VSLb(h2->vsl, SLT_Debug, "H2SETTING %s=0x%08x", s->name, y);
+ AN(s->setfunc);
+ s->setfunc(&h2->remote_settings, y);
+ return (0);
}
static h2_error __match_proto__(h2_frame_f)
h2_rx_settings(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2)
{
- const uint8_t *p = h2->rxf_data;
- unsigned l = h2->rxf_len;
+ const uint8_t *p;
+ unsigned l;
+ h2_error retval = 0;
- (void)wrk;
- (void)r2;
+ AN(wrk);
+ AN(r2);
AZ(h2->rxf_stream);
if (h2->rxf_flags == H2FF_SETTINGS_ACK) {
- XXXAZ(h2->rxf_len);
- } else if (h2->rxf_flags == 0) {
- for (;l >= 6; l -= 6, p += 6)
- h2_setting(h2, p);
- if (l > 0)
- VSLb(h2->vsl, SLT_Debug,
- "NB: SETTINGS had %u dribble-bytes", l);
+ if (h2->rxf_len > 0) // rfc7540,l,2047,2049
+ return (H2CE_FRAME_SIZE_ERROR);
+ return (0);
+ } else {
+ if (h2->rxf_len % 6) // rfc7540,l,2062,2064
+ return (H2CE_PROTOCOL_ERROR);
+ p = h2->rxf_data;
+ for (l = h2->rxf_len; l >= 6; l -= 6, p += 6) {
+ retval = h2_set_setting(h2, p);
+ if (retval)
+ return (retval);
+ }
Lck_Lock(&h2->sess->mtx);
H2_Send_Frame(wrk, h2,
H2_F_SETTINGS, H2FF_SETTINGS_ACK, 0, 0, NULL);
Lck_Unlock(&h2->sess->mtx);
- } else {
- WRONG("SETTINGS FRAME");
}
return (0);
}
diff --git a/bin/varnishd/http2/cache_http2_send.c b/bin/varnishd/http2/cache_http2_send.c
index 75fcc43..197d9f4 100644
--- a/bin/varnishd/http2/cache_http2_send.c
+++ b/bin/varnishd/http2/cache_http2_send.c
@@ -119,7 +119,7 @@ H2_Send(struct worker *wrk, struct h2_req *r2, int flush,
AZ(ftyp->act_snonzero);
Lck_Lock(&h2->sess->mtx);
- mfs = h2->their_settings[H2S_MAX_FRAME_SIZE];
+ mfs = h2->remote_settings.max_frame_size;
if (len < mfs) {
retval = H2_Send_Frame(wrk, h2,
ftyp, flags, len, r2->stream, ptr);
diff --git a/bin/varnishd/http2/cache_http2_session.c b/bin/varnishd/http2/cache_http2_session.c
index 4857500..2681b67 100644
--- a/bin/varnishd/http2/cache_http2_session.c
+++ b/bin/varnishd/http2/cache_http2_session.c
@@ -58,6 +58,12 @@ static const uint8_t H2_settings[] = {
0x00, 0x00, 0xff, 0xff
};
+static const struct h2_settings H2_proto_settings = {
+#define H2_SETTING(U,l,v,d,...) . l = d,
+#include "tbl/h2_settings.h"
+};
+
+
/**********************************************************************
* The h2_sess struct needs many of the same things as a request,
* WS, VSL, HTC &c, but rather than implement all that stuff over, we
@@ -93,18 +99,12 @@ h2_new_sess(const struct worker *wrk, struct sess *sp, struct req *srq)
h2->htc->rfd = &sp->fd;
h2->sess = sp;
VTAILQ_INIT(&h2->streams);
-#define H2_SETTINGS(n,v,d) \
- do { \
- assert(v < H2_SETTINGS_N); \
- h2->their_settings[v] = d; \
- h2->our_settings[v] = d; \
- } while (0);
-#include "tbl/h2_settings.h"
-#undef H2_SETTINGS
+ h2->local_settings = H2_proto_settings;
+ h2->remote_settings = H2_proto_settings;
/* XXX: Lacks a VHT_Fini counterpart. Will leak memory. */
AZ(VHT_Init(h2->dectbl,
- h2->our_settings[H2S_HEADER_TABLE_SIZE]));
+ h2->local_settings.header_table_size));
SES_Reserve_xport_priv(sp, &up);
*up = (uintptr_t)h2;
@@ -199,7 +199,7 @@ h2_b64url_settings(struct h2_sess *h2, struct req *req)
n -= 8;
if (up == u + sizeof u) {
AZ(n);
- h2_setting(h2, (void*)u);
+ h2_set_setting(h2, (void*)u);
up = u;
}
}
More information about the varnish-commit
mailing list