Sanitize H2_Send_Frame error handling.

The only way H2_Send_Frame can fail is if the TCP connection is
dead, so reach out and set the session errored and be done with it,
rather than pass an inconvenient error status down through the callers.

While here use writev(2) instead of two write(2) calls.

bin/varnishd/http2/cache_http2.h

@@ -217,7 +217,7 @@ h2_error h2h_decode_bytes(struct h2_sess *h2, const uint8_t *ptr,
 void H2_Send_Get(struct worker *, struct h2_sess *, struct h2_req *);
 void H2_Send_Rel(struct h2_sess *, const struct h2_req *);
 
-h2_error H2_Send_Frame(struct worker *, const struct h2_sess *,
+void H2_Send_Frame(struct worker *, struct h2_sess *,
     h2_frame type, uint8_t flags, uint32_t len, uint32_t stream,
     const void *);
 

bin/varnishd/http2/cache_http2_proto.c

@@ -132,11 +132,10 @@ h2_connectionerror(uint32_t u)
 
 /**********************************************************************/
 
-static h2_error
+static void
 h2_tx_rst(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2,
     uint32_t stream, h2_error h2e)
 {
-	h2_error ret;
 	char b[4];
 
 	CHECK_OBJ_NOTNULL(h2, H2_SESS_MAGIC);
@@ -148,11 +147,8 @@ h2_tx_rst(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2,
 	vbe32enc(b, h2e->val);
 
 	H2_Send_Get(wrk, h2, r2);
-	ret = H2_Send_Frame(wrk, h2, H2_F_RST_STREAM,
-	    0, sizeof b, stream, b);
+	H2_Send_Frame(wrk, h2, H2_F_RST_STREAM, 0, sizeof b, stream, b);
 	H2_Send_Rel(h2, r2);
-
-	return (ret);
 }
 
 /**********************************************************************
@@ -936,8 +932,9 @@ h2_procframe(struct worker *wrk, struct h2_sess *h2,
 			     "H2: stream %u: Hit maximum number of "
 			     "concurrent streams", h2->rxf_stream);
 			// rfc7540,l,1200,1205
-			return (h2_tx_rst(wrk, h2, h2->req0, h2->rxf_stream,
-				H2SE_REFUSED_STREAM));
+			h2_tx_rst(wrk, h2, h2->req0, h2->rxf_stream,
+				H2SE_REFUSED_STREAM);
+			return (0);
 		}
 		h2->highest_stream = h2->rxf_stream;
 		r2 = h2_new_req(wrk, h2, h2->rxf_stream, NULL);
@@ -954,7 +951,8 @@ h2_procframe(struct worker *wrk, struct h2_sess *h2,
 	if (h2->rxf_stream == 0 || h2e->connection)
 		return (h2e);	// Connection errors one level up
 
-	return (h2_tx_rst(wrk, h2, h2->req0, h2->rxf_stream, h2e));
+	h2_tx_rst(wrk, h2, h2->req0, h2->rxf_stream, h2e);
+	return (0);
 }
 
 static int
@@ -1143,8 +1141,8 @@ h2_rxframe(struct worker *wrk, struct h2_sess *h2)
 		vbe32enc(b, h2->highest_stream);
 		vbe32enc(b + 4, h2e->val);
 		H2_Send_Get(wrk, h2, h2->req0);
-		(void)H2_Send_Frame(wrk, h2, H2_F_GOAWAY, 0, 8, 0, b);
+		H2_Send_Frame(wrk, h2, H2_F_GOAWAY, 0, 8, 0, b);
 		H2_Send_Rel(h2, h2->req0);
 	}
-	return (h2e ? 0 : 1);
+	return (h2->error ? 0 : 1);
 }

bin/varnishd/http2/cache_http2_send.c

@@ -29,6 +29,8 @@
 
 #include "config.h"
 
+#include <sys/uio.h>
+
 #include "cache/cache_varnishd.h"
 
 #include "cache/cache_transport.h"
@@ -112,13 +114,14 @@ h2_mk_hdr(uint8_t *hdr, h2_frame ftyp, uint8_t flags,
  * the session mtx must be held.
  */
 
-h2_error
-H2_Send_Frame(struct worker *wrk, const struct h2_sess *h2,
+void
+H2_Send_Frame(struct worker *wrk, struct h2_sess *h2,
     h2_frame ftyp, uint8_t flags,
     uint32_t len, uint32_t stream, const void *ptr)
 {
 	uint8_t hdr[9];
 	ssize_t s;
+	struct iovec iov[2];
 
 	(void)wrk;
 
@@ -137,18 +140,24 @@ H2_Send_Frame(struct worker *wrk, const struct h2_sess *h2,
 		h2->srq->acct.resp_bodybytes += len;
 	Lck_Unlock(&h2->sess->mtx);
 
-	s = write(h2->sess->fd, hdr, sizeof hdr);
-	if (s != sizeof hdr)
-		return (H2CE_PROTOCOL_ERROR);		// XXX Need private ?
-	if (len > 0) {
-		s = write(h2->sess->fd, ptr, len);
-		if (s != len)
-			return (H2CE_PROTOCOL_ERROR);	// XXX Need private ?
+	memset(iov, 0, sizeof iov);
+	iov[0].iov_base = hdr;
+	iov[0].iov_len = sizeof hdr;
+	iov[1].iov_base = TRUST_ME(ptr);
+	iov[1].iov_len = len;
+	s = writev(h2->sess->fd, iov, len == 0 ? 1 : 2);
+	if (s != sizeof hdr + len) {
+		/*
+		 * There is no point in being nice here, we will be unable
+		 * to send a GOAWAY once the code unrolls, so go directly
+		 * to the finale and be done with it.
+		 */
+		h2->error = H2CE_PROTOCOL_ERROR;
+	} else if (len > 0) {
 		Lck_Lock(&h2->sess->mtx);
 		VSLb_bin(h2->vsl, SLT_H2TxBody, len, ptr);
 		Lck_Unlock(&h2->sess->mtx);
 	}
-	return (0);
 }
 
 static int64_t
@@ -251,7 +260,6 @@ void
 H2_Send(struct worker *wrk, struct h2_req *r2,
     h2_frame ftyp, uint8_t flags, uint32_t len, const void *ptr)
 {
-	h2_error retval;
 	struct h2_sess *h2;
 	uint32_t mfs, tf;
 	const char *p;
@@ -289,8 +297,7 @@ H2_Send(struct worker *wrk, struct h2_req *r2,
 		tf = mfs;
 
 	if (len <= tf) {
-		(void)H2_Send_Frame(wrk, h2,
-		    ftyp, flags, len, r2->stream, ptr);
+		H2_Send_Frame(wrk, h2, ftyp, flags, len, r2->stream, ptr);
 	} else {
 		AN(ptr);
 		p = ptr;
@@ -308,19 +315,19 @@ H2_Send(struct worker *wrk, struct h2_req *r2,
 				assert(VTAILQ_FIRST(&h2->txqueue) == r2);
 			}
 			if (tf < len) {
-				retval = H2_Send_Frame(wrk, h2, ftyp,
+				H2_Send_Frame(wrk, h2, ftyp,
 				    flags, tf, r2->stream, p);
 			} else {
 				if (ftyp->respect_window)
 					assert(tf == len);
 				tf = len;
-				retval = H2_Send_Frame(wrk, h2, ftyp,
+				H2_Send_Frame(wrk, h2, ftyp,
 				    final_flags, tf, r2->stream, p);
 				flags = 0;
 			}
 			p += tf;
 			len -= tf;
 			ftyp = ftyp->continuation;
-		} while (len > 0 && retval == 0);
+		} while (!h2->error && len > 0);
 	}
 }