Clarify directory and ORPort checking functions.

In order to make the OR and dir checking functions in router.c less confusing
we renamed some functions and splitted consider_testing_reachability() into
router_should_check_reachability() and router_do_reachability_checks(). Also we
improved the documentation.

Fixes #18918.

Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>

changes/bug18918

@@ -0,0 +1,6 @@
+  o Code simplification and refactoring:
+    - In order to make the OR and dir checking function in router.c less
+      confusing we renamed some functions and consider_testing_reachability()
+      has been splitted into router_should_check_reachability() and
+      router_do_reachability_checks(). Also we improved the documentation in
+      some functions. Closes ticket 18918.

src/or/circuitbuild.c

@@ -1095,7 +1095,7 @@ circuit_build_no_more_hops(origin_circuit_t *circ)
     clear_broken_connection_map(1);
     if (server_mode(options) && !check_whether_orport_reachable(options)) {
       inform_testing_reachability();
-      consider_testing_reachability(1, 1);
+      router_do_reachability_checks(1, 1);
     }
   }
 
@@ -1651,7 +1651,7 @@ onionskin_answer(or_circuit_t *circ,
  *     rend_service_launch_establish_intro())
  *
  *   - We are a router testing its own reachabiity
- *     (CIRCUIT_PURPOSE_TESTING, via consider_testing_reachability())
+ *     (CIRCUIT_PURPOSE_TESTING, via router_do_reachability_checks())
  *
  * onion_pick_cpath_exit() bypasses us (by not calling
  * new_route_len()) in the one-hop tunnel case, so we don't need to

src/or/circuituse.c

@@ -1630,7 +1630,7 @@ circuit_testing_opened(origin_circuit_t *circ)
     router_perform_bandwidth_test(NUM_PARALLEL_TESTING_CIRCS, time(NULL));
     have_performed_bandwidth_test = 1;
   } else
-    consider_testing_reachability(1, 0);
+    router_do_reachability_checks(1, 0);
 }
 
 /** A testing circuit has failed to build. Take whatever stats we want. */

src/or/main.c

@@ -1142,7 +1142,7 @@ directory_info_has_arrived(time_t now, int from_cache, int suppress_logs)
 
   if (server_mode(options) && !net_is_disabled() && !from_cache &&
       (have_completed_a_circuit() || !any_predicted_circuits(now)))
-    consider_testing_reachability(1, 1);
+   router_do_reachability_checks(1, 1);
 }
 
 /** Perform regular maintenance tasks for a single connection.  This
@@ -2062,7 +2062,7 @@ check_for_reachability_bw_callback(time_t now, const or_options_t *options)
       (have_completed_a_circuit() || !any_predicted_circuits(now)) &&
       !net_is_disabled()) {
     if (stats_n_seconds_working < TIMEOUT_UNTIL_UNREACHABILITY_COMPLAINT) {
-      consider_testing_reachability(1, dirport_reachability_count==0);
+      router_do_reachability_checks(1, dirport_reachability_count==0);
       if (++dirport_reachability_count > 5)
         dirport_reachability_count = 0;
       return 1;

src/or/router.c

@@ -1227,7 +1227,8 @@ check_whether_dirport_reachable(const or_options_t *options)
 /* XXX Should this be increased? */
 #define MIN_BW_TO_ADVERTISE_DIRSERVER 51200
 
-/** Return true iff we have enough configured bandwidth to cache directory
+/** Return true iff we have enough configured bandwidth to advertise or
+ * automatically provide directory services from cache directory
  * information. */
 static int
 router_has_bandwidth_to_be_dirserver(const or_options_t *options)
@@ -1250,7 +1251,7 @@ router_has_bandwidth_to_be_dirserver(const or_options_t *options)
  * MIN_BW_TO_ADVERTISE_DIRSERVER, don't bother trying to serve requests.
  */
 static int
-router_should_be_directory_server(const or_options_t *options, int dir_port)
+router_should_be_dirserver(const or_options_t *options, int dir_port)
 {
   static int advertising=1; /* start out assuming we will advertise */
   int new_choice=1;
@@ -1301,7 +1302,7 @@ router_should_be_directory_server(const or_options_t *options, int dir_port)
     } else {
       tor_assert(reason);
       log_notice(LD_DIR, "Not advertising Dir%s (Reason: %s)",
-                 dir_port ? "Port" : "ectory Service support", reason);
+                 dir_port ? "Port" : "Directory Service support", reason);
     }
     advertising = new_choice;
   }
@@ -1355,7 +1356,7 @@ decide_to_advertise_dir_impl(const or_options_t *options,
 
   /* Part two: consider config options that could make us choose to
    * publish or not publish that the user might find surprising. */
-  return router_should_be_directory_server(options, dir_port);
+  return router_should_be_dirserver(options, dir_port);
 }
 
 /** Front-end to decide_to_advertise_dir_impl(): return 0 if we don't want to
@@ -1363,7 +1364,7 @@ decide_to_advertise_dir_impl(const or_options_t *options,
  * DirPort we want to advertise.
  */
 static int
