commit dfa6808f57e5856edd4e25a957f3f06707264199 Author: Mike Perry <mikeperry-git@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