commit 5ea993fa5a38353900f9cef3b31f695fc70679b5
Author: Fernando Fernandez Mancera <ffmancera(a)riseup.net>
Date: Wed Jan 24 20:07:49 2018 +0100
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(a)riseup.net>
---
changes/bug18918 | 6 +++
src/or/circuitbuild.c | 4 +-
src/or/circuituse.c | 2 +-
src/or/main.c | 4 +-
src/or/router.c | 129 ++++++++++++++++++++++++++++----------------------
src/or/router.h | 2 +-
6 files changed, 84 insertions(+), 63 deletions(-)
diff --git a/changes/bug18918 b/changes/bug18918
new file mode 100644
index 000000000..c939168f4
--- /dev/null
+++ b/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.
diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c
index 9c049a24b..86d7382d6 100644
--- a/src/or/circuitbuild.c
+++ b/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
diff --git a/src/or/circuituse.c b/src/or/circuituse.c
index 5f9567ea1..d7d7e8233 100644
--- a/src/or/circuituse.c
+++ b/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. */
diff --git a/src/or/main.c b/src/or/main.c
index 10e606f3a..9a4aa01ef 100644
--- a/src/or/main.c
+++ b/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;
diff --git a/src/or/router.c b/src/or/router.c
index 9c053cad4..afba8209e 100644
--- a/src/or/router.c
+++ b/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
- * 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().
+/**See if we currently believe our ORPort or DirPort to be
+ * unreachable. If so, return 1 else return 0.
*/
-void
-consider_testing_reachability(int test_or, int test_dir)
+static int
+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,43 +1432,66 @@ 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 */
+ log_info(LD_CIRC, "Testing %s of my ORPort: %s:%d.",
+ !orport_reachable ? "reachability" : "bandwidth",
+ fmt_addr32(me->addr), me->or_port);
+ circuit_launch_by_extend_info(CIRCUIT_PURPOSE_TESTING, ei,
+ CIRCLAUNCH_NEED_CAPACITY|CIRCLAUNCH_IS_INTERNAL);
+ extend_info_free(ei);
+ }
- if (test_or && (!orport_reachable || !circuit_enough_testing_circs())) {
- extend_info_t *ei = extend_info_from_router(me);
/* XXX IPv6 self testing */
- log_info(LD_CIRC, "Testing %s of my ORPort: %s:%d.",
- !orport_reachable ? "reachability" : "bandwidth",
- fmt_addr32(me->addr), me->or_port);
- circuit_launch_by_extend_info(CIRCUIT_PURPOSE_TESTING, ei,
- CIRCLAUNCH_NEED_CAPACITY|CIRCLAUNCH_IS_INTERNAL);
- extend_info_free(ei);
- }
-
- /* XXX IPv6 self testing */
- tor_addr_from_ipv4h(&addr, me->addr);
- if (test_dir && !check_whether_dirport_reachable(options) &&
- !connection_get_by_type_addr_port_purpose(
- CONN_TYPE_DIR, &addr, me->dir_port,
- DIR_PURPOSE_FETCH_SERVERDESC)) {
- tor_addr_port_t my_orport, my_dirport;
- memcpy(&my_orport.addr, &addr, sizeof(addr));
- memcpy(&my_dirport.addr, &addr, sizeof(addr));
- my_orport.port = me->or_port;
- my_dirport.port = me->dir_port;
- /* ask myself, via tor, for my server descriptor. */
- directory_request_t *req =
- directory_request_new(DIR_PURPOSE_FETCH_SERVERDESC);
- directory_request_set_or_addr_port(req, &my_orport);
- directory_request_set_dir_addr_port(req, &my_dirport);
- directory_request_set_directory_id_digest(req,
+ tor_addr_from_ipv4h(&addr, me->addr);
+ if (test_dir && !check_whether_dirport_reachable(options) &&
+ !connection_get_by_type_addr_port_purpose(
+ CONN_TYPE_DIR, &addr, me->dir_port,
+ DIR_PURPOSE_FETCH_SERVERDESC)) {
+ tor_addr_port_t my_orport, my_dirport;
+ memcpy(&my_orport.addr, &addr, sizeof(addr));
+ memcpy(&my_dirport.addr, &addr, sizeof(addr));
+ my_orport.port = me->or_port;
+ my_dirport.port = me->dir_port;
+ /* ask myself, via tor, for my server descriptor. */
+ directory_request_t *req =
+ directory_request_new(DIR_PURPOSE_FETCH_SERVERDESC);
+ directory_request_set_or_addr_port(req, &my_orport);
+ directory_request_set_dir_addr_port(req, &my_dirport);
+ directory_request_set_directory_id_digest(req,
me->cache_info.identity_digest);
- // ask via an anon circuit, connecting to our dirport.
- directory_request_set_indirection(req, DIRIND_ANON_DIRPORT);
- directory_request_set_resource(req, "authority.z");
- directory_initiate_request(req);
- directory_request_free(req);
+ // ask via an anon circuit, connecting to our dirport.
+ directory_request_set_indirection(req, DIRIND_ANON_DIRPORT);
+ directory_request_set_resource(req, "authority.z");
+ directory_initiate_request(req);
+ directory_request_free(req);
+ }
}
}
@@ -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");
}
diff --git a/src/or/router.h b/src/or/router.h
index 696e98366..7f5030895 100644
--- a/src/or/router.h
+++ b/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);
