commit 1ba9b7e0137afc1a4bd6da7dd986ed98999cfbe5
parent 0e8ae82a8757fb8bb2850f1a3d59ad1dfb468556
Author: Nick Mathewson <nickm@torproject.org>
Date: Sun, 22 Apr 2018 20:39:35 -0400
Merge remote-tracking branch 'mikeperry/bug25400_squashed'
Diffstat:
8 files changed, 56 insertions(+), 29 deletions(-)
diff --git a/changes/bug25400 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/common/util.c b/src/common/util.c
@@ -572,6 +572,19 @@ add_laplace_noise(int64_t signal_, double random_, double delta_f,
return signal_ + noise;
}
+/* Helper: safely add two uint32_t's, capping at UINT32_MAX rather
+ * than overflow */
+uint32_t
+tor_add_u32_nowrap(uint32_t a, uint32_t b)
+{
+ /* a+b > UINT32_MAX check, without overflow */
+ if (PREDICT_UNLIKELY(a > UINT32_MAX - b)) {
+ return UINT32_MAX;
+ } else {
+ return a+b;
+ }
+}
+
/* Helper: return greatest common divisor of a,b */
static uint64_t
gcd64(uint64_t a, uint64_t b)
diff --git a/src/common/util.h b/src/common/util.h
@@ -176,6 +176,8 @@ int n_bits_set_u8(uint8_t v);
int64_t clamp_double_to_int64(double number);
void simplify_fraction64(uint64_t *numer, uint64_t *denom);
+uint32_t tor_add_u32_nowrap(uint32_t a, uint32_t b);
+
/* Compute the CEIL of <b>a</b> divided by <b>b</b>, for nonnegative <b>a</b>
* and positive <b>b</b>. Works on integer types only. Not defined if a+(b-1)
* can overflow. */
diff --git a/src/or/command.c b/src/or/command.c
@@ -495,6 +495,17 @@ 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 */
+ ocirc->n_read_circ_bw = tor_add_u32_nowrap(ocirc->n_read_circ_bw,
+ CELL_PAYLOAD_SIZE);
}
if (!CIRCUIT_IS_ORIGIN(circ) &&
diff --git 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
@@ -5803,8 +5803,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)) {
@@ -5820,12 +5818,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
@@ -374,6 +374,12 @@ 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);
+ ocirc->n_written_circ_bw = tor_add_u32_nowrap(ocirc->n_written_circ_bw,
+ CELL_PAYLOAD_SIZE);
+
} else { /* incoming cell */
if (CIRCUIT_IS_ORIGIN(circ)) {
/* We should never package an _incoming_ cell from the circuit
diff --git a/src/test/test_util.c b/src/test/test_util.c
@@ -6050,6 +6050,23 @@ test_util_monotonic_time_add_msec(void *arg)
}
static void
+test_util_nowrap_math(void *arg)
+{
+ (void)arg;
+
+ tt_u64_op(0, OP_EQ, tor_add_u32_nowrap(0, 0));
+ tt_u64_op(1, OP_EQ, tor_add_u32_nowrap(0, 1));
+ tt_u64_op(1, OP_EQ, tor_add_u32_nowrap(1, 0));
+ tt_u64_op(4, OP_EQ, tor_add_u32_nowrap(2, 2));
+ tt_u64_op(UINT32_MAX, OP_EQ, tor_add_u32_nowrap(UINT32_MAX-1, 2));
+ tt_u64_op(UINT32_MAX, OP_EQ, tor_add_u32_nowrap(2, UINT32_MAX-1));
+ tt_u64_op(UINT32_MAX, OP_EQ, tor_add_u32_nowrap(UINT32_MAX, UINT32_MAX));
+
+ done:
+ ;
+}
+
+static void
test_util_htonll(void *arg)
{
(void)arg;
@@ -6243,6 +6260,7 @@ struct testcase_t util_tests[] = {
UTIL_TEST(listdir, 0),
UTIL_TEST(parent_dir, 0),
UTIL_TEST(ftruncate, 0),
+ UTIL_TEST(nowrap_math, 0),
UTIL_TEST(num_cpus, 0),
UTIL_TEST_WIN_ONLY(load_win_lib, 0),
UTIL_TEST_NO_WIN(exit_status, 0),
