[multiple] simplify bytes_in/bytes_out accounting

encapsulate accounting calculations in
 http_request_stats_bytes_in()
 http_request_stats_bytes_out()

more accurate accounting for HTTP/1.1 bytes_in on keep-alive requests
(affects case where client pipelines HTTP/1.1 requests)

remove con->bytes_read and con->bytes_written
(no longer needed since request_st was split from connection struct
 and request bytes_read_ckpt and bytes_written_ckpt are maintained
 for HTTP/1.x bytes_in and bytes_out accounting.  Also, further back,
 chunkqueue internal accounting was simplified to maintain bytes_in
 and bytes_out to always match chunkqueue length)

src/base.h

@@ -36,9 +36,7 @@ struct connection {
 	chunkqueue *write_queue;      /* a large queue for low-level write ( HTTP response ) [ file, mem ] */
 	chunkqueue *read_queue;       /* a small queue for low-level read ( HTTP request ) [ mem ] */
 
-	off_t bytes_written;          /* used by mod_accesslog, mod_rrd */
-	off_t bytes_written_cur_second; /* used by mod_accesslog, mod_rrd */
-	off_t bytes_read;             /* used by mod_accesslog, mod_rrd */
+	off_t bytes_written_cur_second; /* used by rate-limiting and mod_status */
 
 	int (* network_write)(struct connection *con, chunkqueue *cq, off_t max_bytes);
 	int (* network_read)(struct connection *con, chunkqueue *cq, off_t max_bytes);

src/connections.c

@@ -207,8 +207,9 @@ static void connection_handle_response_end_state(request_st * const r, connectio
 		request_reset(r);
 		con->is_readable = 1; /* potentially trigger optimistic read */
 		/*(accounting used by mod_accesslog for HTTP/1.0 and HTTP/1.1)*/
-		r->bytes_read_ckpt = con->bytes_read;
-		r->bytes_written_ckpt = con->bytes_written;
+		/*(overloaded to detect next bytes recv'd on keep-alive con)*/
+		r->bytes_read_ckpt = r->read_queue.bytes_in;
+		r->bytes_written_ckpt = r->write_queue.bytes_out;
 #if 0
 		r->start_hp.tv_sec = log_epoch_secs;
 		con->read_idle_ts = log_monotonic_secs;
@@ -314,7 +315,6 @@ connection_write_chunkqueue (connection * const con, chunkqueue * const restrict
   #endif
 
     written = cq->bytes_out - written;
-    con->bytes_written += written;
     con->bytes_written_cur_second += written;
     request_st * const r = &con->request;
     if (r->conf.global_bytes_per_second_cnt_ptr)
@@ -336,7 +336,6 @@ connection_write_1xx_info (request_st * const r, connection * const con)
     int rc = con->network_write(con, cq, MAX_WRITE_LIMIT);
 
     written = cq->bytes_out - written;
-    con->bytes_written += written;
     con->bytes_written_cur_second += written;
     if (r->conf.global_bytes_per_second_cnt_ptr)
         *(r->conf.global_bytes_per_second_cnt_ptr) += written;
@@ -353,6 +352,9 @@ connection_write_1xx_info (request_st * const r, connection * const con)
              * Note: sending of remainder of 1xx might be delayed
              * until next set of response headers are sent */
             con->write_queue = chunkqueue_init(NULL);
+            /* (copy bytes for accounting purposes in event of failure) */
+            con->write_queue->bytes_in = cq->bytes_out; /*(yes, bytes_out)*/
+            con->write_queue->bytes_out = cq->bytes_out;
             chunkqueue_append_chunkqueue(con->write_queue, cq);
         }
     }
@@ -559,10 +561,7 @@ static void connection_reset(connection *con) {
 	r->bytes_read_ckpt = 0;
 	r->bytes_written_ckpt = 0;
 	con->is_readable = 1;
-
-	con->bytes_written = 0;
 	con->bytes_written_cur_second = 0;
-	con->bytes_read = 0;
 }
 
 
@@ -656,7 +655,7 @@ static int connection_handle_read_state(connection * const con)  {
 
     if (con->request_count > 1) {
         discard_blank = 1;
-        if (con->bytes_read == r->bytes_read_ckpt) {
+        if (cq->bytes_in == r->bytes_read_ckpt) {
             keepalive_request_start = 1;
             if (NULL != c) { /* !chunkqueue_is_empty(cq)) */
                 pipelined_request_start = 1;
@@ -729,7 +728,7 @@ static int connection_handle_read_state(connection * const con)  {
     } while ((c = connection_read_header_more(con, cq, c, clen)));
 
     if (keepalive_request_start) {
-        if (con->bytes_read > r->bytes_read_ckpt) {
+        if (cq->bytes_in > r->bytes_read_ckpt) {
             /* update r->start_hp.tv_sec timestamp when first byte of
              * next request is received on a keep-alive connection */
             r->start_hp.tv_sec = log_epoch_secs;
@@ -751,6 +750,9 @@ static int connection_handle_read_state(connection * const con)  {
     char * const hdrs = c->mem->ptr + hoff[1];
 
     if (con->request_count > 1) {
+        /* adjust r->bytes_read_ckpt for http_request_stats_bytes_in()
+         * (headers_len is still in cq; marked written, bytes_out incr below) */
+        r->bytes_read_ckpt = cq->bytes_out;
         /* clear buffers which may have been kept for reporting on keep-alive,
          * (e.g. mod_status) */
         request_reset_ex(r);
@@ -781,6 +783,7 @@ static int connection_handle_read_state(connection * const con)  {
         /*(Upgrade: h2c over cleartext does not have SNI; no COMP_HTTP_HOST)*/
         r->conditional_is_valid = (1 << COMP_SERVER_SOCKET)
                                 | (1 << COMP_HTTP_REMOTE_IP);
+        r->bytes_read_ckpt = 0;
         /*connection_handle_write(r, con);*//* defer write to network */
         return 0;
     }
@@ -881,18 +884,9 @@ static int connection_read_cq(connection *con, chunkqueue *cq, off_t max_bytes)
         if (len != (ssize_t)mem_len) {
             /* we got less then expected, wait for the next fd-event */
             con->is_readable = 0;
-
-            if (len > 0) {
-                con->bytes_read += len;
-                return 0;
-            }
-            else if (0 == len) /* other end close connection -> KEEP-ALIVE */
-                return -2;     /* (pipelining) */
-            else
-                return connection_read_cq_err(con);
+            return len > 0 ? 0 : 0 == len ? -2 : connection_read_cq_err(con);
         }
 
-        con->bytes_read += len;
         max_bytes -= len;
 
         int frd;
@@ -1505,7 +1499,8 @@ static void connection_check_timeout (connection * const con, const unix_time64_
                   "increase server.max-write-idle",
                   con->dst_addr_buf.ptr,
                   BUFFER_INTLEN_PTR(&r->target),
-                  (long long)con->bytes_written, (int)r->conf.max_write_idle);
+                  (long long)con->write_queue->bytes_out,
+                  (int)r->conf.max_write_idle);
             }
             connection_set_state_error(r, CON_STATE_ERROR);
             changed = 1;
@@ -1519,8 +1514,9 @@ static void connection_check_timeout (connection * const con, const unix_time64_
     if (0 == (t_diff = cur_ts - con->connection_start)) t_diff = 1;
 
     if (con->traffic_limit_reached &&
-        (r->conf.bytes_per_second == 0 ||
-         con->bytes_written < (off_t)r->conf.bytes_per_second * t_diff)) {
+        (r->conf.bytes_per_second == 0
+         || con->write_queue->bytes_out
+              < (off_t)r->conf.bytes_per_second * t_diff)) {
         /* enable connection again */
         con->traffic_limit_reached = 0;
 

src/h2.c

@@ -2846,7 +2846,6 @@ h2_retire_con (request_st * const h2r, connection * const con)
             con->network_write(con, cq, len);
             /*(optional accounting)*/
             written = cq->bytes_out - written;
-            con->bytes_written += written;
             con->bytes_written_cur_second += written;
             if (h2r->conf.global_bytes_per_second_cnt_ptr)
                 *(h2r->conf.global_bytes_per_second_cnt_ptr) += written;

src/mod_accesslog.c

@@ -8,6 +8,7 @@
 #include "log.h"
 #include "buffer.h"
 #include "http_header.h"
+#include "response.h"
 #include "sock_addr.h"
 
 #include "plugin.h"
@@ -854,13 +855,9 @@ static int log_access_record (const request_st * const r, buffer * const b, form
 				break;
 
 			case FORMAT_BYTES_OUT_NO_HEADER:
-			{
-				off_t bytes = r->http_version <= HTTP_VERSION_1_1
-				  ? con->bytes_written - r->bytes_written_ckpt
-				  : r->write_queue.bytes_out;
-				accesslog_append_bytes(b, bytes, r->resp_header_len);
+				accesslog_append_bytes(b, http_request_stats_bytes_out(r),
+				                       r->resp_header_len);
 				break;
-			}
 			case FORMAT_HEADER:
 				vb = http_header_request_get(r, f->opt, BUF_PTR_LEN(&f->string));
 				accesslog_append_buffer(b, vb, esc);
@@ -880,21 +877,11 @@ static int log_access_record (const request_st * const r, buffer * const b, form
 				accesslog_append_buffer(b, &r->physical.path, esc);
 				break;
 			case FORMAT_BYTES_OUT:
-			{
-				off_t bytes = r->http_version <= HTTP_VERSION_1_1
-				  ? con->bytes_written - r->bytes_written_ckpt
-				  : r->write_queue.bytes_out;
-				accesslog_append_bytes(b, bytes, 0);
+				accesslog_append_bytes(b, http_request_stats_bytes_out(r), 0);
 				break;
-			}
 			case FORMAT_BYTES_IN:
-			{
-				off_t bytes = r->http_version <= HTTP_VERSION_1_1
-				  ? con->bytes_read - r->bytes_read_ckpt
-				  : r->read_queue.bytes_in;
-				accesslog_append_bytes(b, bytes, 0);
+				accesslog_append_bytes(b, http_request_stats_bytes_in(r), 0);
 				break;
-			}
 			case FORMAT_SERVER_NAME:
 				accesslog_append_buffer(b, r->server_name, esc);
 				break;

src/mod_gnutls.c

@@ -2578,12 +2578,7 @@ connection_read_cq_ssl (connection * const con, chunkqueue * const cq, off_t max
         mem = chunkqueue_get_memory(cq, &mem_len);
 
         len = gnutls_record_recv(ssl, mem, mem_len);
-        if (len > 0) {
-            chunkqueue_use_memory(cq, ckpt, len);
-            con->bytes_read += len;
-        } else {
-            chunkqueue_use_memory(cq, ckpt, 0);
-        }
+        chunkqueue_use_memory(cq, ckpt, len > 0 ? len : 0);
     } while (len > 0 && (pend = gnutls_record_check_pending(ssl)));
 
     if (len < 0) {

src/mod_magnet.c

@@ -1601,19 +1601,13 @@ magnet_req_item_get (lua_State *L)
     switch (klen) {
       case 8:
         if (0 == memcmp(k, "bytes_in", 8)) {
-            off_t bytes = r->http_version <= HTTP_VERSION_1_1
-              ? r->con->bytes_read - r->bytes_read_ckpt
-              : r->read_queue.bytes_in;
-            lua_pushinteger(L, (lua_Integer)bytes);
+            lua_pushinteger(L, (lua_Integer)http_request_stats_bytes_in(r));
             return 1;
         }
         break;
       case 9:
         if (0 == memcmp(k, "bytes_out", 9)) {
-            off_t bytes = r->http_version <= HTTP_VERSION_1_1
-              ? r->con->bytes_written - r->bytes_written_ckpt
-              : r->write_queue.bytes_out;
-            lua_pushinteger(L, (lua_Integer)bytes);
+            lua_pushinteger(L, (lua_Integer)http_request_stats_bytes_out(r));
             return 1;
         }
         if (0 == memcmp(k, "stream_id", 9)) {

src/mod_mbedtls.c

@@ -2364,12 +2364,7 @@ connection_read_cq_ssl (connection * const con, chunkqueue * const cq, off_t max
         mem = chunkqueue_get_memory(cq, &mem_len);
 
         len = mbedtls_ssl_read(&hctx->ssl, (unsigned char *)mem, mem_len);
-        if (len > 0) {
-            chunkqueue_use_memory(cq, ckpt, len);
-            con->bytes_read += len;
-        } else {
-            chunkqueue_use_memory(cq, ckpt, 0);
-        }
+        chunkqueue_use_memory(cq, ckpt, len > 0 ? len : 0);
     } while (len > 0
              && mbedtls_ssl_check_pending(&hctx->ssl));
 

src/mod_nss.c

@@ -2319,12 +2319,7 @@ connection_read_cq_ssl (connection * const con, chunkqueue * const cq, off_t max
         mem = chunkqueue_get_memory(cq, &mem_len);
 
         len = PR_Read(ssl, mem, (PRInt32)mem_len);
-        if (len > 0) {
-            chunkqueue_use_memory(cq, ckpt, len);
-            con->bytes_read += len;
-        } else {
-            chunkqueue_use_memory(cq, ckpt, 0);
-        }
+        chunkqueue_use_memory(cq, ckpt, len > 0 ? len : 0);
     } while (len > 0);
 
     if (hctx->alpn && hctx->handshake) {

src/mod_openssl.c

@@ -3159,12 +3159,7 @@ connection_read_cq_ssl (connection * const con, chunkqueue * const cq, off_t max
         mem = chunkqueue_get_memory(cq, &mem_len);
 
         len = SSL_read(hctx->ssl, mem, mem_len);
-        if (len > 0) {
-            chunkqueue_use_memory(cq, ckpt, len);
-            con->bytes_read += len;
-        } else {
-            chunkqueue_use_memory(cq, ckpt, 0);
-        }
+        chunkqueue_use_memory(cq, ckpt, len > 0 ? len : 0);
 
         if (hctx->renegotiations > 1
             && hctx->conf.ssl_disable_client_renegotiation) {

src/mod_rrdtool.c

@@ -3,6 +3,7 @@
 #include "base.h"
 #include "fdevent.h"
 #include "log.h"
+#include "response.h"
 
 #include "plugin.h"
 #include <sys/types.h>
@@ -422,17 +423,11 @@ REQUESTDONE_FUNC(mod_rrd_account) {
 
     mod_rrd_patch_config(r, p);
     rrd_config * const rrd = p->conf.rrd;
-    if (NULL == rrd) return HANDLER_GO_ON;
-    ++rrd->requests;
-    if (r->http_version <= HTTP_VERSION_1_1) {
-        rrd->bytes_written += (r->con->bytes_written - r->bytes_written_ckpt);
-        rrd->bytes_read    += (r->con->bytes_read    - r->bytes_read_ckpt);
+    if (NULL != rrd) {
+        ++rrd->requests;
+        rrd->bytes_written += http_request_stats_bytes_out(r);
+        rrd->bytes_read    += http_request_stats_bytes_in(r);
     }
-    else {
-        rrd->bytes_written += r->write_queue.bytes_out;
-        rrd->bytes_read    += r->read_queue.bytes_in;
-    }
-
     return HANDLER_GO_ON;
 }
 

src/mod_wolfssl.c

@@ -2908,12 +2908,7 @@ connection_read_cq_ssl (connection * const con, chunkqueue * const cq, off_t max
         mem = chunkqueue_get_memory(cq, &mem_len);
 
         len = SSL_read(hctx->ssl, mem, mem_len);
-        if (len > 0) {
-            chunkqueue_use_memory(cq, ckpt, len);
-            con->bytes_read += len;
-        } else {
-            chunkqueue_use_memory(cq, ckpt, 0);
-        }
+        chunkqueue_use_memory(cq, ckpt, len > 0 ? len : 0);
 
         if (hctx->renegotiations > 1
             && hctx->conf.ssl_disable_client_renegotiation) {

src/request.h

@@ -245,6 +245,12 @@ int64_t li_restricted_strtoint64 (const char *v, const uint32_t vlen, const char
     ? "k"                             \
     : http_request_state_short((con)->request.state))
 
+#define http_request_stats_bytes_in(r) \
+   ((r)->read_queue.bytes_out - (r)->bytes_read_ckpt)
+
+#define http_request_stats_bytes_out(r) \
+   ((r)->write_queue.bytes_out - (r)->bytes_written_ckpt)
+
 __attribute_pure__
 const char * http_request_state_short (request_state_t state);