vincenzopalazzo
/
arti
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Make CircMgrConfig transparent (and make it a trait) 

See commentary for the rationale.
This commit is contained in:
Ian Jackson 2022-03-16 17:55:40 +00:00
parent 8d54c0f073
commit 8bde40fdd3
4 changed files with 47 additions and 68 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
crates/arti-client/src/client.rs
View File

@ -332,11 +332,9 @@ impl<R: Runtime> TorClient<R> {
autobootstrap: BootstrapBehavior,
dirmgr_builder: &dyn crate::builder::DirProviderBuilder<R>,
) -> StdResult<Self, ErrorDetail> {
let circ_cfg = config.get_circmgr_config()?;
let dir_cfg = config.get_dirmgr_config()?;
let statemgr = FsStateMgr::from_path(config.storage.expand_state_dir()?)?;
let addr_cfg = config.address_filter.clone();
let timeout_cfg = config.stream_timeouts;
let (status_sender, status_receiver) = postage::watch::channel();
let status_receiver = status::BootstrapEvents {
@ -344,9 +342,10 @@ impl<R: Runtime> TorClient<R> {
};
let chanmgr = Arc::new(tor_chanmgr::ChanMgr::new(runtime.clone()));
let circmgr =
tor_circmgr::CircMgr::new(circ_cfg, statemgr.clone(), &runtime, Arc::clone(&chanmgr))
tor_circmgr::CircMgr::new(&config, statemgr.clone(), &runtime, Arc::clone(&chanmgr))
.map_err(ErrorDetail::CircMgrSetup)?;
let timeout_cfg = config.stream_timeouts;
let dirmgr = dirmgr_builder
.build(runtime.clone(), Arc::clone(&circmgr), dir_cfg)
.map_err(crate::Error::into_detail)?;
@ -541,7 +540,6 @@ impl<R: Runtime> TorClient<R> {
_ => {}
}
let circ_cfg = new_config.get_circmgr_config().map_err(wrap_err)?;
let dir_cfg = new_config.get_dirmgr_config().map_err(wrap_err)?;
let state_cfg = new_config.storage.expand_state_dir().map_err(wrap_err)?;
let addr_cfg = &new_config.address_filter;
@ -551,7 +549,9 @@ impl<R: Runtime> TorClient<R> {
how.cannot_change("storage.state_dir").map_err(wrap_err)?;
}
self.circmgr.reconfigure(&circ_cfg, how).map_err(wrap_err)?;
self.circmgr
.reconfigure(new_config, how)
.map_err(wrap_err)?;
self.dirmgr.reconfigure(&dir_cfg, how).map_err(wrap_err)?;
if how == tor_config::Reconfigure::CheckAllOrNothing {

23
crates/arti-client/src/config.rs
View File

@ -12,6 +12,7 @@
//! [#285]: https://gitlab.torproject.org/tpo/core/arti/-/issues/285
use derive_builder::Builder;
use derive_more::AsRef;
use serde::Deserialize;
use std::collections::HashMap;
use std::path::Path;
@ -22,8 +23,8 @@ pub use tor_config::{CfgPath, ConfigBuildError, Reconfigure};
/// Types for configuring how Tor circuits are built.
pub mod circ {
pub use tor_circmgr::{
CircMgrConfig, CircMgrConfigBuilder, CircuitTiming, CircuitTimingBuilder, PathConfig,
PathConfigBuilder, PreemptiveCircuitConfig, PreemptiveCircuitConfigBuilder,
CircMgrConfig, CircuitTiming, CircuitTimingBuilder, PathConfig, PathConfigBuilder,
PreemptiveCircuitConfig, PreemptiveCircuitConfigBuilder,
};
}
@ -270,7 +271,7 @@ impl SystemConfig {
/// to change. For more information see ticket [#285].
///
/// [#285]: https://gitlab.torproject.org/tpo/core/arti/-/issues/285
#[derive(Clone, Debug, Eq, PartialEq)]
#[derive(Clone, Debug, Eq, PartialEq, AsRef)]
pub struct TorClientConfig {
/// Information about the Tor network we want to connect to.
tor_network: dir::NetworkConfig,
@ -286,12 +287,15 @@ pub struct TorClientConfig {
override_net_params: tor_netdoc::doc::netstatus::NetParams<i32>,
/// Information about how to build paths through the network.
#[as_ref]
path_rules: circ::PathConfig,
/// Information about preemptive circuits.
#[as_ref]
preemptive_circuits: circ::PreemptiveCircuitConfig,
/// Information about how to retry and expire circuits and request for circuits.
#[as_ref]
circuit_timing: circ::CircuitTiming,
/// Rules about which addresses the client is willing to connect to.
@ -304,6 +308,8 @@ pub struct TorClientConfig {
pub system: SystemConfig,
}
impl tor_circmgr::CircMgrConfig for TorClientConfig {}
impl Default for TorClientConfig {
fn default() -> Self {
Self::builder()
@ -329,17 +335,6 @@ impl TorClientConfig {
}
dircfg.build()
}
/// Return a [`CircMgrConfig`](circ::CircMgrConfig) object based on the user's selected
/// configuration.
pub(crate) fn get_circmgr_config(&self) -> Result<circ::CircMgrConfig, ConfigBuildError> {
let mut builder = circ::CircMgrConfigBuilder::default();
builder
.path_rules(self.path_rules.clone())
.circuit_timing(self.circuit_timing.clone())
.preemptive_circuits(self.preemptive_circuits.clone())
.build()
}
}
/// Builder object used to construct a [`TorClientConfig`].

47
crates/tor-circmgr/src/config.rs
View File

@ -248,37 +248,26 @@ impl PreemptiveCircuitConfig {
}
}
/// Configuration for a circuit manager.
/// Configuration for a circuit manager
///
/// This configuration includes information about how to build paths
/// on the Tor network, and rules for timeouts and retries on Tor
/// circuits.
/// If the circuit manager gains new configurabilities, this trait will gain additional
/// supertraits, as an API break.
///
/// This type is immutable once constructed. To create an object of
/// this type, use [`CircMgrConfigBuilder`], or deserialize it from a
/// string. (Arti generally uses Toml for configuration, but you can
/// use other formats if you prefer.)
#[derive(Debug, Clone, Builder, Default, Eq, PartialEq)]
#[builder(build_fn(error = "ConfigBuildError"))]
pub struct CircMgrConfig {
/// Override the default required distance for two relays to share
/// the same circuit.
#[builder(default)]
pub(crate) path_rules: PathConfig,
/// Timing and retry information related to circuits themselves.
#[builder(default)]
pub(crate) circuit_timing: CircuitTiming,
/// Information related to preemptive circuits.
#[builder(default)]
pub(crate) preemptive_circuits: PreemptiveCircuitConfig,
}
impl CircMgrConfig {
/// Return a new [`CircMgrConfigBuilder`].
pub fn builder() -> CircMgrConfigBuilder {
CircMgrConfigBuilder::default()
/// Prefer to use `TorClientConfig`, which will always implement this trait.
pub trait CircMgrConfig:
AsRef<PathConfig> + AsRef<CircuitTiming> + AsRef<PreemptiveCircuitConfig>
{
/// Get the PathConfig
fn path_rules(&self) -> &PathConfig {
self.as_ref()
}
/// Get the CircuitTiming
fn circuit_timing(&self) -> &CircuitTiming {
self.as_ref()
}
/// Get the PreemptiveCircuitConfig
fn preemptive_circuits(&self) -> &PreemptiveCircuitConfig {
self.as_ref()
}
}

35
crates/tor-circmgr/src/lib.rs
View File

@ -74,8 +74,8 @@ pub use err::Error;
pub use usage::{IsolationToken, StreamIsolation, StreamIsolationBuilder, TargetPort, TargetPorts};
pub use config::{
CircMgrConfig, CircMgrConfigBuilder, CircuitTiming, CircuitTimingBuilder, PathConfig,
PathConfigBuilder, PreemptiveCircuitConfig, PreemptiveCircuitConfigBuilder,
CircMgrConfig, CircuitTiming, CircuitTimingBuilder, PathConfig, PathConfigBuilder,
PreemptiveCircuitConfig, PreemptiveCircuitConfigBuilder,
};
use crate::preemptive::PreemptiveCircuitPredictor;
@ -157,8 +157,8 @@ pub struct CircMgr<R: Runtime> {
impl<R: Runtime> CircMgr<R> {
/// Construct a new circuit manager.
pub fn new<SM>(
config: CircMgrConfig,
pub fn new<SM, CFG: CircMgrConfig>(
config: &CFG,
storage: SM,
runtime: &R,
chanmgr: Arc<ChanMgr<R>>,
@ -166,14 +166,8 @@ impl<R: Runtime> CircMgr<R> {
where
SM: tor_persist::StateMgr + Send + Sync + 'static,
{
let CircMgrConfig {
path_rules,
circuit_timing,
preemptive_circuits,
} = config;
let preemptive = Arc::new(Mutex::new(PreemptiveCircuitPredictor::new(
preemptive_circuits,
config.preemptive_circuits().clone(),
)));
let guardmgr = tor_guardmgr::GuardMgr::new(runtime.clone(), storage.clone())?;
@ -183,11 +177,12 @@ impl<R: Runtime> CircMgr<R> {
let builder = build::CircuitBuilder::new(
runtime.clone(),
chanmgr,
path_rules,
config.path_rules().clone(),
storage_handle,
guardmgr,
);
let mgr = mgr::AbstractCircMgr::new(builder, runtime.clone(), circuit_timing);
let mgr =
mgr::AbstractCircMgr::new(builder, runtime.clone(), config.circuit_timing().clone());
let circmgr = Arc::new(CircMgr {
mgr: Arc::new(mgr),
predictor: preemptive,
@ -199,16 +194,16 @@ impl<R: Runtime> CircMgr<R> {
/// Try to change our configuration settings to `new_config`.
///
/// The actual behavior here will depend on the value of `how`.
pub fn reconfigure(
pub fn reconfigure<CFG: CircMgrConfig>(
&self,
new_config: &CircMgrConfig,
new_config: &CFG,
how: tor_config::Reconfigure,
) -> std::result::Result<(), tor_config::ReconfigureError> {
let old_path_rules = self.mgr.peek_builder().path_config();
let predictor = self.predictor.lock().expect("poisoned lock");
let preemptive_circuits = predictor.config();
if preemptive_circuits.initial_predicted_ports
!= new_config.preemptive_circuits.initial_predicted_ports
!= new_config.preemptive_circuits().initial_predicted_ports
{
// This change has no effect, since the list of ports was _initial_.
how.cannot_change("preemptive_circuits.initial_predicted_ports")?;
@ -219,15 +214,15 @@ impl<R: Runtime> CircMgr<R> {
}
let discard_circuits = !new_config
.path_rules
.path_rules()
.at_least_as_permissive_as(&old_path_rules);
self.mgr
.peek_builder()
.set_path_config(new_config.path_rules.clone());
.set_path_config(new_config.path_rules().clone());
self.mgr
.set_circuit_timing(new_config.circuit_timing.clone());
predictor.set_config(new_config.preemptive_circuits.clone());
.set_circuit_timing(new_config.circuit_timing().clone());
predictor.set_config(new_config.preemptive_circuits().clone());
if discard_circuits {
// TODO(nickm): Someday, we might want to take a more lenient approach, and only