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

channel padding: Rename ChannelsParams from ChannelsConfig 

As per
  https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/586#note_2814276

Change names and comments and docs everywhere.
This commit is contained in:
Ian Jackson 2022-06-17 14:16:21 +01:00
parent ff228e5d9c
commit 7d676cde02
7 changed files with 98 additions and 98 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
crates/tor-chanmgr/src/builder.rs
View File

@ -11,7 +11,7 @@ use std::time::Duration;
use tor_error::{bad_api_usage, internal};
use tor_linkspec::{ChanTarget, OwnedChanTarget};
use tor_llcrypto::pk;
use tor_proto::channel::config::ChannelsConfigUpdates;
use tor_proto::channel::params::ChannelsParamsUpdates;
use tor_rtcompat::{tls::TlsConnector, Runtime, TcpProvider, TlsProvider};
use async_trait::async_trait;
@ -238,8 +238,8 @@ impl crate::mgr::AbstractChannel for tor_proto::channel::Channel {
fn duration_unused(&self) -> Option<Duration> {
self.duration_unused()
}
fn reconfigure(&mut self, updates: Arc<ChannelsConfigUpdates>) -> StdResult<(), ()> {
self.reconfigure(updates).map_err(|_| ())
fn reparameterize(&mut self, updates: Arc<ChannelsParamsUpdates>) -> StdResult<(), ()> {
self.reparameterize(updates).map_err(|_| ())
}
}

18
crates/tor-chanmgr/src/mgr.rs
View File

@ -12,7 +12,7 @@ use std::result::Result as StdResult;
use std::sync::Arc;
use std::time::Duration;
use tor_error::internal;
use tor_proto::channel::config::ChannelsConfigUpdates;
use tor_proto::channel::params::ChannelsParamsUpdates;
mod map;
@ -34,13 +34,13 @@ pub(crate) trait AbstractChannel: Clone {
/// Return None if the channel is currently in use.
fn duration_unused(&self) -> Option<Duration>;
/// Reconfigure this channel according to the provided `ChannelsConfigUpdates`
/// Reparameterise this channel according to the provided `ChannelsParamsUpdates`
///
/// The changed configuration may not be implemented "immediately",
/// The changed parameters may not be implemented "immediately",
/// but this will be done "reasonably soon".
///
/// Returns `Err` (only) if the channel was closed earlier.
fn reconfigure(&mut self, updates: Arc<ChannelsConfigUpdates>) -> StdResult<(), ()>;
fn reparameterize(&mut self, updates: Arc<ChannelsParamsUpdates>) -> StdResult<(), ()>;
}
/// Trait to describe how channels are created.
@ -210,16 +210,16 @@ impl<CF: ChannelFactory> AbstractChanMgr<CF> {
// The channel got built: remember it, tell the
// others, and return it.
self.channels
.replace_with_config(ident.clone(), |channels_config| {
.replace_with_params(ident.clone(), |channels_params| {
// This isn't great. We context switch to the newly-created
// channel just to tell it how and whether to do padding. Ideally
// we would pass the config at some suitable point during
// we would pass the params at some suitable point during
// building. However, that would involve the channel taking a
// copy of the config, and that must happen in the same channel
// copy of the params, and that must happen in the same channel
// manager lock acquisition span as the one where we insert the
// channel into the table so it will receive updates. I.e.,
// here.
chan.reconfigure(channels_config.total_update().into())
chan.reparameterize(channels_params.total_update().into())
.map_err(|()| internal!("new channel already closed"))?;
Ok(Open(OpenEntry {
channel: chan.clone(),
@ -318,7 +318,7 @@ mod test {
fn duration_unused(&self) -> Option<Duration> {
None
}
fn reconfigure(&mut self, _updates: Arc<ChannelsConfigUpdates>) -> StdResult<(), ()> {
fn reparameterize(&mut self, _updates: Arc<ChannelsParamsUpdates>) -> StdResult<(), ()> {
Ok(())
}
}

54
crates/tor-chanmgr/src/mgr/map.rs
View File

@ -11,7 +11,7 @@ use std::sync::Arc;
use tor_error::{internal, into_internal};
use tor_netdir::{params::CHANNEL_PADDING_TIMEOUT_UPPER_BOUND, NetDir};
use tor_proto::channel::padding::ParametersBuilder as PaddingParametersBuilder;
use tor_proto::ChannelsConfig;
use tor_proto::ChannelsParams;
use tor_units::BoundedInt32;
use tracing::info;
@ -34,14 +34,14 @@ struct Inner<C: AbstractChannel> {
/// must never be held while an await is happening.)
channels: HashMap<C::Ident, ChannelState<C>>,
/// Configuration for channels that we create, and that all existing channels are using
/// Parameters for channels that we create, and that all existing channels are using
///
/// Will be updated by a background task, which also notifies all existing
/// `Open` channels via `channels`.
///
/// (Must be protected by the same lock as `channels`, or a channel might be
/// created using being-replaced configuration, but not get an update.)
channels_config: ChannelsConfig,
/// created using being-replaced parameters, but not get an update.)
channels_params: ChannelsParams,
}
/// Structure that can only be constructed from within this module.
@ -147,11 +147,11 @@ impl<C: AbstractChannel> ChannelState<C> {
impl<C: AbstractChannel> ChannelMap<C> {
/// Create a new empty ChannelMap.
pub(crate) fn new() -> Self {
let channels_config = ChannelsConfig::default();
let channels_params = ChannelsParams::default();
ChannelMap {
inner: std::sync::Mutex::new(Inner {
channels: HashMap::new(),
channels_config,
channels_params,
}),
}
}
@ -183,20 +183,20 @@ impl<C: AbstractChannel> ChannelMap<C> {
/// Replace the channel state for `ident` with the return value from `func`,
/// and return the previous value if any.
///
/// Passes a snapshot of the current global channels configuration to `func`.
/// If that configuration is copied by `func` into an [`AbstractChannel`]
/// Passes a snapshot of the current global channels parameters to `func`.
/// If those parameters are copied by `func` into an [`AbstractChannel`]
/// `func` must ensure that that `AbstractChannel` is returned,
/// so that it will be properly registered and receive config updates.
pub(crate) fn replace_with_config<F>(
/// so that it will be properly registered and receive params updates.
pub(crate) fn replace_with_params<F>(
&self,
ident: C::Ident,
func: F,
) -> Result<Option<ChannelState<C>>>
where
F: FnOnce(&ChannelsConfig) -> Result<ChannelState<C>>,
F: FnOnce(&ChannelsParams) -> Result<ChannelState<C>>,
{
let mut inner = self.inner.lock()?;
let newval = func(&inner.channels_config)?;
let newval = func(&inner.channels_params)?;
newval.check_ident(&ident)?;
Ok(inner.channels.insert(ident, newval))
}
@ -269,7 +269,7 @@ impl<C: AbstractChannel> ChannelMap<C> {
}
}
/// Handle a `NetDir` update (by reconfiguring channels as needed)
/// Handle a `NetDir` update (by reparameterising channels as needed)
pub(crate) fn process_updated_netdir(&self, netdir: Arc<tor_netdir::NetDir>) -> Result<()> {
use ChannelState as CS;
@ -298,7 +298,7 @@ impl<C: AbstractChannel> ChannelMap<C> {
let mut inner = self.inner.lock()?;
let update = inner
.channels_config
.channels_params
.start_update()
.padding_parameters(padding_parameters)
.finish();
@ -315,7 +315,7 @@ impl<C: AbstractChannel> ChannelMap<C> {
CS::Building(_) | CS::Poisoned(_) => continue,
};
// Ignore error (which simply means the channel is closed or gone)
let _ = channel.reconfigure(update.clone());
let _ = channel.reparameterize(update.clone());
}
Ok(())
}
@ -369,13 +369,13 @@ mod test {
use super::*;
use std::result::Result as StdResult;
use std::sync::Arc;
use tor_proto::channel::config::ChannelsConfigUpdates;
use tor_proto::channel::params::ChannelsParamsUpdates;
#[derive(Eq, PartialEq, Clone, Debug)]
struct FakeChannel {
ident: &'static str,
usable: bool,
unused_duration: Option<u64>,
config_update: Option<Arc<ChannelsConfigUpdates>>,
params_update: Option<Arc<ChannelsParamsUpdates>>,
}
impl AbstractChannel for FakeChannel {
type Ident = u8;
@ -388,8 +388,8 @@ mod test {
fn duration_unused(&self) -> Option<Duration> {
self.unused_duration.map(Duration::from_secs)
}
fn reconfigure(&mut self, update: Arc<ChannelsConfigUpdates>) -> StdResult<(), ()> {
self.config_update = Some(update);
fn reparameterize(&mut self, update: Arc<ChannelsParamsUpdates>) -> StdResult<(), ()> {
self.params_update = Some(update);
Ok(())
}
}
@ -398,7 +398,7 @@ mod test {
ident,
usable: true,
unused_duration: None,
config_update: None,
params_update: None,
};
ChannelState::Open(OpenEntry {
channel,
@ -414,7 +414,7 @@ mod test {
ident,
usable: true,
unused_duration,
config_update: None,
params_update: None,
};
ChannelState::Open(OpenEntry {
channel,
@ -426,7 +426,7 @@ mod test {
ident,
usable: false,
unused_duration: None,
config_update: None,
params_update: None,
};
ChannelState::Open(OpenEntry {
channel,
@ -534,7 +534,7 @@ mod test {
}
#[test]
fn reconfigure_via_netdir() {
fn reparameterise_via_netdir() {
let map = ChannelMap::new();
// Set some non-default parameters so that we can tell when an update happens
@ -542,7 +542,7 @@ mod test {
.inner
.lock()
.unwrap()
.channels_config
.channels_params
.start_update()
.padding_parameters(
PaddingParametersBuilder::default()
@ -569,9 +569,9 @@ mod test {
map.process_updated_netdir(netdir.clone()).unwrap();
with_ch(&|ch| {
assert_eq!(
format!("{:?}", ch.config_update.take().unwrap()),
format!("{:?}", ch.params_update.take().unwrap()),
// evade field visibility by (ab)using Debug impl
"ChannelsConfigUpdates { padding_enable: None, \
"ChannelsParamsUpdates { padding_enable: None, \
padding_parameters: Some(Parameters { \
low_ms: IntegerMilliseconds { value: 1500 }, \
high_ms: IntegerMilliseconds { value: 9500 } }) }"
@ -581,7 +581,7 @@ mod test {
eprintln!("-- process a default netdir again, which should *not* send an update --");
map.process_updated_netdir(netdir).unwrap();
with_ch(&|ch| assert_eq!(ch.config_update, None));
with_ch(&|ch| assert_eq!(ch.params_update, None));
}
#[test]

10
crates/tor-proto/src/channel.rs
View File

@ -58,13 +58,13 @@ pub const CHANNEL_BUFFER_SIZE: usize = 128;
mod circmap;
mod codec;
pub mod config;
pub mod params;
mod handshake;
pub mod padding;
mod reactor;
mod unique_id;
pub use crate::channel::config::*;
pub use crate::channel::params::*;
use crate::channel::reactor::{BoxedChannelSink, BoxedChannelStream, CtrlMsg, Reactor};
pub use crate::channel::unique_id::UniqId;
use crate::circuit::celltypes::CreateResponse;
@ -333,12 +333,12 @@ impl Channel {
self.details.clock_skew
}
/// Reconfigure
/// Reparameterise (update parameters; reconfigure)
///
/// Returns `Err` if the channel was closed earlier
pub fn reconfigure(
pub fn reparameterize(
&mut self,
updates: Arc<ChannelsConfigUpdates>,
updates: Arc<ChannelsParamsUpdates>,
) -> StdResult<(), ChannelClosed> {
self.control
.unbounded_send(CtrlMsg::ConfigUpdate(updates))

98
crates/tor-proto/src/channel/config.rs → crates/tor-proto/src/channel/params.rs
View File

@ -1,50 +1,50 @@
//! Configuration influencing all channels in a Tor client
//! Parameters influencing all channels in a Tor client
use educe::Educe;
use super::padding;
/// Generate most of the module: things which contain or process all config fields (or each one)
/// Generate most of the module: things which contain or process all params fields (or each one)
///
/// There is one call to this macro, which has as argument
/// the body of `struct ChannelsConfig`, with the following differences:
/// the body of `struct ChannelsParams`, with the following differences:
///
/// * field visibility specifiers are not specified; they are provided by the macro
/// * non-doc attributes that ought to be applied to fields in `ChannelsConfig`
/// * non-doc attributes that ought to be applied to fields in `ChannelsParams`
/// are prefixed with `field`, e.g. `#[field educe(Default ...)]`;
/// this allows applying doc attributes to other items too.
///
/// Generates, fairly straightforwardly:
///
/// ```ignore
/// pub struct ChannelsConfig { ... } // containing the fields as specified
/// pub struct ChannelsConfigUpdates { ... } // containing `Option` of each field
/// pub fn ChannelsConfig::total_update(&self) -> ChannelsConfigUpdates;
/// pub fn ChannelsConfigUpdatesBuilder::$field(self, new_value: _) -> Self;
/// pub struct ChannelsParams { ... } // containing the fields as specified
/// pub struct ChannelsParamsUpdates { ... } // containing `Option` of each field
/// pub fn ChannelsParams::total_update(&self) -> ChannelsParamsUpdates;
/// pub fn ChannelsParamsUpdatesBuilder::$field(self, new_value: _) -> Self;
/// ```
///
/// Within the macro body, we indent the per-field `$( )*` with 2 spaces.
macro_rules! define_channels_config_and_automatic_impls { { $(
macro_rules! define_channels_params_and_automatic_impls { { $(
$( #[doc $($doc_attr:tt)*] )*
$( #[field $other_attr:meta] )*
$field:ident : $ty:ty
),* $(,)? } => {
/// Initial, and, overall, configuration for channels
/// Initial, and, overall, parameters for channels
///
/// This is used both to generate the initial configuration,
/// This is used both to generate the initial parameters,
/// and to handle updates:
/// when used for handling updates,
/// it contains the last configuration that has been implemented.
/// it contains the last parameters that has been implemented.
///
/// Central code managing all channels will contain a `ChannelsConfig`,
/// and use `ChannelsConfigUpdatesBuilder` to both update that config
/// and generate `ChannelsConfigUpdates` messages representing the changes.
/// Central code managing all channels will contain a `ChannelsParams`,
/// and use `ChannelsParamsUpdatesBuilder` to both update that params
/// and generate `ChannelsParamsUpdates` messages representing the changes.
///
/// `Default` is a placeholder to use pending availability of a netdir etc.
#[derive(Debug, Educe, Clone, Eq, PartialEq)]
#[educe(Default)]
pub struct ChannelsConfig {
pub struct ChannelsParams {
$(
$( #[doc $($doc_attr)*] )*
$( #[$other_attr] )*
@ -52,15 +52,15 @@ macro_rules! define_channels_config_and_automatic_impls { { $(
)*
}
/// Reconfiguration message
/// Reparameterisation message
///
/// Can contain updates to each of the fields in `ChannelsConfig`.
/// Constructed via [`ChannelsConfigUpdatesBuilder`],
/// which is obtained from [`ChannelsConfig::start_update`].
/// Can contain updates to each of the fields in `ChannelsParams`.
/// Constructed via [`ChannelsParamsUpdatesBuilder`],
/// which is obtained from [`ChannelsParams::start_update`].
///
/// Sent to all channel implementations, when they ought to change their behaviour.
#[derive(Debug, Default, Clone, Eq, PartialEq)]
pub struct ChannelsConfigUpdates {
pub struct ChannelsParamsUpdates {
$(
/// New value, if it has changed.
///
@ -71,13 +71,13 @@ macro_rules! define_channels_config_and_automatic_impls { { $(
)*
}
impl ChannelsConfig {
impl ChannelsParams {
/// Create an update message which sets *all* of the settings in `self`
///
/// Used during channel startup.
#[must_use = "total_update makes an updates message that must be sent to have effect"]
pub fn total_update(&self) -> ChannelsConfigUpdates {
ChannelsConfigUpdates {
pub fn total_update(&self) -> ChannelsParamsUpdates {
ChannelsParamsUpdates {
$(
$field: Some(self.$field.clone()),
)*
@ -85,18 +85,18 @@ macro_rules! define_channels_config_and_automatic_impls { { $(
}
}
impl<'c> ChannelsConfigUpdatesBuilder<'c> {
impl<'c> ChannelsParamsUpdatesBuilder<'c> {
$(
$( #[doc $($doc_attr)*] )*
///
/// (Adds this setting to the update, if it has changed.)
pub fn $field(mut self, new_value: $ty) -> Self {
if &new_value != &self.config.$field {
if &new_value != &self.params.$field {
self
.update
.get_or_insert_with(|| Default::default())
.$field = Some(new_value.clone());
self.config.$field = new_value;
self.params.$field = new_value;
}
self
}
@ -104,7 +104,7 @@ macro_rules! define_channels_config_and_automatic_impls { { $(
}
} }
define_channels_config_and_automatic_impls! {
define_channels_params_and_automatic_impls! {
/// Whether to send padding
#[field educe(Default(expression = "interim_enable_by_env_var()"))]
padding_enable: bool,
@ -125,60 +125,60 @@ pub(crate) fn interim_enable_by_env_var() -> bool {
std::env::var("ARTI_EXPERIMENTAL_CHANNEL_PADDING").unwrap_or_default() != ""
}
/// Builder for a channels config update
/// Builder for a channels params update
///
/// Obtain this from `ChannelsConfig::update`,
/// Obtain this from `ChannelsParams::update`,
/// call zero or more setter methods,
/// call [`finish`](ChannelsConfigUpdatesBuilder::finish),
/// call [`finish`](ChannelsParamsUpdatesBuilder::finish),
/// and then send the resulting message.
///
/// # Panics
///
/// Panics if dropped. Instead, call `finish`.
pub struct ChannelsConfigUpdatesBuilder<'c> {
/// Tracking the existing config
config: &'c mut ChannelsConfig,
pub struct ChannelsParamsUpdatesBuilder<'c> {
/// Tracking the existing params
params: &'c mut ChannelsParams,
/// The update we are building
///
/// `None` means nothing has changed yet.
update: Option<ChannelsConfigUpdates>,
update: Option<ChannelsParamsUpdates>,
/// Make it hard to write code paths that drop this
drop_bomb: bool,
}
impl ChannelsConfig {
/// Start building an update to channel configuration
impl ChannelsParams {
/// Start building an update to channel parameters
///
/// The builder **must not be dropped**, once created;
/// instead, [`finish`](ChannelsConfigUpdatesBuilder::finish) must be called.
/// instead, [`finish`](ChannelsParamsUpdatesBuilder::finish) must be called.
/// So prepare your new values first, perhaps fallibly,
/// and only then create and use the builder and send the update, infallibly.
///
/// (This is because the builder uses `self: ChannelsConfig`
/// (This is because the builder uses `self: ChannelsParams`
/// to track which values have changed,
/// and the values in `self` are updated immediately by the field update methods.)
///
/// # Panics
///
/// [`ChannelsConfigUpdatesBuilder`] panics if it is dropped.
pub fn start_update(&mut self) -> ChannelsConfigUpdatesBuilder {
ChannelsConfigUpdatesBuilder {
config: self,
/// [`ChannelsParamsUpdatesBuilder`] panics if it is dropped.
pub fn start_update(&mut self) -> ChannelsParamsUpdatesBuilder {
ChannelsParamsUpdatesBuilder {
params: self,
update: None,
drop_bomb: true,
}
}
}
impl<'c> Drop for ChannelsConfigUpdatesBuilder<'c> {
impl<'c> Drop for ChannelsParamsUpdatesBuilder<'c> {
fn drop(&mut self) {
assert!(!self.drop_bomb, "ChannelsConfigUpdatesBuilder dropped");
assert!(!self.drop_bomb, "ChannelsParamsUpdatesBuilder dropped");
}
}
impl<'c> ChannelsConfigUpdatesBuilder<'c> {
impl<'c> ChannelsParamsUpdatesBuilder<'c> {
/// Finalise the update
///
/// If nothing actually changed, returns `None`.
@ -186,9 +186,9 @@ impl<'c> ChannelsConfigUpdatesBuilder<'c> {
/// every channel with a null update.)
///
/// If `Some` is returned, the update **must** be implemented,
/// since the underlying tracking [`ChannelsConfig`] has already been updated.
#[must_use = "the update from finish() must be sent, to avoid losing config changes"]
pub fn finish(mut self) -> Option<ChannelsConfigUpdates> {
/// since the underlying tracking [`ChannelsParams`] has already been updated.
#[must_use = "the update from finish() must be sent, to avoid losing params changes"]
pub fn finish(mut self) -> Option<ChannelsParamsUpdates> {
self.drop_bomb = false;
self.update.take()
}

8
crates/tor-proto/src/channel/reactor.rs
View File

@ -29,7 +29,7 @@ use std::pin::Pin;
use std::sync::atomic::Ordering;
use std::sync::Arc;
use crate::channel::{codec::CodecError, config::*, padding, unique_id, ChannelDetails};
use crate::channel::{codec::CodecError, params::*, padding, unique_id, ChannelDetails};
use crate::circuit::celltypes::{ClientCircChanMsg, CreateResponse};
use tracing::{debug, trace};
@ -72,11 +72,11 @@ pub(super) enum CtrlMsg {
///
/// The sender of these messages is responsible for the optimisation of
/// ensuring that "no-change" messages are elided.
/// (This is implemented in `ChannelsConfigUpdatesBuilder`.)
/// (This is implemented in `ChannelsParamsUpdatesBuilder`.)
///
/// These updates are done via a control message to avoid adding additional branches to the
/// main reactor `select!`.
ConfigUpdate(Arc<ChannelsConfigUpdates>),
ConfigUpdate(Arc<ChannelsParamsUpdates>),
}
/// Object to handle incoming cells and background tasks on a channel.
@ -225,7 +225,7 @@ impl<S: SleepProvider> Reactor<S> {
self.update_disused_since();
}
CtrlMsg::ConfigUpdate(updates) => {
let ChannelsConfigUpdates {
let ChannelsParamsUpdates {
// List all the fields explicitly; that way the compiler will warn us
// if one is added and we fail to handle it here.
padding_enable,

2
crates/tor-proto/src/lib.rs
View File

@ -124,7 +124,7 @@ mod util;
pub use util::err::{Error, ResolveError};
pub use util::skew::ClockSkew;
pub use channel::config::ChannelsConfig;
pub use channel::params::ChannelsParams;
/// A vector of bytes that gets cleared when it's dropped.
type SecretBytes = zeroize::Zeroizing<Vec<u8>>;