commit d80b881a52644fc05a65c0ff440449ea4c82c61b Author: Mike Perry <mikeperry-git@fscked.org> Date: Fri Jan 18 20:17:23 2013 -0800 Remove a source of error during path bias scaling If any circuits were opened during a scaling event, we were scaling attempts and successes by different amounts. This leads to rounding error. The fix is to record how many circuits are in a state that hasn't been fully counted yet, and subtract that before scaling, and add it back afterwords. --- src/or/circuitbuild.c | 98 +++++++++++++++++++++++++----------------------- 1 files changed, 51 insertions(+), 47 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 8880f14..8c7bd87 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -2071,20 +2071,20 @@ pathbias_count_timeout(origin_circuit_t *circ) } /** - * Return the number of circuits counted as successfully closed for - * this guard. - * - * Also add in the currently open circuits to give them the benefit - * of the doubt. + * Helper function to count all of the currently opened circuits + * for a guard that are in a given path state range. The state + * range is inclusive on both ends. */ -double -pathbias_get_close_success_count(entry_guard_t *guard) +static int +pathbias_count_circs_in_states(entry_guard_t *guard, + path_state_t from, + path_state_t to) { - circuit_t *circ; + circuit_t *circ = global_circuitlist; int open_circuits = 0; - /* Count currently open circuits. Give them the benefit of the doubt. */ - for (circ = global_circuitlist; circ; circ = circ->next) { + /* Count currently open circuits. Give them the benefit of the doubt */ + for ( ; circ; circ = circ->next) { origin_circuit_t *ocirc = NULL; if (!CIRCUIT_IS_ORIGIN(circ) || /* didn't originate here */ circ->marked_for_close) /* already counted */ @@ -2095,20 +2095,33 @@ pathbias_get_close_success_count(entry_guard_t *guard) if (!ocirc->cpath || !ocirc->cpath->extend_info) continue; - /* Don't count known failed or already counted circuits */ - if (ocirc->path_state >= PATH_STATE_USE_FAILED) - continue; - - if (ocirc->path_state >= PATH_STATE_BUILD_SUCCEEDED && + if (ocirc->path_state >= from && + ocirc->path_state <= to && pathbias_should_count(ocirc) && fast_memeq(guard->identity, - ocirc->cpath->extend_info->identity_digest, - DIGEST_LEN)) { + ocirc->cpath->extend_info->identity_digest, + DIGEST_LEN)) { open_circuits++; } } - return guard->successful_circuits_closed + open_circuits; + return open_circuits; +} + +/** + * Return the number of circuits counted as successfully closed for + * this guard. + * + * Also add in the currently open circuits to give them the benefit + * of the doubt. + */ +double +pathbias_get_close_success_count(entry_guard_t *guard) +{ + return guard->successful_circuits_closed + + pathbias_count_circs_in_states(guard, + PATH_STATE_BUILD_SUCCEEDED, + PATH_STATE_USE_SUCCEEDED); } /** @@ -2121,35 +2134,10 @@ pathbias_get_close_success_count(entry_guard_t *guard) double pathbias_get_use_success_count(entry_guard_t *guard) { - circuit_t *circ = global_circuitlist; - int open_circuits = 0; - - /* Count currently open circuits. Give them the benefit of the doubt */ - for ( ; circ; circ = circ->next) { - origin_circuit_t *ocirc = NULL; - if (!CIRCUIT_IS_ORIGIN(circ) || /* didn't originate here */ - circ->marked_for_close) /* already counted */ - continue; - - ocirc = TO_ORIGIN_CIRCUIT(circ); - - if (!ocirc->cpath || !ocirc->cpath->extend_info) - continue; - - /* Don't count known failed or already counted circuits */ - if (ocirc->path_state >= PATH_STATE_USE_FAILED) - continue; - - if (ocirc->path_state >= PATH_STATE_USE_ATTEMPTED && - pathbias_should_count(ocirc) && - fast_memeq(guard->identity, - ocirc->cpath->extend_info->identity_digest, - DIGEST_LEN)) { - open_circuits++; - } - } - - return guard->use_successes + open_circuits; + return guard->use_successes + + pathbias_count_circs_in_states(guard, + PATH_STATE_USE_ATTEMPTED, + PATH_STATE_USE_SUCCEEDED); } /** @@ -2252,6 +2240,10 @@ pathbias_check_use_rate(entry_guard_t *guard) if (guard->use_attempts > pathbias_get_scale_use_threshold(options)) { const int scale_factor = pathbias_get_scale_factor(options); const int mult_factor = pathbias_get_mult_factor(options); + int opened_attempts = pathbias_count_circs_in_states(guard, + PATH_STATE_USE_ATTEMPTED, PATH_STATE_USE_ATTEMPTED); + guard->use_attempts -= opened_attempts; + log_info(LD_CIRC, "Scaling pathbias use counts to (%f/%f)*(%d/%d) for guard %s=%s", guard->use_successes, guard->use_attempts, @@ -2263,6 +2255,8 @@ pathbias_check_use_rate(entry_guard_t *guard) guard->use_attempts /= scale_factor; guard->use_successes /= scale_factor; + + guard->use_attempts += opened_attempts; } return 0; @@ -2393,6 +2387,13 @@ pathbias_check_close_rate(entry_guard_t *guard) if (guard->circ_attempts > pathbias_get_scale_threshold(options)) { const int scale_factor = pathbias_get_scale_factor(options); const int mult_factor = pathbias_get_mult_factor(options); + int opened_attempts = pathbias_count_circs_in_states(guard, + PATH_STATE_BUILD_ATTEMPTED, PATH_STATE_BUILD_ATTEMPTED); + int opened_built = pathbias_count_circs_in_states(guard, + PATH_STATE_BUILD_SUCCEEDED, + PATH_STATE_USE_FAILED); + guard->circ_attempts -= opened_attempts; + guard->circ_successes -= opened_built; log_info(LD_CIRC, "Scaling pathbias counts to (%f/%f)*(%d/%d) for guard %s=%s", guard->circ_successes, guard->circ_attempts, @@ -2412,6 +2413,9 @@ pathbias_check_close_rate(entry_guard_t *guard) guard->successful_circuits_closed /= scale_factor; guard->collapsed_circuits /= scale_factor; guard->unusable_circuits /= scale_factor; + + guard->circ_attempts += opened_attempts; + guard->circ_successes += opened_built; } return 0;