guardmgr: Refactor candidate info; add `full_dir_info`

Previously we always set `dir_info_missing` to `false` for new
guards, since new guards could only be taken from ones that were
present in the NetDir.  But for bridges, we don't download their
info until _after_ we have chosen them as guards.
This commit is contained in:
Nick Mathewson 2022-10-28 12:28:14 -04:00
parent 5867318a35
commit 08473872ab
4 changed files with 72 additions and 30 deletions

View File

@ -9,7 +9,7 @@ use std::time::SystemTime;
use crate::{
bridge::BridgeConfig,
sample::{CandidateStatus, Universe, WeightThreshold},
sample::{Candidate, CandidateStatus, Universe, WeightThreshold},
};
use dyn_clone::DynClone;
use futures::stream::BoxStream;
@ -214,11 +214,12 @@ impl Universe for BridgeSet {
let bridge_relay = self.relay_by_bridge(bridge);
// Logic here is similar to that of "contains"
if bridge_relay.has_all_relay_ids_from(guard) {
CandidateStatus::Present {
CandidateStatus::Present(Candidate {
listed_as_guard: true,
is_dir_cache: true, // all bridges are directory caches.
full_dir_info: bridge_relay.has_descriptor(),
owned_target: OwnedChanTarget::from_chan_target(&bridge_relay),
}
})
} else if bridge_relay.has_descriptor() {
CandidateStatus::Absent
} else {
@ -252,7 +253,7 @@ impl Universe for BridgeSet {
pre_existing: &tor_linkspec::ByRelayIds<T>,
filter: &crate::GuardFilter,
n: usize,
) -> Vec<(tor_linkspec::OwnedChanTarget, tor_netdir::RelayWeight)>
) -> Vec<(Candidate, tor_netdir::RelayWeight)>
where
T: HasRelayIds,
{
@ -268,8 +269,14 @@ impl Universe for BridgeSet {
.choose_multiple(&mut rand::thread_rng(), n)
.into_iter()
.map(|bridge_config| {
let relay = self.relay_by_bridge(bridge_config);
(
OwnedChanTarget::from_chan_target(&self.relay_by_bridge(bridge_config)),
Candidate {
listed_as_guard: true,
is_dir_cache: true,
full_dir_info: relay.has_descriptor(),
owned_target: OwnedChanTarget::from_chan_target(&relay),
},
RelayWeight::from(0),
)
})

View File

@ -10,6 +10,7 @@ use std::time::{Duration, Instant, SystemTime};
use tracing::{trace, warn};
use crate::dirstatus::DirStatus;
use crate::sample::Candidate;
use crate::skew::SkewObservation;
use crate::util::randomize_time;
use crate::{ids::GuardId, GuardParams, GuardRestriction, GuardUsage};
@ -224,11 +225,31 @@ pub(crate) enum NewlyConfirmed {
}
impl Guard {
/// Create a new unused [`Guard`] from a [`Candidate`].
pub(crate) fn from_candidate(
candidate: Candidate,
now: SystemTime,
params: &GuardParams,
) -> Self {
let Candidate {
is_dir_cache,
full_dir_info,
owned_target,
..
} = candidate;
Guard {
is_dir_cache,
dir_info_missing: !full_dir_info,
..Self::from_chan_target(&owned_target, now, params)
}
}
/// Create a new unused [`Guard`] from a [`ChanTarget`].
///
/// This function doesn't check whether the provided relay is a
/// suitable guard node or not: that's up to the caller to decide.
pub(crate) fn from_chan_target<T>(relay: &T, now: SystemTime, params: &GuardParams) -> Self
fn from_chan_target<T>(relay: &T, now: SystemTime, params: &GuardParams) -> Self
where
T: ChanTarget,
{
@ -268,7 +289,7 @@ impl Guard {
disabled: None,
confirmed_at: None,
unlisted_since: None,
dir_info_missing: false, // TODO pt-client this can be wrong for bridges.
dir_info_missing: false,
last_tried_to_connect_at: None,
reachable: Reachable::Unknown,
retry_at: None,
@ -494,11 +515,12 @@ impl Guard {
// for the guard, we won't know its full set of identities.
use sample::CandidateStatus::*;
let listed_as_guard = match universe.status(self) {
Present {
Present(Candidate {
listed_as_guard,
is_dir_cache,
full_dir_info,
owned_target,
} => {
}) => {
// Update address information.
self.orports = owned_target.addrs().into();
// Update Pt information.
@ -512,6 +534,7 @@ impl Guard {
// Update our IDs: the Relay will have strictly more.
assert!(owned_target.has_all_relay_ids_from(self));
self.id = GuardId(RelayIds::from_relay_ids(&owned_target));
self.dir_info_missing = !full_dir_info;
listed_as_guard
}

View File

@ -15,7 +15,7 @@ use crate::{
};
use crate::{FirstHop, GuardSetSelector};
use tor_basic_utils::iter::{FilterCount, IteratorExt as _};
use tor_linkspec::{ByRelayIds, ChanTarget, HasRelayIds};
use tor_linkspec::{ByRelayIds, HasRelayIds};
use itertools::Itertools;
use rand::seq::SliceRandom;
@ -25,7 +25,7 @@ use std::collections::{HashMap, HashSet};
use std::time::{Instant, SystemTime};
use tracing::{debug, info};
pub(crate) use candidate::{CandidateStatus, Universe, WeightThreshold};
pub(crate) use candidate::{Candidate, CandidateStatus, Universe, WeightThreshold};
/// A set of sampled guards, along with various orderings on subsets
/// of the sample.
@ -455,11 +455,11 @@ impl GuardSet {
// We've reached our target; no need to add more.
break;
}
if self.active_filter.permits(&candidate) {
if self.active_filter.permits(&candidate.owned_target) {
n_filtered_usable += 1;
}
current_weight += weight;
self.add_guard(&candidate, now, params);
self.add_guard(candidate, now, params);
any_added = true;
}
self.assert_consistency();
@ -469,13 +469,13 @@ impl GuardSet {
/// Add `relay` as a new guard.
///
/// Does nothing if it is already a guard.
fn add_guard<T: ChanTarget>(&mut self, relay: &T, now: SystemTime, params: &GuardParams) {
let id = GuardId::from_relay_ids(relay);
fn add_guard(&mut self, relay: Candidate, now: SystemTime, params: &GuardParams) {
let id = GuardId::from_relay_ids(&relay.owned_target);
if self.guards.by_all_ids(&id).is_some() {
return;
}
debug!(guard_id=?id, "Adding guard to sample.");
let guard = Guard::from_chan_target(relay, now, params);
let guard = Guard::from_candidate(relay, now, params);
self.guards.insert(guard);
self.sample.push(id);
self.primary_guards_invalidated = true;

View File

@ -40,7 +40,7 @@ pub(crate) trait Universe {
pre_existing: &ByRelayIds<T>,
filter: &GuardFilter,
n: usize,
) -> Vec<(OwnedChanTarget, RelayWeight)>
) -> Vec<(Candidate, RelayWeight)>
where
T: HasRelayIds;
}
@ -50,15 +50,7 @@ pub(crate) trait Universe {
#[derive(Clone, Debug)]
pub(crate) enum CandidateStatus {
/// The candidate is definitely present in some form.
Present {
/// True if the candidate is not currently disabled for use as a guard.
listed_as_guard: bool,
/// True if the candidate can be used as a directory cache.
is_dir_cache: bool,
/// Information about connecting to the candidate and using it to build
/// a channel.
owned_target: OwnedChanTarget,
},
Present(Candidate),
/// The candidate is definitely not in the [`Universe`].
Absent,
/// We would need to download more directory information to be sure whether
@ -66,6 +58,20 @@ pub(crate) enum CandidateStatus {
Uncertain,
}
/// Information about a candidate that we have selected as a guard.
#[derive(Clone, Debug)]
pub(crate) struct Candidate {
/// True if the candidate is not currently disabled for use as a guard.
pub(crate) listed_as_guard: bool,
/// True if the candidate can be used as a directory cache.
pub(crate) is_dir_cache: bool,
/// True if we have complete directory information about this candidate.
pub(crate) full_dir_info: bool,
/// Information about connecting to the candidate and using it to build
/// a channel.
pub(crate) owned_target: OwnedChanTarget,
}
/// Information about how much of the universe we are using in a guard sample,
/// and how much we are allowed to use.
///
@ -93,11 +99,12 @@ impl Universe for NetDir {
fn status<T: ChanTarget>(&self, guard: &T) -> CandidateStatus {
match NetDir::by_ids(self, guard) {
Some(relay) => CandidateStatus::Present {
Some(relay) => CandidateStatus::Present(Candidate {
listed_as_guard: relay.is_flagged_guard(),
is_dir_cache: relay.is_dir_cache(),
owned_target: OwnedChanTarget::from_chan_target(&relay),
},
full_dir_info: true,
}),
None => match NetDir::ids_listed(self, guard) {
Some(true) => panic!("ids_listed said true, but by_ids said none!"),
Some(false) => CandidateStatus::Absent,
@ -139,7 +146,7 @@ impl Universe for NetDir {
pre_existing: &ByRelayIds<T>,
filter: &GuardFilter,
n: usize,
) -> Vec<(OwnedChanTarget, RelayWeight)>
) -> Vec<(Candidate, RelayWeight)>
where
T: HasRelayIds,
{
@ -165,7 +172,12 @@ impl Universe for NetDir {
.iter()
.map(|relay| {
(
OwnedChanTarget::from_chan_target(relay),
Candidate {
listed_as_guard: true,
is_dir_cache: true,
full_dir_info: true,
owned_target: OwnedChanTarget::from_chan_target(relay),
},
weight(self, relay).unwrap_or_else(|| RelayWeight::from(0)),
)
})