Move ChannelUsage from tor_proto to tor_chanmgr

Replace Channel::note_usage with Channel::engage_padding_activities,
which unconditionally causes the channel to (start to) do netflow
padding things.

The condition now lives in chanmgr.

Addresses
  https://gitlab.torproject.org/tpo/core/arti/-/merge_requests/657#note_2826094

crates/tor-chanmgr/src/builder.rs

@@ -12,7 +12,6 @@ use tor_error::{bad_api_usage, internal};
 use tor_linkspec::{HasAddrs, HasRelayIds, OwnedChanTarget};
 use tor_llcrypto::pk;
 use tor_proto::channel::params::ChannelsParamsUpdates;
-use tor_proto::channel::ChannelUsage;
 use tor_rtcompat::{tls::TlsConnector, Runtime, TcpProvider, TlsProvider};
 
 use async_trait::async_trait;
@@ -256,8 +255,8 @@ impl crate::mgr::AbstractChannel for tor_proto::channel::Channel {
     fn reparameterize(&mut self, updates: Arc<ChannelsParamsUpdates>) -> tor_proto::Result<()> {
         self.reparameterize(updates)
     }
-    fn note_usage(&self, usage: ChannelUsage) -> StdResult<(), tor_error::Bug> {
+    fn engage_padding_activities(&self) -> StdResult<(), tor_error::Bug> {
-        self.note_usage(usage)
+        self.engage_padding_activities()
     }
 }
 

crates/tor-chanmgr/src/lib.rs

@@ -69,7 +69,7 @@ use std::time::Duration;
 use tor_config::ReconfigureError;
 use tor_linkspec::{ChanTarget, OwnedChanTarget};
 use tor_netdir::{params::NetParameters, NetDirProvider};
-use tor_proto::channel::{Channel, ChannelUsage};
+use tor_proto::channel::Channel;
 use tracing::{debug, error};
 use void::{ResultVoidErrExt, Void};
 
@@ -127,6 +127,28 @@ pub enum Dormancy {
     Dormant,
 }
 
+/// How a channel is going to be used
+///
+/// A channel may be used in multiple ways.  Each time it is (re)used, a separate
+/// ChannelUsage is passed in.
+///
+/// This type is obtained from a `tor_circmgr::usage::SupportedCircUsage` in
+/// `tor_circmgr::usage`, and it has roughly the same set of variants.
+#[derive(Clone, Debug, Copy, Eq, PartialEq)]
+#[non_exhaustive]
+pub enum ChannelUsage {
+    /// Use for BEGINDIR-based non-anonymous directory connections
+    Dir,
+
+    /// Use to exit
+    ///
+    /// Includes a circuit being constructed preemptively.
+    Exit,
+
+    /// For a channel which is not use for circuit(s), or only for useless circuits
+    UselessCircuit,
+}
+
 impl<R: Runtime> ChanMgr<R> {
     /// Construct a new channel manager.
     ///

crates/tor-chanmgr/src/mgr.rs

@@ -1,7 +1,7 @@
 //! Abstract implementation of a channel manager
 
 use crate::mgr::map::OpenEntry;
-use crate::{ChanProvenance, ChannelConfig, Dormancy, Error, Result};
+use crate::{ChanProvenance, ChannelConfig, ChannelUsage, Dormancy, Error, Result};
 
 use async_trait::async_trait;
 use futures::channel::oneshot;
@@ -14,7 +14,6 @@ use std::time::Duration;
 use tor_error::internal;
 use tor_netdir::params::NetParameters;
 use tor_proto::channel::params::ChannelsParamsUpdates;
-use tor_proto::channel::ChannelUsage;
 
 mod map;
 
@@ -42,8 +41,20 @@ pub(crate) trait AbstractChannel: Clone {
     /// but this will be done "reasonably soon".
     fn reparameterize(&mut self, updates: Arc<ChannelsParamsUpdates>) -> tor_proto::Result<()>;
 
-    /// Note that this channel is about to be used for `usage`
+    /// Specify that this channel should do activities related to channel padding
-    fn note_usage(&self, usage: ChannelUsage) -> StdResult<(), tor_error::Bug>;
+    ///
+    /// Initially, the channel does nothing related to channel padding:
+    /// it neither sends any padding, nor sends any PADDING_NEGOTIATE cells.
+    ///
+    /// After this function has been called, it will do both,
+    /// according to the parameters specified through `reparameterize`.
+    /// Note that this might include *disabling* padding
+    /// (for example, by sending a `PADDING_NEGOTIATE`).
+    ///
+    /// Idempotent.
+    ///
+    /// There is no way to undo the effect of this call.
+    fn engage_padding_activities(&self) -> StdResult<(), tor_error::Bug>;
 }
 
 /// Trait to describe how channels are created.
@@ -131,8 +142,15 @@ impl<CF: ChannelFactory> AbstractChanMgr<CF> {
         target: CF::BuildSpec,
         usage: ChannelUsage,
     ) -> Result<(CF::Channel, ChanProvenance)> {
+        use ChannelUsage as CU;
+
         let chan = self.get_or_launch_internal(ident, target).await?;
-        chan.0.note_usage(usage)?;
+
+        match usage {
+            CU::Dir | CU::UselessCircuit => {}
+            CU::Exit => chan.0.engage_padding_activities()?,
+        }
+
         Ok(chan)
     }
 
@@ -338,7 +356,7 @@ mod test {
     use std::time::Duration;
     use tor_error::bad_api_usage;
 
-    use tor_proto::channel::ChannelUsage as CU;
+    use crate::ChannelUsage as CU;
     use tor_rtcompat::{task::yield_now, test_with_one_runtime, Runtime};
 
     struct FakeChannelFactory<RT> {
@@ -375,7 +393,7 @@ mod test {
             self.last_params = Some((*updates).clone());
             Ok(())
         }
-        fn note_usage(&self, _usage: ChannelUsage) -> StdResult<(), tor_error::Bug> {
+        fn engage_padding_activities(&self) -> StdResult<(), tor_error::Bug> {
             Ok(())
         }
     }

crates/tor-chanmgr/src/mgr/map.rs

@@ -552,7 +552,6 @@ mod test {
     use super::*;
     use std::sync::Arc;
     use tor_proto::channel::params::ChannelsParamsUpdates;
-    use tor_proto::channel::ChannelUsage;
 
     fn new_test_channel_map<C: AbstractChannel>() -> ChannelMap<C> {
         ChannelMap::new(
@@ -584,7 +583,7 @@ mod test {
             self.params_update = Some(update);
             Ok(())
         }
-        fn note_usage(&self, _usage: ChannelUsage) -> StdResult<(), tor_error::Bug> {
+        fn engage_padding_activities(&self) -> StdResult<(), tor_error::Bug> {
             Ok(())
         }
     }

crates/tor-chanmgr/src/mgr/map/padding_test.rs

@@ -22,9 +22,10 @@ use tor_cell::chancell::msg::PaddingNegotiateCmd;
 use tor_config::PaddingLevel;
 use tor_linkspec::HasRelayIds;
 use tor_netdir::NetDir;
-use tor_proto::channel::{Channel, ChannelUsage, CtrlMsg};
+use tor_proto::channel::{Channel, CtrlMsg};
 
 use crate::mgr::{AbstractChanMgr, ChannelFactory};
+use crate::ChannelUsage;
 
 use PaddingLevel as PL;
 
@@ -277,7 +278,7 @@ async fn padding_control_through_layers() {
     c.expect_0();
 
     eprintln!("### Exit ###");
-    c.channel.note_usage(ChannelUsage::Exit).unwrap();
+    c.channel.engage_padding_activities().unwrap();
     c.expect_1(Expected {
         enabled: Some(true), // we now turn on our padding sender
         timing: None,        // with default parameters

crates/tor-circmgr/src/build.rs

@@ -12,10 +12,9 @@ use std::sync::{
     Arc,
 };
 use std::time::{Duration, Instant};
-use tor_chanmgr::{ChanMgr, ChanProvenance};
+use tor_chanmgr::{ChanMgr, ChanProvenance, ChannelUsage};
 use tor_guardmgr::GuardStatus;
 use tor_linkspec::{ChanTarget, OwnedChanTarget, OwnedCircTarget};
-use tor_proto::channel::ChannelUsage;
 use tor_proto::circuit::{CircParameters, ClientCirc, PendingClientCirc};
 use tor_rtcompat::{Runtime, SleepProviderExt};
 
@@ -508,10 +507,10 @@ mod test {
     use crate::timeouts::TimeoutEstimator;
     use futures::channel::oneshot;
     use std::sync::Mutex;
-    use tor_linkspec::{HasRelayIds, RelayIdType, RelayIds};
     use tor_chanmgr::ChannelConfig;
+    use tor_chanmgr::ChannelUsage as CU;
+    use tor_linkspec::{HasRelayIds, RelayIdType, RelayIds};
     use tor_llcrypto::pk::ed25519::Ed25519Identity;
-    use tor_proto::channel::ChannelUsage as CU;
     use tor_rtcompat::{test_with_all_runtimes, SleepProvider};
     use tracing::trace;
 

crates/tor-circmgr/src/mgr.rs

@@ -27,9 +27,9 @@ use crate::{DirInfo, Error, Result};
 
 use retry_error::RetryError;
 use tor_basic_utils::retry::RetryDelay;
+use tor_chanmgr::ChannelUsage;
 use tor_config::MutCfg;
 use tor_error::{internal, AbsRetryTime, HasRetryTime};
-use tor_proto::channel::ChannelUsage;
 use tor_rtcompat::{Runtime, SleepProviderExt};
 
 use async_trait::async_trait;

crates/tor-circmgr/src/usage.rs

@@ -8,10 +8,10 @@ use std::time::SystemTime;
 use tracing::debug;
 
 use crate::path::{dirpath::DirPathBuilder, exitpath::ExitPathBuilder, TorPath};
+use tor_chanmgr::ChannelUsage;
 use tor_guardmgr::{GuardMgr, GuardMonitor, GuardUsable};
 use tor_netdir::Relay;
 use tor_netdoc::types::policy::PortPolicy;
-use tor_proto::channel::ChannelUsage;
 use tor_rtcompat::Runtime;
 
 use crate::isolation::{IsolationHelper, StreamIsolation};

crates/tor-proto/src/channel.rs

@@ -136,28 +136,6 @@ pub struct Channel {
     details: Arc<ChannelDetails>,
 }
 
-/// How a channel is going to be used
-///
-/// A channel may be used in multiple ways.  Each time it is (re)used, a separate
-/// ChannelUsage is passed in.
-///
-/// This type is obtained from a `tor_circmgr::usage::SupportedCircUsage` in
-/// `tor_circmgr::usage`, and it has roughly the same set of variants.
-#[derive(Clone, Debug, Copy, Eq, PartialEq)]
-#[non_exhaustive]
-pub enum ChannelUsage {
-    /// Use for BEGINDIR-based non-anonymous directory connections
-    Dir,
-
-    /// Use to exit
-    ///
-    /// Includes a circuit being constructed preemptively.
-    Exit,
-
-    /// For a channel which is not use for circuit(s), or only for useless circuits
-    UselessCircuit,
-}
-
 /// This is information shared between the reactor and the frontend (`Channel` object).
 ///
 /// This exists to make `Channel` cheap to clone, which is desirable because every circuit wants
@@ -441,44 +419,37 @@ impl Channel {
             .map_err(|_| internal!("channel details poisoned"))
     }
 
-    /// Note that this channel is about to be used for `usage`
+    /// Specify that this channel should do activities related to channel padding
-    pub fn note_usage(&self, usage: ChannelUsage) -> StdResult<(), tor_error::Bug> {
+    ///
+    /// See [`AbstractChannel::set_channel_padding_relevant`]
+    pub fn engage_padding_activities(&self) -> StdResult<(), tor_error::Bug> {
         let mut mutable = self.mutable()?;
 
-        use ChannelUsage as CU;
+        match &mutable.padding {
-        let control_padding = match usage {
+            PCS::UsageDoesNotImplyPadding {
-            CU::Dir => false,
+                padding_params: params,
-            CU::Exit => true,
+            } => {
-            CU::UselessCircuit => false,
+                // Well, apparently the channel usage *does* imply padding now,
-        };
+                // so we need to (belatedly) enable the timer,
+                // send the padding negotiation cell, etc.
+                let mut params = params.clone();
 
-        if control_padding {
+                // Except, maybe the padding we would be requesting is precisely default,
-            match &mutable.padding {
+                // so we wouldn't actually want to send that cell.
-                PCS::UsageDoesNotImplyPadding {
+                if params.padding_negotiate == Some(PaddingNegotiate::start_default()) {
-                    padding_params: params,
+                    params.padding_negotiate = None;
-                } => {
-                    // Well, apparently the channel usage *does* imply padding now,
-                    // so we need to (belatedly) enable the timer,
-                    // send the padding negotiation cell, etc.
-                    let mut params = params.clone();
-
-                    // Except, maybe the padding we would be requesting is precisely default,
-                    // so we wouldn't actually want to send that cell.
-                    if params.padding_negotiate == Some(PaddingNegotiate::start_default()) {
-                        params.padding_negotiate = None;
-                    }
-
-                    match self.send_control(CtrlMsg::ConfigUpdate(Arc::new(params))) {
-                        Ok(()) => {}
-                        Err(ChannelClosed) => return Ok(()),
-                    }
-
-                    mutable.padding = PCS::PaddingConfigured;
                 }
 
-                PCS::PaddingConfigured => {
+                match self.send_control(CtrlMsg::ConfigUpdate(Arc::new(params))) {
-                    // OK, nothing to do
+                    Ok(()) => {}
+                    Err(ChannelClosed) => return Ok(()),
                 }
+
+                mutable.padding = PCS::PaddingConfigured;
+            }
+
+            PCS::PaddingConfigured => {
+                // OK, nothing to do
             }
         }