-decide_to_advertise_dirport(const or_options_t *options, uint16_t dir_port)
+router_should_advertise_dirport(const or_options_t *options, uint16_t dir_port)
 {
   /* supports_tunnelled_dir_requests is not relevant, pass 0 */
   return decide_to_advertise_dir_impl(options, dir_port, 0) ? dir_port : 0;
@@ -1373,7 +1374,7 @@ decide_to_advertise_dirport(const or_options_t *options, uint16_t dir_port)
  * advertise the fact that we support begindir requests, else return 1.
  */
 static int
-decide_to_advertise_begindir(const or_options_t *options,
+router_should_advertise_begindir(const or_options_t *options,
                              int supports_tunnelled_dir_requests)
 {
   /* dir_port is not relevant, pass 0 */
@@ -1406,26 +1407,17 @@ extend_info_from_router(const routerinfo_t *r)
                          &ap.addr, ap.port);
 }
 
-/** Some time has passed, or we just got new directory information.
+/**See if we currently believe our ORPort or DirPort to be
- * See if we currently believe our ORPort or DirPort to be
+ * unreachable. If so, return 1 else return 0.
- * unreachable. If so, launch a new test for it.
- *
- * For ORPort, we simply try making a circuit that ends at ourselves.
- * Success is noticed in onionskin_answer().
- *
- * For DirPort, we make a connection via Tor to our DirPort and ask
- * for our own server descriptor.
- * Success is noticed in connection_dir_client_reached_eof().
  */
-void
+static int
-consider_testing_reachability(int test_or, int test_dir)
+router_should_check_reachability(int test_or, int test_dir)
 {
   const routerinfo_t *me = router_get_my_routerinfo();
   const or_options_t *options = get_options();
-  int orport_reachable = check_whether_orport_reachable(options);
+
-  tor_addr_t addr;
   if (!me)
-    return;
+    return 0;
 
   if (routerset_contains_router(options->ExcludeNodes, me, -1) &&
       options->StrictNodes) {
@@ -1440,9 +1432,31 @@ consider_testing_reachability(int test_or, int test_dir)
                  "We cannot learn whether we are usable, and will not "
                  "be able to advertise ourself.");
     }
-    return;
+    return 0;
   }
+  return 1;
+}
 
+/** Some time has passed, or we just got new directory information.
+ * See if we currently believe our ORPort or DirPort to be
+ * unreachable. If so, launch a new test for it.
+ *
+ * For ORPort, we simply try making a circuit that ends at ourselves.
+ * Success is noticed in onionskin_answer().
+ *
+ * For DirPort, we make a connection via Tor to our DirPort and ask
+ * for our own server descriptor.
+ * Success is noticed in connection_dir_client_reached_eof().
+ */
+void
+router_do_reachability_checks(int test_or, int test_dir)
+{
+  const routerinfo_t *me = router_get_my_routerinfo();
+  const or_options_t *options = get_options();
+  int orport_reachable = check_whether_orport_reachable(options);
+  tor_addr_t addr;
+
+  if (router_should_check_reachability(test_or, test_dir)) {
     if (test_or && (!orport_reachable || !circuit_enough_testing_circs())) {
       extend_info_t *ei = extend_info_from_router(me);
       /* XXX IPv6 self testing */
@@ -1478,6 +1492,7 @@ consider_testing_reachability(int test_or, int test_dir)
       directory_initiate_request(req);
       directory_request_free(req);
     }
+  }
 }
 
 /** Annotate that we found our ORPort reachable. */
@@ -1521,7 +1536,7 @@ router_dirport_found_reachable(void)
                && check_whether_orport_reachable(options) ?
                " Publishing server descriptor." : "");
     can_reach_dir_port = 1;
-    if (decide_to_advertise_dirport(options, me->dir_port)) {
+    if (router_should_advertise_dirport(options, me->dir_port)) {
       mark_my_descriptor_dirty("DirPort found reachable");
       /* This is a significant enough change to upload immediately,
        * at least in a test network */
@@ -2915,7 +2930,7 @@ router_dump_router_to_string(routerinfo_t *router,
     router->nickname,
     address,
     router->or_port,
-    decide_to_advertise_dirport(options, router->dir_port),
+    router_should_advertise_dirport(options, router->dir_port),
     ed_cert_line ? ed_cert_line : "",
     extra_or_address ? extra_or_address : "",
     router->platform,
@@ -2989,7 +3004,7 @@ router_dump_router_to_string(routerinfo_t *router,
     tor_free(p6);
   }
 
-  if (decide_to_advertise_begindir(options,
+  if (router_should_advertise_begindir(options,
                                    router->supports_tunnelled_dir_requests)) {
     smartlist_add_strdup(chunks, "tunnelled-dir-server\n");
   }

src/or/router.h

@@ -47,7 +47,7 @@ int init_keys_client(void);
 int check_whether_orport_reachable(const or_options_t *options);
 int check_whether_dirport_reachable(const or_options_t *options);
 int dir_server_mode(const or_options_t *options);
-void consider_testing_reachability(int test_or, int test_dir);
+void router_do_reachability_checks(int test_or, int test_dir);
 void router_orport_found_reachable(void);
 void router_dirport_found_reachable(void);
 void router_perform_bandwidth_test(int num_circs, time_t now);