[master] 1ad7006 A LOT more H2 plumbing.
Poul-Henning Kamp
phk at FreeBSD.org
Fri Mar 24 12:02:06 CET 2017
commit 1ad7006dd182e33b06fa60816adf3b4f5a761452
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date: Fri Mar 24 11:00:33 2017 +0000
A LOT more H2 plumbing.
diff --git a/bin/varnishd/http2/cache_http2.h b/bin/varnishd/http2/cache_http2.h
index bd9e778..b95ac67 100644
--- a/bin/varnishd/http2/cache_http2.h
+++ b/bin/varnishd/http2/cache_http2.h
@@ -221,6 +221,8 @@ h2_error H2_Send(struct worker *, const struct h2_req *,
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 *);
+void h2_kill_req(struct worker *, const struct h2_sess *,
+ struct h2_req *, h2_error);
int h2_rxframe(struct worker *, struct h2_sess *);
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_deliver.c b/bin/varnishd/http2/cache_http2_deliver.c
index 2d65033..be422e6 100644
--- a/bin/varnishd/http2/cache_http2_deliver.c
+++ b/bin/varnishd/http2/cache_http2_deliver.c
@@ -260,7 +260,6 @@ h2_deliver(struct req *req, struct boc *boc, int sendbody)
WS_Release(req->ws, 0);
-
if (sendbody) {
VDP_push(req, h2_bytes, NULL, 1, "H2");
err = VDP_DeliverObj(req);
diff --git a/bin/varnishd/http2/cache_http2_proto.c b/bin/varnishd/http2/cache_http2_proto.c
index b5f07a2..146754f 100644
--- a/bin/varnishd/http2/cache_http2_proto.c
+++ b/bin/varnishd/http2/cache_http2_proto.c
@@ -190,6 +190,31 @@ h2_del_req(struct worker *wrk, struct h2_req *r2)
SES_Delete(sp, SC_RX_JUNK, NAN);
}
+void
+h2_kill_req(struct worker *wrk, const struct h2_sess *h2,
+ struct h2_req *r2, h2_error h2e)
+{
+
+ VSLb(h2->vsl, SLT_Debug, "KILL st=%u state=%d", r2->stream, r2->state);
+ Lck_Lock(&h2->sess->mtx);
+ r2->error = h2e;
+ switch (r2->state) {
+ case H2_S_CLOS_REM:
+ if (r2->wrk != NULL)
+ AZ(pthread_cond_signal(h2->cond));
+ r2 = NULL;
+ break;
+ case H2_S_OPEN:
+ (void)h2h_decode_fini(h2, r2->decode);
+ break;
+ default:
+ break;
+ }
+ Lck_Unlock(&h2->sess->mtx);
+ if (r2 != NULL)
+ h2_del_req(wrk, r2);
+}
+
/**********************************************************************/
static void
@@ -241,8 +266,7 @@ h2_rx_ping(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2)
(void)r2;
if (h2->rxf_len != 8) // rfc7540,l,2364,2366
return (H2CE_FRAME_SIZE_ERROR);
- if (h2->rxf_stream != 0) // rfc7540,l,2359,2362
- return (H2CE_PROTOCOL_ERROR);
+ AZ(h2->rxf_stream); // rfc7540,l,2359,2362
if (h2->rxf_flags != 0) // We never send pings
return (H2SE_PROTOCOL_ERROR);
H2_Send_Get(wrk, h2, r2);
@@ -275,20 +299,9 @@ h2_rx_rst_stream(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2)
if (h2->rxf_len != 4) // rfc7540,l,2003,2004
return (H2CE_FRAME_SIZE_ERROR);
- if (r2 == NULL)
- return (0);
- Lck_Lock(&h2->sess->mtx);
- r2->error = h2_streamerror(vbe32dec(h2->rxf_data));
-VSLb(h2->vsl, SLT_Debug, "H2RST %u %d %s", r2->stream, r2->state, r2->error->name);
- if (r2->wrk != NULL)
- AZ(pthread_cond_signal(h2->cond));
- if (r2->state != H2_S_IDLE) {
- r2->state = H2_S_CLOSED;
- r2 = NULL;
- }
- Lck_Unlock(&h2->sess->mtx);
if (r2 != NULL)
- h2_del_req(wrk, r2);
+ h2_kill_req(wrk, h2, r2,
+ h2_streamerror(vbe32dec(h2->rxf_data)));
return (0);
}
@@ -463,7 +476,6 @@ h2_end_headers(const struct worker *wrk, const struct h2_sess *h2,
h2_error h2e;
assert(r2->state == H2_S_OPEN);
- r2->state = H2_S_CLOS_REM;
h2e = h2h_decode_fini(h2, r2->decode);
FREE_OBJ(r2->decode);
if (h2e != NULL) {
@@ -480,6 +492,7 @@ h2_end_headers(const struct worker *wrk, const struct h2_sess *h2,
req->req_step = R_STP_TRANSPORT;
req->task.func = h2_do_req;
req->task.priv = req;
+ r2->state = H2_S_CLOS_REM; // XXX: not _quite_ true
XXXAZ(Pool_Task(wrk->pool, &req->task, TASK_QUEUE_REQ));
return (0);
}
@@ -535,7 +548,10 @@ h2_rx_headers(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2)
}
h2e = h2h_decode_bytes(h2, r2->decode, p, l);
if (h2e != NULL) {
- VSL(SLT_Debug, 0, "H2H_DECODE_BYTES %s", h2e->name);
+ VSL(SLT_Debug, 0, "H2H_DECODE_BYTES(hdr) %s", h2e->name);
+ (void)h2h_decode_fini(h2, r2->decode);
+ AZ(r2->req->ws->r);
+ h2_del_req(wrk, r2);
return (h2e);
}
if (h2->rxf_flags & H2FF_HEADERS_END_HEADERS)
@@ -558,7 +574,10 @@ h2_rx_continuation(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2)
req = r2->req;
h2e = h2h_decode_bytes(h2, r2->decode, h2->rxf_data, h2->rxf_len);
if (h2e != NULL) {
- VSL(SLT_Debug, 0, "H2H_DECODE_BYTES %s", h2e->name);
+ VSL(SLT_Debug, 0, "H2H_DECODE_BYTES(cont) %s", h2e->name);
+ (void)h2h_decode_fini(h2, r2->decode);
+ AZ(r2->req->ws->r);
+ h2_del_req(wrk, r2);
return (h2e);
}
if (h2->rxf_flags & H2FF_HEADERS_END_HEADERS)
@@ -677,6 +696,7 @@ h2_req_fail(struct req *req, enum sess_close reason)
{
assert(reason > 0);
assert(req->sp->fd != 0);
+ VSLb(req->vsl, SLT_Debug, "H2FAILREQ");
}
/**********************************************************************/
@@ -836,7 +856,7 @@ h2_rxframe(struct worker *wrk, struct h2_sess *h2)
h2->bogosity++;
VSLb(h2->vsl, SLT_Debug,
"H2: Unknown flags 0x%02x on %s (ignored)",
- (uint8_t)h2->rxf_flags, h2f->name);
+ (uint8_t)h2->rxf_flags & ~h2f->flags, h2f->name);
h2->rxf_flags &= h2f->flags;
}
diff --git a/bin/varnishd/http2/cache_http2_session.c b/bin/varnishd/http2/cache_http2_session.c
index 69fc12f..63732d2 100644
--- a/bin/varnishd/http2/cache_http2_session.c
+++ b/bin/varnishd/http2/cache_http2_session.c
@@ -201,6 +201,10 @@ h2_ou_session(struct worker *wrk, struct h2_sess *h2,
if (h2_b64url_settings(h2, req)) {
VSLb(h2->vsl, SLT_Debug, "H2: Bad HTTP-Settings");
+ AN (req->vcl);
+ VCL_Rel(&req->vcl);
+ CNT_AcctLogCharge(wrk->stats, req);
+ Req_Release(req);
return (0);
}
@@ -317,20 +321,25 @@ h2_new_session(struct worker *wrk, void *arg)
VTAILQ_FOREACH(r2, &h2->streams, list) {
if (r2->error == 0)
r2->error = h2->error;
+#if 0
if (r2->wrk != NULL)
- AZ(pthread_cond_signal(&r2->wrk->cond));
+ AZ(pthread_cond_signal(&r2->wrk->cond)); // XXX Why?
+#endif
}
+ AZ(pthread_cond_signal(h2->cond));
while (1) {
VTAILQ_FOREACH(r2, &h2->streams, list)
- if (r2->state == H2_S_IDLE && r2 != h2->req0)
+ if (r2->state != H2_S_CLOS_REM && r2 != h2->req0)
break;
if (r2 == NULL)
break;
Lck_Unlock(&sp->mtx);
- h2_del_req(wrk, r2);
+ h2_kill_req(wrk, h2, r2, h2->error);
Lck_Lock(&sp->mtx);
}
VSLb(h2->vsl, SLT_Debug, "H2CLEAN done");
+ VTAILQ_FOREACH(r2, &h2->streams, list)
+ VSLb(h2->vsl, SLT_Debug, "ST %u %d", r2->stream, r2->state);
Lck_Unlock(&sp->mtx);
h2->cond = NULL;
h2_del_req(wrk, h2->req0);
diff --git a/bin/varnishtest/tests/t02001.vtc b/bin/varnishtest/tests/t02001.vtc
index a55920a..d744822 100644
--- a/bin/varnishtest/tests/t02001.vtc
+++ b/bin/varnishtest/tests/t02001.vtc
@@ -39,7 +39,12 @@ server s1 {
txresp -status 402 -bodylen 11
} -start
-delay .5
+varnish v1 -vsl_catchup
+
+varnish v1 -expect MEMPOOL.req0.live == 0
+varnish v1 -expect MEMPOOL.req1.live == 0
+varnish v1 -expect MEMPOOL.sess0.live == 0
+varnish v1 -expect MEMPOOL.sess1.live == 0
varnish v1 -cliok "param.set feature +http2"
@@ -58,7 +63,12 @@ client c1 {
expect resp.bodylen == 8
} -run
-delay .5
+varnish v1 -vsl_catchup
+
+varnish v1 -expect MEMPOOL.req0.live == 0
+varnish v1 -expect MEMPOOL.req1.live == 0
+varnish v1 -expect MEMPOOL.sess0.live == 0
+varnish v1 -expect MEMPOOL.sess1.live == 0
client c1 {
send "GET /upgrade2 HTTP/1.1\r\n"
@@ -86,6 +96,13 @@ client c1 {
} -run
} -run
+varnish v1 -vsl_catchup
+
+varnish v1 -expect MEMPOOL.req0.live == 0
+varnish v1 -expect MEMPOOL.req1.live == 0
+varnish v1 -expect MEMPOOL.sess0.live == 0
+varnish v1 -expect MEMPOOL.sess1.live == 0
+
client c1 {
# Illegal HTTP2-Settings
send "GET /noupgrade HTTP/1.1\r\n"
@@ -96,6 +113,13 @@ client c1 {
expect_close
} -run
+varnish v1 -vsl_catchup
+
+varnish v1 -expect MEMPOOL.req0.live == 0
+varnish v1 -expect MEMPOOL.req1.live == 0
+varnish v1 -expect MEMPOOL.sess0.live == 0
+varnish v1 -expect MEMPOOL.sess1.live == 0
+
client c1 {
# PRISM with error in last bit
send "GET /noupgrade HTTP/1.1\r\n"
@@ -111,8 +135,7 @@ client c1 {
expect_close
} -run
-# XXX: Tests temporarily neutered, they are too flakey
-#varnish v1 -expect MEMPOOL.req0.live == 0
-#varnish v1 -expect MEMPOOL.req1.live == 0
-#varnish v1 -expect MEMPOOL.sess0.live == 0
-#varnish v1 -expect MEMPOOL.sess1.live == 0
+varnish v1 -expect MEMPOOL.req0.live == 0
+varnish v1 -expect MEMPOOL.req1.live == 0
+varnish v1 -expect MEMPOOL.sess0.live == 0
+varnish v1 -expect MEMPOOL.sess1.live == 0
diff --git a/bin/varnishtest/tests/t02003.vtc b/bin/varnishtest/tests/t02003.vtc
index 62b0fe7..a3b99e3 100644
--- a/bin/varnishtest/tests/t02003.vtc
+++ b/bin/varnishtest/tests/t02003.vtc
@@ -1,4 +1,4 @@
-varnishtest "H2 Stream error conditions"
+varnishtest "H2 Frame coverage/error conditions"
server s1 {
rxreq
@@ -26,6 +26,13 @@ client c1 {
stream 0 -wait
} -run
+varnish v1 -vsl_catchup
+
+varnish v1 -expect MEMPOOL.req0.live == 0
+varnish v1 -expect MEMPOOL.req1.live == 0
+varnish v1 -expect MEMPOOL.sess0.live == 0
+varnish v1 -expect MEMPOOL.sess1.live == 0
+
#######################################################################
# Test reverse order stream numbers
@@ -44,6 +51,13 @@ client c1 {
stream 0 -wait
} -run
+varnish v1 -vsl_catchup
+
+varnish v1 -expect MEMPOOL.req0.live == 0
+varnish v1 -expect MEMPOOL.req1.live == 0
+varnish v1 -expect MEMPOOL.sess0.live == 0
+varnish v1 -expect MEMPOOL.sess1.live == 0
+
#######################################################################
# Test WINDOW_UPDATE error conditions
@@ -68,7 +82,13 @@ client c1 {
} -start
stream 5 {
txprio
- sendhex "000003 08 00 00000005 010203"
+ sendhex "000003 08 00 00000005"
+ delay .1
+ sendhex 01
+ delay .1
+ sendhex 02
+ delay .1
+ sendhex 03
} -run
stream 0 -wait
} -run
@@ -93,6 +113,13 @@ client c1 {
} -run
} -run
+varnish v1 -vsl_catchup
+
+varnish v1 -expect MEMPOOL.req0.live == 0
+varnish v1 -expect MEMPOOL.req1.live == 0
+varnish v1 -expect MEMPOOL.sess0.live == 0
+varnish v1 -expect MEMPOOL.sess1.live == 0
+
#######################################################################
# Test PING error conditions
@@ -114,6 +141,22 @@ client c1 {
} -run
} -run
+client c1 {
+ stream 0 {
+ sendhex "000007 06 80 00000000 01020304050607"
+ rxgoaway
+ expect goaway.laststream == 0
+ expect goaway.err == FRAME_SIZE_ERROR
+ } -run
+} -run
+
+varnish v1 -vsl_catchup
+
+varnish v1 -expect MEMPOOL.req0.live == 0
+varnish v1 -expect MEMPOOL.req1.live == 0
+varnish v1 -expect MEMPOOL.sess0.live == 0
+varnish v1 -expect MEMPOOL.sess1.live == 0
+
#######################################################################
# Test PUSH_PROMISE error conditions
@@ -130,6 +173,14 @@ client c1 {
stream 0 -wait
} -run
+varnish v1 -vsl_catchup
+
+varnish v1 -expect MEMPOOL.req0.live == 0
+varnish v1 -expect MEMPOOL.req1.live == 0
+varnish v1 -expect MEMPOOL.sess0.live == 0
+varnish v1 -expect MEMPOOL.sess1.live == 0
+
+#######################################################################
# Test RST_STREAM error conditions
client c1 {
@@ -166,6 +217,20 @@ client c1 {
} -run
} -run
+client c1 {
+ stream 1 {
+ txreq -nohdrend
+ txrst -err 0x666
+ } -run
+} -run
+
+varnish v1 -vsl_catchup
+
+varnish v1 -expect MEMPOOL.req0.live == 0
+varnish v1 -expect MEMPOOL.req1.live == 0
+varnish v1 -expect MEMPOOL.sess0.live == 0
+varnish v1 -expect MEMPOOL.sess1.live == 0
+
#######################################################################
# Test SETTING error conditions
@@ -186,8 +251,7 @@ client c1 {
rxgoaway
expect goaway.err == PROTOCOL_ERROR
expect goaway.laststream == 0
- } -run
-} -run
+ } -run } -run
client c1 {
stream 0 {
@@ -207,3 +271,112 @@ client c1 {
rxping
} -run
} -run
+
+varnish v1 -vsl_catchup
+
+varnish v1 -expect MEMPOOL.req0.live == 0
+varnish v1 -expect MEMPOOL.req1.live == 0
+varnish v1 -expect MEMPOOL.sess0.live == 0
+varnish v1 -expect MEMPOOL.sess1.live == 0
+
+#######################################################################
+# Test GOAWAY error conditions
+
+client c1 {
+ stream 0 {
+ txgoaway -err 2
+ } -run
+ expect_close
+} -run
+
+client c1 {
+ stream 0 {
+ txgoaway -err 2222
+ } -run
+ expect_close
+} -run
+
+
+varnish v1 -vsl_catchup
+
+varnish v1 -expect MEMPOOL.req0.live == 0
+varnish v1 -expect MEMPOOL.req1.live == 0
+varnish v1 -expect MEMPOOL.sess0.live == 0
+varnish v1 -expect MEMPOOL.sess1.live == 0
+
+#######################################################################
+# Test HEADERS error conditions
+
+client c1 {
+ stream 1 {
+ txreq -nostrend
+ txreq -nostrend
+ } -run
+ stream 0 {
+ rxgoaway
+ } -run
+ expect_close
+} -run
+
+client c1 {
+ stream 0 {
+ sendhex 00000c
+ sendhex 01
+ sendhex 05
+ sendhex 00000001
+ sendhex ffffffff
+ sendhex ffffffff
+ sendhex ffffffff
+ rxgoaway
+ expect goaway.err == COMPRESSION_ERROR
+ } -run
+} -run
+
+varnish v1 -vsl_catchup
+
+varnish v1 -expect MEMPOOL.req0.live == 0
+varnish v1 -expect MEMPOOL.req1.live == 0
+varnish v1 -expect MEMPOOL.sess0.live == 0
+varnish v1 -expect MEMPOOL.sess1.live == 0
+
+#######################################################################
+# Test CONTINUATION error conditions
+
+client c1 {
+ stream 1 {
+ txreq -nostrend
+ txcont -hdr "foo" "bar"
+ } -run
+ stream 0 {
+ rxgoaway
+ } -run
+ expect_close
+} -run
+
+client c1 {
+ stream 0 {
+ sendhex 000014
+ sendhex 01
+ sendhex 01
+ sendhex 00000001
+ sendhex {8286 8441 0f77 7777 2e65 7861 6d70 6c65 2e63 6f6d}
+
+ sendhex 00000c
+ sendhex 09
+ sendhex 04
+ sendhex 00000001
+ sendhex ffffffff
+ sendhex ffffffff
+ sendhex ffffffff
+ rxgoaway
+ expect goaway.err == COMPRESSION_ERROR
+ } -run
+} -run
+
+varnish v1 -vsl_catchup
+
+varnish v1 -expect MEMPOOL.req0.live == 0
+varnish v1 -expect MEMPOOL.req1.live == 0
+varnish v1 -expect MEMPOOL.sess0.live == 0
+varnish v1 -expect MEMPOOL.sess1.live == 0
+
diff --git a/bin/varnishtest/tests/t02008.vtc b/bin/varnishtest/tests/t02008.vtc
index a1bccba..75cee51 100644
--- a/bin/varnishtest/tests/t02008.vtc
+++ b/bin/varnishtest/tests/t02008.vtc
@@ -27,7 +27,7 @@ client c1 {
txprio
} -run
stream 0 {
- txgoaway -err 7
+ txgoaway -err 2
} -run
expect_close
} -run
More information about the varnish-commit
mailing list