diff options
| -rw-r--r-- | changes/bug1882 | 4 | ||||
| -rw-r--r-- | src/or/circuitbuild.c | 76 | ||||
| -rw-r--r-- | src/or/circuitbuild.h | 8 | ||||
| -rw-r--r-- | src/or/circuituse.c | 21 | ||||
| -rw-r--r-- | src/or/connection_or.c | 45 | ||||
| -rw-r--r-- | src/or/connection_or.h | 2 | ||||
| -rw-r--r-- | src/or/main.c | 6 | ||||
| -rw-r--r-- | src/or/or.h | 5 | ||||
| -rw-r--r-- | src/or/routerlist.c | 2 |
9 files changed, 99 insertions, 70 deletions
diff --git a/changes/bug1882 b/changes/bug1882 new file mode 100644 index 000000000..d495d2f41 --- /dev/null +++ b/changes/bug1882 @@ -0,0 +1,4 @@ + o Minor features: + - If we've configured EntryNodes and our network goes away and/or all + our entrynodes get marked down, optimistically retry them all when + a new socks application request appears. Fixes bug 1882. diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index d696fe1b9..2b4540bf3 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -3209,8 +3209,6 @@ entry_guard_set_status(entry_guard_t *e, routerinfo_t *ri, char buf[HEX_DIGEST_LEN+1]; int changed = 0; - tor_assert(options); - *reason = NULL; /* Do we want to mark this guard as bad? */ @@ -3468,9 +3466,8 @@ add_an_entry_guard(routerinfo_t *chosen, int reset_status) /** If the use of entry guards is configured, choose more entry guards * until we have enough in the list. */ static void -pick_entry_guards(void) +pick_entry_guards(or_options_t *options) { - or_options_t *options = get_options(); int changed = 0; tor_assert(entry_guards); @@ -3502,10 +3499,9 @@ entry_guard_free(entry_guard_t *e) * or which was selected by a version of Tor that's known to select * entry guards badly. */ static int -remove_obsolete_entry_guards(void) +remove_obsolete_entry_guards(time_t now) { int changed = 0, i; - time_t now = time(NULL); for (i = 0; i < smartlist_len(entry_guards); ++i) { entry_guard_t *entry = smartlist_get(entry_guards, i); @@ -3565,11 +3561,10 @@ remove_obsolete_entry_guards(void) * long that we don't think they'll come up again. Return 1 if we * removed any, or 0 if we did nothing. */ static int -remove_dead_entry_guards(void) +remove_dead_entry_guards(time_t now) { char dbuf[HEX_DIGEST_LEN+1]; char tbuf[ISO_TIME_LEN+1]; - time_t now = time(NULL); int i; int changed = 0; @@ -3604,23 +3599,18 @@ remove_dead_entry_guards(void) * think that things are unlisted. */ void -entry_guards_compute_status(void) +entry_guards_compute_status(or_options_t *options, time_t now) { - time_t now; int changed = 0; int severity = LOG_DEBUG; - or_options_t *options; digestmap_t *reasons; if (! entry_guards) return; - options = get_options(); if (options->EntryNodes) /* reshuffle the entry guard list if needed */ entry_nodes_should_be_added(); - now = time(NULL); - reasons = digestmap_new(); SMARTLIST_FOREACH_BEGIN(entry_guards, entry_guard_t *, entry) { @@ -3636,7 +3626,7 @@ entry_guards_compute_status(void) } SMARTLIST_FOREACH_END(entry); - if (remove_dead_entry_guards()) + if (remove_dead_entry_guards(now)) changed = 1; severity = changed ? LOG_DEBUG : LOG_INFO; @@ -3801,9 +3791,8 @@ entry_nodes_should_be_added(void) /** Add all nodes in EntryNodes that aren't currently guard nodes to the list * of guard nodes, at the front. */ static void -entry_guards_prepend_from_config(void) +entry_guards_prepend_from_config(or_options_t *options) { - or_options_t *options = get_options(); smartlist_t *entry_routers, *entry_fps; smartlist_t *old_entry_guards_on_list, *old_entry_guards_not_on_list; tor_assert(entry_guards); @@ -3931,11 +3920,11 @@ choose_random_entry(cpath_build_state_t *state) entry_guards = smartlist_create(); if (should_add_entry_nodes) - entry_guards_prepend_from_config(); + entry_guards_prepend_from_config(options); if (!entry_list_is_constrained(options) && smartlist_len(entry_guards) < options->NumEntryGuards) - pick_entry_guards(); + pick_entry_guards(options); retry: smartlist_clear(live_entry_guards); @@ -4164,7 +4153,7 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg) entry_guards_dirty = 0; /* XXX022 hand new_entry_guards to this func, and move it up a * few lines, so we don't have to re-dirty it */ - if (remove_obsolete_entry_guards()) + if (remove_obsolete_entry_guards(now)) entry_guards_dirty = 1; } digestmap_free(added_by, _tor_free); @@ -4469,9 +4458,8 @@ retry_bridge_descriptor_fetch_directly(const char *digest) * descriptor, fetch a new copy of its descriptor -- either directly * from the bridge or via a bridge authority. */ void -fetch_bridge_descriptors(time_t now) +fetch_bridge_descriptors(or_options_t *options, time_t now) { - or_options_t *options = get_options(); int num_bridge_auths = get_n_authorities(BRIDGE_AUTHORITY); int ask_bridge_directly; int can_use_bridge_authority; @@ -4595,26 +4583,38 @@ any_pending_bridge_descriptor_fetches(void) return 0; } -/** Return 1 if we have at least one descriptor for a bridge and - * all descriptors we know are down. Else return 0. If <b>act</b> is - * 1, then mark the down bridges up; else just observe and report. */ +/** Return 1 if we have at least one descriptor for an entry guard + * (bridge or member of EntryNodes) and all descriptors we know are + * down. Else return 0. If <b>act</b> is 1, then mark the down guards + * up; else just observe and report. */ static int -bridges_retry_helper(int act) +entries_retry_helper(or_options_t *options, int act) { routerinfo_t *ri; int any_known = 0; int any_running = 0; + int purpose = options->UseBridges ? + ROUTER_PURPOSE_BRIDGE : ROUTER_PURPOSE_GENERAL; if (!entry_guards) entry_guards = smartlist_create(); SMARTLIST_FOREACH(entry_guards, entry_guard_t *, e, { ri = router_get_by_digest(e->identity); - if (ri && ri->purpose == ROUTER_PURPOSE_BRIDGE) { + if (ri && ri->purpose == purpose) { any_known = 1; if (ri->is_running) - any_running = 1; /* some bridge is both known and running */ - else if (act) { /* mark it for retry */ - ri->is_running = 1; + any_running = 1; /* some entry is both known and running */ + else if (act) { + /* Mark all current connections to this OR as unhealthy, since + * otherwise there could be one that started 30 seconds + * ago, and in 30 seconds it will time out, causing us to mark + * the node down and undermine the retry attempt. We mark even + * the established conns, since if the network just came back + * we'll want to attach circuits to fresh conns. */ + connection_or_set_bad_connections(ri->cache_info.identity_digest, 1); + + /* mark this entry node for retry */ + router_set_status(ri->cache_info.identity_digest, 1); e->can_retry = 1; e->bad_since = 0; } @@ -4625,19 +4625,21 @@ bridges_retry_helper(int act) return any_known && !any_running; } -/** Do we know any descriptors for our bridges, and are they all - * down? */ +/** Do we know any descriptors for our bridges / entrynodes, and are + * all the ones we have descriptors for down? */ int -bridges_known_but_down(void) +entries_known_but_down(or_options_t *options) { - return bridges_retry_helper(0); + tor_assert(entry_list_is_constrained(options)); + return entries_retry_helper(options, 0); } -/** Mark all down known bridges up. */ +/** Mark all down known bridges / entrynodes up. */ void -bridges_retry_all(void) +entries_retry_all(or_options_t *options) { - bridges_retry_helper(1); + tor_assert(entry_list_is_constrained(options)); + entries_retry_helper(options, 1); } /** Release all storage held by the list of entry guards and related diff --git a/src/or/circuitbuild.h b/src/or/circuitbuild.h index f4cc2a904..033dd79c4 100644 --- a/src/or/circuitbuild.h +++ b/src/or/circuitbuild.h @@ -49,7 +49,7 @@ void extend_info_free(extend_info_t *info); routerinfo_t *build_state_get_exit_router(cpath_build_state_t *state); const char *build_state_get_exit_nickname(cpath_build_state_t *state); -void entry_guards_compute_status(void); +void entry_guards_compute_status(or_options_t *options, time_t now); int entry_guard_register_connect_status(const char *digest, int succeeded, int mark_relay_status, time_t now); void entry_nodes_should_be_added(void); @@ -68,12 +68,12 @@ learned_router_identity(tor_addr_t *addr, uint16_t port, const char *digest); void bridge_add_from_config(const tor_addr_t *addr, uint16_t port, char *digest); void retry_bridge_descriptor_fetch_directly(const char *digest); -void fetch_bridge_descriptors(time_t now); +void fetch_bridge_descriptors(or_options_t *options, time_t now); void learned_bridge_descriptor(routerinfo_t *ri, int from_cache); int any_bridge_descriptors_known(void); int any_pending_bridge_descriptor_fetches(void); -int bridges_known_but_down(void); -void bridges_retry_all(void); +int entries_known_but_down(or_options_t *options); +void entries_retry_all(or_options_t *options); void entry_guards_free_all(void); diff --git a/src/or/circuituse.c b/src/or/circuituse.c index e9335b18d..f369678ab 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -955,8 +955,19 @@ circuit_build_failed(origin_circuit_t *circ) * to blame, blame it. Also, avoid this relay for a while, and * fail any one-hop directory fetches destined for it. */ const char *n_conn_id = circ->cpath->extend_info->identity_digest; + int already_marked = 0; if (circ->_base.n_conn) { or_connection_t *n_conn = circ->_base.n_conn; + if (n_conn->is_bad_for_new_circs) { + /* We only want to blame this router when a fresh healthy + * connection fails. So don't mark this router as newly failed, + * since maybe this was just an old circuit attempt that's + * finally timing out now. Also, there's no need to blow away + * circuits/streams/etc, since the failure of an unhealthy conn + * doesn't tell us much about whether a healthy conn would + * succeed. */ + already_marked = 1; + } log_info(LD_OR, "Our circuit failed to get a response from the first hop " "(%s:%d). I'm going to try to rotate to a better connection.", @@ -966,7 +977,7 @@ circuit_build_failed(origin_circuit_t *circ) log_info(LD_OR, "Our circuit died before the first hop with no connection"); } - if (n_conn_id) { + if (n_conn_id && !already_marked) { entry_guard_register_connect_status(n_conn_id, 0, 1, time(NULL)); /* if there are any one-hop streams waiting on this circuit, fail * them now so they can retry elsewhere. */ @@ -1192,11 +1203,13 @@ circuit_get_open_circ_or_launch(edge_connection_t *conn, int severity = LOG_NOTICE; /* FFFF if this is a tunneled directory fetch, don't yell * as loudly. the user doesn't even know it's happening. */ - if (options->UseBridges && bridges_known_but_down()) { + if (entry_list_is_constrained(options) && + entries_known_but_down(options)) { log_fn(severity, LD_APP|LD_DIR, "Application request when we haven't used client functionality " - "lately. Optimistically trying known bridges again."); - bridges_retry_all(); + "lately. Optimistically trying known %s again.", + options->UseBridges ? "bridges" : "entrynodes"); + entries_retry_all(options); } else if (!options->UseBridges || any_bridge_descriptors_known()) { log_fn(severity, LD_APP|LD_DIR, "Application request when we haven't used client functionality " diff --git a/src/or/connection_or.c b/src/or/connection_or.c index 90e60e49f..069c3e133 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -632,11 +632,24 @@ connection_or_get_for_extend(const char *digest, #define TIME_BEFORE_OR_CONN_IS_TOO_OLD (60*60*24*7) /** Given the head of the linked list for all the or_connections with a given - * identity, set elements of that list as is_bad_for_new_circs() as - * appropriate. Helper for connection_or_set_bad_connections(). + * identity, set elements of that list as is_bad_for_new_circs as + * appropriate. Helper for connection_or_set_bad_connections(). + * + * Specifically, we set the is_bad_for_new_circs flag on: + * - all connections if <b>force</b> is true. + * - all connections that are too old. + * - all open non-canonical connections for which a canonical connection + * exists to the same router. + * - all open canonical connections for which a 'better' canonical + * connection exists to the same router. + * - all open non-canonical connections for which a 'better' non-canonical + * connection exists to the same router at the same address. + * + * See connection_or_is_better() for our idea of what makes one OR connection + * better than another. */ static void -connection_or_group_set_badness(or_connection_t *head) +connection_or_group_set_badness(or_connection_t *head, int force) { or_connection_t *or_conn = NULL, *best = NULL; int n_old = 0, n_inprogress = 0, n_canonical = 0, n_other = 0; @@ -648,8 +661,9 @@ connection_or_group_set_badness(or_connection_t *head) if (or_conn->_base.marked_for_close || or_conn->is_bad_for_new_circs) continue; - if (or_conn->_base.timestamp_created + TIME_BEFORE_OR_CONN_IS_TOO_OLD - < now) { + if (force || + or_conn->_base.timestamp_created + TIME_BEFORE_OR_CONN_IS_TOO_OLD + < now) { log_info(LD_OR, "Marking OR conn to %s:%d as too old for new circuits " "(fd %d, %d secs old).", @@ -744,27 +758,20 @@ connection_or_group_set_badness(or_connection_t *head) } } -/** Go through all the OR connections, and set the is_bad_for_new_circs - * flag on: - * - all connections that are too old. - * - all open non-canonical connections for which a canonical connection - * exists to the same router. - * - all open canonical connections for which a 'better' canonical - * connection exists to the same router. - * - all open non-canonical connections for which a 'better' non-canonical - * connection exists to the same router at the same address. - * - * See connection_or_is_better() for our idea of what makes one OR connection - * better than another. +/** Go through all the OR connections (or if <b>digest</b> is non-NULL, just + * the OR connections with that digest), and set the is_bad_for_new_circs + * flag based on the rules in connection_or_group_set_badness() (or just + * always set it if <b>force</b> is true). */ void -connection_or_set_bad_connections(void) +connection_or_set_bad_connections(const char *digest, int force) { if (!orconn_identity_map) return; DIGESTMAP_FOREACH(orconn_identity_map, identity, or_connection_t *, conn) { - connection_or_group_set_badness(conn); + if (!digest || !memcmp(digest, conn->identity_digest, DIGEST_LEN)) + connection_or_group_set_badness(conn, force); } DIGESTMAP_FOREACH_END; } diff --git a/src/or/connection_or.h b/src/or/connection_or.h index 717630217..216a9bd64 100644 --- a/src/or/connection_or.h +++ b/src/or/connection_or.h @@ -18,7 +18,7 @@ or_connection_t *connection_or_get_for_extend(const char *digest, const tor_addr_t *target_addr, const char **msg_out, int *launch_out); -void connection_or_set_bad_connections(void); +void connection_or_set_bad_connections(const char *digest, int force); int connection_or_reached_eof(or_connection_t *conn); int connection_or_process_inbuf(or_connection_t *conn); diff --git a/src/or/main.c b/src/or/main.c index 8ea1728fb..23daf1366 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -861,7 +861,7 @@ directory_info_has_arrived(time_t now, int from_cache) /* if we have enough dir info, then update our guard status with * whatever we just learned. */ - entry_guards_compute_status(); + entry_guards_compute_status(options, now); /* Don't even bother trying to get extrainfo until the rest of our * directory info is up-to-date */ if (options->DownloadExtraInfo) @@ -1067,7 +1067,7 @@ run_scheduled_events(time_t now) update_extrainfo_downloads(now); update_microdesc_downloads(now); if (options->UseBridges) - fetch_bridge_descriptors(now); + fetch_bridge_descriptors(options, now); if (router_have_minimum_dir_info()) time_to_try_getting_descriptors = now + LAZY_DESCRIPTOR_RETRY_INTERVAL; else @@ -1328,7 +1328,7 @@ run_scheduled_events(time_t now) circuit_expire_old_circuits_serverside(now); /** 5. We do housekeeping for each connection... */ - connection_or_set_bad_connections(); + connection_or_set_bad_connections(NULL, 0); for (i=0;i<smartlist_len(connection_array);i++) { run_connection_housekeeping(i, now); } diff --git a/src/or/or.h b/src/or/or.h index e5ca88fc5..8913dbc83 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -1062,7 +1062,10 @@ typedef struct or_connection_t { * NETINFO cell listed the address we're connected to as recognized. */ unsigned int is_canonical:1; /** True iff this connection shouldn't get any new circs attached to it, - * because the connection is too old, or because there's a better one, etc. + * because the connection is too old, or because there's a better one. + * More generally, this flag is used to note an unhealthy connection; + * for example, if a bad connection fails we shouldn't assume that the + * router itself has a problem. */ unsigned int is_bad_for_new_circs:1; uint8_t link_proto; /**< What protocol version are we using? 0 for diff --git a/src/or/routerlist.c b/src/or/routerlist.c index ff3606c47..a6cccabf3 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -4852,7 +4852,7 @@ update_router_have_minimum_dir_info(void) count_usable_descriptors(&num_present, &num_usable, consensus, options, now, options->EntryNodes); - if (num_usable && (num_present == 0)) { + if (!num_usable || !num_present) { tor_snprintf(dir_info_status, sizeof(dir_info_status), "We have only %d/%d usable entry node descriptors.", num_present, num_usable); |