r8973@Kushana: nickm | 2006-09-28 16:53:19 -0400

 Refactor entry guard status logic a lot; allow more factors [like not
 having a Guard flag or being listed in ExcludeNodes] to render a guard
 "unlisted" (now called "unusable"); track guard down status (now
 called "unreachable") separately from is_running.


svn:r8519

ChangeLog

@@ -23,6 +23,10 @@ Changes in version 0.1.2.2-alpha - 2006-??-??
       servers on the same /16" behavior.  It's still on by default; this
       is mostly for people who want to operate private test networks with
       all the machines on the same subnet.
+    - If one of our entry guards is on the ExcludeNodes list, or the
+      directory authorities don't think it's a good guard, treat it as if it
+      were unlisted: stop using it as a guard, and throw it off the guards
+      list if it stays that way for a long time.
 
   o Security Fixes, minor:
     - If a client asked for a server by name, and we didn't have a
@@ -59,13 +63,16 @@ Changes in version 0.1.2.2-alpha - 2006-??-??
       we don't recognize.
     - Avoid a memory corruption bug when creating a hash table for the first
       time.
+    - Track unreachable entry guards correctly: don't conflate 'unreachable
+      by us right now' with 'listed as down by the directory authorities'.
+      With the old code, if a guard was unreachable by us but listed as
+      running, it would clog our guard list forever.
 
   o Documentation
     - Documented (and renamed) ServerDNSSearchDomains and
       ServerDNSResolvConfFile options.
 
 
-
   o Packaging:
     - Patches so Tor builds with MinGW on Windows.
     - The Debian package now uses --verify-config when (re)starting,

doc/TODO

@@ -24,8 +24,9 @@ x - When we've been idle a long time, we stop fetching server
     until we've fetched correct ones.
 x - If the client's clock is too far in the past, it will drop (or
     just not try to get) descriptors, so it'll never build circuits.
-N - when we start, remove any entryguards that are listed in excludenodes.
-N - Remember the last time we saw one of our entry guards labelled with
+  o when we start, remove^Wmark as unusable any entryguards that are listed
+    in excludenodes.
+  o Remember the last time we saw one of our entry guards labeled with
     the GUARD flag. If it's been too long, it is not suitable for use.
     If it's been really too long, remove it from the list.
   o Figure out avoiding duplicate /24 lines

src/or/circuitbuild.c

@@ -27,10 +27,13 @@ typedef struct {
   char identity[DIGEST_LEN];
   uint8_t made_contact; /**< 0 if we have never connected to this router,
                          * 1 if we have. */
-  time_t down_since; /**< 0 if this router is currently up, or the time at
-                      * which it was observed to go down. */
-  time_t unlisted_since; /**< 0 if this router is currently listed, or the
-                          * time at which it became unlisted */
+  time_t bad_since; /**< 0 if this guard is currently usable, or the time at
+                      * which it was observed to become (according to the
+                      * directory or the user configuration) unusable. */
+  time_t unreachable_since; /**< 0 if we can connect to this guard, or the
+                             * time at which we first noticed we couldn't
+                             * connect to it. */
+  time_t last_attempted;
 } entry_guard_t;
 
 /** A list of our chosen entry guards. */
@@ -1716,6 +1719,65 @@ build_state_get_exit_nickname(cpath_build_state_t *state)
   return state->chosen_exit->nickname;
 }
 
