[tor-commits] [tor/master] Make sure primary-guards are up-to-date when we inspect them.
nickm at torproject.org
nickm at torproject.org
Fri Dec 16 16:26:18 UTC 2016
commit ac67819396ac9e96c3dd65a5b5b23715e11eeec5
Author: Nick Mathewson <nickm at torproject.org>
Date: Wed Nov 23 10:04:23 2016 -0500
Make sure primary-guards are up-to-date when we inspect them.
(Plus some magic to prevent and detect recursive invocation of
entry_guards_update_primary(), since that can cause some pretty
tricky misbehavior.)
---
src/or/entrynodes.c | 58 +++++++++++++++++++++++++++++++++-------------
src/or/entrynodes.h | 8 +++++++
src/test/test_entrynodes.c | 3 ++-
3 files changed, 52 insertions(+), 17 deletions(-)
diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c
index 9a753e6..bd30078 100644
--- a/src/or/entrynodes.c
+++ b/src/or/entrynodes.c
@@ -79,9 +79,6 @@
**/
/* DOCDOC -- expand this.
*
- * XXXX prop271 -- make sure we check all of these properties everywhere we
- * should.
- *
* Information invariants:
*
* [x] whenever a guard becomes unreachable, clear its usable_filtered flag.
@@ -100,11 +97,11 @@
* [x] Whenever we remove a guard from the sample, remove it from the primary
* and confirmed lists.
*
- * [ ] When we make a guard confirmed, update the primary list.
+ * [x] When we make a guard confirmed, update the primary list.
*
- * [ ] When we make a guard filtered or unfiltered, update the primary list.
+ * [x] When we make a guard filtered or unfiltered, update the primary list.
*
- * [ ] When we are about to pick a guard, make sure that the primary list is
+ * [x] When we are about to pick a guard, make sure that the primary list is
* full.
*
* [x] Before calling sample_reachable_filtered_entry_guards(), make sure
@@ -682,9 +679,12 @@ sampled_guards_update_from_consensus(guard_selection_t *gs)
} SMARTLIST_FOREACH_END(guard);
if (n_changes) {
- /* Regnerate other things. XXXXXX prop271 */
- // XXXX prop271 rebuild confirmed list.
+ gs->primary_guards_up_to_date = 0;
entry_guards_update_filtered_sets(gs);
+ /* We don't need to rebuild the confirmed list right here -- we may have
+ * removed confirmed guards above, but we can't have added any new
+ * confirmed guards.
+ */
entry_guards_changed_for_guard_selection(gs);
}
}
@@ -749,6 +749,7 @@ entry_guard_set_filtered_flags(const or_options_t *options,
guard_selection_t *gs,
entry_guard_t *guard)
{
+ unsigned was_filtered = guard->is_filtered_guard;
guard->is_filtered_guard = 0;
guard->is_usable_filtered_guard = 0;
@@ -763,6 +764,11 @@ entry_guard_set_filtered_flags(const or_options_t *options,
log_debug(LD_GUARD, "Updated sampled guard %s: filtered=%d; "
"reachable_filtered=%d.", entry_guard_describe(guard),
guard->is_filtered_guard, guard->is_usable_filtered_guard);
+
+ if (!bool_eq(was_filtered, guard->is_filtered_guard)) {
+ /* This guard might now be primary or nonprimary. */
+ gs->primary_guards_up_to_date = 0;
+ }
}
/**
@@ -795,6 +801,7 @@ sample_reachable_filtered_entry_guards(guard_selection_t *gs,
const unsigned exclude_confirmed = flags & SAMPLE_EXCLUDE_CONFIRMED;
const unsigned exclude_primary = flags & SAMPLE_EXCLUDE_PRIMARY;
const unsigned exclude_pending = flags & SAMPLE_EXCLUDE_PENDING;
+ const unsigned no_update_primary = flags & SAMPLE_NO_UPDATE_PRIMARY;
SMARTLIST_FOREACH_BEGIN(gs->sampled_entry_guards, entry_guard_t *, guard) {
entry_guard_consider_retry(guard);
@@ -810,6 +817,9 @@ sample_reachable_filtered_entry_guards(guard_selection_t *gs,
entry_guards_expand_sample(gs);
}
+ if (exclude_primary && !gs->primary_guards_up_to_date && !no_update_primary)
+ entry_guards_update_primary(gs);
+
/* Build the set of reachable filtered guards. */
smartlist_t *reachable_filtered_sample = smartlist_new();
SMARTLIST_FOREACH_BEGIN(gs->sampled_entry_guards, entry_guard_t *, guard) {
@@ -908,24 +918,34 @@ make_guard_confirmed(guard_selection_t *gs, entry_guard_t *guard)
guard->confirmed_idx = gs->next_confirmed_idx++;
smartlist_add(gs->confirmed_entry_guards, guard);
+ // This confirmed guard might kick something else out of the primary
+ // guards.
+ gs->primary_guards_up_to_date = 0;
+
entry_guards_changed_for_guard_selection(gs);
}
/**
* Recalculate the list of primary guards (the ones we'd prefer to use) from
* the filtered sample and the confirmed list.
- *
- * XXXXX prop271 are calling this enough ???
*/
STATIC void
entry_guards_update_primary(guard_selection_t *gs)
{
tor_assert(gs);
+ // prevent recursion. Recursion is potentially very bad here.
+ static int running = 0;
+ tor_assert(!running);
+ running = 1;
+
smartlist_t *new_primary_guards = smartlist_new();
smartlist_t *old_primary_guards = smartlist_new();
smartlist_add_all(old_primary_guards, gs->primary_entry_guards);
+ /* Set this flag now, to prevent the calls below from recursing. */
+ gs->primary_guards_up_to_date = 1;
+
/* First, can we fill it up with confirmed guards? */
SMARTLIST_FOREACH_BEGIN(gs->confirmed_entry_guards, entry_guard_t *, guard) {
if (smartlist_len(new_primary_guards) >= N_PRIMARY_GUARDS)
@@ -964,7 +984,8 @@ entry_guards_update_primary(guard_selection_t *gs)
while (smartlist_len(new_primary_guards) < N_PRIMARY_GUARDS) {
entry_guard_t *guard = sample_reachable_filtered_entry_guards(gs,
SAMPLE_EXCLUDE_CONFIRMED|
- SAMPLE_EXCLUDE_PRIMARY);
+ SAMPLE_EXCLUDE_PRIMARY|
+ SAMPLE_NO_UPDATE_PRIMARY);
if (!guard)
break;
guard->is_primary = 1;
@@ -1007,6 +1028,8 @@ entry_guards_update_primary(guard_selection_t *gs)
smartlist_free(old_primary_guards);
smartlist_free(gs->primary_entry_guards);
gs->primary_entry_guards = new_primary_guards;
+ gs->primary_guards_up_to_date = 1;
+ running = 0;
}
/**
@@ -1104,6 +1127,9 @@ select_entry_guard_for_circuit(guard_selection_t *gs, unsigned *state_out)
tor_assert(gs);
tor_assert(state_out);
+ if (!gs->primary_guards_up_to_date)
+ entry_guards_update_primary(gs);
+
/* "If any entry in PRIMARY_GUARDS has {is_reachable} status of
<maybe> or <yes>, return the first such guard." */
SMARTLIST_FOREACH_BEGIN(gs->primary_entry_guards, entry_guard_t *, guard) {
@@ -1191,6 +1217,9 @@ entry_guards_note_guard_failure(guard_selection_t *gs,
STATIC void
mark_primary_guards_maybe_reachable(guard_selection_t *gs)
{
+ if (!gs->primary_guards_up_to_date)
+ entry_guards_update_primary(gs);
+
SMARTLIST_FOREACH_BEGIN(gs->primary_entry_guards, entry_guard_t *, guard) {
if (guard->is_reachable != GUARD_REACHABLE_NO)
continue;
@@ -1230,7 +1259,6 @@ entry_guards_note_guard_success(guard_selection_t *gs,
guard->is_usable_filtered_guard = 1;
if (guard->confirmed_idx < 0) {
- // XXXX prop271 XXXX update primary guards, since we confirmed something?
make_guard_confirmed(gs, guard);
}
@@ -1248,9 +1276,6 @@ entry_guards_note_guard_success(guard_selection_t *gs,
}
}
- // XXXX prop271 XXXX update primary guards, since we confirmed something?
- // XXXX prop261 XXXX if so, here or above?
-
log_info(LD_GUARD, "Recorded success for %s%sguard %s",
guard->is_primary?"primary ":"",
guard->confirmed_idx>=0?"confirmed ":"",
@@ -1468,8 +1493,9 @@ entry_guard_chan_failed(guard_selection_t *gs,
static int
entry_guards_all_primary_guards_are_down(guard_selection_t *gs)
{
- /* XXXXX prop271 do we have to call entry_guards_update_primary() ?? */
tor_assert(gs);
+ if (!gs->primary_guards_up_to_date)
+ entry_guards_update_primary(gs);
SMARTLIST_FOREACH_BEGIN(gs->primary_entry_guards, entry_guard_t *, guard) {
entry_guard_consider_retry(guard);
if (guard->is_reachable != GUARD_REACHABLE_NO)
diff --git a/src/or/entrynodes.h b/src/or/entrynodes.h
index 4cbfbf5..a514c13 100644
--- a/src/or/entrynodes.h
+++ b/src/or/entrynodes.h
@@ -223,6 +223,13 @@ struct guard_selection_s {
int dirty;
/**
+ * A value of 1 means that primary_entry_guards is up-to-date; 0
+ * means we need to recalculate it before using primary_entry_guards
+ * or the is_primary flag on any guard.
+ */
+ int primary_guards_up_to_date;
+
+ /**
* A list of the sampled entry guards, as entry_guard_t structures.
* Not in any particular order. When we 'sample' a guard, we are
* noting it as a possible guard to pick in the future. The use of
@@ -428,6 +435,7 @@ STATIC void entry_guards_update_filtered_sets(guard_selection_t *gs);
#define SAMPLE_EXCLUDE_CONFIRMED (1u<<0)
#define SAMPLE_EXCLUDE_PRIMARY (1u<<1)
#define SAMPLE_EXCLUDE_PENDING (1u<<2)
+#define SAMPLE_NO_UPDATE_PRIMARY (1u<<3)
/**@}*/
STATIC entry_guard_t *sample_reachable_filtered_entry_guards(
guard_selection_t *gs,
diff --git a/src/test/test_entrynodes.c b/src/test/test_entrynodes.c
index d8b9ccb..cdf8672 100644
--- a/src/test/test_entrynodes.c
+++ b/src/test/test_entrynodes.c
@@ -1839,6 +1839,7 @@ test_entry_guard_sample_reachable_filtered(void *arg)
g->pb.path_bias_disabled = 1;
entry_guards_update_filtered_sets(gs);
+ gs->primary_guards_up_to_date = 1;
tt_int_op(num_reachable_filtered_guards(gs), OP_EQ, n_guards - 1);
tt_int_op(smartlist_len(gs->sampled_entry_guards), OP_EQ, n_guards);
@@ -1851,7 +1852,7 @@ test_entry_guard_sample_reachable_filtered(void *arg)
} tests[] = {
{ 0, -1 },
{ SAMPLE_EXCLUDE_CONFIRMED, 1 },
- { SAMPLE_EXCLUDE_PRIMARY, 2 },
+ { SAMPLE_EXCLUDE_PRIMARY|SAMPLE_NO_UPDATE_PRIMARY, 2 },
{ SAMPLE_EXCLUDE_PENDING, 0 },
{ -1, -1},
};
More information about the tor-commits
mailing list