commit bea0a31c1c859a563f065d8b868570560abc5135 Author: Nick Mathewson <nickm@torproject.org> Date: Mon Nov 22 12:30:33 2010 -0500 Treat routers whose IPs have changed as having been down for MTBF/routerinfo calculation purposes. --- changes/bug1035 | 7 +++++++ src/or/dirserv.c | 16 +++++++++++----- src/or/rephist.c | 38 +++++++++++++++++++++++++++++++++++++- src/or/rephist.h | 3 ++- 4 files changed, 57 insertions(+), 7 deletions(-) diff --git a/changes/bug1035 b/changes/bug1035 new file mode 100644 index 0000000..4b62b46 --- /dev/null +++ b/changes/bug1035 @@ -0,0 +1,7 @@ + o Minor features (authorities) + - Take altered router IPs into account when determining router stability. + Previously, if a router changed its IP, the authorities would not + treat it as having any downtime for the purposes of stability + calculation, whereas clients would experience downtime since the + IP could take a while to propagate to them. Resolves issue 1035. + diff --git a/src/or/dirserv.c b/src/or/dirserv.c index 42d7d56..52e59cd 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -3108,19 +3108,25 @@ dirserv_orconn_tls_done(const char *address, tor_assert(address); tor_assert(digest_rcvd); - SMARTLIST_FOREACH(rl->routers, routerinfo_t *, ri, { + /* XXX023 Doing a loop like this is stupid. We should just look up the + * router by digest_rcvd, and see if address, orport, and as_advertised + * match up. -NM */ + SMARTLIST_FOREACH_BEGIN(rl->routers, routerinfo_t *, ri) { if (!strcasecmp(address, ri->address) && or_port == ri->or_port && as_advertised && !memcmp(ri->cache_info.identity_digest, digest_rcvd, DIGEST_LEN)) { /* correct digest. mark this router reachable! */ if (!bridge_auth || ri->purpose == ROUTER_PURPOSE_BRIDGE) { - log_info(LD_DIRSERV, "Found router %s to be reachable. Yay.", - ri->nickname); - rep_hist_note_router_reachable(digest_rcvd, now); + tor_addr_t addr, *addrp=NULL; + log_info(LD_DIRSERV, "Found router %s to be reachable at %s. Yay.", + ri->nickname, address); + if (tor_addr_from_str(&addr, ri->address) != -1) + addrp = &addr; + rep_hist_note_router_reachable(digest_rcvd, addrp, now); ri->last_reachable = now; } } - }); + } SMARTLIST_FOREACH_END(ri); /* FFFF Maybe we should reinstate the code that dumps routers with the same * addr/port but with nonmatching keys, but instead of dumping, we should * skip testing. */ diff --git a/src/or/rephist.c b/src/or/rephist.c index 22b3ec5..a0c9c9f 100644 --- a/src/or/rephist.c +++ b/src/or/rephist.c @@ -14,6 +14,7 @@ #include "circuitlist.h" #include "circuituse.h" #include "config.h" +#include "networkstatus.h" #include "rephist.h" #include "router.h" #include "routerlist.h" @@ -73,6 +74,10 @@ typedef struct or_history_t { /** If nonzero, we have been unable to connect since this time. */ time_t down_since; + /** The address at which we most recently connected to this OR + * sucessfully. */ + tor_addr_t last_reached_addr; + /* === For MTBF tracking: */ /** Weighted sum total of all times that this router has been online. */ @@ -119,6 +124,7 @@ get_or_history(const char* id) rephist_total_num++; hist->link_history_map = digestmap_new(); hist->since = hist->changed = time(NULL); + tor_addr_make_unspec(&hist->last_reached_addr); digestmap_set(history_map, id, hist); } return hist; @@ -289,14 +295,19 @@ rep_hist_note_connection_died(const char* id, time_t when) /** We have just decided that this router with identity digest <b>id</b> is * reachable, meaning we will give it a "Running" flag for the next while. */ void -rep_hist_note_router_reachable(const char *id, time_t when) +rep_hist_note_router_reachable(const char *id, const tor_addr_t *at_addr, + time_t when) { or_history_t *hist = get_or_history(id); int was_in_run = 1; char tbuf[ISO_TIME_LEN+1]; + int addr_changed; tor_assert(hist); + addr_changed = at_addr && + tor_addr_compare(at_addr, &hist->last_reached_addr, CMP_EXACT) != 0; + if (!started_tracking_stability) started_tracking_stability = time(NULL); if (!hist->start_of_run) { @@ -315,6 +326,29 @@ rep_hist_note_router_reachable(const char *id, time_t when) down_length = when - hist->start_of_downtime; hist->total_weighted_time += down_length; hist->start_of_downtime = 0; + } else if (addr_changed) { + /* If we're reachable, but the address changed, treat this as some + * downtime. */ +#define MIN_DOWNTIME_FOR_ADDR_CHANGE 600 + int penalty = get_options()->TestingTorNetwork ? 240 : 3600; + networkstatus_t *ns; + tor_assert(at_addr); + + if ((ns = networkstatus_get_latest_consensus())) { + int fresh_interval = ns->fresh_until - ns->valid_after; + int live_interval = ns->valid_until - ns->valid_after; + /* on average, a descriptor addr change takes .5 intervals to make it + * into a consensus, and half a liveness period to make it to + * clients. */ + penalty = (fresh_interval + live_interval) / 2; + } + format_local_iso_time(tbuf, hist->start_of_run); + log_info(LD_HIST,"Router %s still seems Running, but its address appears " + "to have changed since the last time it was reachable. I'm " + "going to treat it as having been down for %d seconds", + hex_str(id, DIGEST_LEN), penalty); + rep_hist_note_router_unreachable(id, when-penalty); + rep_hist_note_router_reachable(id, NULL, when); } else { format_local_iso_time(tbuf, hist->start_of_run); if (was_in_run) @@ -324,6 +358,8 @@ rep_hist_note_router_reachable(const char *id, time_t when) log_info(LD_HIST,"Router %s is now Running; it was previously untracked", hex_str(id, DIGEST_LEN)); } + if (at_addr) + tor_addr_copy(&hist->last_reached_addr, at_addr); } /** We have just decided that this router is unreachable, meaning diff --git a/src/or/rephist.h b/src/or/rephist.h index 8f5a34d..1ddbac4 100644 --- a/src/or/rephist.h +++ b/src/or/rephist.h @@ -33,7 +33,8 @@ void rep_hist_update_state(or_state_t *state); int rep_hist_load_state(or_state_t *state, char **err); void rep_history_clean(time_t before); -void rep_hist_note_router_reachable(const char *id, time_t when); +void rep_hist_note_router_reachable(const char *id, const tor_addr_t *at_addr, + time_t when); void rep_hist_note_router_unreachable(const char *id, time_t when); int rep_hist_record_mtbf_data(time_t now, int missing_means_down); int rep_hist_load_mtbf_data(time_t now);