+/** DOCDOC */
+static int
+entry_guard_set_status(entry_guard_t *e, routerinfo_t *ri,
+                       or_options_t *options)
+{
+  const char *reason = NULL;
+  char buf[HEX_DIGEST_LEN+1];
+  int changed = 0;
+
+  /* Do we want to mark this guard as bad? */
+  if (!ri)
+    reason = "unlisted";
+  else if (!ri->is_running)
+    reason = "down";
+  else if (!ri->is_possible_guard)
+    reason = "not recommended as a guard";
+  else if (options && ri &&
+           router_nickname_is_in_list(ri, options->ExcludeNodes))
+    reason = "excluded";
+
+  if (reason && ! e->bad_since) {
+    /* Router is newly bad. */
+    base16_encode(buf, sizeof(buf), e->identity, DIGEST_LEN);
+    log_info(LD_CIRC, "Entry guard %s (%s) is %s: marking as unusable",
+             e->nickname, buf, reason);
+
+    e->bad_since = time(NULL);
+    changed = 1;
+  } else if (!reason && e->bad_since) {
+    /* There's nothing wrong with the router any more. */
+    base16_encode(buf, sizeof(buf), e->identity, DIGEST_LEN);
+    log_info(LD_CIRC, "Entry guard %s (%s) is no longer unusable: "
+             "marking as ok", e->nickname, buf);
+
+    e->bad_since = 0;
+    changed = 1;
+  }
+
+  return changed;
+}
+
+/** DOCDOC */
+static int
+entry_is_time_to_retry(entry_guard_t *e, time_t now)
+{
+  long diff;
+  if (e->last_attempted < e->unreachable_since)
+    return 1;
+  diff = now - e->unreachable_since;
+  if (diff < 6*60*60)
+    return now > (e->last_attempted + 60*60);
+  else if (diff < 3*24*60*60)
+    return now > (e->last_attempted + 4*60*60);
+  else if (diff < 7*24*60*60)
+    return now > (e->last_attempted + 18*60*60);
+  else
+    return now > (e->last_attempted + 36*60*60);
+}
+
 /** Return the router corresponding to <b>e</b>, if <b>e</b> is
  * working well enough that we are willing to use it as an entry
  * right now. (Else return NULL.) In particular, it must be
@@ -1724,16 +1786,22 @@ build_state_get_exit_nickname(cpath_build_state_t *state)
  * - Listed as 'stable' or 'fast' by the current dirserver concensus,
  *   if demanded by <b>need_uptime</b> or <b>need_capacity</b>; and
  * - Allowed by our current ReachableAddresses config option.
+ * DOCDOC assume_reachable.
  */
 static INLINE routerinfo_t *
