From c86013291bd26f3497e9efcf96125342d97cba33 Mon Sep 17 00:00:00 2001 From: teor Date: Mon, 11 Sep 2017 14:21:48 +1000 Subject: [PATCH] Stop relays calling directory_fetches_from_authorities on dir downloads This change refactors find_dl_schedule() to only call dependent functions as needed. In particular, directory_fetches_from_authorities() only needs to be called on clients. Stopping spurious directory_fetches_from_authorities() calls on every download on public relays has the following impacts: * fewer address resolution attempts, particularly those mentioned in 21789 * fewer descriptor rebuilds * fewer log messages, particularly those limited in 20610 Fixes 23470 in 0.2.8.1-alpha. The original bug was introduced in commit 35bbf2e as part of prop210. --- changes/bug23470 | 6 ++++++ src/or/directory.c | 22 +++++++++++----------- 2 files changed, 17 insertions(+), 11 deletions(-) create mode 100644 changes/bug23470 diff --git a/changes/bug23470 b/changes/bug23470 new file mode 100644 index 000000000..33367b3a3 --- /dev/null +++ b/changes/bug23470 @@ -0,0 +1,6 @@ + o Minor bugfix (relay address resolution): + - Avoid unnecessary calls to directory_fetches_from_authorities() + on relays. This avoids spurious address resolutions and + descriptor rebuilds. This is a mitigation for 21789. The original + bug was introduced in commit 35bbf2e as part of prop210. + Fixes 23470 in 0.2.8.1-alpha. diff --git a/src/or/directory.c b/src/or/directory.c index fce48c6e9..f285e4c6e 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -3703,26 +3703,24 @@ connection_dir_finished_connecting(dir_connection_t *conn) STATIC const smartlist_t * find_dl_schedule(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( - options); - const int we_are_bootstrapping = networkstatus_consensus_is_bootstrapping( - time(NULL)); - const int use_fallbacks = networkstatus_consensus_can_use_extra_fallbacks( - options); switch (dls->schedule) { case DL_SCHED_GENERIC: - if (dir_server) { + /* Any other directory document */ + if (dir_server_mode(options)) { + /* A directory authority or directory mirror */ return options->TestingServerDownloadSchedule; } else { return options->TestingClientDownloadSchedule; } case DL_SCHED_CONSENSUS: - if (!multi_d) { + if (!networkstatus_consensus_can_use_multiple_directories(options)) { + /* A public relay */ return options->TestingServerConsensusDownloadSchedule; } else { - if (we_are_bootstrapping) { - if (!use_fallbacks) { + /* A client or bridge */ + if (networkstatus_consensus_is_bootstrapping(time(NULL))) { + /* During bootstrapping */ + if (!networkstatus_consensus_can_use_extra_fallbacks(options)) { /* A bootstrapping client without extra fallback directories */ return options->ClientBootstrapConsensusAuthorityOnlyDownloadSchedule; @@ -3738,6 +3736,8 @@ find_dl_schedule(download_status_t *dls, const or_options_t *options) options->ClientBootstrapConsensusFallbackDownloadSchedule; } } else { + /* A client with a reasonably live consensus, with or without + * certificates */ return options->TestingClientConsensusDownloadSchedule; } }