commit 9b64227ffd38e9406c5c88ace137a0eae010771d Author: Sebastian Hahn sebastian@torproject.org Date: Mon Feb 7 16:31:20 2011 +0100

Routers count as down when they change ORPort, too

rransom noticed that a change of ORPort is just as bad as a change of IP address from a client's perspective, because both mean that the relay is not available to them while the new information hasn't propagated. Change the bug1035 fix accordingly.

Also make sure we don't log a bridge's IP address (which might happen when we are the bridge authority). --- changes/bug1035 | 11 ++++++----- src/or/dirserv.c | 8 +++++--- src/or/rephist.c | 23 +++++++++++++++-------- src/or/rephist.h | 2 +- 4 files changed, 27 insertions(+), 17 deletions(-)

diff --git a/changes/bug1035 b/changes/bug1035 index 041e3b3..3d86330 100644 --- a/changes/bug1035 +++ b/changes/bug1035 @@ -1,9 +1,10 @@ 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. + - Take altered router IP addresses and ORPorts into account when + determining router stability. Previously, if a router changed + its IP or ORPort, the authorities would not treat it as having + any downtime for the purposes of stability calculation, whereas + clients would experience downtime since the change could take a + while to propagate to them. Resolves issue 1035. o Minor bugfixes (authorities) - Try to be more robust to hops back in time when calculating router stability. Previously, if a run of uptime or downtime diff --git a/src/or/dirserv.c b/src/or/dirserv.c index 52e59cd..f426881 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -3118,11 +3118,13 @@ dirserv_orconn_tls_done(const char *address, /* correct digest. mark this router reachable! */ if (!bridge_auth || ri->purpose == ROUTER_PURPOSE_BRIDGE) { tor_addr_t addr, *addrp=NULL; - log_info(LD_DIRSERV, "Found router %s to be reachable at %s. Yay.", - ri->nickname, address); + log_info(LD_DIRSERV, "Found router %s to be reachable at %s:%d. Yay.", + ri->nickname, address, ri->or_port ); if (tor_addr_from_str(&addr, ri->address) != -1) addrp = &addr; - rep_hist_note_router_reachable(digest_rcvd, addrp, now); + else + log_warn(LD_BUG, "Couldn't parse IP address "%s"", ri->address); + rep_hist_note_router_reachable(digest_rcvd, addrp, or_port, now); ri->last_reachable = now; } } diff --git a/src/or/rephist.c b/src/or/rephist.c index 2869990..7c570e2 100644 --- a/src/or/rephist.c +++ b/src/or/rephist.c @@ -78,6 +78,9 @@ typedef struct or_history_t { * successfully. */ tor_addr_t last_reached_addr;

+ /** The port at which we most recently connected to this OR successfully */ + uint16_t last_reached_port; + /* === For MTBF tracking: */ /** Weighted sum total of all times that this router has been online. */ @@ -296,17 +299,18 @@ rep_hist_note_connection_died(const char* id, time_t when) * reachable, meaning we will give it a "Running" flag for the next while. */ void rep_hist_note_router_reachable(const char *id, const tor_addr_t *at_addr, - time_t when) + const uint16_t at_port, 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; + int addr_changed, port_changed;

tor_assert(hist);

addr_changed = at_addr && tor_addr_compare(at_addr, &hist->last_reached_addr, CMP_EXACT) != 0; + port_changed = at_port && at_port != hist->last_reached_port;

if (!started_tracking_stability) started_tracking_stability = time(NULL); @@ -326,7 +330,7 @@ rep_hist_note_router_reachable(const char *id, const tor_addr_t *at_addr, down_length = when - hist->start_of_downtime; hist->total_weighted_time += down_length; hist->start_of_downtime = 0; - } else if (addr_changed) { + } else if (addr_changed || port_changed) { /* If we're reachable, but the address changed, treat this as some * downtime. */ int penalty = get_options()->TestingTorNetwork ? 240 : 3600; @@ -342,12 +346,13 @@ rep_hist_note_router_reachable(const char *id, const tor_addr_t *at_addr, penalty = (int)(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); + if (!authdir_mode_bridge(get_options())) + 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); + rep_hist_note_router_reachable(id, NULL, 0, when); } else { format_local_iso_time(tbuf, hist->start_of_run); if (was_in_run) @@ -359,6 +364,8 @@ rep_hist_note_router_reachable(const char *id, const tor_addr_t *at_addr, } if (at_addr) tor_addr_copy(&hist->last_reached_addr, at_addr); + if (at_port) + hist->last_reached_port = at_port; }

/** We have just decided that this router is unreachable, meaning diff --git a/src/or/rephist.h b/src/or/rephist.h index 1ddbac4..610c1a0 100644 --- a/src/or/rephist.h +++ b/src/or/rephist.h @@ -34,7 +34,7 @@ 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, const tor_addr_t *at_addr, - time_t when); + uint16_t at_port, 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);