From d118d5069a183574c8d03a7e9e5559e47a9ddaa4 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sun, 18 Sep 2005 02:51:12 +0000 Subject: [PATCH] Prevent a "try forever to download an unobtainable descriptor" bug. Still remaining: the "give up forever" bug. svn:r5090 --- doc/TODO | 5 +++-- src/or/directory.c | 13 ++++++++++--- src/or/or.h | 7 ++++++- src/or/routerlist.c | 22 +++++++++++++--------- src/or/routerparse.c | 2 -- 5 files changed, 32 insertions(+), 17 deletions(-) diff --git a/doc/TODO b/doc/TODO index 965f08406c..380c2dc3ae 100644 --- a/doc/TODO +++ b/doc/TODO @@ -133,7 +133,7 @@ R - check reachability as soon as you hear about a new server X By 'if-newer-than' (Does the spec require this??) o Support compression. N - Alice acts on network-status objects - . Alice downloads descriptors as needed. + o Alice downloads descriptors as needed. o Figure out what's needed o Store it o Implement store @@ -146,7 +146,8 @@ N - Alice acts on network-status objects o Call directory_has_arrived as needed; rename it. o Set has_fetched_directory properly. o Retry descriptors on failure - - Give up after a while. + o Give up after a while. + - But try again after a long while (???) - Check software versions according to some sane plan. o Alice sets descriptor status from network-status o Implement diff --git a/src/or/directory.c b/src/or/directory.c index 031828be8d..3bf2cf857d 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -283,7 +283,7 @@ connection_dir_request_failed(connection_t *conn) log_fn(LOG_INFO, "Giving up on directory server at '%s'; retrying", conn->address); connection_dir_download_networkstatus_failed(conn); - } else if (conn->purpose == DIR_PURPOSE_FETCH_NETWORKSTATUS) { + } else if (conn->purpose == DIR_PURPOSE_FETCH_SERVERDESC) { log_fn(LOG_INFO, "Giving up on directory server at '%s'; retrying", conn->address); connection_dir_download_routerdesc_failed(conn); @@ -320,7 +320,8 @@ connection_dir_download_networkstatus_failed(connection_t *conn) static void connection_dir_download_routerdesc_failed(connection_t *conn) { - /* try again. */ + /* Try again. */ + /*XXXX011 plays poorly with multiple conns. */ update_router_descriptor_downloads(time(NULL)); } @@ -1547,7 +1548,13 @@ dir_networkstatus_download_failed(smartlist_t *failed) static void dir_routerdesc_download_failed(smartlist_t *failed) { - /* XXXX writeme! Give up after a while! */ + SMARTLIST_FOREACH(failed, const char *, cp, + { + routerstatus_t *rs = router_get_combined_status_by_digest(cp); + if (!rs || rs->n_download_failures >= MAX_ROUTERDESC_DOWNLOAD_FAILURES) + continue; + ++rs->n_download_failures; + }); } /* DOCDOC */ diff --git a/src/or/or.h b/src/or/or.h index 14004a9020..a710df06ae 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -805,6 +805,9 @@ typedef struct routerstatus_t { * recent descriptor. */ } routerstatus_t; +/*XXXX001 make this configurable! */ +#define MAX_ROUTERDESC_DOWNLOAD_FAILURES 16 + /** Contents of a (v2 or later) network status object. */ typedef struct networkstatus_t { /** When did we receive the network-status document? */ @@ -2107,11 +2110,13 @@ void add_trusted_dir_server(const char *addr, uint16_t port, const char *digest, int supports_v1); void clear_trusted_dir_servers(void); networkstatus_t *networkstatus_get_by_digest(const char *digest); +routerstatus_t *router_get_combined_status_by_digest(const char *digest); void update_networkstatus_cache_downloads(time_t now); void update_networkstatus_client_downloads(time_t now); void update_router_descriptor_downloads(time_t now); void routers_update_all_from_networkstatus(void); -void routers_update_status_from_networkstatus(smartlist_t *routers); +void routers_update_status_from_networkstatus(smartlist_t *routers, + int reset_failures); smartlist_t *router_list_superseded(void); int router_have_minimum_dir_info(void); void networkstatus_list_update_recent(time_t now); diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 9f3068fa34..e4d4f2d92f 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -29,7 +29,6 @@ static trusted_dir_server_t *router_pick_trusteddirserver_impl( static void mark_all_trusteddirservers_up(void); static int router_nickname_is_in_list(routerinfo_t *router, const char *list); static int router_nickname_matches(routerinfo_t *router, const char *nickname); -static routerstatus_t *router_get_combined_status_by_digest(const char *d); static void routerstatus_list_update_from_networkstatus(time_t now); /****************************************************************************/ @@ -1292,7 +1291,7 @@ router_load_routers_from_string(const char *s, int from_cache, router_parse_list_from_string(&s, routers); - routers_update_status_from_networkstatus(routers); + routers_update_status_from_networkstatus(routers, !from_cache); SMARTLIST_FOREACH(routers, routerinfo_t *, ri, { @@ -1554,7 +1553,8 @@ networkstatus_find_entry(networkstatus_t *ns, const char *digest) _compare_digest_to_routerstatus_entry); } -static routerstatus_t * +/*DOCDOC*/ +routerstatus_t * router_get_combined_status_by_digest(const char *digest) { if (!routerstatus_list) @@ -1563,7 +1563,6 @@ router_get_combined_status_by_digest(const char *digest) _compare_digest_to_routerstatus_entry); } - /* XXXX These should be configurable, perhaps? NM */ #define AUTHORITY_NS_CACHE_INTERVAL 10*60 #define NONAUTHORITY_NS_CACHE_INTERVAL 15*60 @@ -1995,7 +1994,7 @@ routers_update_all_from_networkstatus(void) if (networkstatus_list_has_changed) routerstatus_list_update_from_networkstatus(now); - routers_update_status_from_networkstatus(routerlist->routers); + routers_update_status_from_networkstatus(routerlist->routers, 0); me = router_get_my_routerinfo(); if (me) { @@ -2003,7 +2002,7 @@ routers_update_all_from_networkstatus(void) * dirservers list us as named, valid, etc. */ smartlist_t *lst = smartlist_create(); smartlist_add(lst, me); - routers_update_status_from_networkstatus(lst); + routers_update_status_from_networkstatus(lst, 1); if (me->is_verified == 0) { log_fn(LOG_WARN, "Many directory servers list us as unverified. Please consider sending your identity fingerprint to the tor-ops."); have_warned_about_unverified_status = 1; @@ -2179,7 +2178,7 @@ routerstatus_list_update_from_networkstatus(time_t now) * is_named, is_verified, and is_running fields according to our current * networkstatus_t documents. */ void -routers_update_status_from_networkstatus(smartlist_t *routers) +routers_update_status_from_networkstatus(smartlist_t *routers, int reset_failures) { trusted_dir_server_t *ds; routerstatus_t *rs; @@ -2198,6 +2197,9 @@ routers_update_status_from_networkstatus(smartlist_t *routers) if (!rs) continue; + if (reset_failures) + rs->n_download_failures = 0; + if (!namingdir) router->is_named = rs->is_named; @@ -2250,8 +2252,10 @@ router_list_downloadable(void) // log_fn(LOG_NOTICE, "No status for %s", fp); continue; } - if (!memcmp(ri->signed_descriptor_digest,rs->descriptor_digest,DIGEST_LEN) - || rs->published_on <= ri->published_on) { + /*XXXX001 reset max_routerdesc_download_failures somewhere! */ + if (rs->n_download_failures >= MAX_ROUTERDESC_DOWNLOAD_FAILURES || + !memcmp(ri->signed_descriptor_digest,rs->descriptor_digest,DIGEST_LEN)|| + rs->published_on <= ri->published_on) { /* Same digest, or earlier. No need to download it. */ // log_fn(LOG_NOTICE, "Up-to-date status for %s", fp); strmap_remove(status_map, fp); diff --git a/src/or/routerparse.c b/src/or/routerparse.c index b3f780c7ba..58342d3df2 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -692,8 +692,6 @@ router_parse_list_from_string(const char **s, smartlist_t *dest) smartlist_add(routers, router); } - routers_update_status_from_networkstatus(routers); - smartlist_add_all(dest, routers); return 0;