aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--changes/ticket271696
-rw-r--r--src/core/or/connection_or.c1
-rw-r--r--src/core/or/relay.c4
-rw-r--r--src/feature/control/control.c39
-rw-r--r--src/feature/control/control.h2
-rw-r--r--src/feature/dircache/directory.c8
-rw-r--r--src/feature/nodelist/nodelist.c4
-rw-r--r--src/test/test_controller_events.c68
8 files changed, 124 insertions, 8 deletions
diff --git a/changes/ticket27169 b/changes/ticket27169
new file mode 100644
index 0000000000..7854532a66
--- /dev/null
+++ b/changes/ticket27169
@@ -0,0 +1,6 @@
+ o Minor features (bootstrap):
+ - Improve user experience by deferring directory progress
+ reporting until after a connection to a relay or bridge has
+ succeeded. This avoids reporting 80% progress based on cached
+ directory information when we can't even connect to a bridge or
+ relay. Closes ticket 27169.
diff --git a/src/core/or/connection_or.c b/src/core/or/connection_or.c
index 08371d1ad7..e6b2354012 100644
--- a/src/core/or/connection_or.c
+++ b/src/core/or/connection_or.c
@@ -705,6 +705,7 @@ connection_or_finished_connecting(or_connection_t *or_conn)
log_debug(LD_HANDSHAKE,"OR connect() to router at %s:%u finished.",
conn->address,conn->port);
control_event_bootstrap(BOOTSTRAP_STATUS_HANDSHAKE, 0);
+ control_event_boot_first_orconn();
if (proxy_type != PROXY_NONE) {
/* start proxy handshake */
diff --git a/src/core/or/relay.c b/src/core/or/relay.c
index 6e1adfaff3..1d0be0c894 100644
--- a/src/core/or/relay.c
+++ b/src/core/or/relay.c
@@ -1389,8 +1389,8 @@ connection_edge_process_relay_cell_not_open(
case DIR_PURPOSE_FETCH_SERVERDESC:
case DIR_PURPOSE_FETCH_MICRODESC:
if (TO_DIR_CONN(dirconn)->router_purpose == ROUTER_PURPOSE_GENERAL)
- control_event_bootstrap(BOOTSTRAP_STATUS_LOADING_DESCRIPTORS,
- count_loading_descriptors_progress());
+ control_event_boot_dir(BOOTSTRAP_STATUS_LOADING_DESCRIPTORS,
+ count_loading_descriptors_progress());
break;
}
}
diff --git a/src/feature/control/control.c b/src/feature/control/control.c
index d68f1e3b07..2fac3bd6de 100644
--- a/src/feature/control/control.c
+++ b/src/feature/control/control.c
@@ -7178,6 +7178,45 @@ control_event_bootstrap(bootstrap_status_t status, int progress)
}
}
+/** Flag whether we've opened an OR_CONN yet */
+static int bootstrap_first_orconn = 0;
+
+/** Like bootstrap_phase, but for (possibly deferred) directory progress */
+static int bootstrap_dir_phase = BOOTSTRAP_STATUS_UNDEF;
+
+/** Like bootstrap_problems, but for (possibly deferred) directory progress */
+static int bootstrap_dir_progress = BOOTSTRAP_STATUS_UNDEF;
+
+/** Defer directory info bootstrap events until we have successfully
+ * completed our first connection to a router. */
+void
+control_event_boot_dir(bootstrap_status_t status, int progress)
+{
+ if (status > bootstrap_dir_progress) {
+ bootstrap_dir_progress = status;
+ bootstrap_dir_phase = status;
+ }
+ if (progress && progress >= bootstrap_dir_progress) {
+ bootstrap_dir_progress = progress;
+ }
+
+ /* Don't report unless we have successfully opened at least one OR_CONN */
+ if (!bootstrap_first_orconn)
+ return;
+
+ control_event_bootstrap(status, progress);
+}
+
+/** Set a flag to allow reporting of directory bootstrap progress.
+ * (Code that reports completion of an OR_CONN calls this.) Also,
+ * report directory progress so far. */
+void
+control_event_boot_first_orconn(void)
+{
+ bootstrap_first_orconn = 1;
+ control_event_bootstrap(bootstrap_dir_phase, bootstrap_dir_progress);
+}
+
/** Called when Tor has failed to make bootstrapping progress in a way
* that indicates a problem. <b>warn</b> gives a human-readable hint
* as to why, and <b>reason</b> provides a controller-facing short
diff --git a/src/feature/control/control.h b/src/feature/control/control.h
index 7f57228e5c..3445eb0a9d 100644
--- a/src/feature/control/control.h
+++ b/src/feature/control/control.h
@@ -195,6 +195,8 @@ void control_event_bootstrap(bootstrap_status_t status, int progress);
MOCK_DECL(void, control_event_bootstrap_prob_or,(const char *warn,
int reason,
or_connection_t *or_conn));
+void control_event_boot_dir(bootstrap_status_t status, int progress);
+void control_event_boot_first_orconn(void);
void control_event_bootstrap_problem(const char *warn, const char *reason,
const connection_t *conn, int dowarn);
diff --git a/src/feature/dircache/directory.c b/src/feature/dircache/directory.c
index de0bcdbfa7..b94c5317af 100644
--- a/src/feature/dircache/directory.c
+++ b/src/feature/dircache/directory.c
@@ -2226,8 +2226,8 @@ load_downloaded_routers(const char *body, smartlist_t *which,
added = router_load_routers_from_string(body, NULL, SAVED_NOWHERE, which,
descriptor_digests, buf);
if (added && general)
- control_event_bootstrap(BOOTSTRAP_STATUS_LOADING_DESCRIPTORS,
- count_loading_descriptors_progress());
+ control_event_boot_dir(BOOTSTRAP_STATUS_LOADING_DESCRIPTORS,
+ count_loading_descriptors_progress());
return added;
}
@@ -2949,8 +2949,8 @@ handle_response_fetch_microdesc(dir_connection_t *conn,
dir_microdesc_download_failed(which, status_code, conn->identity_digest);
}
if (mds && smartlist_len(mds)) {
- control_event_bootstrap(BOOTSTRAP_STATUS_LOADING_DESCRIPTORS,
- count_loading_descriptors_progress());
+ control_event_boot_dir(BOOTSTRAP_STATUS_LOADING_DESCRIPTORS,
+ count_loading_descriptors_progress());
directory_info_has_arrived(now, 0, 1);
}
SMARTLIST_FOREACH(which, char *, cp, tor_free(cp));
diff --git a/src/feature/nodelist/nodelist.c b/src/feature/nodelist/nodelist.c
index 6dd501ea34..50dc8f7d3c 100644
--- a/src/feature/nodelist/nodelist.c
+++ b/src/feature/nodelist/nodelist.c
@@ -2528,7 +2528,7 @@ update_router_have_minimum_dir_info(void)
(int)(paths*100), status);
tor_free(status);
res = 0;
- control_event_bootstrap(BOOTSTRAP_STATUS_REQUESTING_DESCRIPTORS, 0);
+ control_event_boot_dir(BOOTSTRAP_STATUS_REQUESTING_DESCRIPTORS, 0);
goto done;
}
@@ -2553,7 +2553,7 @@ update_router_have_minimum_dir_info(void)
/* If paths have just become available in this update. */
if (res && !have_min_dir_info) {
control_event_client_status(LOG_NOTICE, "ENOUGH_DIR_INFO");
- control_event_bootstrap(BOOTSTRAP_STATUS_CONN_OR, 0);
+ control_event_boot_dir(BOOTSTRAP_STATUS_CONN_OR, 0);
log_info(LD_DIR,
"We now have enough directory information to build circuits.");
}
diff --git a/src/test/test_controller_events.c b/src/test/test_controller_events.c
index e935b70428..4c404876b0 100644
--- a/src/test/test_controller_events.c
+++ b/src/test/test_controller_events.c
@@ -322,6 +322,72 @@ test_cntev_event_mask(void *arg)
;
}
+static char *saved_event_str = NULL;
+
+static void
+mock_queue_control_event_string(uint16_t event, char *msg)
+{
+ (void)event;
+
+ tor_free(saved_event_str);
+ saved_event_str = msg;
+}
+
+/* Helper macro for checking bootstrap control event strings */
+#define assert_bootmsg(s) \
+ tt_ptr_op(strstr(saved_event_str, "650 STATUS_CLIENT NOTICE " \
+ "BOOTSTRAP PROGRESS=" s), OP_EQ, saved_event_str)
+
+/* Test deferral of directory bootstrap messages (requesting_descriptors) */
+static void
+test_cntev_dirboot_defer_desc(void *arg)
+{
+ (void)arg;
+
+ MOCK(queue_control_event_string, mock_queue_control_event_string);
+ control_testing_set_global_event_mask(EVENT_MASK_(EVENT_STATUS_CLIENT));
+ control_event_bootstrap(BOOTSTRAP_STATUS_STARTING, 0);
+ assert_bootmsg("0 TAG=starting");
+ /* This event should get deferred */
+ control_event_boot_dir(BOOTSTRAP_STATUS_REQUESTING_DESCRIPTORS, 0);
+ assert_bootmsg("0 TAG=starting");
+ control_event_bootstrap(BOOTSTRAP_STATUS_CONN_DIR, 0);
+ assert_bootmsg("5 TAG=conn_dir");
+ control_event_bootstrap(BOOTSTRAP_STATUS_HANDSHAKE, 0);
+ assert_bootmsg("10 TAG=handshake_dir");
+ /* The deferred event should appear */
+ control_event_boot_first_orconn();
+ assert_bootmsg("45 TAG=requesting_descriptors");
+ done:
+ tor_free(saved_event_str);
+ UNMOCK(queue_control_event_string);
+}
+
+/* Test deferral of directory bootstrap messages (conn_or) */
+static void
+test_cntev_dirboot_defer_orconn(void *arg)
+{
+ (void)arg;
+
+ MOCK(queue_control_event_string, mock_queue_control_event_string);
+ control_testing_set_global_event_mask(EVENT_MASK_(EVENT_STATUS_CLIENT));
+ control_event_bootstrap(BOOTSTRAP_STATUS_STARTING, 0);
+ assert_bootmsg("0 TAG=starting");
+ /* This event should get deferred */
+ control_event_boot_dir(BOOTSTRAP_STATUS_CONN_OR, 0);
+ assert_bootmsg("0 TAG=starting");
+ control_event_bootstrap(BOOTSTRAP_STATUS_CONN_DIR, 0);
+ assert_bootmsg("5 TAG=conn_dir");
+ control_event_bootstrap(BOOTSTRAP_STATUS_HANDSHAKE, 0);
+ assert_bootmsg("10 TAG=handshake_dir");
+ /* The deferred event should appear */
+ control_event_boot_first_orconn();
+ assert_bootmsg("80 TAG=conn_or");
+ done:
+ tor_free(saved_event_str);
+ UNMOCK(queue_control_event_string);
+}
+
#define TEST(name, flags) \
{ #name, test_cntev_ ## name, flags, 0, NULL }
@@ -330,5 +396,7 @@ struct testcase_t controller_event_tests[] = {
TEST(append_cell_stats, TT_FORK),
TEST(format_cell_stats, TT_FORK),
TEST(event_mask, TT_FORK),
+ TEST(dirboot_defer_desc, TT_FORK),
+ TEST(dirboot_defer_orconn, TT_FORK),
END_OF_TESTCASES
};
7apu2bq3rhoylwmucxizk54afw5jyda7pho4j5zdcqpwkufke7zdzwad.onion