diff --git a/ChangeLog b/ChangeLog index ece9aaf78a..9869479e53 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,4 +1,11 @@ Changes in version 0.2.0.13-alpha - 2007-11-?? + o Major bugfixes: + - Only update guard status (usable / not usable) once we have + enough directory information. This was causing us to always pick + two new guards on startup (bugfix on 0.2.0.9-alpha), and it was + causing us to discard all our guards on startup if we hadn't been + running for a few weeks (bugfix on 0.1.2.x). Fixes bug 448. + o Minor bugfixes: - The fix in 0.2.0.12-alpha cleared the "hsdir" flag in v3 network consensus documents when there are too many relays at a single diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 6b159ec6f5..995978b57e 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -1844,40 +1844,44 @@ build_state_get_exit_nickname(cpath_build_state_t *state) /** Check whether the entry guard e is usable, given the directory * authorities' opinion about the router (stored in ri) and the user's * configuration (in options). Set e->bad_since - * accordingly. Return true iff the entry guard's status changes. */ + * accordingly. Return true iff the entry guard's status changes. + * + * If it's not usable, set *reason to a static string explaining why. + */ static int entry_guard_set_status(entry_guard_t *e, routerinfo_t *ri, - time_t now, or_options_t *options) + time_t now, or_options_t *options, const char **reason) { - const char *reason = NULL; char buf[HEX_DIGEST_LEN+1]; int changed = 0; tor_assert(options); + *reason = NULL; + /* Do we want to mark this guard as bad? */ if (!ri) - reason = "unlisted"; + *reason = "unlisted"; else if (!ri->is_running) - reason = "down"; + *reason = "down"; else if (options->UseBridges && ri->purpose != ROUTER_PURPOSE_BRIDGE) - reason = "not a bridge"; + *reason = "not a bridge"; else if (!options->UseBridges && !ri->is_possible_guard && !router_nickname_is_in_list(ri, options->EntryNodes)) - reason = "not recommended as a guard"; + *reason = "not recommended as a guard"; else if (router_nickname_is_in_list(ri, options->ExcludeNodes)) - reason = "excluded"; + *reason = "excluded"; - if (reason && ! e->bad_since) { + if (*reason && ! e->bad_since) { /* Router is newly bad. */ base16_encode(buf, sizeof(buf), e->identity, DIGEST_LEN); log_info(LD_CIRC, "Entry guard %s (%s) is %s: marking as unusable.", - e->nickname, buf, reason); + e->nickname, buf, *reason); e->bad_since = now; control_event_guard(e->nickname, e->identity, "BAD"); changed = 1; - } else if (!reason && e->bad_since) { + } else if (!*reason && e->bad_since) { /* There's nothing wrong with the router any more. */ base16_encode(buf, sizeof(buf), e->identity, DIGEST_LEN); log_info(LD_CIRC, "Entry guard %s (%s) is no longer unusable: " @@ -2185,12 +2189,13 @@ remove_dead_entry_guards(void) * * An entry is 'down' if the directory lists it as nonrunning. * An entry is 'unlisted' if the directory doesn't include it. + * + * Don't call this on startup; only on a fresh download. Otherwise we'll + * think that things are unlisted. */ void entry_guards_compute_status(void) { - /* Don't call this on startup; only on a fresh download. Otherwise we'll - * think that things are unlisted. */ time_t now; int changed = 0; int severity = LOG_INFO; @@ -2205,13 +2210,18 @@ entry_guards_compute_status(void) SMARTLIST_FOREACH(entry_guards, entry_guard_t *, entry, { routerinfo_t *r = router_get_by_digest(entry->identity); - if (entry_guard_set_status(entry, r, now, options)) + const char *reason = NULL; + if (entry_guard_set_status(entry, r, now, options, &reason)) changed = 1; - log_info(LD_CIRC, "Summary: Entry '%s' is %s, %s and %s.", + if (entry->bad_since) + tor_assert(reason); + + log_info(LD_CIRC, "Summary: Entry '%s' is %s, %s%s, and %s.", entry->nickname, entry->unreachable_since ? "unreachable" : "reachable", - entry->bad_since ? "unusable" : "usable", + entry->bad_since ? "unusable: " : "usable", + entry->bad_since ? reason : "", entry_is_live(entry, 0, 1, 0) ? "live" : "not live"); }); diff --git a/src/or/directory.c b/src/or/directory.c index 34162ce6bc..4743f95782 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -1181,7 +1181,7 @@ load_downloaded_routers(const char *body, smartlist_t *which, } /** We are a client, and we've finished reading the server's - * response. Parse and it and act appropriately. + * response. Parse it and act appropriately. * * If we're still happy with using this directory server in the future, return * 0. Otherwise return -1; and the caller should consider trying the request diff --git a/src/or/main.c b/src/or/main.c index 775c5f656f..9d88b76217 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -663,6 +663,9 @@ directory_info_has_arrived(time_t now, int from_cache) update_router_descriptor_downloads(now); return; } else { + /* if we have enough dir info, then update our guard status with + * whatever we just learned. */ + entry_guards_compute_status(); /* Don't even bother trying to get extrainfo until the rest of our * directory info is up-to-date */ if (options->DownloadExtraInfo) diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c index 150a5d1b81..3ede96eafc 100644 --- a/src/or/networkstatus.c +++ b/src/or/networkstatus.c @@ -1423,7 +1423,6 @@ routers_update_all_from_networkstatus(time_t now, int dir_version) ri->cache_info.routerlist_index = ri_sl_idx); if (rl->old_routers) signed_descs_update_status_from_consensus_networkstatus(rl->old_routers); - entry_guards_compute_status(); me = router_get_my_routerinfo(); if (me && !have_warned_about_invalid_status) {