mirror of
https://gitlab.torproject.org/tpo/core/tor.git
synced 2024-11-10 21:23:58 +01:00
Merge branch 'bug17750_029_squashed'
This commit is contained in:
commit
948158df33
4
changes/bug17750
Normal file
4
changes/bug17750
Normal file
@ -0,0 +1,4 @@
|
||||
o Minor bugfixes (directory downloads):
|
||||
- Make clients wait for 6 seconds before trying to download their
|
||||
consensus from an authority.
|
||||
Fixes bug 17750, bugfix on 0.2.8.1-alpha.
|
@ -18,7 +18,6 @@
|
||||
#include "consdiffmgr.h"
|
||||
#include "control.h"
|
||||
#include "compat.h"
|
||||
#define DIRECTORY_PRIVATE
|
||||
#include "directory.h"
|
||||
#include "dirserv.h"
|
||||
#include "dirvote.h"
|
||||
@ -5124,7 +5123,7 @@ connection_dir_finished_connecting(dir_connection_t *conn)
|
||||
* Helper function for download_status_increment_failure(),
|
||||
* download_status_reset(), and download_status_increment_attempt(). */
|
||||
STATIC const smartlist_t *
|
||||
find_dl_schedule(download_status_t *dls, const or_options_t *options)
|
||||
find_dl_schedule(const download_status_t *dls, const or_options_t *options)
|
||||
{
|
||||
const int dir_server = dir_server_mode(options);
|
||||
const int multi_d = networkstatus_consensus_can_use_multiple_directories(
|
||||
@ -5193,6 +5192,8 @@ 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 */
|
||||
if (dls->backoff == DL_SCHED_DETERMINISTIC)
|
||||
*max = *((int *)((smartlist_get(schedule, smartlist_len(schedule) - 1))));
|
||||
else
|
||||
@ -5201,8 +5202,9 @@ find_dl_min_and_max_delay(download_status_t *dls, const or_options_t *options,
|
||||
|
||||
/** Advance one delay step. The algorithm is to use the previous delay to
|
||||
* compute an increment, we construct a value uniformly at random between
|
||||
* delay and MAX(delay*2,delay+1). We then clamp that value to be no larger
|
||||
* than max_delay, and return it.
|
||||
* delay+1 and (delay*(DIR_DEFAULT_RANDOM_MULTIPLIER+1))+1 (or
|
||||
* DIR_TEST_NET_RANDOM_MULTIPLIER in test networks).
|
||||
* We then clamp that value to be no larger than max_delay, and return it.
|
||||
*
|
||||
* Requires that delay is less than INT_MAX, and delay is in [0,max_delay].
|
||||
*/
|
||||
@ -5221,11 +5223,11 @@ next_random_exponential_delay(int delay, int max_delay)
|
||||
|
||||
/* How much are we willing to add to the delay? */
|
||||
int max_increment;
|
||||
int multiplier = 3; /* no more than quadruple the previous delay */
|
||||
int multiplier = DIR_DEFAULT_RANDOM_MULTIPLIER;
|
||||
if (get_options()->TestingTorNetwork) {
|
||||
/* Decrease the multiplier in testing networks. This reduces the variance,
|
||||
* so that bootstrap is more reliable. */
|
||||
multiplier = 2; /* no more than triple the previous delay */
|
||||
multiplier = DIR_TEST_NET_RANDOM_MULTIPLIER;
|
||||
}
|
||||
|
||||
if (delay && delay < (INT_MAX-1) / multiplier) {
|
||||
@ -5377,6 +5379,11 @@ download_status_increment_failure(download_status_t *dls, int status_code,
|
||||
|
||||
tor_assert(dls);
|
||||
|
||||
/* dls wasn't reset before it was used */
|
||||
if (dls->next_attempt_at == 0) {
|
||||
download_status_reset(dls);
|
||||
}
|
||||
|
||||
/* count the failure */
|
||||
if (dls->n_download_failures < IMPOSSIBLE_TO_DOWNLOAD-1) {
|
||||
++dls->n_download_failures;
|
||||
@ -5401,14 +5408,16 @@ download_status_increment_failure(download_status_t *dls, int status_code,
|
||||
|
||||
download_status_log_helper(item, !dls->increment_on, "failed",
|
||||
"concurrently", dls->n_download_failures,
|
||||
increment, dls->next_attempt_at, now);
|
||||
increment,
|
||||
download_status_get_next_attempt_at(dls),
|
||||
now);
|
||||
|
||||
if (dls->increment_on == DL_SCHED_INCREMENT_ATTEMPT) {
|
||||
/* stop this schedule retrying on failure, it will launch concurrent
|
||||
* connections instead */
|
||||
return TIME_MAX;
|
||||
} else {
|
||||
return dls->next_attempt_at;
|
||||
return download_status_get_next_attempt_at(dls);
|
||||
}
|
||||
}
|
||||
|
||||
@ -5429,6 +5438,11 @@ download_status_increment_attempt(download_status_t *dls, const char *item,
|
||||
|
||||
tor_assert(dls);
|
||||
|
||||
/* dls wasn't reset before it was used */
|
||||
if (dls->next_attempt_at == 0) {
|
||||
download_status_reset(dls);
|
||||
}
|
||||
|
||||
if (dls->increment_on == DL_SCHED_INCREMENT_FAILURE) {
|
||||
/* this schedule should retry on failure, and not launch any concurrent
|
||||
attempts */
|
||||
@ -5447,9 +5461,19 @@ download_status_increment_attempt(download_status_t *dls, const char *item,
|
||||
|
||||
download_status_log_helper(item, dls->increment_on, "attempted",
|
||||
"on failure", dls->n_download_attempts,
|
||||
delay, dls->next_attempt_at, now);
|
||||
delay, download_status_get_next_attempt_at(dls),
|
||||
now);
|
||||
|
||||
return dls->next_attempt_at;
|
||||
return download_status_get_next_attempt_at(dls);
|
||||
}
|
||||
|
||||
static time_t
|
||||
download_status_get_initial_delay_from_now(const download_status_t *dls)
|
||||
{
|
||||
const smartlist_t *schedule = find_dl_schedule(dls, get_options());
|
||||
/* We use constant initial delays, even in exponential backoff
|
||||
* schedules. */
|
||||
return time(NULL) + *(int *)smartlist_get(schedule, 0);
|
||||
}
|
||||
|
||||
/** Reset <b>dls</b> so that it will be considered downloadable
|
||||
@ -5470,11 +5494,9 @@ download_status_reset(download_status_t *dls)
|
||||
|| dls->n_download_attempts == IMPOSSIBLE_TO_DOWNLOAD)
|
||||
return; /* Don't reset this. */
|
||||
|
||||
const smartlist_t *schedule = find_dl_schedule(dls, get_options());
|
||||
|
||||
dls->n_download_failures = 0;
|
||||
dls->n_download_attempts = 0;
|
||||
dls->next_attempt_at = time(NULL) + *(int *)smartlist_get(schedule, 0);
|
||||
dls->next_attempt_at = download_status_get_initial_delay_from_now(dls);
|
||||
dls->last_backoff_position = 0;
|
||||
dls->last_delay_used = 0;
|
||||
/* Don't reset dls->want_authority or dls->increment_on */
|
||||
@ -5501,6 +5523,12 @@ download_status_get_n_attempts(const download_status_t *dls)
|
||||
time_t
|
||||
download_status_get_next_attempt_at(const download_status_t *dls)
|
||||
{
|
||||
/* dls wasn't reset before it was used */
|
||||
if (dls->next_attempt_at == 0) {
|
||||
/* so give the answer we would have given if it had been */
|
||||
return download_status_get_initial_delay_from_now(dls);
|
||||
}
|
||||
|
||||
return dls->next_attempt_at;
|
||||
}
|
||||
|
||||
|
@ -123,12 +123,19 @@ time_t download_status_increment_attempt(download_status_t *dls,
|
||||
void download_status_reset(download_status_t *dls);
|
||||
static int download_status_is_ready(download_status_t *dls, time_t now,
|
||||
int max_failures);
|
||||
time_t download_status_get_next_attempt_at(const download_status_t *dls);
|
||||
|
||||
/** Return true iff, as of <b>now</b>, the resource tracked by <b>dls</b> is
|
||||
* ready to get its download reattempted. */
|
||||
static inline int
|
||||
download_status_is_ready(download_status_t *dls, time_t now,
|
||||
int max_failures)
|
||||
{
|
||||
/* dls wasn't reset before it was used */
|
||||
if (dls->next_attempt_at == 0) {
|
||||
download_status_reset(dls);
|
||||
}
|
||||
|
||||
if (dls->backoff == DL_SCHED_DETERMINISTIC) {
|
||||
/* Deterministic schedules can hit an endpoint; exponential backoff
|
||||
* schedules just wait longer and longer. */
|
||||
@ -137,7 +144,7 @@ download_status_is_ready(download_status_t *dls, time_t now,
|
||||
if (!under_failure_limit)
|
||||
return 0;
|
||||
}
|
||||
return dls->next_attempt_at <= now;
|
||||
return download_status_get_next_attempt_at(dls) <= now;
|
||||
}
|
||||
|
||||
static void download_status_mark_impossible(download_status_t *dl);
|
||||
@ -151,7 +158,6 @@ download_status_mark_impossible(download_status_t *dl)
|
||||
|
||||
int download_status_get_n_failures(const download_status_t *dls);
|
||||
int download_status_get_n_attempts(const download_status_t *dls);
|
||||
time_t download_status_get_next_attempt_at(const download_status_t *dls);
|
||||
|
||||
int purpose_needs_anonymity(uint8_t dir_purpose, uint8_t router_purpose,
|
||||
const char *resource);
|
||||
@ -193,7 +199,7 @@ STATIC char* authdir_type_to_string(dirinfo_type_t auth);
|
||||
STATIC const char * dir_conn_purpose_to_string(int purpose);
|
||||
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(download_status_t *dls,
|
||||
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,
|
||||
@ -206,5 +212,15 @@ STATIC int parse_hs_version_from_post(const char *url, const char *prefix,
|
||||
STATIC unsigned parse_accept_encoding_header(const char *h);
|
||||
#endif
|
||||
|
||||
#if defined(TOR_UNIT_TESTS) || defined(DIRECTORY_PRIVATE)
|
||||
/* Used only by directory.c and test_dir.c */
|
||||
|
||||
/* no more than quadruple the previous delay (multiplier + 1) */
|
||||
#define DIR_DEFAULT_RANDOM_MULTIPLIER (3)
|
||||
/* no more than triple the previous delay */
|
||||
#define DIR_TEST_NET_RANDOM_MULTIPLIER (2)
|
||||
|
||||
#endif
|
||||
|
||||
#endif
|
||||
|
||||
|
@ -2073,7 +2073,9 @@ typedef struct download_status_t {
|
||||
* or after each failure? */
|
||||
download_schedule_backoff_bitfield_t backoff : 1; /**< do we use the
|
||||
* deterministic schedule, or random
|
||||
* exponential backoffs? */
|
||||
* exponential backoffs?
|
||||
* Increment on failure schedules
|
||||
* always use exponential backoff. */
|
||||
uint8_t last_backoff_position; /**< number of attempts/failures, depending
|
||||
* on increment_on, when we last recalculated
|
||||
* the delay. Only updated if backoff
|
||||
|
@ -4109,32 +4109,59 @@ test_dir_download_status_schedule(void *arg)
|
||||
}
|
||||
|
||||
static void
|
||||
test_dir_download_status_random_backoff(void *arg)
|
||||
download_status_random_backoff_helper(int min_delay, int max_delay)
|
||||
{
|
||||
download_status_t dls_random =
|
||||
{ 0, 0, 0, DL_SCHED_GENERIC, DL_WANT_AUTHORITY,
|
||||
DL_SCHED_INCREMENT_FAILURE, DL_SCHED_RANDOM_EXPONENTIAL, 0, 0 };
|
||||
int increment = -1;
|
||||
int old_increment;
|
||||
int old_increment = -1;
|
||||
time_t current_time = time(NULL);
|
||||
const int min_delay = 0;
|
||||
const int max_delay = 1000000;
|
||||
|
||||
(void)arg;
|
||||
const int exponent = DIR_DEFAULT_RANDOM_MULTIPLIER + 1;
|
||||
|
||||
/* Check the random backoff cases */
|
||||
old_increment = 0;
|
||||
do {
|
||||
increment = download_status_schedule_get_delay(&dls_random,
|
||||
NULL,
|
||||
min_delay, max_delay,
|
||||
current_time);
|
||||
|
||||
log_debug(LD_DIR, "Min: %d, Max: %d, Inc: %d, Old Inc: %d",
|
||||
min_delay, max_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) {
|
||||
/* 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);
|
||||
tt_int_op(increment, OP_GE, old_increment);
|
||||
/* We at most quadruple, and maybe add one */
|
||||
tt_int_op(increment, OP_LE, 4 * old_increment + 1);
|
||||
if (dls_random.last_backoff_position == 0) {
|
||||
/* regression tests for 17750
|
||||
* Always use the minimum delay for the first increment */
|
||||
tt_int_op(increment, OP_EQ, min_delay);
|
||||
} else {
|
||||
/* It's times like these I'd love a good saturating arithmetic
|
||||
* implementation */
|
||||
int min_inc = INT_MAX;
|
||||
if (old_increment <= INT_MAX - 1) {
|
||||
min_inc = old_increment + 1;
|
||||
}
|
||||
|
||||
int max_inc = INT_MAX;
|
||||
if (old_increment <= (INT_MAX - 1)/exponent) {
|
||||
max_inc = (exponent * old_increment) + 1;
|
||||
}
|
||||
|
||||
/* Regression test for 20534 and friends:
|
||||
* increment must always increase after the first */
|
||||
tt_int_op(increment, OP_GE, min_inc);
|
||||
/* We at most quadruple, and always add one */
|
||||
tt_int_op(increment, OP_LE, max_inc);
|
||||
}
|
||||
|
||||
/* Advance */
|
||||
current_time += increment;
|
||||
@ -4149,6 +4176,27 @@ test_dir_download_status_random_backoff(void *arg)
|
||||
return;
|
||||
}
|
||||
|
||||
static void
|
||||
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);
|
||||
/* regression tests for 17750: initial delay */
|
||||
download_status_random_backoff_helper(10, 1000);
|
||||
download_status_random_backoff_helper(20, 30);
|
||||
|
||||
/* 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);
|
||||
}
|
||||
|
||||
static void
|
||||
test_dir_download_status_increment(void *arg)
|
||||
{
|
||||
@ -4161,32 +4209,97 @@ test_dir_download_status_increment(void *arg)
|
||||
DL_WANT_ANY_DIRSERVER,
|
||||
DL_SCHED_INCREMENT_ATTEMPT,
|
||||
DL_SCHED_DETERMINISTIC, 0, 0 };
|
||||
download_status_t dls_exp = { 0, 0, 0, DL_SCHED_GENERIC,
|
||||
DL_WANT_ANY_DIRSERVER,
|
||||
DL_SCHED_INCREMENT_ATTEMPT,
|
||||
DL_SCHED_RANDOM_EXPONENTIAL, 0, 0 };
|
||||
int no_delay = 0;
|
||||
int delay0 = -1;
|
||||
int delay1 = -1;
|
||||
int delay2 = -1;
|
||||
smartlist_t *schedule = smartlist_new();
|
||||
smartlist_t *schedule_no_initial_delay = smartlist_new();
|
||||
or_options_t test_options;
|
||||
time_t next_at = TIME_MAX;
|
||||
time_t current_time = time(NULL);
|
||||
|
||||
/* Provide some values for the schedule */
|
||||
/* Provide some values for the schedules */
|
||||
delay0 = 10;
|
||||
delay1 = 99;
|
||||
delay2 = 20;
|
||||
|
||||
/* Make the schedule */
|
||||
/* Make the schedules */
|
||||
smartlist_add(schedule, (void *)&delay0);
|
||||
smartlist_add(schedule, (void *)&delay1);
|
||||
smartlist_add(schedule, (void *)&delay2);
|
||||
|
||||
smartlist_add(schedule_no_initial_delay, (void *)&no_delay);
|
||||
smartlist_add(schedule_no_initial_delay, (void *)&delay1);
|
||||
smartlist_add(schedule_no_initial_delay, (void *)&delay2);
|
||||
|
||||
/* Put it in the options */
|
||||
mock_options = &test_options;
|
||||
reset_options(mock_options, &mock_get_options_calls);
|
||||
mock_options->TestingClientDownloadSchedule = schedule;
|
||||
mock_options->TestingBridgeDownloadSchedule = schedule;
|
||||
mock_options->TestingClientDownloadSchedule = schedule;
|
||||
|
||||
MOCK(get_options, mock_get_options);
|
||||
|
||||
/* Check that the initial value of the schedule is the first value used,
|
||||
* whether or not it was reset before being used */
|
||||
|
||||
/* regression test for 17750: no initial delay */
|
||||
mock_options->TestingClientDownloadSchedule = schedule_no_initial_delay;
|
||||
mock_get_options_calls = 0;
|
||||
/* we really want to test that it's equal to time(NULL) + delay0, but that's
|
||||
* an unrealiable test, because time(NULL) might change. */
|
||||
tt_assert(download_status_get_next_attempt_at(&dls_failure)
|
||||
>= current_time + no_delay);
|
||||
tt_assert(download_status_get_next_attempt_at(&dls_failure)
|
||||
!= TIME_MAX);
|
||||
tt_assert(download_status_get_n_failures(&dls_failure) == 0);
|
||||
tt_assert(download_status_get_n_attempts(&dls_failure) == 0);
|
||||
tt_assert(mock_get_options_calls >= 1);
|
||||
|
||||
/* regression test for 17750: initial delay */
|
||||
mock_options->TestingClientDownloadSchedule = schedule;
|
||||
mock_get_options_calls = 0;
|
||||
/* we really want to test that it's equal to time(NULL) + delay0, but that's
|
||||
* an unrealiable test, because time(NULL) might change. */
|
||||
tt_assert(download_status_get_next_attempt_at(&dls_failure)
|
||||
>= current_time + delay0);
|
||||
tt_assert(download_status_get_next_attempt_at(&dls_failure)
|
||||
!= TIME_MAX);
|
||||
tt_assert(download_status_get_n_failures(&dls_failure) == 0);
|
||||
tt_assert(download_status_get_n_attempts(&dls_failure) == 0);
|
||||
tt_assert(mock_get_options_calls >= 1);
|
||||
|
||||
/* regression test for 17750: exponential, no initial delay */
|
||||
mock_options->TestingClientDownloadSchedule = schedule_no_initial_delay;
|
||||
mock_get_options_calls = 0;
|
||||
/* we really want to test that it's equal to time(NULL) + delay0, but that's
|
||||
* an unrealiable test, because time(NULL) might change. */
|
||||
tt_assert(download_status_get_next_attempt_at(&dls_exp)
|
||||
>= current_time + no_delay);
|
||||
tt_assert(download_status_get_next_attempt_at(&dls_exp)
|
||||
!= TIME_MAX);
|
||||
tt_assert(download_status_get_n_failures(&dls_exp) == 0);
|
||||
tt_assert(download_status_get_n_attempts(&dls_exp) == 0);
|
||||
tt_assert(mock_get_options_calls >= 1);
|
||||
|
||||
/* regression test for 17750: exponential, initial delay */
|
||||
mock_options->TestingClientDownloadSchedule = schedule;
|
||||
mock_get_options_calls = 0;
|
||||
/* we really want to test that it's equal to time(NULL) + delay0, but that's
|
||||
* an unrealiable test, because time(NULL) might change. */
|
||||
tt_assert(download_status_get_next_attempt_at(&dls_exp)
|
||||
>= current_time + delay0);
|
||||
tt_assert(download_status_get_next_attempt_at(&dls_exp)
|
||||
!= TIME_MAX);
|
||||
tt_assert(download_status_get_n_failures(&dls_exp) == 0);
|
||||
tt_assert(download_status_get_n_attempts(&dls_exp) == 0);
|
||||
tt_assert(mock_get_options_calls >= 1);
|
||||
|
||||
/* Check that a failure reset works */
|
||||
mock_get_options_calls = 0;
|
||||
download_status_reset(&dls_failure);
|
||||
@ -4313,7 +4426,7 @@ test_dir_download_status_increment(void *arg)
|
||||
tt_assert(next_at == TIME_MAX);
|
||||
tt_assert(download_status_get_n_failures(&dls_attempt) == 1);
|
||||
tt_assert(download_status_get_n_attempts(&dls_attempt) == 0);
|
||||
tt_assert(mock_get_options_calls == 0);
|
||||
tt_assert(mock_get_options_calls >= 1);
|
||||
|
||||
/* Check that an attempt reset works */
|
||||
mock_get_options_calls = 0;
|
||||
@ -4440,6 +4553,7 @@ test_dir_download_status_increment(void *arg)
|
||||
done:
|
||||
/* the pointers in schedule are allocated on the stack */
|
||||
smartlist_free(schedule);
|
||||
smartlist_free(schedule_no_initial_delay);
|
||||
UNMOCK(get_options);
|
||||
mock_options = NULL;
|
||||
mock_get_options_calls = 0;
|
||||
|
Loading…
Reference in New Issue
Block a user