Merge remote branch 'origin/maint-0.2.2'

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.

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

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);
 

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 "

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;
 }
 

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);

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);
   }

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

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);