-entry_is_live(entry_guard_t *e, int need_uptime, int need_capacity)
+entry_is_live(entry_guard_t *e, int need_uptime, int need_capacity,
+              int assume_reachable)
 {
   routerinfo_t *r;
-  if (e->down_since && e->made_contact)
+  if (e->bad_since)
+    return NULL;
+  if (!assume_reachable &&
+      e->unreachable_since && !entry_is_time_to_retry(e, time(NULL)))
     return NULL;
   r = router_get_by_digest(e->identity);
   if (!r)
     return NULL;
+  /* Remove this check -- it seems redundant wrt the Guard flag? XXXX NM */
   if (router_is_unreliable(r, need_uptime, need_capacity, 0))
     return NULL;
   if (firewall_is_fascist_or() &&
@@ -1751,7 +1819,7 @@ num_live_entry_guards(void)
     return 0;
   SMARTLIST_FOREACH(entry_guards, entry_guard_t *, entry,
     {
-      if (entry_is_live(entry, 0, 1))
+      if (entry_is_live(entry, 0, 1, 0))
         ++n;
     });
   return n;
@@ -1778,10 +1846,9 @@ log_entry_guards(int severity)
 
   SMARTLIST_FOREACH(entry_guards, entry_guard_t *, e,
     {
-      tor_snprintf(buf, sizeof(buf), "%s (%s%s%s)",
+      tor_snprintf(buf, sizeof(buf), "%s (%s%s)",
                    e->nickname,
-                   (e->down_since || e->unlisted_since) ? "down " : "up ",
-                   e->unlisted_since ? "unlisted " : "listed ",
+                   e->bad_since ? "down " : "up ",
                    e->made_contact ? "made-contact" : "never-contacted");
       smartlist_add(elements, tor_strdup(buf));
     });
@@ -1859,12 +1926,9 @@ entry_guards_free_all(void)
   }
 }
 
-/** How long (in seconds) do we allow an entry guard to be nonfunctional
- * before we give up on it? */
-#define ENTRY_ALLOW_DOWNTIME (30*24*60*60)
-/** How long (in seconds) do we allow an entry guard to be unlisted in the
- * directory before we give up on it? */
-#define ENTRY_ALLOW_UNLISTED (30*24*60*60)
+/** How long (in seconds) do we allow an entry guard to be nonfunctional,
+ * unlisted, excuded, or otherwise nonusable before we give up on it? */
+#define ENTRY_GUARD_REMOVE_AFTER (30*24*60*60)
 
 /** Remove all entry guards that have been down or unlisted for so
  * long that we don't think they'll come up again. Return 1 if we
@@ -1880,23 +1944,14 @@ remove_dead_entries(void)
 
   for (i = 0; i < smartlist_len(entry_guards); ) {
     entry_guard_t *entry = smartlist_get(entry_guards, i);
-    const char *why = NULL;
-    time_t since = 0;
-    if (entry->unlisted_since &&
-        entry->unlisted_since + ENTRY_ALLOW_UNLISTED < now) {
-      why = "unlisted";
-      since = entry->unlisted_since;
-    } else if (entry->down_since &&
-               entry->down_since + ENTRY_ALLOW_DOWNTIME < now) {
-      why = "down";
-      since = entry->down_since;
-    }
-    if (why) {
+    if (entry->bad_since &&
+        entry->bad_since + ENTRY_GUARD_REMOVE_AFTER < now) {
+
       base16_encode(dbuf, sizeof(dbuf), entry->identity, DIGEST_LEN);
-      format_local_iso_time(tbuf, since);
-      log_info(LD_CIRC,
-               "Entry guard '%s' (%s) has been %s since %s; removing.",
-               entry->nickname, dbuf, why, tbuf);
+      format_local_iso_time(tbuf, entry->bad_since);
+      log_info(LD_CIRC, "Entry guard '%s' (%s) has been down or unlisted "
+               "since %s; removing.",
+               entry->nickname, dbuf, tbuf);
       tor_free(entry);
       smartlist_del_keeporder(entry_guards, i);
       log_entry_guards(LOG_INFO);
@@ -1914,62 +1969,32 @@ remove_dead_entries(void)
  * An entry is 'unlisted' if the directory doesn't include it.
  */
 void
-entry_guards_set_status_from_directory(void)
+entry_guards_compute_status(void)
 {
   /* Don't call this on startup; only on a fresh download.  Otherwise we'll
    * think that things are unlisted. */
-  routerlist_t *routers;
   time_t now;
   int changed = 0;
   int severity = LOG_INFO;
+  or_options_t *options;
   if (! entry_guards)
     return;
 
-  routers = router_get_routerlist();
+  options = get_options();
 
   now = time(NULL);
 
   SMARTLIST_FOREACH(entry_guards, entry_guard_t *, entry,
     {
       routerinfo_t *r = router_get_by_digest(entry->identity);
-      if (! r) {
-        if (! entry->unlisted_since) {
-          entry->unlisted_since = time(NULL);
-          changed = 1;
-          log_info(LD_CIRC,"Entry guard '%s' is not listed by directories.",
-                   entry->nickname);
-          severity = LOG_INFO;
-        }
-      } else {
-        if (entry->unlisted_since) {
-          log_info(LD_CIRC,"Entry guard '%s' is listed again by directories.",
-                   entry->nickname);
-          changed = 1;
-          severity = LOG_INFO;
-        }
-        entry->unlisted_since = 0;
-        if (! r->is_running) {
-          if (! entry->down_since) {
-            entry->down_since = now;
-            log_info(LD_CIRC, "Entry guard '%s' is now down.",
-                     entry->nickname);
-            changed = 1;
-            severity = LOG_INFO;
-          }
-        } else {
-          if (entry->down_since) {
-            log_info(LD_CIRC,"Entry guard '%s' is up in latest directories.",
-                     entry->nickname);
-            changed = 1;
-          }
-          entry->down_since = 0;
-        }
-      }
-      log_info(LD_CIRC, "Summary: Entry '%s' is %s, %s, and %s.",
+      if (entry_guard_set_status(entry, r, options))
+        changed = 1;
+
+      log_info(LD_CIRC, "Summary: Entry '%s' is %s, %s and %s.",
                entry->nickname,
-               (entry->down_since || entry->unlisted_since) ? "down" : "up",
-               entry->unlisted_since ? "unlisted" : "listed",
-               entry_is_live(entry, 0, 1) ? "live" : "not live");
+               entry->unreachable_since ? "unreachable" : "reachable",
+               entry->bad_since ? "unusable" : "usable",
+               entry_is_live(entry, 0, 1, 0) ? "live" : "not live");
     });
 
   if (remove_dead_entries())
@@ -1989,79 +2014,98 @@ entry_guards_set_status_from_directory(void)
  * Return 0 normally, or -1 if we want to tear down the new connection.
  */
 int
-entry_guard_set_status(const char *digest, int succeeded)
+entry_guard_register_connect_status(const char *digest, int succeeded)
 {
   int changed = 0;
   int refuse_conn = 0;
+  int first_contact = 0;
+  entry_guard_t *entry = NULL;
+  int idx = -1;
+  char buf[HEX_DIGEST_LEN+1];
 
   if (! entry_guards)
     return 0;
 
-  SMARTLIST_FOREACH(entry_guards, entry_guard_t *, entry,
+  SMARTLIST_FOREACH(entry_guards, entry_guard_t *, e,
     {
-      if (!memcmp(entry->identity, digest, DIGEST_LEN)) {
-        if (succeeded) {
-          if (!entry->made_contact) {
-            /* We've just added a new long-term entry guard. Perhaps
-             * the network just came back? We should give our earlier
-             * entries another try too, and close this connection so
-             * we don't use it before we've given the others a shot. */
-            entry->made_contact = 1;
-            SMARTLIST_FOREACH(entry_guards, entry_guard_t *, e,
-              {
-                routerinfo_t *r;
-                if (e == entry)
-                  break;
-                if (e->made_contact) {
-                  e->down_since = 0;
-                  r = entry_is_live(e, 0, 1);
-                  if (r) {
-                    refuse_conn = 1;
-                    r->is_running = 1;
-                  }
-                }
-              });
-            log_info(LD_CIRC,
-                     "Connected to new entry guard '%s'. Marking earlier "
-                     "entry guards up. %d/%d entry guards usable/new.",
-                     entry->nickname,
-                     num_live_entry_guards(), smartlist_len(entry_guards));
-            log_entry_guards(LOG_INFO);
-            changed = 1;
-          }
-          if (entry->down_since) {
-            entry->down_since = 0;
-            log_info(LD_CIRC,
-                  "Connection to formerly down entry guard '%s' succeeded. "
-                  "%d/%d entry guards usable/new.", entry->nickname,
-                  num_live_entry_guards(), smartlist_len(entry_guards));
-            log_entry_guards(LOG_INFO);
-            changed = 1;
-          }
-        } else {
-          if (!entry->made_contact) { /* dump him */
-            log_info(LD_CIRC,
-                   "Connection to never-contacted entry guard '%s' failed. "
-                   "Removing from the list. %d/%d entry guards usable/new.",
-                   entry->nickname,
-                   num_live_entry_guards()-1, smartlist_len(entry_guards)-1);
-            tor_free(entry);
-            smartlist_del_keeporder(entry_guards, entry_sl_idx);
-            log_entry_guards(LOG_INFO);
-            changed = 1;
-          } else if (!entry->down_since) {
-            entry->down_since = time(NULL);
-            log_info(LD_CIRC, "Connection to entry guard '%s' failed. "
-                     "%d/%d entry guards usable/new.",
-                     entry->nickname,
-                     num_live_entry_guards(), smartlist_len(entry_guards));
-            log_entry_guards(LOG_INFO);
-            changed = 1;
-          }
-        }
+      if (!memcmp(e->identity, digest, DIGEST_LEN)) {
+        entry = e;
+        idx = e_sl_idx;
+        break;
       }
     });
 
+  if (!entry)
+    return 0;
+
+  base16_encode(buf, sizeof(buf), entry->identity, DIGEST_LEN);
+
+  if (succeeded) {
+    if (entry->unreachable_since) {
+      log_info(LD_CIRC, "Entry guard '%s' (%s) is now reachable again. Good.",
+               entry->nickname, buf);
+      entry->unreachable_since = 0;
+      entry->last_attempted = time(NULL);
+      changed = 1;
+    }
+    if (!entry->made_contact) {
+      entry->made_contact = 1;
+      first_contact = changed = 1;
+    }
+  } else { /* ! succeeded */
+    if (!entry->made_contact) {
+      /* We've never connected to this one. */
+      log_info(LD_CIRC,
+               "Connection to never-contacted entry guard '%s' (%s) failed. "
+               "Removing from the list. %d/%d entry guards usable/new.",
+               entry->nickname, buf,
+               num_live_entry_guards()-1, smartlist_len(entry_guards)-1);
+      tor_free(entry);
+      smartlist_del_keeporder(entry_guards, idx);
+      log_entry_guards(LOG_INFO);
+      changed = 1;
+    } else if (!entry->unreachable_since) {
+      log_info(LD_CIRC, "Unable to connect to entry guard '%s' (%s). "
+               "Marking as unreachable.", entry->nickname, buf);
+      changed = 1;
+    } else {
+      char tbuf[ISO_TIME_LEN+1];
+      format_iso_time(tbuf, entry->unreachable_since);
+      log_debug(LD_CIRC, "Failed to connect to unreachable entry guard "
+                "'%s' (%s).  It has been unreachable since %s.",
+                entry->nickname, buf, tbuf);
+      entry->last_attempted = time(NULL);
+    }
+  }
+
+  if (first_contact) {
+    /* We've just added a new long-term entry guard. Perhaps the network just
+     * came back? We should give our earlier entries another try too,
+     * and close this connection so we don't use it before we've given
+     * the others a shot. */
+    SMARTLIST_FOREACH(entry_guards, entry_guard_t *, e, {
+        routerinfo_t *r;
+        if (e == entry)
+          break;
+        if (e->made_contact) {
+          r = entry_is_live(e, 0, 1, 1);
+          if (r && !r->is_running) {
+            refuse_conn = 1;
+            r->is_running = 1;
+          }
+        }
+      });
+    if (refuse_conn) {
+      log_info(LD_CIRC,
+               "Connected to new entry guard '%s' (%s). Marking earlier "
+               "entry guards up. %d/%d entry guards usable/new.",
+               entry->nickname, buf,
+               num_live_entry_guards(), smartlist_len(entry_guards));
+      log_entry_guards(LOG_INFO);
+      changed = 1;
+    }
+  }
+
   if (changed)
     entry_guards_changed();
   return refuse_conn ? -1 : 0;
@@ -2150,7 +2194,7 @@ choose_random_entry(cpath_build_state_t *state)
   smartlist_clear(live_entry_guards);
   SMARTLIST_FOREACH(entry_guards, entry_guard_t *, entry,
     {
-      r = entry_is_live(entry, need_uptime, need_capacity);
+      r = entry_is_live(entry, need_uptime, need_capacity, 0);
       if (r && r != chosen_exit) {
         smartlist_add(live_entry_guards, r);
         if (smartlist_len(live_entry_guards) >= options->NumEntryGuards)
@@ -2242,9 +2286,9 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg)
         break;
       }
       if (!strcasecmp(line->key, "EntryGuardDownSince"))
-        node->down_since = when;
+        node->unreachable_since = when;
       else
-        node->unlisted_since = when;
+        node->bad_since = when;
     }
   }
 
@@ -2301,18 +2345,18 @@ entry_guards_update_state(or_state_t *state)
       tor_snprintf(line->value,HEX_DIGEST_LEN+MAX_NICKNAME_LEN+2,
                    "%s %s", e->nickname, dbuf);
       next = &(line->next);
-      if (e->down_since) {
+      if (e->unreachable_since) {
         *next = line = tor_malloc_zero(sizeof(config_line_t));
         line->key = tor_strdup("EntryGuardDownSince");
         line->value = tor_malloc(ISO_TIME_LEN+1);
-        format_iso_time(line->value, e->down_since);
+        format_iso_time(line->value, e->unreachable_since);
         next = &(line->next);
       }
-      if (e->unlisted_since) {
+      if (e->bad_since) {
         *next = line = tor_malloc_zero(sizeof(config_line_t));
         line->key = tor_strdup("EntryGuardUnlistedSince");
         line->value = tor_malloc(ISO_TIME_LEN+1);
-        format_iso_time(line->value, e->unlisted_since);
+        format_iso_time(line->value, e->bad_since);
         next = &(line->next);
       }
     });
@@ -2344,12 +2388,9 @@ entry_guards_getinfo(const char *question, char **answer)
         time_t when = 0;
         if (!e->made_contact) {
           status = "never-connected";
-        } else if (e->unlisted_since) {
-          when = e->unlisted_since;
-          status = "unlisted";
-        } else if (e->down_since) {
-          when = e->down_since;
-          status = "down";
+        } else if (e->bad_since) {
+          when = e->bad_since;
+          status = "unusable";
         } else {
           status = "up";
         }

src/or/circuituse.c

@@ -740,7 +740,7 @@ circuit_build_failed(origin_circuit_t *circ)
                "(%s:%d). I'm going to try to rotate to a better connection.",
                n_conn->_base.address, n_conn->_base.port);
       n_conn->_base.or_is_obsolete = 1;
-      entry_guard_set_status(n_conn->identity_digest, 0);
+      entry_guard_register_connect_status(n_conn->identity_digest, 0);
     }
   }
 

src/or/connection.c

@@ -428,7 +428,7 @@ connection_about_to_close_connection(connection_t *conn)
       if (conn->state != OR_CONN_STATE_OPEN) {
         if (connection_or_nonopen_was_started_here(or_conn)) {
           rep_hist_note_connect_failed(or_conn->identity_digest, time(NULL));
-          entry_guard_set_status(or_conn->identity_digest, 0);
+          entry_guard_register_connect_status(or_conn->identity_digest, 0);
           router_set_status(or_conn->identity_digest, 0);
           control_event_or_conn_status(or_conn, OR_CONN_EVENT_FAILED);
         }

src/or/connection_or.c

@@ -448,7 +448,7 @@ connection_or_connect(uint32_t addr, uint16_t port, const char *id_digest)
        * an https proxy, our https proxy is down. Don't blame the
        * Tor server. */
       if (!options->HttpsProxy) {
-        entry_guard_set_status(conn->identity_digest, 0);
+        entry_guard_register_connect_status(conn->identity_digest, 0);
         router_set_status(conn->identity_digest, 0);
       }
       control_event_or_conn_status(conn, OR_CONN_EVENT_FAILED);
@@ -622,7 +622,7 @@ connection_or_check_valid_handshake(or_connection_t *conn, char *digest_rcvd)
              "Identity key not as expected for router at %s:%d: wanted %s "
              "but got %s",
              conn->_base.address, conn->_base.port, expected, seen);
-      entry_guard_set_status(conn->identity_digest, 0);
+      entry_guard_register_connect_status(conn->identity_digest, 0);
       router_set_status(conn->identity_digest, 0);
       control_event_or_conn_status(conn, OR_CONN_EVENT_FAILED);
       as_advertised = 0;
@@ -684,7 +684,7 @@ connection_tls_finish_handshake(or_connection_t *conn)
   control_event_or_conn_status(conn, OR_CONN_EVENT_CONNECTED);
   if (started_here) {
     rep_hist_note_connect_succeeded(conn->identity_digest, time(NULL));
-    if (entry_guard_set_status(conn->identity_digest, 1) < 0) {
+    if (entry_guard_register_connect_status(conn->identity_digest, 1) < 0) {
       /* pending circs get closed in circuit_about_to_close_connection() */
       return -1;
     }

src/or/or.h

@@ -1707,8 +1707,8 @@ 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_set_status_from_directory(void);
-int entry_guard_set_status(const char *digest, int succeeded);
+void entry_guards_compute_status(void);
+int entry_guard_register_connect_status(const char *digest, int succeeded);
 void entry_nodes_should_be_added(void);
 void entry_guards_prepend_from_config(void);
 void entry_guards_update_state(or_state_t *state);
@@ -2534,6 +2534,7 @@ void routerlist_add_family(smartlist_t *sl, routerinfo_t *router);
 void add_nickname_list_to_smartlist(smartlist_t *sl, const char *list,
                                     int must_be_running,
                                     int warn_if_down, int warn_if_unnamed);
+int router_nickname_is_in_list(routerinfo_t *router, const char *list);
 routerinfo_t *routerlist_find_my_routerinfo(void);
 routerinfo_t *router_find_exact_exit_enclave(const char *address,
                                              uint16_t port);

src/or/routerlist.c

@@ -24,7 +24,6 @@ static routerstatus_t *router_pick_directory_server_impl(int requireother,
 static routerstatus_t *router_pick_trusteddirserver_impl(
                  int need_v1_authority, int requireother, int fascistfirewall);
 static void mark_all_trusteddirservers_up(void);
-static int router_nickname_is_in_list(routerinfo_t *router, const char *list);
 static int router_nickname_matches(routerinfo_t *router, const char *nickname);
 static void routerstatus_list_update_from_networkstatus(time_t now);
 static void local_routerstatus_free(local_routerstatus_t *rs);
@@ -662,9 +661,12 @@ routerlist_add_family(smartlist_t *sl, routerinfo_t *router)
   }
 }
 
-/** Given a comma-and-whitespace separated list of nicknames, see which
- * nicknames in <b>list</b> name routers in our routerlist that are
- * currently running.  Add the routerinfos for those routers to <b>sl</b>.
+/** Given a (possibly NULL) comma-and-whitespace separated list of nicknames,
+ * see which nicknames in <b>list</b> name routers in our routerlist, and add
+ * the routerinfos for those routers to <b>sl</b>.  If <b>must_be_running</b>,
+ * only include routers that we think are running.  If <b>warn_if_down</b>,
+ * warn if some included routers aren't running.  If <b>warn_if_unnamed</b>,
+ * warn if any non-Named routers are specified by nickname.
  */
 void
 add_nickname_list_to_smartlist(smartlist_t *sl, const char *list,
@@ -718,10 +720,11 @@ add_nickname_list_to_smartlist(smartlist_t *sl, const char *list,
   smartlist_free(nickname_list);
 }
 
-/** Return 1 iff any member of the comma-separated list <b>list</b> is an
- * acceptable nickname or hexdigest for <b>router</b>.  Else return 0.
+/** Return 1 iff any member of the (possibly NULL) comma-separated list
+ * <b>list</b> is an acceptable nickname or hexdigest for <b>router</b>.  Else
+ * return 0.
  */
-static int
+int
 router_nickname_is_in_list(routerinfo_t *router, const char *list)
 {
   smartlist_t *nickname_list;
@@ -2954,7 +2957,7 @@ routers_update_all_from_networkstatus(void)
     }
   }
 
-  entry_guards_set_status_from_directory();
+  entry_guards_compute_status();
 
   if (!have_warned_about_old_version &&
       have_tried_downloading_all_statuses(4)) {