[tor-commits] [tor/release-0.3.1] Stop relays calling directory_fetches_from_authorities on dir downloads

[nickm at torproject.org]

commit c86013291bd26f3497e9efcf96125342d97cba33
Author: teor <teor2345 at gmail.com>
Date:   Mon Sep 11 14:21:48 2017 +1000

    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(-)

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;
         }
       }