[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

[tor-commits] [tor/master] Refactor reachability test logging: log exactly what we launch.



commit 2238b6320b6e33b5c661012fcbe6831b42a8e123
Author: Nick Mathewson <nickm@xxxxxxxxxxxxxx>
Date:   Tue Jun 30 11:12:46 2020 -0400

    Refactor reachability test logging: log exactly what we launch.
    
    Previously we had two chains of logic for reachability tests: one
    for launching them, and one for telling the user that we had
    launched them.  Now, we simply have the launch code inform the user:
    this way, we can't get out of sync.
    
    Closes ticket 34137.
---
 changes/ticket34137              |  5 +++
 src/core/or/circuitbuild.c       |  1 -
 src/feature/relay/relay_config.c |  2 -
 src/feature/relay/selftest.c     | 90 ++++++++++++++--------------------------
 src/feature/relay/selftest.h     |  1 -
 5 files changed, 35 insertions(+), 64 deletions(-)

diff --git a/changes/ticket34137 b/changes/ticket34137
new file mode 100644
index 000000000..0982d9dd3
--- /dev/null
+++ b/changes/ticket34137
@@ -0,0 +1,5 @@
+  o Minor features (relay):
+    - Log immediately when launching a relay self-check.  Previously
+      we would try to log before launching checks, or approximately
+      when we intended to launch checks, but this tended to be
+      error-prone.  Closes ticket 34137.
diff --git a/src/core/or/circuitbuild.c b/src/core/or/circuitbuild.c
index 652d85b1c..e971fb086 100644
--- a/src/core/or/circuitbuild.c
+++ b/src/core/or/circuitbuild.c
@@ -1056,7 +1056,6 @@ circuit_build_no_more_hops(origin_circuit_t *circ)
     clear_broken_connection_map(1);
     if (server_mode(options) &&
         !router_all_orports_seem_reachable(options)) {
-      inform_testing_reachability();
       router_do_reachability_checks(1, 1);
     }
   }
diff --git a/src/feature/relay/relay_config.c b/src/feature/relay/relay_config.c
index 7b43b5742..bd1c1e8fe 100644
--- a/src/feature/relay/relay_config.c
+++ b/src/feature/relay/relay_config.c
@@ -1114,8 +1114,6 @@ options_act_relay(const or_options_t *old_options)
 
       if (server_mode_turned_on) {
         ip_address_changed(0);
-        if (have_completed_a_circuit() || !any_predicted_circuits(time(NULL)))
-          inform_testing_reachability();
       }
       cpuworkers_rotate_keyinfo();
     }
diff --git a/src/feature/relay/selftest.c b/src/feature/relay/selftest.c
index 5602ac1d4..6420b29fb 100644
--- a/src/feature/relay/selftest.c
+++ b/src/feature/relay/selftest.c
@@ -45,6 +45,9 @@
 #include "feature/relay/selftest.h"
 
 static bool have_orport_for_family(int family);
+static void inform_testing_reachability(const tor_addr_t *addr,
+                                        uint16_t port,
+                                        bool is_dirport);
 
 /** Whether we can reach our IPv4 ORPort from the outside. */
 static bool can_reach_or_port_ipv4 = false;
@@ -255,6 +258,9 @@ router_do_orport_reachability_checks(const routerinfo_t *me,
     log_info(LD_CIRC, "Testing %s of my %s ORPort: %s.",
              !orport_reachable ? "reachability" : "bandwidth",
              family_name, fmt_addrport(&ei->addr, ei->port));
+    if (!orport_reachable) {
+      inform_testing_reachability(&ei->addr, ei->port, false);
+    }
     circuit_launch_by_extend_info(CIRCUIT_PURPOSE_TESTING, ei,
                                   CIRCLAUNCH_NEED_CAPACITY|
                                   CIRCLAUNCH_IS_INTERNAL|
@@ -292,6 +298,8 @@ router_do_dirport_reachability_checks(const routerinfo_t *me)
     directory_request_set_resource(req, "authority.z");
     directory_initiate_request(req);
     directory_request_free(req);
+
+    inform_testing_reachability(&my_dirport.addr, my_dirport.port, true);
   }
 }
 
@@ -335,73 +343,35 @@ router_do_reachability_checks(int test_or, int test_dir)
   }
 }
 
