diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 7dc544d47d..90b4f523c7 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -139,6 +139,15 @@ circuit_build_times_quantile_cutoff(void) return num/100.0; } +static double +circuit_build_times_close_quantile(void) +{ + int32_t num = networkstatus_get_param(NULL, "cbtclosequantile", + CBT_DEFAULT_CLOSE_QUANTILE); + + return num/100.0; +} + static int32_t circuit_build_times_test_frequency(void) { @@ -539,7 +548,10 @@ circuit_build_times_shuffle_and_store_array(circuit_build_times_t *cbt, /** * Filter old synthetic timeouts that were created before the - * right censored new Pareto calculation was deployed. + * new right-censored Pareto calculation was deployed. + * + * Once all clients before 0.2.1.13-alpha are gone, this code + * will be unused. */ static int circuit_build_times_filter_timeouts(circuit_build_times_t *cbt) @@ -549,7 +561,7 @@ circuit_build_times_filter_timeouts(circuit_build_times_t *cbt) build_time_t max_timeout = 0; timeout_rate = circuit_build_times_timeout_rate(cbt); - max_timeout = (build_time_t)cbt->timeout_ms; + max_timeout = (build_time_t)cbt->close_ms; for (i = 0; i < CBT_NCIRCUITS_TO_OBSERVE; i++) { if (cbt->circuit_build_times[i] > max_timeout) { @@ -1097,6 +1109,15 @@ circuit_build_times_set_timeout_worker(circuit_build_times_t *cbt) cbt->timeout_ms = circuit_build_times_calculate_timeout(cbt, circuit_build_times_quantile_cutoff()); + + cbt->close_ms = circuit_build_times_calculate_timeout(cbt, + circuit_build_times_close_quantile()); + + /* Sometimes really fast guard nodes give us such a steep curve + * that this ends up being not that much greater than timeout_ms. + * Make it be at least 1 min to handle this case. */ + cbt->close_ms = MAX(cbt->close_ms, 60*1000); + cbt->have_computed_timeout = 1; return 1; } @@ -1132,8 +1153,9 @@ circuit_build_times_set_timeout(circuit_build_times_t *cbt) cbt->total_build_times, tor_lround(cbt->timeout_ms/1000)); log_info(LD_CIRC, - "Circuit timeout data: %lfms, Xm: %d, a: %lf, r: %lf", - cbt->timeout_ms, cbt->Xm, cbt->alpha, timeout_rate); + "Circuit timeout data: %lfms, %lfms, Xm: %d, a: %lf, r: %lf", + cbt->timeout_ms, cbt->close_ms, cbt->Xm, cbt->alpha, + timeout_rate); } else if (prev_timeout < tor_lround(cbt->timeout_ms/1000)) { log_notice(LD_CIRC, "Based on %d circuit times, it looks like we need to wait " @@ -1142,14 +1164,15 @@ circuit_build_times_set_timeout(circuit_build_times_t *cbt) cbt->total_build_times, tor_lround(cbt->timeout_ms/1000)); log_info(LD_CIRC, - "Circuit timeout data: %lfms, Xm: %d, a: %lf, r: %lf", - cbt->timeout_ms, cbt->Xm, cbt->alpha, timeout_rate); + "Circuit timeout data: %lfms, %lfms, Xm: %d, a: %lf, r: %lf", + cbt->timeout_ms, cbt->close_ms, cbt->Xm, cbt->alpha, + timeout_rate); } else { log_info(LD_CIRC, - "Set circuit build timeout to %lds (%lfms, Xm: %d, a: %lf, " - "r: %lf) based on %d circuit times", + "Set circuit build timeout to %lds (%lfms, %lfms, Xm: %d, a: %lf," + " r: %lf) based on %d circuit times", tor_lround(cbt->timeout_ms/1000), - cbt->timeout_ms, cbt->Xm, cbt->alpha, timeout_rate, + cbt->timeout_ms, cbt->close_ms, cbt->Xm, cbt->alpha, timeout_rate, cbt->total_build_times); } } @@ -1747,7 +1770,7 @@ circuit_send_next_onion_skin(origin_circuit_t *circ) * it off at, we probably had a suspend event along this codepath, * and we should discard the value. */ - if (timediff < 0 || timediff > 2*circ_times.timeout_ms+1000) { + if (timediff < 0 || timediff > 2*circ_times.close_ms+1000) { log_notice(LD_CIRC, "Strange value for circuit build time: %ld. " "Assuming clock jump.", timediff); } else if (!circuit_build_times_disabled()) { diff --git a/src/or/circuituse.c b/src/or/circuituse.c index e7d10a22f2..5696d9c6f0 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -270,6 +270,7 @@ circuit_expire_building(time_t now) * decided on a customized one yet */ time_t general_cutoff = now - lround(circ_times.timeout_ms/1000); time_t begindir_cutoff = now - lround(circ_times.timeout_ms/2000); + time_t close_cutoff = now - lround(circ_times.close_ms/1000); time_t introcirc_cutoff = begindir_cutoff; cpath_build_state_t *build_state; @@ -286,8 +287,11 @@ circuit_expire_building(time_t now) cutoff = begindir_cutoff; else if (victim->purpose == CIRCUIT_PURPOSE_C_INTRODUCING) cutoff = introcirc_cutoff; + else if (victim->purpose == CIRCUIT_PURPOSE_C_MEASURE_TIMEOUT) + cutoff = close_cutoff; else cutoff = general_cutoff; + if (victim->timestamp_created > cutoff) continue; /* it's still young, leave it alone */ @@ -350,12 +354,29 @@ circuit_expire_building(time_t now) } else { /* circuit not open, consider recording failure as timeout */ int first_hop_succeeded = TO_ORIGIN_CIRCUIT(victim)->cpath && TO_ORIGIN_CIRCUIT(victim)->cpath->state == CPATH_STATE_OPEN; + + if (TO_ORIGIN_CIRCUIT(victim)->p_streams != NULL) { + log_warn(LD_BUG, "Circuit %d (purpose %d) has timed out, " + "yet has attached streams!", + TO_ORIGIN_CIRCUIT(victim)->global_identifier, + victim->purpose); + tor_fragile_assert(); + continue; + } + + /* circuits are allowed to last longer for measurement. + * Switch their purpose and wait. */ + if (victim->purpose != CIRCUIT_PURPOSE_C_MEASURE_TIMEOUT) { + victim->purpose = CIRCUIT_PURPOSE_C_MEASURE_TIMEOUT; + continue; + } + /* * 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 (now - victim->timestamp_created > (2*circ_times.timeout_ms)/1000+1) { + if (now - victim->timestamp_created > 2*circ_times.close_ms/1000+1) { log_notice(LD_CIRC, "Extremely large value for circuit build timeout: %ld. " "Assuming clock jump.", now - victim->timestamp_created); diff --git a/src/or/or.h b/src/or/or.h index e99b2e91dd..8e8ea18694 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -467,23 +467,23 @@ typedef enum { #define CIRCUIT_PURPOSE_C_REND_READY_INTRO_ACKED 11 /** Client-side circuit purpose: at Alice, rendezvous established. */ #define CIRCUIT_PURPOSE_C_REND_JOINED 12 - -#define _CIRCUIT_PURPOSE_C_MAX 12 - +/** This circuit is used for build time measurement only */ +#define CIRCUIT_PURPOSE_C_MEASURE_TIMEOUT 13 +#define _CIRCUIT_PURPOSE_C_MAX 13 /** Hidden-service-side circuit purpose: at Bob, waiting for introductions. */ -#define CIRCUIT_PURPOSE_S_ESTABLISH_INTRO 13 +#define CIRCUIT_PURPOSE_S_ESTABLISH_INTRO 14 /** Hidden-service-side circuit purpose: at Bob, successfully established * intro. */ -#define CIRCUIT_PURPOSE_S_INTRO 14 +#define CIRCUIT_PURPOSE_S_INTRO 15 /** Hidden-service-side circuit purpose: at Bob, connecting to rend point. */ -#define CIRCUIT_PURPOSE_S_CONNECT_REND 15 +#define CIRCUIT_PURPOSE_S_CONNECT_REND 16 /** Hidden-service-side circuit purpose: at Bob, rendezvous established. */ -#define CIRCUIT_PURPOSE_S_REND_JOINED 16 +#define CIRCUIT_PURPOSE_S_REND_JOINED 17 /** A testing circuit; not meant to be used for actual traffic. */ -#define CIRCUIT_PURPOSE_TESTING 17 +#define CIRCUIT_PURPOSE_TESTING 18 /** A controller made this circuit and Tor should not use it. */ -#define CIRCUIT_PURPOSE_CONTROLLER 18 -#define _CIRCUIT_PURPOSE_MAX 18 +#define CIRCUIT_PURPOSE_CONTROLLER 19 +#define _CIRCUIT_PURPOSE_MAX 19 /** A catch-all for unrecognized purposes. Currently we don't expect * to make or see any circuits with this purpose. */ #define CIRCUIT_PURPOSE_UNKNOWN 255 @@ -3055,6 +3055,12 @@ typedef uint32_t build_time_t; /* Circuit build times consensus parameters */ +/** + * How long to wait before actually closing circuits that take too long to + * build in terms of CDF quantile. + */ +#define CBT_DEFAULT_CLOSE_QUANTILE 95 + /** * How many circuits count as recent when considering if the * connection has gone gimpy or changed. @@ -3134,6 +3140,8 @@ typedef struct { /** The exact value for that timeout in milliseconds. Stored as a double * to maintain precision from calculations to and from quantile value. */ double timeout_ms; + /** How long we wait before actually closing the circuit. */ + double close_ms; } circuit_build_times_t; extern circuit_build_times_t circ_times;