From 631ec5c4fe4d5535d91e8e1e3597fbaa687b8790 Mon Sep 17 00:00:00 2001 From: Linus Nordberg Date: Tue, 27 Mar 2012 15:00:34 +0200 Subject: [PATCH] Move last_reachable and testing_since from routerinfo_t to node_t. --- changes/bug5529 | 3 +++ src/or/dirserv.c | 31 +++++++++++++++---------- src/or/nodelist.c | 55 ++++++++++++++++++++++++++++++++++++++++----- src/or/nodelist.h | 3 ++- src/or/or.h | 17 +++++++------- src/or/routerlist.c | 10 ++------- 6 files changed, 84 insertions(+), 35 deletions(-) create mode 100644 changes/bug5529 diff --git a/changes/bug5529 b/changes/bug5529 new file mode 100644 index 0000000000..3f56e82047 --- /dev/null +++ b/changes/bug5529 @@ -0,0 +1,3 @@ + o Code refactoring: + - Move last_reachable and testing_since from routerinfo_t to + node_t. Implements enhancement 5529. diff --git a/src/or/dirserv.c b/src/or/dirserv.c index e21f5113f2..8f65b7fce4 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -988,7 +988,7 @@ dirserv_set_router_is_running(routerinfo_t *router, time_t now) answer = ! we_are_hibernating(); } else if (router->is_hibernating && (router->cache_info.published_on + - HIBERNATION_PUBLICATION_SKEW) > router->last_reachable) { + HIBERNATION_PUBLICATION_SKEW) > node->last_reachable) { /* A hibernating router is down unless we (somehow) had contact with it * since it declared itself to be hibernating. */ answer = 0; @@ -998,7 +998,7 @@ dirserv_set_router_is_running(routerinfo_t *router, time_t now) } else { /* Otherwise, a router counts as up if we found it reachable in the last REACHABLE_TIMEOUT seconds. */ - answer = (now < router->last_reachable + REACHABLE_TIMEOUT); + answer = (now < node->last_reachable + REACHABLE_TIMEOUT); } if (!answer && running_long_enough_to_decide_unreachable()) { @@ -1010,9 +1010,9 @@ dirserv_set_router_is_running(routerinfo_t *router, time_t now) it. */ time_t when = now; - if (router->last_reachable && - router->last_reachable + REACHABILITY_TEST_CYCLE_PERIOD < now) - when = router->last_reachable + REACHABILITY_TEST_CYCLE_PERIOD; + if (node->last_reachable && + node->last_reachable + REACHABILITY_TEST_CYCLE_PERIOD < now) + when = node->last_reachable + REACHABILITY_TEST_CYCLE_PERIOD; rep_hist_note_router_unreachable(router->cache_info.identity_digest, when); } @@ -3277,15 +3277,17 @@ dirserv_orconn_tls_done(const char *address, uint16_t or_port, const char *digest_rcvd) { - routerinfo_t *ri; + node_t *node = NULL; + routerinfo_t *ri = NULL; time_t now = time(NULL); tor_assert(address); tor_assert(digest_rcvd); - ri = router_get_mutable_by_digest(digest_rcvd); - - if (ri == NULL) + node = node_get_mutable_by_id(digest_rcvd); + if (node == NULL) return; + ri = node->ri; + tor_assert(ri); if (!strcasecmp(address, ri->address) && or_port == ri->or_port) { /* Found the right router. */ @@ -3302,7 +3304,7 @@ dirserv_orconn_tls_done(const char *address, 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; + node->last_reachable = now; } } } @@ -3338,12 +3340,17 @@ dirserv_should_launch_reachability_test(const routerinfo_t *ri, void dirserv_single_reachability_test(time_t now, routerinfo_t *router) { + node_t *node = NULL; tor_addr_t router_addr; + + tor_assert(router); + node = node_get_mutable_by_id(router->cache_info.identity_digest); + tor_assert(node); log_debug(LD_OR,"Testing reachability of %s at %s:%u.", router->nickname, router->address, router->or_port); /* Remember when we started trying to determine reachability */ - if (!router->testing_since) - router->testing_since = now; + if (!node->testing_since) + node->testing_since = now; tor_addr_from_ipv4h(&router_addr, router->addr); connection_or_connect(&router_addr, router->or_port, router->cache_info.identity_digest); diff --git a/src/or/nodelist.c b/src/or/nodelist.c index d17850888d..1e07c82e35 100644 --- a/src/or/nodelist.c +++ b/src/or/nodelist.c @@ -115,13 +115,58 @@ node_get_or_create(const char *identity_digest) return node; } -/** Add ri to the nodelist. */ +/** Replace old router with new in nodelist. If + * old and new in fact are the same relays (having the + * same identity_digest) the node_t of old is used for + * new. Otherwise the node_t of old is dropped and + * new gets a new one (which might be a recycled node_t in + * case we already have one matching its identity). + */ node_t * -nodelist_add_routerinfo(routerinfo_t *ri) +nodelist_replace_routerinfo(routerinfo_t *old, routerinfo_t *new) { - node_t *node; - init_nodelist(); - node = node_get_or_create(ri->cache_info.identity_digest); + node_t *node = NULL; + tor_assert(old); + tor_assert(new); + + if (tor_memeq(old->cache_info.identity_digest, + new->cache_info.identity_digest, DIGEST_LEN)) { + /* NEW == OLD, reuse node_t. */ + node = node_get_mutable_by_id(old->cache_info.identity_digest); + if (node) { + tor_assert(node->ri == old); + /* XXXX prop186 we may have more than one address. */ + if (!routers_have_same_or_addr(old, new)) { + /* These mustn't carry over when the address and orport + change. */ + node->last_reachable = 0; + node->testing_since = 0; + } + } + } else { + /* NEW != OLD, get a new node_t. */ + nodelist_remove_routerinfo(old); + } + node = nodelist_add_routerinfo(node, new); + + return node; +} + + +/** Add ri to the nodelist. If node_in is not NULL, use + that node rather than creating a new. */ +node_t * +nodelist_add_routerinfo(node_t *node_in, routerinfo_t *ri) +{ + node_t *node = NULL; + + if (node_in) { + node = node_in; + } else { + tor_assert(ri); + init_nodelist(); + node = node_get_or_create(ri->cache_info.identity_digest); + } node->ri = ri; if (node->country == -1) diff --git a/src/or/nodelist.h b/src/or/nodelist.h index 1e9da88d4e..b110fe52f7 100644 --- a/src/or/nodelist.h +++ b/src/or/nodelist.h @@ -15,7 +15,8 @@ node_t *node_get_mutable_by_id(const char *identity_digest); const node_t *node_get_by_id(const char *identity_digest); const node_t *node_get_by_hex_id(const char *identity_digest); -node_t *nodelist_add_routerinfo(routerinfo_t *ri); +node_t *nodelist_replace_routerinfo(routerinfo_t *old, routerinfo_t *new); +node_t *nodelist_add_routerinfo(node_t *node, routerinfo_t *ri); node_t *nodelist_add_microdesc(microdesc_t *md); void nodelist_set_consensus(networkstatus_t *ns); diff --git a/src/or/or.h b/src/or/or.h index 3a53e5ed86..a330f770e8 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -1793,15 +1793,6 @@ typedef struct { * things; see notes on ROUTER_PURPOSE_* macros above. */ uint8_t purpose; - - /* The below items are used only by authdirservers for - * reachability testing. */ - - /** When was the last time we could reach this OR? */ - time_t last_reachable; - /** When did we start testing reachability for this OR? */ - time_t testing_since; - } routerinfo_t; /** Information needed to keep and cache a signed extra-info document. */ @@ -2037,6 +2028,14 @@ typedef struct node_t { /** According to the geoip db what country is this router in? */ country_t country; + + /* The below items are used only by authdirservers for + * reachability testing. */ + + /** When was the last time we could reach this OR? */ + time_t last_reachable; /* IPv4 */ + /** When did we start testing reachability for this OR? */ + time_t testing_since; /* IPv4 */ } node_t; /** How many times will we try to download a router's descriptor before giving diff --git a/src/or/routerlist.c b/src/or/routerlist.c index de1a66ce16..f984d93e8e 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -2875,7 +2875,7 @@ routerlist_insert(routerlist_t *rl, routerinfo_t *ri) &ri->cache_info); smartlist_add(rl->routers, ri); ri->cache_info.routerlist_index = smartlist_len(rl->routers) - 1; - nodelist_add_routerinfo(ri); + nodelist_add_routerinfo(NULL, ri); router_dir_info_changed(); #ifdef DEBUG_ROUTERLIST routerlist_assert_ok(rl); @@ -3104,8 +3104,7 @@ routerlist_replace(routerlist_t *rl, routerinfo_t *ri_old, tor_assert(0 <= idx && idx < smartlist_len(rl->routers)); tor_assert(smartlist_get(rl->routers, idx) == ri_old); - nodelist_remove_routerinfo(ri_old); - nodelist_add_routerinfo(ri_new); + nodelist_replace_routerinfo(ri_old, ri_new); router_dir_info_changed(); if (idx >= 0) { @@ -3442,11 +3441,6 @@ router_add_to_routerlist(routerinfo_t *router, const char **msg, /* Same key, and either new, or listed in the consensus. */ log_debug(LD_DIR, "Replacing entry for router %s", router_describe(router)); - if (routers_have_same_or_addr(router, old_router)) { - /* these carry over when the address and orport are unchanged. */ - router->last_reachable = old_router->last_reachable; - router->testing_since = old_router->testing_since; - } routerlist_replace(routerlist, old_router, router); if (!from_cache) { signed_desc_append_to_journal(&router->cache_info,