[tor-commits] [tor/master] Refactor reachability test logging: log exactly what we launch.
nickm at torproject.org
nickm at torproject.org
Wed Jul 1 14:34:48 UTC 2020
commit 2238b6320b6e33b5c661012fcbe6831b42a8e123
Author: Nick Mathewson <nickm at torproject.org>
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);
More information about the tor-commits
mailing list