commit 5b55e15707fd49079b9c127b339976c4f446e131 Author: Nick Mathewson <nickm@torproject.org> Date: Thu Jan 25 16:05:20 2018 -0500 Remove all the old max_delay logic. We had tests for it, but it was always INT_MAX. --- src/or/directory.c | 53 +++++++++++++++++------------------------------------ src/or/directory.h | 10 ++++------ src/test/test_dir.c | 27 ++++++++++----------------- 3 files changed, 31 insertions(+), 59 deletions(-) diff --git a/src/or/directory.c b/src/or/directory.c index 3d5321157..3dbed28e7 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -5362,17 +5362,14 @@ find_dl_schedule(const download_status_t *dls, const or_options_t *options) return NULL; } -/** Decide which minimum and maximum delay step we want to use based on +/** Decide which minimum delay step we want to use based on * descriptor type in <b>dls</b> and <b>options</b>. * Helper function for download_status_schedule_get_delay(). */ -STATIC void -find_dl_min_and_max_delay(download_status_t *dls, const or_options_t *options, - int *min, int *max) +STATIC int +find_dl_min_delay(download_status_t *dls, const or_options_t *options) { tor_assert(dls); tor_assert(options); - tor_assert(min); - tor_assert(max); /* * For now, just use the existing schedule config stuff and pick the @@ -5380,10 +5377,7 @@ find_dl_min_and_max_delay(download_status_t *dls, const or_options_t *options, */ const smartlist_t *schedule = find_dl_schedule(dls, options); tor_assert(schedule != NULL && smartlist_len(schedule) >= 2); - *min = *((int *)(smartlist_get(schedule, 0))); - /* Increment on failure schedules always use exponential backoff, but they - * have a smaller limit when they're deterministic */ - *max = INT_MAX; + return *(int *)(smartlist_get(schedule, 0)); } /** As next_random_exponential_delay() below, but does not compute a random @@ -5421,49 +5415,39 @@ next_random_exponential_delay_range(int *low_bound_out, * zero); the <b>backoff_position</b> parameter is the number of times we've * generated a delay; and the <b>delay</b> argument is the most recently used * delay. - * - * Requires that delay is less than INT_MAX, and delay is in [0,max_delay]. */ STATIC int next_random_exponential_delay(int delay, - int base_delay, - int max_delay) + int base_delay) { /* Check preconditions */ - if (BUG(max_delay < 0)) - max_delay = 0; - if (BUG(delay > max_delay)) - delay = max_delay; if (BUG(delay < 0)) delay = 0; if (base_delay < 1) base_delay = 1; - int low_bound=0, high_bound=max_delay; + int low_bound=0, high_bound=INT_MAX; next_random_exponential_delay_range(&low_bound, &high_bound, delay, base_delay); - int rand_delay = crypto_rand_int_range(low_bound, high_bound); - - return MIN(rand_delay, max_delay); + return crypto_rand_int_range(low_bound, high_bound); } -/** Find the current delay for dls based on min_delay/ - * max_delay. Requires that 0 <= min_delay <= max_delay <= INT_MAX. +/** Find the current delay for dls based on min_delay. * * This function sets dls->next_attempt_at based on now, and returns the delay. * Helper for download_status_increment_failure and * download_status_increment_attempt. */ STATIC int download_status_schedule_get_delay(download_status_t *dls, - int min_delay, int max_delay, + int min_delay, time_t now) { tor_assert(dls); /* If we're using random exponential backoff, we do need min/max delay */ - tor_assert(min_delay >= 0 && max_delay >= min_delay); + tor_assert(min_delay >= 0); int delay = INT_MAX; uint8_t dls_schedule_position = (dls->increment_on @@ -5482,7 +5466,7 @@ download_status_schedule_get_delay(download_status_t *dls, while (dls->last_backoff_position < dls_schedule_position) { /* Do one increment step */ - delay = next_random_exponential_delay(delay, min_delay, max_delay); + delay = next_random_exponential_delay(delay, min_delay); /* Update our position */ ++(dls->last_backoff_position); } @@ -5493,7 +5477,6 @@ download_status_schedule_get_delay(download_status_t *dls, /* Clamp it within min/max if we have them */ if (min_delay >= 0 && delay < min_delay) delay = min_delay; - if (max_delay != INT_MAX && delay > max_delay) delay = max_delay; /* Store it for next time */ dls->last_backoff_position = dls_schedule_position; @@ -5560,7 +5543,7 @@ download_status_increment_failure(download_status_t *dls, int status_code, (void) status_code; // XXXX no longer used. (void) server; // XXXX no longer used. int increment = -1; - int min_delay = 0, max_delay = INT_MAX; + int min_delay = 0; tor_assert(dls); @@ -5585,9 +5568,8 @@ download_status_increment_failure(download_status_t *dls, int status_code, /* only return a failure retry time if this schedule increments on failures */ - find_dl_min_and_max_delay(dls, get_options(), &min_delay, &max_delay); - increment = download_status_schedule_get_delay(dls, - min_delay, max_delay, now); + min_delay = find_dl_min_delay(dls, get_options()); + increment = download_status_schedule_get_delay(dls, min_delay, now); } download_status_log_helper(item, !dls->increment_on, "failed", @@ -5618,7 +5600,7 @@ download_status_increment_attempt(download_status_t *dls, const char *item, time_t now) { int delay = -1; - int min_delay = 0, max_delay = INT_MAX; + int min_delay = 0; tor_assert(dls); @@ -5638,9 +5620,8 @@ download_status_increment_attempt(download_status_t *dls, const char *item, if (dls->n_download_attempts < IMPOSSIBLE_TO_DOWNLOAD-1) ++dls->n_download_attempts; - find_dl_min_and_max_delay(dls, get_options(), &min_delay, &max_delay); - delay = download_status_schedule_get_delay(dls, - min_delay, max_delay, now); + min_delay = find_dl_min_delay(dls, get_options()); + delay = download_status_schedule_get_delay(dls, min_delay, now); download_status_log_helper(item, dls->increment_on, "attempted", "on failure", dls->n_download_attempts, diff --git a/src/or/directory.h b/src/or/directory.h index 6008d3a88..e223d51ae 100644 --- a/src/or/directory.h +++ b/src/or/directory.h @@ -254,7 +254,7 @@ MOCK_DECL(STATIC int, directory_handle_command_post,(dir_connection_t *conn, const char *body, size_t body_len)); STATIC int download_status_schedule_get_delay(download_status_t *dls, - int min_delay, int max_delay, + int min_delay, time_t now); STATIC int handle_post_hs_descriptor(const char *url, const char *body); @@ -265,13 +265,11 @@ STATIC int should_use_directory_guards(const or_options_t *options); STATIC compression_level_t choose_compression_level(ssize_t n_bytes); STATIC const smartlist_t *find_dl_schedule(const download_status_t *dls, const or_options_t *options); -STATIC void find_dl_min_and_max_delay(download_status_t *dls, - const or_options_t *options, - int *min, int *max); +STATIC int find_dl_min_delay(download_status_t *dls, + const or_options_t *options); STATIC int next_random_exponential_delay(int delay, - int base_delay, - int max_delay); + int base_delay); STATIC void next_random_exponential_delay_range(int *low_bound_out, int *high_bound_out, diff --git a/src/test/test_dir.c b/src/test/test_dir.c index dc8d2a3bc..929ccd6c5 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -3965,7 +3965,7 @@ test_dir_packages(void *arg) } static void -download_status_random_backoff_helper(int min_delay, int max_delay) +download_status_random_backoff_helper(int min_delay) { download_status_t dls_random = { 0, 0, 0, DL_SCHED_GENERIC, DL_WANT_AUTHORITY, @@ -3978,22 +3978,21 @@ download_status_random_backoff_helper(int min_delay, int max_delay) int n_attempts = 0; do { increment = download_status_schedule_get_delay(&dls_random, - min_delay, max_delay, + min_delay, current_time); - log_debug(LD_DIR, "Min: %d, Max: %d, Inc: %d, Old Inc: %d", - min_delay, max_delay, increment, old_increment); + log_debug(LD_DIR, "Min: %d, Inc: %d, Old Inc: %d", + min_delay, increment, old_increment); /* Regression test for 20534 and friends * increment must always increase after the first */ - if (dls_random.last_backoff_position > 0 && max_delay > 0) { + if (dls_random.last_backoff_position > 0) { /* Always increment the exponential backoff */ tt_int_op(increment, OP_GE, 1); } /* Test */ tt_int_op(increment, OP_GE, min_delay); - tt_int_op(increment, OP_LE, max_delay); /* Advance */ if (dls_random.n_download_attempts < IMPOSSIBLE_TO_DOWNLOAD - 1) { @@ -4003,7 +4002,7 @@ download_status_random_backoff_helper(int min_delay, int max_delay) /* Try another maybe */ old_increment = increment; - } while (increment < max_delay && ++n_attempts < 1000); + } while (++n_attempts < 1000); done: return; @@ -4015,19 +4014,13 @@ test_dir_download_status_random_backoff(void *arg) (void)arg; /* Do a standard test */ - download_status_random_backoff_helper(0, 1000000); - /* Regression test for 20534 and friends: - * try tighter bounds */ - download_status_random_backoff_helper(0, 100); + download_status_random_backoff_helper(0); /* regression tests for 17750: initial delay */ - download_status_random_backoff_helper(10, 1000); - download_status_random_backoff_helper(20, 30); + download_status_random_backoff_helper(10); + download_status_random_backoff_helper(20); /* Pathological cases */ - download_status_random_backoff_helper(0, 0); - download_status_random_backoff_helper(1, 1); - download_status_random_backoff_helper(0, INT_MAX); - download_status_random_backoff_helper(INT_MAX/2, INT_MAX); + download_status_random_backoff_helper(INT_MAX/2); } static void