Mailing List Archive

[6.0] 62db24848 respect `send_timeout` for "dripping" http1 writes
commit 62db248488e450c8dc9b276ab1b9420201deb958
Author: Nils Goroll <nils.goroll@uplex.de>
Date: Mon Jan 27 14:18:28 2020 +0100

respect `send_timeout` for "dripping" http1 writes

Previously, we only checked `v1l->deadline` (which gets initialized
from the `send_timeout` session attribute or parameter) only for short
writes, so for successful "dripping" http1 writes (streaming from a
backend busy object with small chunks), we did not respect the
timeout.

This patch restructures `V1L_Flush()` to always check the deadline
before a write by turning a `while() { ... }` into a `do { ... }
while` with the same continuation criteria: `while (i != v1l->liov)`
is turned into `if (i == v1l->liov) break;` and `while (i > 0 || errno
== EWOULDBLOCK)` is kept to retry short writes.

This also reduces the `writev()` call sites to one.

Fixes #3189

Conflicts:
bin/varnishd/http1/cache_http1_line.c
bin/varnishtest/tests/s00011.vtc

diff --git a/bin/varnishd/http1/cache_http1_line.c b/bin/varnishd/http1/cache_http1_line.c
index e7be7c8c2..be0214afb 100644
--- a/bin/varnishd/http1/cache_http1_line.c
+++ b/bin/varnishd/http1/cache_http1_line.c
@@ -201,18 +201,8 @@ V1L_Flush(const struct worker *wrk)
v1l->iov[v1l->ciov].iov_len = 0;
}

- i = writev(*v1l->wfd, v1l->iov, v1l->niov);
- if (i > 0)
- v1l->cnt += i;
- while (i != v1l->liov && (i > 0 || errno == EWOULDBLOCK)) {
- /* Remove sent data from start of I/O vector,
- * then retry; we hit a timeout, but some data
- * was sent.
- *
- * XXX: Add a "minimum sent data per timeout
- * counter to prevent slowlaris attacks
- */
-
+ i = 0;
+ do {
if (VTIM_real() - v1l->t0 > cache_param->send_timeout) {
VSLb(v1l->vsl, SLT_Debug,
"Hit total send timeout, "
@@ -222,16 +212,28 @@ V1L_Flush(const struct worker *wrk)
break;
}

+ i = writev(*v1l->wfd, v1l->iov, v1l->niov);
+ if (i > 0)
+ v1l->cnt += i;
+
+ if (i == v1l->liov)
+ break;
+
+ /* we hit a timeout, and some data may have been sent:
+ * Remove sent data from start of I/O vector, then retry
+ *
+ * XXX: Add a "minimum sent data per timeout counter to
+ * prevent slowloris attacks
+ */
+
VSLb(v1l->vsl, SLT_Debug,
"Hit idle send timeout, wrote = %zd/%zd; retrying",
i, v1l->liov);

if (i > 0)
v1l_prune(v1l, i);
- i = writev(*v1l->wfd, v1l->iov, v1l->niov);
- if (i > 0)
- v1l->cnt += i;
- }
+ } while (i > 0 || errno == EWOULDBLOCK);
+
if (i <= 0) {
v1l->werr++;
VSLb(v1l->vsl, SLT_Debug,
diff --git a/bin/varnishtest/tests/r03189.vtc b/bin/varnishtest/tests/r03189.vtc
new file mode 100644
index 000000000..cf3d466c9
--- /dev/null
+++ b/bin/varnishtest/tests/r03189.vtc
@@ -0,0 +1,28 @@
+varnishtest "h1 send_timeout and streaming of dripping chunks"
+
+barrier b cond 2
+
+server s1 {
+ rxreq
+ txresp -nolen -hdr "Transfer-Encoding: chunked"
+ chunkedlen 1
+ delay 1
+ non_fatal
+ barrier b sync
+ chunkedlen 1
+ delay 1
+ chunkedlen 0
+} -start
+
+varnish v1 \
+ -arg "-p idle_send_timeout=.1" \
+ -arg "-p send_timeout=.8" \
+ -vcl+backend { } -start
+
+client c1 {
+ txreq
+ rxresphdrs
+ rxchunk
+ barrier b sync
+ expect_close
+} -run
_______________________________________________
varnish-commit mailing list
varnish-commit@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-commit