From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id AB5811FF17A for ; Tue, 11 Nov 2025 11:44:21 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id A58195C89; Tue, 11 Nov 2025 11:45:07 +0100 (CET) Message-ID: <65aac2e6-3bc8-4694-89b4-beaa6a430545@proxmox.com> Date: Tue, 11 Nov 2025 11:45:02 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Proxmox Backup Server development discussion , Christian Ebner References: <20250916124147.513342-1-c.ebner@proxmox.com> <20250916124147.513342-6-c.ebner@proxmox.com> Content-Language: en-US From: Hannes Laimer In-Reply-To: <20250916124147.513342-6-c.ebner@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1762857879828 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.046 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pbs-devel] [PATCH proxmox-backup v2 1/4] traffic control: use factored out shared rate limiter X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Backup Server development discussion Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" Reviewed-by: Hannes Laimer Tested-by: Hannes Laimer On 9/16/25 14:42, Christian Ebner wrote: > The shared rate limiter implementation was factored out into a > dedicated crate so it can be easily reused for other code paths, e.g. > the s3 client implementation. > > Use that implementation and drop the now dead code. > > No functional changes intended. > > Signed-off-by: Christian Ebner > --- > Changes since version 1: > - depend on proxmox-rate-limiter > > Cargo.toml | 3 + > debian/control | 1 - > pbs-client/Cargo.toml | 3 +- > pbs-client/src/http_client.rs | 3 +- > src/tools/mod.rs | 4 - > src/tools/shared_rate_limiter.rs | 122 ------------------------------- > src/traffic_control_cache.rs | 8 +- > 7 files changed, 11 insertions(+), 133 deletions(-) > delete mode 100644 src/tools/shared_rate_limiter.rs > > diff --git a/Cargo.toml b/Cargo.toml > index b3f55b4db..e276c11de 100644 > --- a/Cargo.toml > +++ b/Cargo.toml > @@ -74,6 +74,7 @@ proxmox-network-api = "1" > proxmox-notify = "1" > proxmox-openid = "1" > proxmox-product-config = "1" > +proxmox-rate-limiter = "1.0.0" > proxmox-rest-server = { version = "1.0.1", features = [ "templates" ] } > # some use "cli", some use "cli" and "server", pbs-config uses nothing > proxmox-router = { version = "3.2.2", default-features = false } > @@ -227,6 +228,7 @@ proxmox-network-api = { workspace = true, features = [ "impl" ] } > proxmox-notify = { workspace = true, features = [ "pbs-context" ] } > proxmox-openid.workspace = true > proxmox-product-config.workspace = true > +proxmox-rate-limiter = { workspace = true, features = [ "shared-rate-limiter" ] } > proxmox-rest-server = { workspace = true, features = [ "rate-limited-stream" ] } > proxmox-router = { workspace = true, features = [ "cli", "server"] } > proxmox-s3-client.workspace = true > @@ -288,6 +290,7 @@ proxmox-rrd-api-types.workspace = true > #proxmox-notify = { path = "../proxmox/proxmox-notify" } > #proxmox-openid = { path = "../proxmox/proxmox-openid" } > #proxmox-product-config = { path = "../proxmox/proxmox-product-config" } > +#proxmox-rate-limiter = { path = "../proxmox/proxmox-rate-limiter" } > #proxmox-rest-server = { path = "../proxmox/proxmox-rest-server" } > #proxmox-router = { path = "../proxmox/proxmox-router" } > #proxmox-rrd = { path = "../proxmox/proxmox-rrd" } > diff --git a/debian/control b/debian/control > index 4346acd38..58ea000c6 100644 > --- a/debian/control > +++ b/debian/control > @@ -81,7 +81,6 @@ Build-Depends: bash-completion, > librust-proxmox-http-1+http-helpers-dev (>= 1.0.2-~~), > librust-proxmox-http-1+proxmox-async-dev (>= 1.0.2-~~), > librust-proxmox-http-1+rate-limited-stream-dev (>= 1.0.2-~~), > - librust-proxmox-http-1+rate-limiter-dev (>= 1.0.2-~~), > librust-proxmox-http-1+websocket-dev (>= 1.0.2-~~), > librust-proxmox-human-byte-1+default-dev, > librust-proxmox-io-1+default-dev (>= 1.0.1-~~), > diff --git a/pbs-client/Cargo.toml b/pbs-client/Cargo.toml > index 4c6c80983..233969f65 100644 > --- a/pbs-client/Cargo.toml > +++ b/pbs-client/Cargo.toml > @@ -36,10 +36,11 @@ pathpatterns.workspace = true > proxmox-async.workspace = true > proxmox-auth-api.workspace = true > proxmox-compression.workspace = true > -proxmox-http = { workspace = true, features = [ "body", "rate-limiter" ] } > +proxmox-http = { workspace = true, features = [ "body" ] } > proxmox-human-byte.workspace = true > proxmox-io = { workspace = true, features = [ "tokio" ] } > proxmox-log = { workspace = true } > +proxmox-rate-limiter = { workspace = true, features = [ "rate-limiter" ] } > proxmox-router = { workspace = true, features = [ "cli", "server" ] } > proxmox-schema.workspace = true > proxmox-sys.workspace = true > diff --git a/pbs-client/src/http_client.rs b/pbs-client/src/http_client.rs > index 74b076244..3cdae2c01 100644 > --- a/pbs-client/src/http_client.rs > +++ b/pbs-client/src/http_client.rs > @@ -31,8 +31,9 @@ use proxmox_async::broadcast_future::BroadcastFuture; > use proxmox_http::client::HttpsConnector; > use proxmox_http::uri::{build_authority, json_object_to_query}; > use proxmox_http::Body; > -use proxmox_http::{ProxyConfig, RateLimiter}; > +use proxmox_http::ProxyConfig; > use proxmox_log::{error, info, warn}; > +use proxmox_rate_limiter::RateLimiter; > > use pbs_api_types::percent_encoding::DEFAULT_ENCODE_SET; > use pbs_api_types::{Authid, RateLimitConfig, Userid}; > diff --git a/src/tools/mod.rs b/src/tools/mod.rs > index 6556effe3..a97aba5b6 100644 > --- a/src/tools/mod.rs > +++ b/src/tools/mod.rs > @@ -16,10 +16,6 @@ use pbs_datastore::manifest::BackupManifest; > pub mod config; > pub mod disks; > pub mod fs; > - > -mod shared_rate_limiter; > -pub use shared_rate_limiter::SharedRateLimiter; > - > pub mod statistics; > pub mod systemd; > pub mod ticket; > diff --git a/src/tools/shared_rate_limiter.rs b/src/tools/shared_rate_limiter.rs > deleted file mode 100644 > index db754728b..000000000 > --- a/src/tools/shared_rate_limiter.rs > +++ /dev/null > @@ -1,122 +0,0 @@ > -use std::mem::MaybeUninit; > -use std::path::PathBuf; > -use std::time::{Duration, Instant}; > - > -use anyhow::{bail, Error}; > -use nix::sys::stat::Mode; > - > -use proxmox_sys::fs::{create_path, CreateOptions}; > - > -use proxmox_http::{RateLimit, RateLimiter, ShareableRateLimit}; > -use proxmox_shared_memory::{check_subtype, initialize_subtype}; > -use proxmox_shared_memory::{Init, SharedMemory, SharedMutex}; > - > -// openssl::sha::sha256(b"Proxmox Backup SharedRateLimiter v1.0")[0..8]; > -pub const PROXMOX_BACKUP_SHARED_RATE_LIMITER_MAGIC_1_0: [u8; 8] = > - [6, 58, 213, 96, 161, 122, 130, 117]; > - > -const BASE_PATH: &str = pbs_buildcfg::rundir!("/shmem/tbf"); > - > -// Wrap RateLimiter, so that we can provide an Init impl > -#[repr(C)] > -struct WrapLimiter(RateLimiter); > - > -impl Init for WrapLimiter { > - fn initialize(this: &mut MaybeUninit) { > - // default does not matter here, because we override later > - this.write(WrapLimiter(RateLimiter::new(1_000_000, 1_000_000))); > - } > -} > - > -#[repr(C)] > -struct SharedRateLimiterData { > - magic: [u8; 8], > - tbf: SharedMutex, > - padding: [u8; 4096 - 104], > -} > - > -impl Init for SharedRateLimiterData { > - fn initialize(this: &mut MaybeUninit) { > - unsafe { > - let me = &mut *this.as_mut_ptr(); > - me.magic = PROXMOX_BACKUP_SHARED_RATE_LIMITER_MAGIC_1_0; > - initialize_subtype(&mut me.tbf); > - } > - } > - > - fn check_type_magic(this: &MaybeUninit) -> Result<(), Error> { > - unsafe { > - let me = &*this.as_ptr(); > - if me.magic != PROXMOX_BACKUP_SHARED_RATE_LIMITER_MAGIC_1_0 { > - bail!("SharedRateLimiterData: wrong magic number"); > - } > - check_subtype(&me.tbf)?; > - Ok(()) > - } > - } > -} > - > -/// Rate limiter designed for shared memory ([SharedMemory]) > -/// > -/// The actual [RateLimiter] is protected by a [SharedMutex] and > -/// implements [Init]. This way we can share the limiter between > -/// different processes. > -pub struct SharedRateLimiter { > - shmem: SharedMemory, > -} > - > -impl SharedRateLimiter { > - /// Creates a new mmap'ed instance. > - /// > - /// Data is mapped in `/var/run/proxmox-backup/shmem/tbf/` using > - /// `TMPFS`. > - pub fn mmap_shmem(name: &str, rate: u64, burst: u64) -> Result { > - let mut path = PathBuf::from(BASE_PATH); > - > - let user = pbs_config::backup_user()?; > - > - let dir_opts = CreateOptions::new() > - .perm(Mode::from_bits_truncate(0o770)) > - .owner(user.uid) > - .group(user.gid); > - > - create_path(&path, Some(dir_opts), Some(dir_opts))?; > - > - path.push(name); > - > - let file_opts = CreateOptions::new() > - .perm(Mode::from_bits_truncate(0o660)) > - .owner(user.uid) > - .group(user.gid); > - > - let shmem: SharedMemory = SharedMemory::open(&path, file_opts)?; > - > - shmem.data().tbf.lock().0.update_rate(rate, burst); > - > - Ok(Self { shmem }) > - } > -} > - > -impl ShareableRateLimit for SharedRateLimiter { > - fn update_rate(&self, rate: u64, bucket_size: u64) { > - self.shmem > - .data() > - .tbf > - .lock() > - .0 > - .update_rate(rate, bucket_size); > - } > - > - fn traffic(&self) -> u64 { > - self.shmem.data().tbf.lock().0.traffic() > - } > - > - fn register_traffic(&self, current_time: Instant, data_len: u64) -> Duration { > - self.shmem > - .data() > - .tbf > - .lock() > - .0 > - .register_traffic(current_time, data_len) > - } > -} > diff --git a/src/traffic_control_cache.rs b/src/traffic_control_cache.rs > index 830a8c043..bc52d37ee 100644 > --- a/src/traffic_control_cache.rs > +++ b/src/traffic_control_cache.rs > @@ -8,7 +8,7 @@ use std::time::Instant; > use anyhow::Error; > use cidr::IpInet; > > -use proxmox_http::{RateLimiter, ShareableRateLimit}; > +use proxmox_rate_limiter::{RateLimiter, ShareableRateLimit, SharedRateLimiter}; > use proxmox_section_config::SectionConfigData; > > use proxmox_time::{parse_daily_duration, DailyDuration, TmEditor}; > @@ -17,8 +17,6 @@ use pbs_api_types::TrafficControlRule; > > use pbs_config::ConfigVersionCache; > > -use crate::tools::SharedRateLimiter; > - > pub type SharedRateLimit = Arc; > > /// Shared traffic control cache singleton. > @@ -110,7 +108,9 @@ fn create_limiter( > burst: u64, > ) -> Result { > if use_shared_memory { > - let limiter = SharedRateLimiter::mmap_shmem(name, rate, burst)?; > + let user = pbs_config::backup_user()?; > + let base_path = pbs_buildcfg::rundir!("/shmem/tbf"); > + let limiter = SharedRateLimiter::mmap_shmem(name, rate, burst, user, base_path)?; > Ok(Arc::new(limiter)) > } else { > Ok(Arc::new(Mutex::new(RateLimiter::new(rate, burst)))) _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel