Patch from Sebiastian for bug 888: mark a descriptor as "Impossible" if we reject it after downloading it so that we do not download it again

svn:r17756

4 files changed, 50 insertions, 12 deletions

diff --git a/ChangeLog b/ChangeLog
index 7e0fe2325f..762430c476 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -47,6 +47,12 @@ Changes in version 0.2.1.9-alpha - 2008-12-2?
       for every cell. On systems where time() is a slow syscalls, this will
       be slightly helpful.
     - Exit servers can now answer resolve requests for ip6.arpa addresses.
+    - When we download a descriptor that we then immediately (as a directory
+      authority) reject, do not retry downloading it right away.  Should
+      save some bandwidth on authorities.  Fix for bug 888.  Patch by
+      Sebastian Hahn.
+    - When a download gets us zero good descriptors, do not notify Tor that
+      new directory information has arrived.
 
   o Minor features (controller):
     - New CONSENSUS_ARRIVED event to note when a new consensus has
diff --git a/src/or/directory.c b/src/or/directory.c
index b383ad5e89..aa9b76c68f 100644
--- a/src/or/directory.c
+++ b/src/or/directory.c
@@ -1349,8 +1349,10 @@ body_is_plausible(const char *body, size_t len, int purpose)
  * descriptors we requested: descriptor digests if <b>descriptor_digests</b>
  * is true, or identity digests otherwise.  Parse the descriptors, validate
  * them, and annotate them as having purpose <b>purpose</b> and as having been
