diff --git a/changes/bug23670 b/changes/bug23670
new file mode 100644
index 0000000000..039bc39478
--- /dev/null
+++ b/changes/bug23670
@@ -0,0 +1,3 @@
+ o Minor features (entry guards):
+ - Improve logs issued when we are missing descriptors of primary guards.
+ Resolves ticket 23670.
diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c
index 129b983334..9fbf426433 100644
--- a/src/or/entrynodes.c
+++ b/src/or/entrynodes.c
@@ -3440,15 +3440,20 @@ guards_retry_optimistic(const or_options_t *options)
}
/**
- * Return true iff we know enough directory information to construct
- * circuits through all of the primary guards we'd currently use.
- */
-int
-guard_selection_have_enough_dir_info_to_build_circuits(guard_selection_t *gs)
+ * Check if we are missing any crucial dirinfo for the guard subsystem to
+ * work. Return NULL if everything went well, otherwise return a newly
+ * allocated string with an informative error message. In the latter case, use
+ * the genreal descriptor information using_mds, num_present and
+ * num_usable to improve the error message. */
+char *
+guard_selection_get_err_str_if_dir_info_missing(guard_selection_t *gs,
+ int using_mds,
+ int num_present, int num_usable)
{
if (!gs->primary_guards_up_to_date)
entry_guards_update_primary(gs);
+ char *ret_str = NULL;
int n_missing_descriptors = 0;
int n_considered = 0;
int num_primary_to_check;
@@ -3470,16 +3475,30 @@ guard_selection_have_enough_dir_info_to_build_circuits(guard_selection_t *gs)
break;
} SMARTLIST_FOREACH_END(guard);
- return n_missing_descriptors == 0;
+ /* If we are not missing any descriptors, return NULL. */
+ if (!n_missing_descriptors) {
+ return NULL;
+ }
+
+ /* otherwise return a helpful error string */
+ tor_asprintf(&ret_str, "We're missing descriptors for %d/%d of our "
+ "primary entry guards (total %sdescriptors: %d/%d).",
+ n_missing_descriptors, num_primary_to_check,
+ using_mds?"micro":"", num_present, num_usable);
+
+ return ret_str;
}
/** As guard_selection_have_enough_dir_info_to_build_circuits, but uses
* the default guard selection. */
-int
-entry_guards_have_enough_dir_info_to_build_circuits(void)
+char *
+entry_guards_get_err_str_if_dir_info_missing(int using_mds,
+ int num_present, int num_usable)
{
- return guard_selection_have_enough_dir_info_to_build_circuits(
- get_guard_selection_info());
+ return guard_selection_get_err_str_if_dir_info_missing(
+ get_guard_selection_info(),
+ using_mds,
+ num_present, num_usable);
}
/** Free one guard selection context */
diff --git a/src/or/entrynodes.h b/src/or/entrynodes.h
index f74ccd97f4..9e1e729930 100644
--- a/src/or/entrynodes.h
+++ b/src/or/entrynodes.h
@@ -572,9 +572,11 @@ int getinfo_helper_entry_guards(control_connection_t *conn,
int entries_known_but_down(const or_options_t *options);
void entries_retry_all(const or_options_t *options);
-int guard_selection_have_enough_dir_info_to_build_circuits(
- guard_selection_t *gs);
-int entry_guards_have_enough_dir_info_to_build_circuits(void);
+char *entry_guards_get_err_str_if_dir_info_missing(int using_mds,
+ int num_present, int num_usable);
+char *guard_selection_get_err_str_if_dir_info_missing(guard_selection_t *gs,
+ int using_mds,
+ int num_present, int num_usable);
void entry_guards_free_all(void);
diff --git a/src/or/nodelist.c b/src/or/nodelist.c
index df735a9d24..eae74e18b5 100644
--- a/src/or/nodelist.c
+++ b/src/or/nodelist.c
@@ -2282,6 +2282,7 @@ update_router_have_minimum_dir_info(void)
{
time_t now = time(NULL);
int res;
+ int num_present=0, num_usable=0;
const or_options_t *options = get_options();
const networkstatus_t *consensus =
networkstatus_get_reasonably_live_consensus(now,usable_consensus_flavor());
@@ -2300,17 +2301,9 @@ update_router_have_minimum_dir_info(void)
using_md = consensus->flavor == FLAV_MICRODESC;
- if (! entry_guards_have_enough_dir_info_to_build_circuits()) {
- strlcpy(dir_info_status, "We're missing descriptors for some of our "
- "primary entry guards", sizeof(dir_info_status));
- res = 0;
- goto done;
- }
-
/* Check fraction of available paths */
{
char *status = NULL;
- int num_present=0, num_usable=0;
double paths = compute_frac_paths_available(consensus, options, now,
&num_present, &num_usable,
&status);
@@ -2331,6 +2324,18 @@ update_router_have_minimum_dir_info(void)
res = 1;
}
+ { /* Check entry guard dirinfo status */
+ char *guard_error = entry_guards_get_err_str_if_dir_info_missing(using_md,
+ num_present,
+ num_usable);
+ if (guard_error) {
+ strlcpy(dir_info_status, guard_error, sizeof(dir_info_status));
+ tor_free(guard_error);
+ res = 0;
+ goto done;
+ }
+ }
+
done:
/* If paths have just become available in this update. */
diff --git a/src/test/test_entrynodes.c b/src/test/test_entrynodes.c
index 1e008c3a2f..f9d981953d 100644
--- a/src/test/test_entrynodes.c
+++ b/src/test/test_entrynodes.c
@@ -1639,6 +1639,27 @@ test_entry_guard_manage_primary(void *arg)
tt_ptr_op(g, OP_EQ, smartlist_get(prev_guards, g_sl_idx));
});
+ /* Do some dirinfo checks */
+ {
+ /* Check that we have all required dirinfo for the primaries (that's done
+ * in big_fake_network_setup()) */
+ char *dir_info_str =
+ guard_selection_get_err_str_if_dir_info_missing(gs, 0, 0, 0);
+ tt_assert(!dir_info_str);
+
+ /* Now artificially remove the first primary's descriptor and re-check */
+ entry_guard_t *first_primary;
+ first_primary = smartlist_get(gs->primary_entry_guards, 0);
+ /* Change the first primary's identity digest so that the mocked functions
+ * can't find its descriptor */
+ memset(first_primary->identity, 9, sizeof(first_primary->identity));
+ dir_info_str =guard_selection_get_err_str_if_dir_info_missing(gs, 1, 2, 3);
+ tt_str_op(dir_info_str, OP_EQ,
+ "We're missing descriptors for 1/2 of our primary entry guards "
+ "(total microdescriptors: 2/3).");
+ tor_free(dir_info_str);
+ }
+
done:
guard_selection_free(gs);
smartlist_free(prev_guards);