[tor-commits] [tor/master] Bug 25400: Make CIRC_BW event properly total everything on a circ.

[nickm at torproject.org]

commit dfa6808f57e5856edd4e25a957f3f06707264199
Author: Mike Perry <mikeperry-git at torproject.org>
Date:   Mon Feb 12 09:11:14 2018 +0000

    Bug 25400: Make CIRC_BW event properly total everything on a circ.
---
 changes/bug25400    |  5 +++++
 src/or/command.c    | 14 ++++++++++++++
 src/or/connection.c | 22 +---------------------
 src/or/control.c    |  8 --------
 src/or/relay.c      |  9 +++++++++
 5 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/changes/bug25400 b/changes/bug25400
new file mode 100644
index 000000000..cee7ea83b
--- /dev/null
+++ b/changes/bug25400
@@ -0,0 +1,5 @@
+  o Minor bugfix (controler):
+    - Make CIRC_BW event reflect the total of all data sent on a circuit,
+      including padding and dropped cells. Also fix a mis-counting bug
+      when STREAM_BW events were enabled. Fixes bug 25400; bugfix on
+      0.2.5.2-alpha.
diff --git a/src/or/command.c b/src/or/command.c
index 4f99462f3..18cbbca25 100644
--- a/src/or/command.c
+++ b/src/or/command.c
@@ -495,6 +495,20 @@ command_process_relay_cell(cell_t *cell, channel_t *chan)
     /* if we're a relay and treating connections with recent local
      * traffic better, then this is one of them. */
     channel_timestamp_client(chan);
+
+    /* Count all circuit bytes here for control port accuracy. We want
+     * to count even invalid/dropped relay cells, hence counting
+     * before the recognized check and the connection_edge_process_relay
+     * cell checks.
+     */
+    origin_circuit_t *ocirc = TO_ORIGIN_CIRCUIT(circ);
+
+    /* Count the payload bytes only. We don't care about cell headers */
+    if (PREDICT_LIKELY(UINT32_MAX - ocirc->n_read_circ_bw >
+                CELL_PAYLOAD_SIZE))
+      ocirc->n_read_circ_bw += (int)CELL_PAYLOAD_SIZE;
+    else
+      ocirc->n_read_circ_bw = UINT32_MAX;
   }
 
   if (!CIRCUIT_IS_ORIGIN(circ) &&
diff --git a/src/or/connection.c b/src/or/connection.c
index 957398985..e8ecf0db9 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -3479,25 +3479,15 @@ connection_buf_read_from_socket(connection_t *conn, ssize_t *max_to_read,
      /* change *max_to_read */
     *max_to_read = at_most - n_read;
 
-    /* Update edge_conn->n_read and ocirc->n_read_circ_bw */
+    /* Update edge_conn->n_read */
     if (conn->type == CONN_TYPE_AP) {
       edge_connection_t *edge_conn = TO_EDGE_CONN(conn);
-      circuit_t *circ = circuit_get_by_edge_conn(edge_conn);
-      origin_circuit_t *ocirc;
 
       /* Check for overflow: */
       if (PREDICT_LIKELY(UINT32_MAX - edge_conn->n_read > n_read))
         edge_conn->n_read += (int)n_read;
       else
         edge_conn->n_read = UINT32_MAX;
-
-      if (circ && CIRCUIT_IS_ORIGIN(circ)) {
-        ocirc = TO_ORIGIN_CIRCUIT(circ);
-        if (PREDICT_LIKELY(UINT32_MAX - ocirc->n_read_circ_bw > n_read))
-          ocirc->n_read_circ_bw += (int)n_read;
-        else
-          ocirc->n_read_circ_bw = UINT32_MAX;
-      }
     }
 
     /* If CONN_BW events are enabled, update conn->n_read_conn_bw for
@@ -3815,22 +3805,12 @@ connection_handle_write_impl(connection_t *conn, int force)
 
   if (n_written && conn->type == CONN_TYPE_AP) {
     edge_connection_t *edge_conn = TO_EDGE_CONN(conn);
-    circuit_t *circ = circuit_get_by_edge_conn(edge_conn);
-    origin_circuit_t *ocirc;
 
     /* Check for overflow: */
     if (PREDICT_LIKELY(UINT32_MAX - edge_conn->n_written > n_written))
       edge_conn->n_written += (int)n_written;
     else
       edge_conn->n_written = UINT32_MAX;
-
-    if (circ && CIRCUIT_IS_ORIGIN(circ)) {
-      ocirc = TO_ORIGIN_CIRCUIT(circ);
-      if (PREDICT_LIKELY(UINT32_MAX - ocirc->n_written_circ_bw > n_written))
-        ocirc->n_written_circ_bw += (int)n_written;
-      else
-        ocirc->n_written_circ_bw = UINT32_MAX;
-    }
   }
 
   /* If CONN_BW events are enabled, update conn->n_written_conn_bw for
diff --git a/src/or/control.c b/src/or/control.c
index 0539ddaca..016fd46a4 100644
--- a/src/or/control.c
+++ b/src/or/control.c
@@ -5800,8 +5800,6 @@ control_event_or_conn_status(or_connection_t *conn, or_conn_status_event_t tp,
 int
 control_event_stream_bandwidth(edge_connection_t *edge_conn)
 {
-  circuit_t *circ;
-  origin_circuit_t *ocirc;
   struct timeval now;
   char tbuf[ISO_TIME_USEC_LEN+1];
   if (EVENT_IS_INTERESTING(EVENT_STREAM_BANDWIDTH_USED)) {
@@ -5817,12 +5815,6 @@ control_event_stream_bandwidth(edge_connection_t *edge_conn)
                        (unsigned long)edge_conn->n_written,
                        tbuf);
 
-    circ = circuit_get_by_edge_conn(edge_conn);
-    if (circ && CIRCUIT_IS_ORIGIN(circ)) {
-      ocirc = TO_ORIGIN_CIRCUIT(circ);
-      ocirc->n_read_circ_bw += edge_conn->n_read;
-      ocirc->n_written_circ_bw += edge_conn->n_written;
-    }
     edge_conn->n_written = edge_conn->n_read = 0;
   }
 
diff --git a/src/or/relay.c b/src/or/relay.c
index 72a902d8b..d43720478 100644
--- a/src/or/relay.c
+++ b/src/or/relay.c
@@ -371,6 +371,15 @@ circuit_package_relay_cell(cell_t *cell, circuit_t *circ,
     }
 
     relay_encrypt_cell_outbound(cell, TO_ORIGIN_CIRCUIT(circ), layer_hint);
+
+    /* Update circ written totals for control port */
+    origin_circuit_t *ocirc = TO_ORIGIN_CIRCUIT(circ);
+    if (PREDICT_LIKELY(UINT32_MAX - ocirc->n_written_circ_bw
+          > CELL_PAYLOAD_SIZE))
+      ocirc->n_written_circ_bw += (int)CELL_PAYLOAD_SIZE;
+    else
+      ocirc->n_written_circ_bw = UINT32_MAX;
+
   } else { /* incoming cell */
     if (CIRCUIT_IS_ORIGIN(circ)) {
       /* We should never package an _incoming_ cell from the circuit