diff --git a/rita_common/src/debt_keeper/mod.rs b/rita_common/src/debt_keeper/mod.rs index 10c292fe3..5499976f1 100755 --- a/rita_common/src/debt_keeper/mod.rs +++ b/rita_common/src/debt_keeper/mod.rs @@ -17,10 +17,8 @@ use crate::simulated_txfee_manager::add_tx_to_total; use crate::tunnel_manager::tm_tunnel_state_change; use crate::tunnel_manager::TunnelAction; use crate::tunnel_manager::TunnelChange; -use crate::tunnel_manager::TunnelStateChange; use crate::RitaCommonError; use crate::KI; - use althea_types::Denom; use althea_types::Identity; use althea_types::SystemChain; @@ -32,7 +30,6 @@ use num_traits::Signed; use settings::get_rita_common; use settings::DEBT_KEEPER_DENOM; use settings::DEBT_KEEPER_DENOM_DECIMAL; - use std::collections::HashMap; use std::fs; use std::fs::File; @@ -357,9 +354,7 @@ pub fn send_debt_update() -> Result<(), RitaCommonError> { } } - if let Err(e) = tm_tunnel_state_change(TunnelStateChange { - tunnels: debts_message, - }) { + if let Err(e) = tm_tunnel_state_change(debts_message) { warn!("Error during tunnel state change: {}", e); } Ok(()) diff --git a/rita_common/src/tunnel_manager/mod.rs b/rita_common/src/tunnel_manager/mod.rs index b86bda12f..ff4c54fb5 100644 --- a/rita_common/src/tunnel_manager/mod.rs +++ b/rita_common/src/tunnel_manager/mod.rs @@ -294,6 +294,16 @@ pub fn tm_common_slow_loop_helper(babel_interfaces: Vec) { tunnel_manager.tunnel_gc(TUNNEL_TIMEOUT, TUNNEL_HANDSHAKE_TIMEOUT, babel_interfaces); } +/// Called by DebtKeeper with the updated billing status of every tunnel every round +pub fn tm_tunnel_state_change(msg: Vec) -> Result<(), RitaCommonError> { + let tm_pin = &mut *TUNNEL_MANAGER.write().unwrap(); + let tunnel_manager = get_tunnel_manager_write_ref(tm_pin); + for tunnel in msg { + tunnel_manager.tunnel_payment_state_change(tunnel); + } + Ok(()) +} + impl TunnelManager { pub fn new() -> Self { TunnelManager { @@ -500,7 +510,6 @@ impl TunnelManager { id, action, ); - let mut tunnel_bw_limits_need_change = false; // Find a tunnel match self.tunnels.get_mut(&id) { @@ -520,7 +529,6 @@ impl TunnelManager { tunnel.neigh_id.global.wg_public_key ); tunnel.payment_state = PaymentState::Paid; - tunnel_bw_limits_need_change = true; // latency detector probably got confused while enforcement // occurred tunnel.speed_limit = None; @@ -536,7 +544,6 @@ impl TunnelManager { tunnel.neigh_id.global.wg_public_key ); tunnel.payment_state = PaymentState::Overdue; - tunnel_bw_limits_need_change = true; } PaymentState::Overdue => { continue; @@ -545,6 +552,13 @@ impl TunnelManager { } } } + // update the bw limits if required, don't gate calling this function + // if payment issues are occuring it may fail to enforce, it must be called + // again later even if there are no changes to ensure everything is in a proper state + let res = tunnel_bw_limit_update(tunnels); + if res.is_err() { + error!("Bandwidth limiting failed with {:?}", res); + } } None => { // This is now pretty common since there's no more none action @@ -553,75 +567,36 @@ impl TunnelManager { trace!("Couldn't find tunnel for identity {:?}", id); } } - - // this is done outside of the match to make the borrow checker happy - if tunnel_bw_limits_need_change { - if potential_payment_issues_detected() { - warn!("Potential payment issue detected"); - return; - } - let res = tunnel_bw_limit_update(&self.tunnels); - // if this fails consistently it could be a wallet draining attack - // TODO check for that case - if res.is_err() { - error!("Bandwidth limiting failed with {:?}", res); - } - } } } +/// A single use internal struct used to flag what tunnels need to be updated pub struct TunnelChange { pub identity: Identity, pub action: TunnelAction, } -pub struct TunnelStateChange { - pub tunnels: Vec, -} - -/// Called by DebtKeeper with the updated billing status of every tunnel every round -pub fn tm_tunnel_state_change(msg: TunnelStateChange) -> Result<(), RitaCommonError> { - let tm_pin = &mut *TUNNEL_MANAGER.write().unwrap(); - let tunnel_manager = get_tunnel_manager_write_ref(tm_pin); - for tunnel in msg.tunnels { - tunnel_manager.tunnel_payment_state_change(tunnel); - } - Ok(()) -} - -/// Takes the tunnels list and iterates over it to update all of the traffic control settings -/// since we can't figure out how to combine interfaces bandwidth budgets we're subdividing it -/// here with manual terminal commands whenever there is a change -fn tunnel_bw_limit_update(tunnels: &HashMap>) -> Result<(), RitaCommonError> { +/// Takes a vec of tunnels and then updates the bandwidth limits on them. The calling functions +/// ensure that this is only done when required. We further optimize by checking the qdisc before +/// performing the update here +fn tunnel_bw_limit_update(tunnels: &[Tunnel]) -> Result<(), RitaCommonError> { info!("Running tunnel bw limit update!"); - // number of interfaces over which we will have to divide free tier BW - let mut limited_interfaces = 0u16; - for sublist in tunnels.iter() { - for tunnel in sublist.1.iter() { - if tunnel.payment_state == PaymentState::Overdue { - limited_interfaces += 1; - } - } - } let payment = settings::get_rita_common().payment; - let bw_per_iface = if limited_interfaces > 0 { - payment.free_tier_throughput / u32::from(limited_interfaces) - } else { - payment.free_tier_throughput - }; - - for sublist in tunnels.iter() { - for tunnel in sublist.1.iter() { - let payment_state = &tunnel.payment_state; - let iface_name = &tunnel.iface_name; - let has_limit = KI.has_limit(iface_name)?; - - if *payment_state == PaymentState::Overdue { - KI.set_classless_limit(iface_name, bw_per_iface)?; - } else if *payment_state == PaymentState::Paid && has_limit { - KI.set_codel_shaping(iface_name, None)?; - } + let bw_per_iface = payment.free_tier_throughput; + + for tunnel in tunnels { + let payment_state = &tunnel.payment_state; + let iface_name = &tunnel.iface_name; + let has_limit = KI.has_limit(iface_name)?; + + if *payment_state == PaymentState::Overdue + && !has_limit + && !potential_payment_issues_detected() + { + KI.set_classless_limit(iface_name, bw_per_iface)?; + } else if *payment_state == PaymentState::Paid && has_limit { + KI.set_codel_shaping(iface_name, None)?; } } Ok(())