From ac2f6b608a18a8595f62384788196d7c3f2875fd Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 23 Dec 2008 21:17:52 +0000 Subject: [PATCH] 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 --- ChangeLog | 6 ++++++ src/or/directory.c | 26 +++++++++++++++++--------- src/or/or.h | 13 ++++++++++++- src/or/routerlist.c | 17 +++++++++++++++-- 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 descriptor_digests * is true, or identity digests otherwise. Parse the descriptors, validate * them, and annotate them as having purpose purpose and as having been - * downloaded from source. */ -static void + * downloaded from source. + * + * 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 dl 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 requested_fingerprints 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 descriptor_digests 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 s (ending immediately before -- 2.11.4.GIT