- * downloaded from <b>source</b>. */
-static void
+ * downloaded from <b>source</b>.
+ *
+ * Return the number of routers actually added. */
+static int
 load_downloaded_routers(const char *body, smartlist_t *which,
                         int descriptor_digests,
                         int router_purpose,
@@ -1358,6 +1360,7 @@ load_downloaded_routers(const char *body, smartlist_t *which,
 {
   char buf[256];
   char time_buf[ISO_TIME_LEN+1];
+  int added = 0;
   int general = router_purpose == ROUTER_PURPOSE_GENERAL;
   format_iso_time(time_buf, time(NULL));
   tor_assert(source);
@@ -1369,12 +1372,13 @@ load_downloaded_routers(const char *body, smartlist_t *which,
                    !general ? "@purpose " : "",
                    !general ? router_purpose_to_string(router_purpose) : "",
                    !general ? "\n" : "")<0)
-    return;
+    return added;
 
-  router_load_routers_from_string(body, NULL, SAVED_NOWHERE, which,
+  added = router_load_routers_from_string(body, NULL, SAVED_NOWHERE, which,
                                   descriptor_digests, buf);
   control_event_bootstrap(BOOTSTRAP_STATUS_LOADING_DESCRIPTORS,
                           count_loading_descriptors_progress());
+  return added;
 }
 
 /** We are a client, and we've finished reading the server's
@@ -1772,10 +1776,10 @@ connection_dir_client_reached_eof(dir_connection_t *conn)
       } else {
         //router_load_routers_from_string(body, NULL, SAVED_NOWHERE, which,
         //                       descriptor_digests, conn->router_purpose);
-        load_downloaded_routers(body, which, descriptor_digests,
+        if (load_downloaded_routers(body, which, descriptor_digests,
                                 conn->router_purpose,
-                                conn->_base.address);
-        directory_info_has_arrived(now, 0);
+                                conn->_base.address))
+          directory_info_has_arrived(now, 0);
       }
     }
     if (which) { /* mark remaining ones as failed */
@@ -3297,8 +3301,10 @@ download_status_increment_failure(download_status_t *dls, int status_code,
   size_t schedule_len;
   int increment;
   tor_assert(dls);
-  if (status_code != 503 || server)
-    ++dls->n_download_failures;
+  if (status_code != 503 || server) {
+    if (dls->n_download_failures < IMPOSSIBLE_TO_DOWNLOAD)
+      ++dls->n_download_failures;
+  }
 
   switch (dls->schedule) {
     case DL_SCHED_GENERIC:
@@ -3325,6 +3331,8 @@ download_status_increment_failure(download_status_t *dls, int status_code,
 
   if (dls->n_download_failures < schedule_len)
     increment = schedule[dls->n_download_failures];
+  else if (dls->n_download_failures == IMPOSSIBLE_TO_DOWNLOAD)
+    increment = TIME_MAX;
   else
     increment = schedule[schedule_len-1];
 
diff --git a/src/or/or.h b/src/or/or.h
index 1082a79c6a..423a42ae74 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -1299,6 +1299,9 @@ typedef struct download_status_t {
   download_schedule_t schedule : 8;
 } download_status_t;
 
+/** If n_download_failures is this high, the download can never happen. */
+#define IMPOSSIBLE_TO_DOWNLOAD 255
+
 /** The max size we expect router descriptor annotations we create to
  * be. We'll accept larger ones if we see them on disk, but we won't
  * create any that are larger than this. */
@@ -3361,6 +3364,14 @@ download_status_is_ready(download_status_t *dls, time_t now,
           && dls->next_attempt_at <= now);
 }
 
+static void download_status_mark_impossible(download_status_t *dl);
+/** Mark <b>dl</b> as never downloadable. */
+static INLINE void
+download_status_mark_impossible(download_status_t *dl)
+{
+  dl->n_download_failures = IMPOSSIBLE_TO_DOWNLOAD;
+}
+
 /********************************* dirserv.c ***************************/
 /** Maximum length of an exit policy summary. */
 #define MAX_EXITPOLICY_SUMMARY_LEN (1000)
@@ -4424,7 +4435,7 @@ was_router_added_t router_add_extrainfo_to_routerlist(
 void routerlist_remove_old_routers(void);
 int router_load_single_router(const char *s, uint8_t purpose, int cache,
                               const char **msg);
-void router_load_routers_from_string(const char *s, const char *eos,
+int router_load_routers_from_string(const char *s, const char *eos,
                                      saved_location_t saved_location,
                                      smartlist_t *requested_fingerprints,
                                      int descriptor_digests,
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index 7004d938a8..f91af473f2 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -3441,6 +3441,8 @@ router_load_single_router(const char *s, uint8_t purpose, int cache,
  * are in response to a query to the network: cache them by adding them to
  * the journal.
  *
+ * Return the number of routers actually added.
+ *
  * If <b>requested_fingerprints</b> is provided, it must contain a list of
  * uppercased fingerprints.  Do not update any router whose
  * fingerprint is not on the list; after updating a router, remove its
@@ -3449,7 +3451,7 @@ router_load_single_router(const char *s, uint8_t purpose, int cache,
  * If <b>descriptor_digests</b> is non-zero, then the requested_fingerprints
  * are descriptor digests. Otherwise they are identity digests.
  */
-void
+int
 router_load_routers_from_string(const char *s, const char *eos,
                                 saved_location_t saved_location,
                                 smartlist_t *requested_fingerprints,
@@ -3494,10 +3496,19 @@ router_load_routers_from_string(const char *s, const char *eos,
 
     r = router_add_to_routerlist(ri, &msg, from_cache, !from_cache);
     if (WRA_WAS_ADDED(r)) {
-      any_changed = 1;
+      any_changed++;
       smartlist_add(changed, ri);
       routerlist_descriptors_added(changed, from_cache);
       smartlist_clear(changed);
+    } else if (WRA_WAS_REJECTED(r)) {
+      download_status_t *dl_status;
+      dl_status = router_get_dl_status_by_descriptor_digest(
+          ri->cache_info.signed_descriptor_digest);
+      if (dl_status) {
+        log_info(LD_GENERAL, "Marking router %s as never downloadable",
+            ri->cache_info.signed_descriptor_digest);
+        download_status_mark_impossible(dl_status);
+      }
     }
   } SMARTLIST_FOREACH_END(ri);
 
@@ -3508,6 +3519,8 @@ router_load_routers_from_string(const char *s, const char *eos,
 
   smartlist_free(routers);
   smartlist_free(changed);
+
+  return any_changed;
 }
 
 /** Parse one or more extrainfos from <b>s</b> (ending immediately before