diff --git a/changes/bug23100 b/changes/bug23100
new file mode 100644
index 0000000000..22e2485d64
--- /dev/null
+++ b/changes/bug23100
@@ -0,0 +1,7 @@
+ o Minor bugfixes (Performance):
+ - Use hidden service circuits (and other circuits longer than 3 hops)
+ to calculate a circuit build timeout. Previously, Tor only calculated
+ its build timeout based on circuits that planned to be exactly 3 hops
+ long. With this change, we include measurements from all circuits at
+ the point where they complete their third hop. Fixes bug 23100;
+ bugfix on 0.2.2.2-alpha.
diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c
index b36fed63b3..0be54c533c 100644
--- a/src/or/circuitbuild.c
+++ b/src/or/circuitbuild.c
@@ -825,16 +825,25 @@ should_use_create_fast_for_circuit(origin_circuit_t *circ)
return networkstatus_get_param(NULL, "usecreatefast", 0, 0, 1);
}
-/** Return true if circ is the type of circuit we want to count
- * timeouts from. In particular, we want it to have not completed yet
- * (already completing indicates we cannibalized it), and we want it to
- * have exactly three hops.
+/**
+ * Return true if circ is the type of circuit we want to count
+ * timeouts from.
+ *
+ * In particular, we want to consider any circuit that plans to build
+ * at least 3 hops (but maybe more), but has 3 or fewer hops built
+ * so far.
+ *
+ * We still want to consider circuits before 3 hops, because we need
+ * to decide if we should convert them to a measurement circuit in
+ * circuit_build_times_handle_completed_hop(), rather than letting
+ * slow circuits get killed right away.
*/
int
-circuit_timeout_want_to_count_circ(origin_circuit_t *circ)
+circuit_timeout_want_to_count_circ(const origin_circuit_t *circ)
{
return !circ->has_opened
- && circ->build_state->desired_path_len == DEFAULT_ROUTE_LEN;
+ && circ->build_state->desired_path_len >= DEFAULT_ROUTE_LEN
+ && circuit_get_cpath_opened_len(circ) <= DEFAULT_ROUTE_LEN;
}
/** Decide whether to use a TAP or ntor handshake for connecting to ei
@@ -938,7 +947,9 @@ circuit_send_next_onion_skin(origin_circuit_t *circ)
tor_assert(circ->cpath->state == CPATH_STATE_OPEN);
tor_assert(circ->base_.state == CIRCUIT_STATE_BUILDING);
+
crypt_path_t *hop = onion_next_hop_in_cpath(circ->cpath);
+ circuit_build_times_handle_completed_hop(circ);
if (hop) {
/* Case two: we're on a hop after the first. */
@@ -1053,38 +1064,6 @@ circuit_build_no_more_hops(origin_circuit_t *circ)
* I think I got them right, but more checking would be wise. -NM
*/
- if (circuit_timeout_want_to_count_circ(circ)) {
- struct timeval end;
- long timediff;
- tor_gettimeofday(&end);
- timediff = tv_mdiff(&circ->base_.timestamp_began, &end);
-
- /*
- * If the circuit build time is much greater than we would have cut
- * it off at, we probably had a suspend event along this codepath,
- * and we should discard the value.
- */
- if (timediff < 0 ||
- timediff > 2*get_circuit_build_close_time_ms()+1000) {
- log_notice(LD_CIRC, "Strange value for circuit build time: %ldmsec. "
- "Assuming clock jump. Purpose %d (%s)", timediff,
- circ->base_.purpose,
- circuit_purpose_to_string(circ->base_.purpose));
- } else if (!circuit_build_times_disabled(get_options())) {
- /* Only count circuit times if the network is live */
- if (circuit_build_times_network_check_live(
- get_circuit_build_times())) {
- circuit_build_times_add_time(get_circuit_build_times_mutable(),
- (build_time_t)timediff);
- circuit_build_times_set_timeout(get_circuit_build_times_mutable());
- }
-
- if (circ->base_.purpose != CIRCUIT_PURPOSE_C_MEASURE_TIMEOUT) {
- circuit_build_times_network_circ_success(
- get_circuit_build_times_mutable());
- }
- }
- }
log_info(LD_CIRC,"circuit built!");
circuit_reset_failure_count(0);
diff --git a/src/or/circuitbuild.h b/src/or/circuitbuild.h
index b8a651e055..3c28781bc4 100644
--- a/src/or/circuitbuild.h
+++ b/src/or/circuitbuild.h
@@ -27,7 +27,7 @@ int circuit_handle_first_hop(origin_circuit_t *circ);
void circuit_n_chan_done(channel_t *chan, int status,
int close_origin_circuits);
int inform_testing_reachability(void);
-int circuit_timeout_want_to_count_circ(origin_circuit_t *circ);
+int circuit_timeout_want_to_count_circ(const origin_circuit_t *circ);
int circuit_send_next_onion_skin(origin_circuit_t *circ);
void circuit_note_clock_jumped(int seconds_elapsed);
int circuit_extend(cell_t *cell, circuit_t *circ);
diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c
index d37d986f17..20fa03306e 100644
--- a/src/or/circuitlist.c
+++ b/src/or/circuitlist.c
@@ -1789,6 +1789,25 @@ circuit_get_cpath_len(origin_circuit_t *circ)
return n;
}
+/** Return the number of opened hops in circuit's path.
+ * If circ has no entries, or is NULL, returns 0. */
+int
+circuit_get_cpath_opened_len(origin_circuit_t *circ)
+{
+ int n = 0;
+ if (circ && circ->cpath) {
+ crypt_path_t *cpath, *cpath_next = NULL;
+ for (cpath = circ->cpath;
+ cpath->state == CPATH_STATE_OPEN
+ && cpath_next != circ->cpath;
+ cpath = cpath_next) {
+ cpath_next = cpath->next;
+ ++n;
+ }
+ }
+ return n;
+}
+
/** Return the hopnumth hop in circ->cpath, or NULL if there
* aren't that many hops in the list. hopnum starts at 1.
* Returns NULL if hopnum is 0 or negative. */
diff --git a/src/or/circuitlist.h b/src/or/circuitlist.h
index 5d0da15ca8..c90f5445d8 100644
--- a/src/or/circuitlist.h
+++ b/src/or/circuitlist.h
@@ -58,6 +58,7 @@ void circuit_mark_all_dirty_circs_as_unusable(void);
MOCK_DECL(void, circuit_mark_for_close_, (circuit_t *circ, int reason,
int line, const char *file));
int circuit_get_cpath_len(origin_circuit_t *circ);
+int circuit_get_cpath_opened_len(origin_circuit_t *);
void circuit_clear_cpath(origin_circuit_t *circ);
crypt_path_t *circuit_get_cpath_hop(origin_circuit_t *circ, int hopnum);
void circuit_get_all_pending_on_channel(smartlist_t *out,
diff --git a/src/or/circuitstats.c b/src/or/circuitstats.c
index 923a6d794f..5cb90c5000 100644
--- a/src/or/circuitstats.c
+++ b/src/or/circuitstats.c
@@ -36,6 +36,7 @@
#include "rendclient.h"
#include "rendservice.h"
#include "statefile.h"
+#include "circuitlist.h"
#undef log
#include
@@ -610,6 +611,77 @@ circuit_build_times_rewind_history(circuit_build_times_t *cbt, int n)
}
#endif /* 0 */
+/**
+ * Perform the build time work that needs to be done when a circuit
+ * completes a hop.
+ *
+ * This function decides if we should record a circuit's build time
+ * in our histogram data and other statistics, and if so, records it.
+ * It also will mark circuits that have already timed out as
+ * measurement-only circuits, so they can continue to build but
+ * not get used.
+ *
+ * For this, we want to consider circuits that will eventually make
+ * it to the third hop. For circuits longer than 3 hops, we want to
+ * record their build time when they reach the third hop, but let
+ * them continue (and not count them later). For circuits that are
+ * exactly 3 hops, this will count them when they are completed. We
+ * do this so that CBT is always gathering statistics on circuits
+ * of the same length, regardless of their type.
+ */
+void
+circuit_build_times_handle_completed_hop(origin_circuit_t *circ)
+{
+ struct timeval end;
+ long timediff;
+
+ /* If circuit build times are disabled, let circuit_expire_building()
+ * handle it.. */
+ if (circuit_build_times_disabled(get_options())) {
+ return;
+ }
+
+ /* Is this a circuit for which the timeout applies in a straight-forward
+ * way? If so, handle it below. If not, just return (and let
+ * circuit_expire_building() eventually take care of it).
+ */
+ if (!circuit_timeout_want_to_count_circ(circ)) {
+ return;
+ }
+
+ /* If the circuit is built to exactly the DEFAULT_ROUTE_LEN,
+ * add it to our buildtimes. */
+ if (circuit_get_cpath_opened_len(circ) == DEFAULT_ROUTE_LEN) {
+ tor_gettimeofday(&end);
+ timediff = tv_mdiff(&circ->base_.timestamp_began, &end);
+
+ /* If the circuit build time is much greater than we would have cut
+ * it off at, we probably had a suspend event along this codepath,
+ * and we should discard the value.
+ */
+ if (timediff < 0 ||
+ timediff > 2*get_circuit_build_close_time_ms()+1000) {
+ log_notice(LD_CIRC, "Strange value for circuit build time: %ldmsec. "
+ "Assuming clock jump. Purpose %d (%s)", timediff,
+ circ->base_.purpose,
+ circuit_purpose_to_string(circ->base_.purpose));
+ } else {
+ /* Only count circuit times if the network is live */
+ if (circuit_build_times_network_check_live(
+ get_circuit_build_times())) {
+ circuit_build_times_add_time(get_circuit_build_times_mutable(),
+ (build_time_t)timediff);
+ circuit_build_times_set_timeout(get_circuit_build_times_mutable());
+ }
+
+ if (circ->base_.purpose != CIRCUIT_PURPOSE_C_MEASURE_TIMEOUT) {
+ circuit_build_times_network_circ_success(
+ get_circuit_build_times_mutable());
+ }
+ }
+ }
+}
+
/**
* Add a new build time value time to the set of build times. Time
* units are milliseconds.
@@ -1285,9 +1357,14 @@ circuit_build_times_network_is_live(circuit_build_times_t *cbt)
}
/**
- * Called to indicate that we completed a circuit. Because this circuit
+ * Called to indicate that we "completed" a circuit. Because this circuit
* succeeded, it doesn't count as a timeout-after-the-first-hop.
*
+ * (For the purposes of the cbt code, we consider a circuit "completed" if
+ * it has 3 hops, regardless of its final hop count. We do this because
+ * we're trying to answer the question, "how long should a circuit take to
+ * reach the 3-hop count".)
+ *
* This is used by circuit_build_times_network_check_changed() to determine
* if we had too many recent timeouts and need to reset our learned timeout
* to something higher.
diff --git a/src/or/circuitstats.h b/src/or/circuitstats.h
index 92dc6405ba..68b66af1ba 100644
--- a/src/or/circuitstats.h
+++ b/src/or/circuitstats.h
@@ -34,6 +34,7 @@ void circuit_build_times_set_timeout(circuit_build_times_t *cbt);
int circuit_build_times_add_time(circuit_build_times_t *cbt,
build_time_t time);
int circuit_build_times_needs_circuits(const circuit_build_times_t *cbt);
+void circuit_build_times_handle_completed_hop(origin_circuit_t *circ);
int circuit_build_times_needs_circuits_now(const circuit_build_times_t *cbt);
void circuit_build_times_init(circuit_build_times_t *cbt);