-/** If reachability testing is in progress, let the user know that it's
- * happening.
+/** Log a message informing the user that we are testing a port for
+ * reachability.
  *
- * If all is set, log a notice-level message. Return 1 if we did, or 0 if
- * we chose not to log anything, because we were unable to test reachability.
- */
-int
-inform_testing_reachability(void)
+ * If @a is_dirport is true, then the port is a DirPort; otherwise it is an
+ * ORPort. */
+static void
+inform_testing_reachability(const tor_addr_t *addr,
+                            uint16_t port,
+                            bool is_dirport)
 {
-  char ipv4_or_buf[TOR_ADDRPORT_BUF_LEN];
-  char ipv6_or_buf[TOR_ADDRPORT_BUF_LEN];
-  char ipv4_dir_buf[TOR_ADDRPORT_BUF_LEN];
-
-  /* There's a race condition here, between:
-   *  - tor launching reachability tests,
-   *  - any circuits actually completing,
-   *  - routerinfo updates, and
-   *  - these log messages.
-   * In rare cases, we might log the wrong ports, log when we didn't actually
-   * start reachability tests, or fail to log after we actually started
-   * reachability tests.
-   *
-   * We'll deal with late disabled IPv6 ORPorts and IPv4 DirPorts, and
-   * extra or skipped log messages in #34137.
-   */
-  const routerinfo_t *me = router_get_my_routerinfo();
-  if (!me)
-    return 0;
+  if (!router_get_my_routerinfo())
+    return;
+
+  char addr_buf[TOR_ADDRPORT_BUF_LEN];
+  strlcpy(addr_buf, fmt_addrport(addr, port), sizeof(addr_buf));
+
+  const char *control_addr_type = is_dirport ? "DIRADDRESS" : "ORADDRESS";
+  const char *port_type = is_dirport ? "DirPort" : "ORPort";
+  const char *afname = fmt_af_family(tor_addr_family(addr));
 
-  /* IPv4 ORPort */
-  strlcpy(ipv4_or_buf, fmt_addr32_port(me->addr, me->or_port),
-          sizeof(ipv4_or_buf));
   control_event_server_status(LOG_NOTICE,
-                              "CHECKING_REACHABILITY ORADDRESS=%s",
-                              ipv4_or_buf);
-  /* IPv6 ORPort */
-  const bool has_ipv6 = tor_addr_port_is_valid(&me->ipv6_addr,
-                                               me->ipv6_orport, 0);
-  if (has_ipv6) {
-    strlcpy(ipv6_or_buf, fmt_addrport(&me->ipv6_addr, me->ipv6_orport),
-            sizeof(ipv6_or_buf));
-    control_event_server_status(LOG_NOTICE,
-                                "CHECKING_REACHABILITY ORADDRESS=%s",
-                                ipv6_or_buf);
-    /* We'll add an IPv6 control event in #34068. */
-  }
-  /* IPv4 DirPort (there are no advertised IPv6 DirPorts) */
-  if (me->dir_port) {
-    strlcpy(ipv4_dir_buf, fmt_addr32_port(me->addr, me->dir_port),
-            sizeof(ipv4_dir_buf));
-    control_event_server_status(LOG_NOTICE,
-                                "CHECKING_REACHABILITY DIRADDRESS=%s",
-                                ipv4_dir_buf);
-  }
-  log_notice(LD_OR, "Now checking whether ORPort%s %s%s%s%s%s %s reachable... "
+                              "CHECKING_REACHABILITY %s=%s",
+                              control_addr_type, addr_buf);
+
+  log_notice(LD_OR, "Now checking whether %s %s %s is reachable... "
              "(this may take up to %d minutes -- look for log "
              "messages indicating success)",
-             has_ipv6 ? "s" : "",
-             ipv4_or_buf,
-             has_ipv6 ? " and " : "",
-             has_ipv6 ? ipv6_or_buf : "",
-             me->dir_port ? " and DirPort " : "",
-             me->dir_port ? ipv4_dir_buf : "",
-             has_ipv6 || me->dir_port ? "are" : "is",
+             afname, port_type, addr_buf,
              TIMEOUT_UNTIL_UNREACHABILITY_COMPLAINT/60);
-
-  return 1;
 }
 
 /**
diff --git a/src/feature/relay/selftest.h b/src/feature/relay/selftest.h
index 911d8bdc3..e09c0e789 100644
--- a/src/feature/relay/selftest.h
+++ b/src/feature/relay/selftest.h
@@ -25,7 +25,6 @@ int router_dirport_seems_reachable(
 
 void router_do_reachability_checks(int test_or, int test_dir);
 void router_perform_bandwidth_test(int num_circs, time_t now);
-int inform_testing_reachability(void);
 
 void router_orport_found_reachable(int family);
 void router_dirport_found_reachable(void);



_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits