all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH proxmox{, -backup} v3 0/9] shared rate limiter for s3 client instances
@ 2025-11-12 11:53 Christian Ebner
  2025-11-12 11:53 ` [pbs-devel] [PATCH proxmox v3 1/5] rate-limiter: add crate for traffic rate limiter implementations Christian Ebner
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Christian Ebner @ 2025-11-12 11:53 UTC (permalink / raw)
  To: pbs-devel

This patches implement a shared bandwidth rate limiter for the s3
client instances, with the goal to allow users to avoid network
congestion by traffic to the s3 backend.

The limiter is shared accorss all the clients using the same s3
endpoint id, setting the limits on client instantiation. To utilize
the pre-existing shared rate limiter implementation from PBS, factor
it out into a dedicated crate, together with the non-shared rate limiter
implementation from proxmox-http.

Expose the rate limit configuration in the s3 endpoint configuration
and provide it to the client's https connector. Expose the settings
also in the advanced options of the s3 endpoint edit/crate window.

Changes since version 2 (thanks @Hannes for review and testing):
- Fix possible division by zero if rate is set to 0, consider as unlimited
  instead
- Rebased onto current master

Changes since version 1 (thanks @Thomas for feedback):
- Move shared/non-shared rate limiter into dedicated crate
- Use a more generic name for the magic number constant of the mmapped
  state file

proxmox:

Christian Ebner (5):
  rate-limiter: add crate for traffic rate limiter implementations
  rate-limiter: avoid division by zero in traffic updater
  http: drop factored out rate limiter implementation
  rest-server: optionally depend on factored out shared rate limiter
  s3-client: add shared rate limiter via https connector

 Cargo.toml                                    |   2 +
 proxmox-http/Cargo.toml                       |   4 +-
 proxmox-http/debian/control                   |  19 +--
 proxmox-http/src/client/connector.rs          |   3 +-
 proxmox-http/src/lib.rs                       |   5 -
 proxmox-http/src/rate_limited_stream.rs       |   2 +-
 proxmox-rate-limiter/Cargo.toml               |  29 ++++
 proxmox-rate-limiter/debian/changelog         |   5 +
 proxmox-rate-limiter/debian/control           |  69 ++++++++++
 proxmox-rate-limiter/debian/copyright         |  18 +++
 proxmox-rate-limiter/debian/debcargo.toml     |   7 +
 proxmox-rate-limiter/src/lib.rs               |  13 ++
 .../src/rate_limiter.rs                       |   3 +
 .../src/shared_rate_limiter.rs                | 130 ++++++++++++++++++
 proxmox-rest-server/Cargo.toml                |   2 +
 proxmox-rest-server/debian/control            |   4 +-
 proxmox-rest-server/src/connection.rs         |   4 +-
 proxmox-s3-client/Cargo.toml                  |   7 +-
 proxmox-s3-client/debian/control              |   7 +-
 proxmox-s3-client/examples/s3_client.rs       |   1 +
 proxmox-s3-client/src/api_types.rs            |  29 ++++
 proxmox-s3-client/src/client.rs               |  63 ++++++++-
 proxmox-s3-client/src/lib.rs                  |   4 +-
 23 files changed, 396 insertions(+), 34 deletions(-)
 create mode 100644 proxmox-rate-limiter/Cargo.toml
 create mode 100644 proxmox-rate-limiter/debian/changelog
 create mode 100644 proxmox-rate-limiter/debian/control
 create mode 100644 proxmox-rate-limiter/debian/copyright
 create mode 100644 proxmox-rate-limiter/debian/debcargo.toml
 create mode 100644 proxmox-rate-limiter/src/lib.rs
 rename {proxmox-http => proxmox-rate-limiter}/src/rate_limiter.rs (98%)
 create mode 100644 proxmox-rate-limiter/src/shared_rate_limiter.rs


proxmox-backup:

Christian Ebner (4):
  traffic control: use factored out shared rate limiter
  api: config: update s3 endpoint rate limits in config
  datastore: s3: set rate limiter options for s3 client
  ui: expose rate and burst limits for s3 endpoints

 Cargo.toml                       |   3 +
 debian/control                   |   1 -
 pbs-client/Cargo.toml            |   3 +-
 pbs-client/src/http_client.rs    |   3 +-
 pbs-datastore/src/datastore.rs   |  18 ++++-
 src/api2/admin/s3.rs             |   9 ++-
 src/api2/config/s3.rs            |  34 ++++++++-
 src/tools/mod.rs                 |   4 -
 src/tools/shared_rate_limiter.rs | 122 -------------------------------
 src/traffic_control_cache.rs     |   8 +-
 www/window/S3ClientEdit.js       |  34 +++++++++
 11 files changed, 102 insertions(+), 137 deletions(-)
 delete mode 100644 src/tools/shared_rate_limiter.rs


Summary over all repositories:
  34 files changed, 498 insertions(+), 171 deletions(-)

-- 
Generated by git-murpp 0.8.1


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [pbs-devel] [PATCH proxmox v3 1/5] rate-limiter: add crate for traffic rate limiter implementations
  2025-11-12 11:53 [pbs-devel] [PATCH proxmox{, -backup} v3 0/9] shared rate limiter for s3 client instances Christian Ebner
@ 2025-11-12 11:53 ` Christian Ebner
  2025-11-12 11:53 ` [pbs-devel] [PATCH proxmox v3 2/5] rate-limiter: avoid division by zero in traffic updater Christian Ebner
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Christian Ebner @ 2025-11-12 11:53 UTC (permalink / raw)
  To: pbs-devel

Factors out the traffic rate limiter implementations currently tied
to the proxmox-backup and proxmox-http crates to make them
independent and easily reusable, e.g. for the s3-client
implementation.

The shared rate limiter implementation from PBS relies on mmapping
for state sharing, the file exposed having a predefined magic number.
In order to be backwards compatible, leave the magic number as is and
only adapt the constant name to be more generic, although the string
the magic number is derived from is PBS specific.

Further, the user for file ownership and base path of the mmapped
file are now passed as parameters during shared rate limiter
instantiation.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
Changes since version 2:
- rebased onto current master

 Cargo.toml                                    |   1 +
 proxmox-rate-limiter/Cargo.toml               |  29 +++
 proxmox-rate-limiter/debian/changelog         |   5 +
 proxmox-rate-limiter/debian/control           |  69 ++++++
 proxmox-rate-limiter/debian/copyright         |  18 ++
 proxmox-rate-limiter/debian/debcargo.toml     |   7 +
 proxmox-rate-limiter/src/lib.rs               |  13 ++
 proxmox-rate-limiter/src/rate_limiter.rs      | 214 ++++++++++++++++++
 .../src/shared_rate_limiter.rs                | 130 +++++++++++
 9 files changed, 486 insertions(+)
 create mode 100644 proxmox-rate-limiter/Cargo.toml
 create mode 100644 proxmox-rate-limiter/debian/changelog
 create mode 100644 proxmox-rate-limiter/debian/control
 create mode 100644 proxmox-rate-limiter/debian/copyright
 create mode 100644 proxmox-rate-limiter/debian/debcargo.toml
 create mode 100644 proxmox-rate-limiter/src/lib.rs
 create mode 100644 proxmox-rate-limiter/src/rate_limiter.rs
 create mode 100644 proxmox-rate-limiter/src/shared_rate_limiter.rs

diff --git a/Cargo.toml b/Cargo.toml
index 37e8696c..942e15c7 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -30,6 +30,7 @@ members = [
     "proxmox-notify",
     "proxmox-openid",
     "proxmox-product-config",
+    "proxmox-rate-limiter",
     "proxmox-resource-scheduling",
     "proxmox-rest-server",
     "proxmox-router",
diff --git a/proxmox-rate-limiter/Cargo.toml b/proxmox-rate-limiter/Cargo.toml
new file mode 100644
index 00000000..6d8d96cc
--- /dev/null
+++ b/proxmox-rate-limiter/Cargo.toml
@@ -0,0 +1,29 @@
+[package]
+name = "proxmox-rate-limiter"
+description = "Token bucket based traffic rate limiter implementation"
+version = "1.0.0"
+
+authors.workspace = true
+edition.workspace = true
+exclude.workspace = true
+homepage.workspace = true
+license.workspace = true
+repository.workspace = true
+rust-version.workspace = true
+
+[dependencies]
+anyhow.workspace = true
+hyper = { workspace = true, optional = true }
+nix = { workspace = true, optional = true }
+
+proxmox-shared-memory = { workspace = true, optional = true }
+proxmox-sys = { workspace = true, optional = true }
+
+[features]
+default = []
+rate-limiter = ["dep:hyper"]
+shared-rate-limiter = [
+    "dep:nix",
+    "dep:proxmox-shared-memory",
+    "dep:proxmox-sys",
+]
diff --git a/proxmox-rate-limiter/debian/changelog b/proxmox-rate-limiter/debian/changelog
new file mode 100644
index 00000000..0bffa551
--- /dev/null
+++ b/proxmox-rate-limiter/debian/changelog
@@ -0,0 +1,5 @@
+rust-proxmox-rate-limiter (1.0.0-1) bookworm; urgency=medium
+
+  * initial packaging
+
+ -- Proxmox Support Team <support@proxmox.com>  Tue, 16 Sep 2025 11:06:23 +0200
diff --git a/proxmox-rate-limiter/debian/control b/proxmox-rate-limiter/debian/control
new file mode 100644
index 00000000..8bd0c80a
--- /dev/null
+++ b/proxmox-rate-limiter/debian/control
@@ -0,0 +1,69 @@
+Source: rust-proxmox-rate-limiter
+Section: rust
+Priority: optional
+Build-Depends: debhelper-compat (= 13),
+ dh-sequence-cargo
+Build-Depends-Arch: cargo:native <!nocheck>,
+ rustc:native (>= 1.82) <!nocheck>,
+ libstd-rust-dev <!nocheck>,
+ librust-anyhow-1+default-dev <!nocheck>
+Maintainer: Proxmox Support Team <support@proxmox.com>
+Standards-Version: 4.7.2
+Vcs-Git: git://git.proxmox.com/git/proxmox.git
+Vcs-Browser: https://git.proxmox.com/?p=proxmox.git
+Homepage: https://proxmox.com
+X-Cargo-Crate: proxmox-rate-limiter
+
+Package: librust-proxmox-rate-limiter-dev
+Architecture: any
+Multi-Arch: same
+Depends:
+ ${misc:Depends},
+ librust-anyhow-1+default-dev
+Suggests:
+ librust-proxmox-rate-limiter+rate-limiter-dev (= ${binary:Version}),
+ librust-proxmox-rate-limiter+shared-rate-limiter-dev (= ${binary:Version})
+Provides:
+ librust-proxmox-rate-limiter+default-dev (= ${binary:Version}),
+ librust-proxmox-rate-limiter-1-dev (= ${binary:Version}),
+ librust-proxmox-rate-limiter-1+default-dev (= ${binary:Version}),
+ librust-proxmox-rate-limiter-1.0-dev (= ${binary:Version}),
+ librust-proxmox-rate-limiter-1.0+default-dev (= ${binary:Version}),
+ librust-proxmox-rate-limiter-1.0.0-dev (= ${binary:Version}),
+ librust-proxmox-rate-limiter-1.0.0+default-dev (= ${binary:Version})
+Description: Token bucket based traffic rate limiter implementation - Rust source code
+ Source code for Debianized Rust crate "proxmox-rate-limiter"
+
+Package: librust-proxmox-rate-limiter+rate-limiter-dev
+Architecture: any
+Multi-Arch: same
+Depends:
+ ${misc:Depends},
+ librust-proxmox-rate-limiter-dev (= ${binary:Version}),
+ librust-hyper-1+default-dev
+Provides:
+ librust-proxmox-rate-limiter-1+rate-limiter-dev (= ${binary:Version}),
+ librust-proxmox-rate-limiter-1.0+rate-limiter-dev (= ${binary:Version}),
+ librust-proxmox-rate-limiter-1.0.0+rate-limiter-dev (= ${binary:Version})
+Description: Token bucket based traffic rate limiter implementation - feature "rate-limiter"
+ This metapackage enables feature "rate-limiter" for the Rust proxmox-rate-
+ limiter crate, by pulling in any additional dependencies needed by that
+ feature.
+
+Package: librust-proxmox-rate-limiter+shared-rate-limiter-dev
+Architecture: any
+Multi-Arch: same
+Depends:
+ ${misc:Depends},
+ librust-proxmox-rate-limiter-dev (= ${binary:Version}),
+ librust-nix-0.29+default-dev,
+ librust-proxmox-shared-memory-1+default-dev,
+ librust-proxmox-sys-1+default-dev
+Provides:
+ librust-proxmox-rate-limiter-1+shared-rate-limiter-dev (= ${binary:Version}),
+ librust-proxmox-rate-limiter-1.0+shared-rate-limiter-dev (= ${binary:Version}),
+ librust-proxmox-rate-limiter-1.0.0+shared-rate-limiter-dev (= ${binary:Version})
+Description: Token bucket based traffic rate limiter implementation - feature "shared-rate-limiter"
+ This metapackage enables feature "shared-rate-limiter" for the Rust proxmox-
+ rate-limiter crate, by pulling in any additional dependencies needed by that
+ feature.
diff --git a/proxmox-rate-limiter/debian/copyright b/proxmox-rate-limiter/debian/copyright
new file mode 100644
index 00000000..d6e3c304
--- /dev/null
+++ b/proxmox-rate-limiter/debian/copyright
@@ -0,0 +1,18 @@
+Format: https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/
+
+Files:
+ *
+Copyright: 2025 Proxmox Server Solutions GmbH <support@proxmox.com>
+License: AGPL-3.0-or-later
+ This program is free software: you can redistribute it and/or modify it under
+ the terms of the GNU Affero General Public License as published by the Free
+ Software Foundation, either version 3 of the License, or (at your option) any
+ later version.
+ .
+ This program is distributed in the hope that it will be useful, but WITHOUT
+ ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
+ details.
+ .
+ You should have received a copy of the GNU Affero General Public License along
+ with this program. If not, see <https://www.gnu.org/licenses/>.
diff --git a/proxmox-rate-limiter/debian/debcargo.toml b/proxmox-rate-limiter/debian/debcargo.toml
new file mode 100644
index 00000000..b7864cdb
--- /dev/null
+++ b/proxmox-rate-limiter/debian/debcargo.toml
@@ -0,0 +1,7 @@
+overlay = "."
+crate_src_path = ".."
+maintainer = "Proxmox Support Team <support@proxmox.com>"
+
+[source]
+vcs_git = "git://git.proxmox.com/git/proxmox.git"
+vcs_browser = "https://git.proxmox.com/?p=proxmox.git"
diff --git a/proxmox-rate-limiter/src/lib.rs b/proxmox-rate-limiter/src/lib.rs
new file mode 100644
index 00000000..a8d7cfdd
--- /dev/null
+++ b/proxmox-rate-limiter/src/lib.rs
@@ -0,0 +1,13 @@
+//! Token bucket based traffic rate limiter implementations.
+
+#![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))]
+
+#[cfg(feature = "rate-limiter")]
+mod rate_limiter;
+#[cfg(feature = "rate-limiter")]
+pub use rate_limiter::{RateLimit, RateLimiter, RateLimiterVec, ShareableRateLimit};
+
+#[cfg(feature = "shared-rate-limiter")]
+mod shared_rate_limiter;
+#[cfg(feature = "shared-rate-limiter")]
+pub use shared_rate_limiter::SharedRateLimiter;
diff --git a/proxmox-rate-limiter/src/rate_limiter.rs b/proxmox-rate-limiter/src/rate_limiter.rs
new file mode 100644
index 00000000..945c77a6
--- /dev/null
+++ b/proxmox-rate-limiter/src/rate_limiter.rs
@@ -0,0 +1,214 @@
+use std::convert::TryInto;
+use std::time::{Duration, Instant};
+
+use anyhow::{bail, Error};
+
+/// Rate limiter interface.
+pub trait RateLimit {
+    /// Update rate and bucket size
+    fn update_rate(&mut self, rate: u64, bucket_size: u64);
+
+    /// Returns the overall traffic (since started)
+    fn traffic(&self) -> u64;
+
+    /// Register traffic, returning a proposed delay to reach the
+    /// expected rate.
+    fn register_traffic(&mut self, current_time: Instant, data_len: u64) -> Duration;
+}
+
+/// Like [`RateLimit`], but does not require self to be mutable.
+///
+/// This is useful for types providing internal mutability (Mutex).
+pub trait ShareableRateLimit: Send + Sync {
+    fn update_rate(&self, rate: u64, bucket_size: u64);
+    fn traffic(&self) -> u64;
+    fn register_traffic(&self, current_time: Instant, data_len: u64) -> Duration;
+}
+
+/// IMPORTANT: We use this struct in shared memory, so please do not
+/// change/modify the layout (do not add fields)
+#[derive(Clone)]
+#[repr(C)]
+struct TbfState {
+    traffic: u64, // overall traffic
+    last_update: Instant,
+    consumed_tokens: u64,
+}
+
+impl TbfState {
+    const NO_DELAY: Duration = Duration::from_millis(0);
+
+    fn refill_bucket(&mut self, rate: u64, current_time: Instant) {
+        let time_diff = match current_time.checked_duration_since(self.last_update) {
+            Some(duration) => duration.as_nanos(),
+            None => return,
+        };
+
+        if time_diff == 0 {
+            return;
+        }
+
+        self.last_update = current_time;
+
+        let allowed_traffic = ((time_diff.saturating_mul(rate as u128)) / 1_000_000_000)
+            .try_into()
+            .unwrap_or(u64::MAX);
+
+        self.consumed_tokens = self.consumed_tokens.saturating_sub(allowed_traffic);
+    }
+
+    fn register_traffic(
+        &mut self,
+        rate: u64,
+        bucket_size: u64,
+        current_time: Instant,
+        data_len: u64,
+    ) -> Duration {
+        self.refill_bucket(rate, current_time);
+
+        self.traffic += data_len;
+        self.consumed_tokens += data_len;
+
+        if self.consumed_tokens <= bucket_size {
+            return Self::NO_DELAY;
+        }
+        Duration::from_nanos(
+            (self.consumed_tokens - bucket_size).saturating_mul(1_000_000_000) / rate,
+        )
+    }
+}
+
+/// Token bucket based rate limiter
+///
+/// IMPORTANT: We use this struct in shared memory, so please do not
+/// change/modify the layout (do not add fields)
+#[repr(C)]
+pub struct RateLimiter {
+    rate: u64,        // tokens/second
+    bucket_size: u64, // TBF bucket size
+    state: TbfState,
+}
+
+impl RateLimiter {
+    /// Creates a new instance, using [Instant::now] as start time.
+    pub fn new(rate: u64, bucket_size: u64) -> Self {
+        let start_time = Instant::now();
+        Self::with_start_time(rate, bucket_size, start_time)
+    }
+
+    /// Creates a new instance with specified `rate`, `bucket_size` and `start_time`.
+    pub fn with_start_time(rate: u64, bucket_size: u64, start_time: Instant) -> Self {
+        Self {
+            rate,
+            bucket_size,
+            state: TbfState {
+                traffic: 0,
+                last_update: start_time,
+                // start with empty bucket (all tokens consumed)
+                consumed_tokens: bucket_size,
+            },
+        }
+    }
+}
+
+impl RateLimit for RateLimiter {
+    fn update_rate(&mut self, rate: u64, bucket_size: u64) {
+        self.rate = rate;
+
+        if bucket_size < self.bucket_size && self.state.consumed_tokens > bucket_size {
+            self.state.consumed_tokens = bucket_size; // start again
+        }
+
+        self.bucket_size = bucket_size;
+    }
+
+    fn traffic(&self) -> u64 {
+        self.state.traffic
+    }
+
+    fn register_traffic(&mut self, current_time: Instant, data_len: u64) -> Duration {
+        self.state
+            .register_traffic(self.rate, self.bucket_size, current_time, data_len)
+    }
+}
+
+impl<R: RateLimit + Send> ShareableRateLimit for std::sync::Mutex<R> {
+    fn update_rate(&self, rate: u64, bucket_size: u64) {
+        self.lock().unwrap().update_rate(rate, bucket_size);
+    }
+
+    fn traffic(&self) -> u64 {
+        self.lock().unwrap().traffic()
+    }
+
+    fn register_traffic(&self, current_time: Instant, data_len: u64) -> Duration {
+        self.lock()
+            .unwrap()
+            .register_traffic(current_time, data_len)
+    }
+}
+
+/// Array of rate limiters.
+///
+/// A group of rate limiters with same configuration.
+pub struct RateLimiterVec {
+    rate: u64,        // tokens/second
+    bucket_size: u64, // TBF bucket size
+    state: Vec<TbfState>,
+}
+
+impl RateLimiterVec {
+    /// Creates a new instance, using [Instant::now] as start time.
+    pub fn new(group_size: usize, rate: u64, bucket_size: u64) -> Self {
+        let start_time = Instant::now();
+        Self::with_start_time(group_size, rate, bucket_size, start_time)
+    }
+
+    /// Creates a new instance with specified `rate`, `bucket_size` and `start_time`.
+    pub fn with_start_time(
+        group_size: usize,
+        rate: u64,
+        bucket_size: u64,
+        start_time: Instant,
+    ) -> Self {
+        let state = TbfState {
+            traffic: 0,
+            last_update: start_time,
+            // start with empty bucket (all tokens consumed)
+            consumed_tokens: bucket_size,
+        };
+        Self {
+            rate,
+            bucket_size,
+            state: vec![state; group_size],
+        }
+    }
+
+    #[allow(clippy::len_without_is_empty)]
+    /// Return the number of TBF entries (group_size)
+    pub fn len(&self) -> usize {
+        self.state.len()
+    }
+
+    /// Traffic for the specified index
+    pub fn traffic(&self, index: usize) -> Result<u64, Error> {
+        if index >= self.state.len() {
+            bail!("RateLimiterVec::traffic - index out of range");
+        }
+        Ok(self.state[index].traffic)
+    }
+
+    /// Register traffic at the specified index
+    pub fn register_traffic(
+        &mut self,
+        index: usize,
+        current_time: Instant,
+        data_len: u64,
+    ) -> Result<Duration, Error> {
+        if index >= self.state.len() {
+            bail!("RateLimiterVec::register_traffic - index out of range");
+        }
+
+        Ok(self.state[index].register_traffic(self.rate, self.bucket_size, current_time, data_len))
+    }
+}
diff --git a/proxmox-rate-limiter/src/shared_rate_limiter.rs b/proxmox-rate-limiter/src/shared_rate_limiter.rs
new file mode 100644
index 00000000..2822e7ea
--- /dev/null
+++ b/proxmox-rate-limiter/src/shared_rate_limiter.rs
@@ -0,0 +1,130 @@
+//! Rate limiter designed for shared memory
+
+use std::mem::MaybeUninit;
+use std::path::Path;
+use std::time::{Duration, Instant};
+
+use anyhow::{bail, Error};
+use nix::sys::stat::Mode;
+use nix::unistd::User;
+
+use proxmox_shared_memory::{check_subtype, initialize_subtype};
+use proxmox_shared_memory::{Init, SharedMemory, SharedMutex};
+use proxmox_sys::fs::{create_path, CreateOptions};
+
+use crate::{RateLimit, RateLimiter, ShareableRateLimit};
+
+/// Magic number for shared rate limiter exposed file mappings
+///
+/// Generated by `openssl::sha::sha256(b"Proxmox Backup SharedRateLimiter v1.0")[0..8];`
+/// Original magic number kept when factored out from the initial
+/// PBS implementation for full backwards compatibility.
+pub const PROXMOX_SHARED_RATE_LIMITER_MAGIC_1_0: [u8; 8] = [6, 58, 213, 96, 161, 122, 130, 117];
+
+// Wrap RateLimiter, so that we can provide an Init impl
+#[repr(C)]
+struct WrapLimiter(RateLimiter);
+
+impl Init for WrapLimiter {
+    fn initialize(this: &mut MaybeUninit<Self>) {
+        // 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<WrapLimiter>,
+    padding: [u8; 4096 - 104],
+}
+
+impl Init for SharedRateLimiterData {
+    fn initialize(this: &mut MaybeUninit<Self>) {
+        unsafe {
+            let me = &mut *this.as_mut_ptr();
+            me.magic = PROXMOX_SHARED_RATE_LIMITER_MAGIC_1_0;
+            initialize_subtype(&mut me.tbf);
+        }
+    }
+
+    fn check_type_magic(this: &MaybeUninit<Self>) -> Result<(), Error> {
+        unsafe {
+            let me = &*this.as_ptr();
+            if me.magic != PROXMOX_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<SharedRateLimiterData>,
+}
+
+impl SharedRateLimiter {
+    /// Creates a new mmap'ed instance.
+    ///
+    /// Data is mapped in `<base_path>/<name>` using
+    /// `TMPFS`.
+    pub fn mmap_shmem<P: AsRef<Path>>(
+        name: &str,
+        rate: u64,
+        burst: u64,
+        user: User,
+        base_path: P,
+    ) -> Result<Self, Error> {
+        let mut path = base_path.as_ref().to_path_buf();
+
+        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<SharedRateLimiterData> = 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)
+    }
+}
-- 
2.47.3



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [pbs-devel] [PATCH proxmox v3 2/5] rate-limiter: avoid division by zero in traffic updater
  2025-11-12 11:53 [pbs-devel] [PATCH proxmox{, -backup} v3 0/9] shared rate limiter for s3 client instances Christian Ebner
  2025-11-12 11:53 ` [pbs-devel] [PATCH proxmox v3 1/5] rate-limiter: add crate for traffic rate limiter implementations Christian Ebner
@ 2025-11-12 11:53 ` Christian Ebner
  2025-11-12 11:53 ` [pbs-devel] [PATCH proxmox v3 3/5] http: drop factored out rate limiter implementation Christian Ebner
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Christian Ebner @ 2025-11-12 11:53 UTC (permalink / raw)
  To: pbs-devel

Do not calculate the proposed delay if the given rate is set to zero,
but rather return with no-delay similar to when the number of
consumed tokens is below the bucket size.

This effectively sets the bandwidth to be unlimited if the rate is set
to zero.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
Changes since version 2:
- not present in previous version

 proxmox-rate-limiter/src/rate_limiter.rs | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/proxmox-rate-limiter/src/rate_limiter.rs b/proxmox-rate-limiter/src/rate_limiter.rs
index 945c77a6..8d879757 100644
--- a/proxmox-rate-limiter/src/rate_limiter.rs
+++ b/proxmox-rate-limiter/src/rate_limiter.rs
@@ -72,6 +72,9 @@ impl TbfState {
         if self.consumed_tokens <= bucket_size {
             return Self::NO_DELAY;
         }
+        if rate == 0 {
+            return Self::NO_DELAY;
+        }
         Duration::from_nanos(
             (self.consumed_tokens - bucket_size).saturating_mul(1_000_000_000) / rate,
         )
-- 
2.47.3



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [pbs-devel] [PATCH proxmox v3 3/5] http: drop factored out rate limiter implementation
  2025-11-12 11:53 [pbs-devel] [PATCH proxmox{, -backup} v3 0/9] shared rate limiter for s3 client instances Christian Ebner
  2025-11-12 11:53 ` [pbs-devel] [PATCH proxmox v3 1/5] rate-limiter: add crate for traffic rate limiter implementations Christian Ebner
  2025-11-12 11:53 ` [pbs-devel] [PATCH proxmox v3 2/5] rate-limiter: avoid division by zero in traffic updater Christian Ebner
@ 2025-11-12 11:53 ` Christian Ebner
  2025-11-12 11:53 ` [pbs-devel] [PATCH proxmox v3 4/5] rest-server: optionally depend on factored out shared rate limiter Christian Ebner
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Christian Ebner @ 2025-11-12 11:53 UTC (permalink / raw)
  To: pbs-devel

The rate limiter implementation has been moved together with the shared
rate limiter into a dedicated crate. Depend on that and drop the now
dead code from the proxmox-http crate.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
Reviewed-by: Hannes Laimer <h.laimer@proxmox.com>
Tested-by: Hannes Laimer <h.laimer@proxmox.com>
---
Changes since version 2:
- rebased onto current master

 Cargo.toml                              |   1 +
 proxmox-http/Cargo.toml                 |   4 +-
 proxmox-http/debian/control             |  19 +--
 proxmox-http/src/client/connector.rs    |   3 +-
 proxmox-http/src/lib.rs                 |   5 -
 proxmox-http/src/rate_limited_stream.rs |   2 +-
 proxmox-http/src/rate_limiter.rs        | 214 ------------------------
 7 files changed, 8 insertions(+), 240 deletions(-)
 delete mode 100644 proxmox-http/src/rate_limiter.rs

diff --git a/Cargo.toml b/Cargo.toml
index 942e15c7..f2802bb1 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -158,6 +158,7 @@ proxmox-login = { version = "1.0.0", path = "proxmox-login" }
 proxmox-network-types = { version = "0.1.0", path = "proxmox-network-types" }
 proxmox-product-config = { version = "1.0.0", path = "proxmox-product-config" }
 proxmox-config-digest = { version = "1.0.0", path = "proxmox-config-digest" }
+proxmox-rate-limiter = { version = "1.0.0", path = "proxmox-rate-limiter" }
 proxmox-rest-server = { version = "1.0.0", path = "proxmox-rest-server" }
 proxmox-router = { version = "3.2.2", path = "proxmox-router" }
 proxmox-s3-client = { version = "1.2", path = "proxmox-s3-client" }
diff --git a/proxmox-http/Cargo.toml b/proxmox-http/Cargo.toml
index 744e5586..e76a769f 100644
--- a/proxmox-http/Cargo.toml
+++ b/proxmox-http/Cargo.toml
@@ -32,6 +32,7 @@ url = { workspace = true, optional = true }
 
 proxmox-async = { workspace = true, optional = true }
 proxmox-base64 = { workspace = true, optional = true }
+proxmox-rate-limiter = { workspace = true, optional = true, features = [ "rate-limiter" ] }
 proxmox-sys = { workspace = true, optional = true }
 proxmox-io = { workspace = true, optional = true }
 proxmox-lang = { workspace = true, optional = true }
@@ -53,7 +54,6 @@ body = [
     "dep:sync_wrapper",
     "sync_wrapper?/futures",
 ]
-rate-limiter = ["dep:hyper"]
 rate-limited-stream = [
     "dep:tokio",
     "dep:hyper-util",
@@ -61,7 +61,7 @@ rate-limited-stream = [
     "hyper-util?/client-legacy",
     "hyper-util?/http1",
     "tokio?/time",
-    "rate-limiter",
+    "dep:proxmox-rate-limiter",
 ]
 client = [
     "dep:bytes",
diff --git a/proxmox-http/debian/control b/proxmox-http/debian/control
index 43d8a894..fb8bce20 100644
--- a/proxmox-http/debian/control
+++ b/proxmox-http/debian/control
@@ -28,7 +28,6 @@ Suggests:
  librust-proxmox-http+http-helpers-dev (= ${binary:Version}),
  librust-proxmox-http+proxmox-async-dev (= ${binary:Version}),
  librust-proxmox-http+rate-limited-stream-dev (= ${binary:Version}),
- librust-proxmox-http+rate-limiter-dev (= ${binary:Version}),
  librust-proxmox-http+websocket-dev (= ${binary:Version})
 Provides:
  librust-proxmox-http+default-dev (= ${binary:Version}),
@@ -171,12 +170,13 @@ Multi-Arch: same
 Depends:
  ${misc:Depends},
  librust-proxmox-http-dev (= ${binary:Version}),
- librust-proxmox-http+rate-limiter-dev (= ${binary:Version}),
  librust-hyper-util-0.1+client-dev (>= 0.1.12-~~),
  librust-hyper-util-0.1+client-legacy-dev (>= 0.1.12-~~),
  librust-hyper-util-0.1+default-dev (>= 0.1.12-~~),
  librust-hyper-util-0.1+http1-dev (>= 0.1.12-~~),
  librust-hyper-util-0.1+http2-dev (>= 0.1.12-~~),
+ librust-proxmox-rate-limiter-1+default-dev,
+ librust-proxmox-rate-limiter-1+rate-limiter-dev,
  librust-tokio-1+default-dev (>= 1.6-~~),
  librust-tokio-1+time-dev (>= 1.6-~~)
 Provides:
@@ -187,21 +187,6 @@ Description: Proxmox HTTP library - feature "rate-limited-stream"
  This metapackage enables feature "rate-limited-stream" for the Rust proxmox-
  http crate, by pulling in any additional dependencies needed by that feature.
 
-Package: librust-proxmox-http+rate-limiter-dev
-Architecture: any
-Multi-Arch: same
-Depends:
- ${misc:Depends},
- librust-proxmox-http-dev (= ${binary:Version}),
- librust-hyper-1+default-dev
-Provides:
- librust-proxmox-http-1+rate-limiter-dev (= ${binary:Version}),
- librust-proxmox-http-1.0+rate-limiter-dev (= ${binary:Version}),
- librust-proxmox-http-1.0.4+rate-limiter-dev (= ${binary:Version})
-Description: Proxmox HTTP library - feature "rate-limiter"
- This metapackage enables feature "rate-limiter" for the Rust proxmox-http
- crate, by pulling in any additional dependencies needed by that feature.
-
 Package: librust-proxmox-http+websocket-dev
 Architecture: any
 Multi-Arch: same
diff --git a/proxmox-http/src/client/connector.rs b/proxmox-http/src/client/connector.rs
index 1aa3de19..70efd23d 100644
--- a/proxmox-http/src/client/connector.rs
+++ b/proxmox-http/src/client/connector.rs
@@ -13,13 +13,14 @@ use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt};
 use tokio::net::TcpStream;
 use tokio_openssl::SslStream;
 
+use proxmox_rate_limiter::ShareableRateLimit;
 use proxmox_sys::linux::socket::set_tcp_keepalive;
 
 use crate::proxy_config::ProxyConfig;
 use crate::uri::build_authority;
 
 use super::tls::MaybeTlsStream;
-use crate::{RateLimitedStream, ShareableRateLimit};
+use crate::RateLimitedStream;
 
 type SharedRateLimit = Arc<dyn ShareableRateLimit>;
 
diff --git a/proxmox-http/src/lib.rs b/proxmox-http/src/lib.rs
index 8b6953b0..7f6f67f8 100644
--- a/proxmox-http/src/lib.rs
+++ b/proxmox-http/src/lib.rs
@@ -26,11 +26,6 @@ mod client_trait;
 #[cfg(feature = "client-trait")]
 pub use client_trait::HttpClient;
 
-#[cfg(feature = "rate-limiter")]
-mod rate_limiter;
-#[cfg(feature = "rate-limiter")]
-pub use rate_limiter::{RateLimit, RateLimiter, RateLimiterVec, ShareableRateLimit};
-
 #[cfg(feature = "rate-limited-stream")]
 mod rate_limited_stream;
 #[cfg(feature = "rate-limited-stream")]
diff --git a/proxmox-http/src/rate_limited_stream.rs b/proxmox-http/src/rate_limited_stream.rs
index e24df7af..cc64e61a 100644
--- a/proxmox-http/src/rate_limited_stream.rs
+++ b/proxmox-http/src/rate_limited_stream.rs
@@ -11,7 +11,7 @@ use tokio::time::Sleep;
 
 use std::task::{Context, Poll};
 
-use super::{RateLimiter, ShareableRateLimit};
+use proxmox_rate_limiter::{RateLimiter, ShareableRateLimit};
 
 type SharedRateLimit = Arc<dyn ShareableRateLimit>;
 
diff --git a/proxmox-http/src/rate_limiter.rs b/proxmox-http/src/rate_limiter.rs
deleted file mode 100644
index 945c77a6..00000000
--- a/proxmox-http/src/rate_limiter.rs
+++ /dev/null
@@ -1,214 +0,0 @@
-use std::convert::TryInto;
-use std::time::{Duration, Instant};
-
-use anyhow::{bail, Error};
-
-/// Rate limiter interface.
-pub trait RateLimit {
-    /// Update rate and bucket size
-    fn update_rate(&mut self, rate: u64, bucket_size: u64);
-
-    /// Returns the overall traffic (since started)
-    fn traffic(&self) -> u64;
-
-    /// Register traffic, returning a proposed delay to reach the
-    /// expected rate.
-    fn register_traffic(&mut self, current_time: Instant, data_len: u64) -> Duration;
-}
-
-/// Like [`RateLimit`], but does not require self to be mutable.
-///
-/// This is useful for types providing internal mutability (Mutex).
-pub trait ShareableRateLimit: Send + Sync {
-    fn update_rate(&self, rate: u64, bucket_size: u64);
-    fn traffic(&self) -> u64;
-    fn register_traffic(&self, current_time: Instant, data_len: u64) -> Duration;
-}
-
-/// IMPORTANT: We use this struct in shared memory, so please do not
-/// change/modify the layout (do not add fields)
-#[derive(Clone)]
-#[repr(C)]
-struct TbfState {
-    traffic: u64, // overall traffic
-    last_update: Instant,
-    consumed_tokens: u64,
-}
-
-impl TbfState {
-    const NO_DELAY: Duration = Duration::from_millis(0);
-
-    fn refill_bucket(&mut self, rate: u64, current_time: Instant) {
-        let time_diff = match current_time.checked_duration_since(self.last_update) {
-            Some(duration) => duration.as_nanos(),
-            None => return,
-        };
-
-        if time_diff == 0 {
-            return;
-        }
-
-        self.last_update = current_time;
-
-        let allowed_traffic = ((time_diff.saturating_mul(rate as u128)) / 1_000_000_000)
-            .try_into()
-            .unwrap_or(u64::MAX);
-
-        self.consumed_tokens = self.consumed_tokens.saturating_sub(allowed_traffic);
-    }
-
-    fn register_traffic(
-        &mut self,
-        rate: u64,
-        bucket_size: u64,
-        current_time: Instant,
-        data_len: u64,
-    ) -> Duration {
-        self.refill_bucket(rate, current_time);
-
-        self.traffic += data_len;
-        self.consumed_tokens += data_len;
-
-        if self.consumed_tokens <= bucket_size {
-            return Self::NO_DELAY;
-        }
-        Duration::from_nanos(
-            (self.consumed_tokens - bucket_size).saturating_mul(1_000_000_000) / rate,
-        )
-    }
-}
-
-/// Token bucket based rate limiter
-///
-/// IMPORTANT: We use this struct in shared memory, so please do not
-/// change/modify the layout (do not add fields)
-#[repr(C)]
-pub struct RateLimiter {
-    rate: u64,        // tokens/second
-    bucket_size: u64, // TBF bucket size
-    state: TbfState,
-}
-
-impl RateLimiter {
-    /// Creates a new instance, using [Instant::now] as start time.
-    pub fn new(rate: u64, bucket_size: u64) -> Self {
-        let start_time = Instant::now();
-        Self::with_start_time(rate, bucket_size, start_time)
-    }
-
-    /// Creates a new instance with specified `rate`, `bucket_size` and `start_time`.
-    pub fn with_start_time(rate: u64, bucket_size: u64, start_time: Instant) -> Self {
-        Self {
-            rate,
-            bucket_size,
-            state: TbfState {
-                traffic: 0,
-                last_update: start_time,
-                // start with empty bucket (all tokens consumed)
-                consumed_tokens: bucket_size,
-            },
-        }
-    }
-}
-
-impl RateLimit for RateLimiter {
-    fn update_rate(&mut self, rate: u64, bucket_size: u64) {
-        self.rate = rate;
-
-        if bucket_size < self.bucket_size && self.state.consumed_tokens > bucket_size {
-            self.state.consumed_tokens = bucket_size; // start again
-        }
-
-        self.bucket_size = bucket_size;
-    }
-
-    fn traffic(&self) -> u64 {
-        self.state.traffic
-    }
-
-    fn register_traffic(&mut self, current_time: Instant, data_len: u64) -> Duration {
-        self.state
-            .register_traffic(self.rate, self.bucket_size, current_time, data_len)
-    }
-}
-
-impl<R: RateLimit + Send> ShareableRateLimit for std::sync::Mutex<R> {
-    fn update_rate(&self, rate: u64, bucket_size: u64) {
-        self.lock().unwrap().update_rate(rate, bucket_size);
-    }
-
-    fn traffic(&self) -> u64 {
-        self.lock().unwrap().traffic()
-    }
-
-    fn register_traffic(&self, current_time: Instant, data_len: u64) -> Duration {
-        self.lock()
-            .unwrap()
-            .register_traffic(current_time, data_len)
-    }
-}
-
-/// Array of rate limiters.
-///
-/// A group of rate limiters with same configuration.
-pub struct RateLimiterVec {
-    rate: u64,        // tokens/second
-    bucket_size: u64, // TBF bucket size
-    state: Vec<TbfState>,
-}
-
-impl RateLimiterVec {
-    /// Creates a new instance, using [Instant::now] as start time.
-    pub fn new(group_size: usize, rate: u64, bucket_size: u64) -> Self {
-        let start_time = Instant::now();
-        Self::with_start_time(group_size, rate, bucket_size, start_time)
-    }
-
-    /// Creates a new instance with specified `rate`, `bucket_size` and `start_time`.
-    pub fn with_start_time(
-        group_size: usize,
-        rate: u64,
-        bucket_size: u64,
-        start_time: Instant,
-    ) -> Self {
-        let state = TbfState {
-            traffic: 0,
-            last_update: start_time,
-            // start with empty bucket (all tokens consumed)
-            consumed_tokens: bucket_size,
-        };
-        Self {
-            rate,
-            bucket_size,
-            state: vec![state; group_size],
-        }
-    }
-
-    #[allow(clippy::len_without_is_empty)]
-    /// Return the number of TBF entries (group_size)
-    pub fn len(&self) -> usize {
-        self.state.len()
-    }
-
-    /// Traffic for the specified index
-    pub fn traffic(&self, index: usize) -> Result<u64, Error> {
-        if index >= self.state.len() {
-            bail!("RateLimiterVec::traffic - index out of range");
-        }
-        Ok(self.state[index].traffic)
-    }
-
-    /// Register traffic at the specified index
-    pub fn register_traffic(
-        &mut self,
-        index: usize,
-        current_time: Instant,
-        data_len: u64,
-    ) -> Result<Duration, Error> {
-        if index >= self.state.len() {
-            bail!("RateLimiterVec::register_traffic - index out of range");
-        }
-
-        Ok(self.state[index].register_traffic(self.rate, self.bucket_size, current_time, data_len))
-    }
-}
-- 
2.47.3



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [pbs-devel] [PATCH proxmox v3 4/5] rest-server: optionally depend on factored out shared rate limiter
  2025-11-12 11:53 [pbs-devel] [PATCH proxmox{, -backup} v3 0/9] shared rate limiter for s3 client instances Christian Ebner
                   ` (2 preceding siblings ...)
  2025-11-12 11:53 ` [pbs-devel] [PATCH proxmox v3 3/5] http: drop factored out rate limiter implementation Christian Ebner
@ 2025-11-12 11:53 ` Christian Ebner
  2025-11-12 11:53 ` [pbs-devel] [PATCH proxmox v3 5/5] s3-client: add shared rate limiter via https connector Christian Ebner
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Christian Ebner @ 2025-11-12 11:53 UTC (permalink / raw)
  To: pbs-devel

The rate limiter implementation has been moved to a dedicated crate,
so depend on it for the rest servers rate limited stream connection.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
Reviewed-by: Hannes Laimer <h.laimer@proxmox.com>
Tested-by: Hannes Laimer <h.laimer@proxmox.com>
---
Changes since version 2:
- rebased onto current master

 proxmox-rest-server/Cargo.toml        | 2 ++
 proxmox-rest-server/debian/control    | 4 +++-
 proxmox-rest-server/src/connection.rs | 4 +++-
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/proxmox-rest-server/Cargo.toml b/proxmox-rest-server/Cargo.toml
index 81d93ca1..7478f6a8 100644
--- a/proxmox-rest-server/Cargo.toml
+++ b/proxmox-rest-server/Cargo.toml
@@ -44,6 +44,7 @@ proxmox-daemon.workspace = true
 proxmox-http = { workspace = true, features = ["body"] }
 proxmox-lang.workspace = true
 proxmox-log.workspace = true
+proxmox-rate-limiter = { workspace = true, optional = true, features = [ "shared-rate-limiter" ] }
 proxmox-router.workspace = true
 proxmox-schema = { workspace = true, features = [ "api-macro", "upid-api-impl" ] }
 proxmox-sys = { workspace = true, features = [ "logrotate", "timer" ] }
@@ -54,5 +55,6 @@ proxmox-worker-task.workspace = true
 default = []
 templates = ["dep:handlebars"]
 rate-limited-stream = [
+    "dep:proxmox-rate-limiter",
     "proxmox-http/rate-limited-stream",
 ]
diff --git a/proxmox-rest-server/debian/control b/proxmox-rest-server/debian/control
index 78304cef..6433c01b 100644
--- a/proxmox-rest-server/debian/control
+++ b/proxmox-rest-server/debian/control
@@ -137,7 +137,9 @@ Depends:
  ${misc:Depends},
  librust-proxmox-rest-server-dev (= ${binary:Version}),
  librust-proxmox-http-1+body-dev (>= 1.0.3-~~),
- librust-proxmox-http-1+rate-limited-stream-dev (>= 1.0.3-~~)
+ librust-proxmox-http-1+rate-limited-stream-dev (>= 1.0.3-~~),
+ librust-proxmox-rate-limiter-1+default-dev,
+ librust-proxmox-rate-limiter-1+shared-rate-limiter-dev
 Provides:
  librust-proxmox-rest-server-1+rate-limited-stream-dev (= ${binary:Version}),
  librust-proxmox-rest-server-1.0+rate-limited-stream-dev (= ${binary:Version}),
diff --git a/proxmox-rest-server/src/connection.rs b/proxmox-rest-server/src/connection.rs
index 9511b7cb..6ffaac73 100644
--- a/proxmox-rest-server/src/connection.rs
+++ b/proxmox-rest-server/src/connection.rs
@@ -24,7 +24,9 @@ use tokio_openssl::SslStream;
 use tokio_stream::wrappers::ReceiverStream;
 
 #[cfg(feature = "rate-limited-stream")]
-use proxmox_http::{RateLimitedStream, ShareableRateLimit};
+use proxmox_http::RateLimitedStream;
+#[cfg(feature = "rate-limited-stream")]
+use proxmox_rate_limiter::ShareableRateLimit;
 
 #[cfg(feature = "rate-limited-stream")]
 pub type SharedRateLimit = Arc<dyn ShareableRateLimit>;
-- 
2.47.3



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [pbs-devel] [PATCH proxmox v3 5/5] s3-client: add shared rate limiter via https connector
  2025-11-12 11:53 [pbs-devel] [PATCH proxmox{, -backup} v3 0/9] shared rate limiter for s3 client instances Christian Ebner
                   ` (3 preceding siblings ...)
  2025-11-12 11:53 ` [pbs-devel] [PATCH proxmox v3 4/5] rest-server: optionally depend on factored out shared rate limiter Christian Ebner
@ 2025-11-12 11:53 ` Christian Ebner
  2025-11-12 11:53 ` [pbs-devel] [PATCH proxmox-backup v3 1/4] traffic control: use factored out shared rate limiter Christian Ebner
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Christian Ebner @ 2025-11-12 11:53 UTC (permalink / raw)
  To: pbs-devel

Allows to configure a shared rate limiter for the s3 client to limit
upload and download bandwidth. This will help users which suffer from
issues due to network congestion by the unlimited s3 client data
transfers.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
Reviewed-by: Hannes Laimer <h.laimer@proxmox.com>
Tested-by: Hannes Laimer <h.laimer@proxmox.com>
---
Changes since version 2:
- no changes

 proxmox-s3-client/Cargo.toml            |  7 ++-
 proxmox-s3-client/debian/control        |  7 ++-
 proxmox-s3-client/examples/s3_client.rs |  1 +
 proxmox-s3-client/src/api_types.rs      | 29 ++++++++++++
 proxmox-s3-client/src/client.rs         | 63 +++++++++++++++++++++++--
 proxmox-s3-client/src/lib.rs            |  4 +-
 6 files changed, 105 insertions(+), 6 deletions(-)

diff --git a/proxmox-s3-client/Cargo.toml b/proxmox-s3-client/Cargo.toml
index d428df47..0b0de240 100644
--- a/proxmox-s3-client/Cargo.toml
+++ b/proxmox-s3-client/Cargo.toml
@@ -22,6 +22,7 @@ hyper-util = { workspace = true, features = [ "client-legacy", "tokio", "http1"
 hyper = { workspace = true, optional = true }
 iso8601 = { workspace = true, optional = true }
 md5 = { workspace = true, optional = true }
+nix = { workspace = true, optional = true }
 openssl = { workspace = true, optional = true }
 quick-xml = { workspace = true, features = [ "async-tokio" ], optional = true }
 regex.workspace = true
@@ -34,7 +35,9 @@ tracing = { workspace = true, optional = true }
 url = {workspace = true, optional = true }
 
 proxmox-base64 = { workspace = true, optional = true }
-proxmox-http = { workspace = true, features = [ "body", "client", "client-trait", "rate-limiter" ], optional = true }
+proxmox-http = { workspace = true, features = [ "body", "client", "client-trait" ], optional = true }
+proxmox-human-byte.workspace = true
+proxmox-rate-limiter = { workspace = true, features = [ "rate-limiter", "shared-rate-limiter" ], optional = true }
 proxmox-schema = { workspace = true, features = [ "api-macro", "api-types" ] }
 proxmox-serde.workspace = true
 proxmox-time = {workspace = true, optional = true }
@@ -51,6 +54,7 @@ impl = [
     "dep:hyper",
     "dep:iso8601",
     "dep:md5",
+    "dep:nix",
     "dep:openssl",
     "dep:quick-xml",
     "dep:serde-xml-rs",
@@ -60,6 +64,7 @@ impl = [
     "dep:url",
     "dep:proxmox-base64",
     "dep:proxmox-http",
+    "dep:proxmox-rate-limiter",
     "dep:proxmox-time",
 ]
 
diff --git a/proxmox-s3-client/debian/control b/proxmox-s3-client/debian/control
index 499857a4..9209da21 100644
--- a/proxmox-s3-client/debian/control
+++ b/proxmox-s3-client/debian/control
@@ -8,6 +8,7 @@ Build-Depends-Arch: cargo:native <!nocheck>,
  libstd-rust-dev <!nocheck>,
  librust-anyhow-1+default-dev <!nocheck>,
  librust-const-format-0.2+default-dev <!nocheck>,
+ librust-proxmox-human-byte-1+default-dev <!nocheck>,
  librust-proxmox-schema-5+api-macro-dev (>= 5.0.1-~~) <!nocheck>,
  librust-proxmox-schema-5+api-types-dev (>= 5.0.1-~~) <!nocheck>,
  librust-proxmox-schema-5+default-dev (>= 5.0.1-~~) <!nocheck>,
@@ -30,6 +31,7 @@ Depends:
  ${misc:Depends},
  librust-anyhow-1+default-dev,
  librust-const-format-0.2+default-dev,
+ librust-proxmox-human-byte-1+default-dev,
  librust-proxmox-schema-5+api-macro-dev (>= 5.0.1-~~),
  librust-proxmox-schema-5+api-types-dev (>= 5.0.1-~~),
  librust-proxmox-schema-5+default-dev (>= 5.0.1-~~),
@@ -73,13 +75,16 @@ Depends:
  librust-hyper-util-0.1+tokio-dev (>= 0.1.12-~~),
  librust-iso8601-0.6+default-dev (>= 0.6.1-~~),
  librust-md5-0.7+default-dev,
+ librust-nix-0.29+default-dev,
  librust-openssl-0.10+default-dev,
  librust-proxmox-base64-1+default-dev,
  librust-proxmox-http-1+body-dev (>= 1.0.3-~~),
  librust-proxmox-http-1+client-dev (>= 1.0.3-~~),
  librust-proxmox-http-1+client-trait-dev (>= 1.0.3-~~),
  librust-proxmox-http-1+default-dev (>= 1.0.3-~~),
- librust-proxmox-http-1+rate-limiter-dev (>= 1.0.3-~~),
+ librust-proxmox-rate-limiter-1+default-dev,
+ librust-proxmox-rate-limiter-1+rate-limiter-dev,
+ librust-proxmox-rate-limiter-1+shared-rate-limiter-dev,
  librust-proxmox-time-2+default-dev (>= 2.1.0-~~),
  librust-quick-xml-0.36+async-tokio-dev (>= 0.36.1-~~),
  librust-quick-xml-0.36+default-dev (>= 0.36.1-~~),
diff --git a/proxmox-s3-client/examples/s3_client.rs b/proxmox-s3-client/examples/s3_client.rs
index 1e19aa1b..ca69971c 100644
--- a/proxmox-s3-client/examples/s3_client.rs
+++ b/proxmox-s3-client/examples/s3_client.rs
@@ -39,6 +39,7 @@ async fn run() -> Result<(), anyhow::Error> {
         fingerprint: Some("<s3-api-fingerprint>".to_string()),
         put_rate_limit: None,
         provider_quirks: Vec::new(),
+        rate_limiter_config: None,
     };
 
     // Creating a client instance and connect to api endpoint
diff --git a/proxmox-s3-client/src/api_types.rs b/proxmox-s3-client/src/api_types.rs
index e4705f92..93573fbd 100644
--- a/proxmox-s3-client/src/api_types.rs
+++ b/proxmox-s3-client/src/api_types.rs
@@ -2,6 +2,7 @@ use anyhow::bail;
 use const_format::concatcp;
 use serde::{Deserialize, Serialize};
 
+use proxmox_human_byte::HumanByte;
 use proxmox_schema::api_types::{
     CERT_FINGERPRINT_SHA256_SCHEMA, DNS_LABEL_STR, IPRE_STR, SAFE_ID_FORMAT,
 };
@@ -126,6 +127,22 @@ serde_plain::derive_fromstr_from_deserialize!(ProviderQuirks);
                 type: ProviderQuirks,
             },
         },
+        "rate-in": {
+            type: HumanByte,
+            optional: true,
+        },
+        "burst-in": {
+            type: HumanByte,
+            optional: true,
+        },
+        "rate-out": {
+            type: HumanByte,
+            optional: true,
+        },
+        "burst-out": {
+            type: HumanByte,
+            optional: true,
+        },
     },
 )]
 #[derive(Serialize, Deserialize, Updater, Clone, PartialEq)]
@@ -154,6 +171,18 @@ pub struct S3ClientConfig {
     /// List of provider specific feature implementation quirks.
     #[serde(skip_serializing_if = "Option::is_none")]
     pub provider_quirks: Option<Vec<ProviderQuirks>>,
+    /// Download rate limit.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub rate_in: Option<HumanByte>,
+    /// Download burst.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub burst_in: Option<HumanByte>,
+    /// Upload rate limit.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub rate_out: Option<HumanByte>,
+    /// Upload burst
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub burst_out: Option<HumanByte>,
 }
 
 impl S3ClientConfig {
diff --git a/proxmox-s3-client/src/client.rs b/proxmox-s3-client/src/client.rs
index abb35f2f..83176b39 100644
--- a/proxmox-s3-client/src/client.rs
+++ b/proxmox-s3-client/src/client.rs
@@ -1,4 +1,4 @@
-use std::path::Path;
+use std::path::{Path, PathBuf};
 use std::str::FromStr;
 use std::sync::{Arc, Mutex};
 use std::time::{Duration, Instant};
@@ -12,6 +12,7 @@ use hyper::{Request, Response};
 use hyper_util::client::legacy::connect::HttpConnector;
 use hyper_util::client::legacy::Client;
 use hyper_util::rt::TokioExecutor;
+use nix::unistd::User;
 use openssl::hash::MessageDigest;
 use openssl::sha::Sha256;
 use openssl::ssl::{SslConnector, SslMethod, SslVerifyMode};
@@ -19,7 +20,8 @@ use openssl::x509::X509StoreContextRef;
 use tracing::error;
 
 use proxmox_http::client::HttpsConnector;
-use proxmox_http::{Body, RateLimit, RateLimiter};
+use proxmox_http::Body;
+use proxmox_rate_limiter::{RateLimit, RateLimiter, SharedRateLimiter};
 use proxmox_schema::api_types::CERT_FINGERPRINT_SHA256_SCHEMA;
 
 use crate::api_types::{ProviderQuirks, S3ClientConfig};
@@ -53,6 +55,25 @@ pub enum S3PathPrefix {
     None,
 }
 
+/// Options for the https connector's rate limiter
+pub struct S3RateLimiterOptions {
+    /// ID for the shared rate limiter.
+    pub id: String,
+    /// Base path for the shared memory mapped file
+    pub base_path: PathBuf,
+    /// User for the to be created shared memory mapped file and folders
+    pub user: User,
+}
+
+/// Configuration  for the https connector's rate limiter
+pub struct S3RateLimiterConfig {
+    options: S3RateLimiterOptions,
+    rate_in: Option<u64>,
+    burst_in: Option<u64>,
+    rate_out: Option<u64>,
+    burst_out: Option<u64>,
+}
+
 /// Configuration options for client
 pub struct S3ClientOptions {
     /// Endpoint to access S3 object store.
@@ -77,6 +98,8 @@ pub struct S3ClientOptions {
     pub put_rate_limit: Option<u64>,
     /// Provider implementation specific features and limitations
     pub provider_quirks: Vec<ProviderQuirks>,
+    /// Configuration options for the shared rate limiter.
+    pub rate_limiter_config: Option<S3RateLimiterConfig>,
 }
 
 impl S3ClientOptions {
@@ -86,7 +109,15 @@ impl S3ClientOptions {
         secret_key: String,
         bucket: Option<String>,
         common_prefix: String,
+        rate_limiter_options: Option<S3RateLimiterOptions>,
     ) -> Self {
+        let rate_limiter_config = rate_limiter_options.map(|options| S3RateLimiterConfig {
+            options,
+            rate_in: config.rate_in.map(|human_bytes| human_bytes.as_u64()),
+            burst_in: config.burst_in.map(|human_bytes| human_bytes.as_u64()),
+            rate_out: config.rate_out.map(|human_bytes| human_bytes.as_u64()),
+            burst_out: config.burst_out.map(|human_bytes| human_bytes.as_u64()),
+        });
         Self {
             endpoint: config.endpoint,
             port: config.port,
@@ -99,6 +130,7 @@ impl S3ClientOptions {
             secret_key,
             put_rate_limit: config.put_rate_limit,
             provider_quirks: config.provider_quirks.unwrap_or_default(),
+            rate_limiter_config,
         }
     }
 }
@@ -151,11 +183,36 @@ impl S3Client {
         // want communication to object store backend api to always use https
         http_connector.enforce_http(false);
         http_connector.set_connect_timeout(Some(S3_HTTP_CONNECT_TIMEOUT));
-        let https_connector = HttpsConnector::with_connector(
+        let mut https_connector = HttpsConnector::with_connector(
             http_connector,
             ssl_connector_builder.build(),
             S3_TCP_KEEPIDLE_TIME,
         );
+
+        if let Some(limiter_config) = &options.rate_limiter_config {
+            if let Some(limit) = limiter_config.rate_in {
+                let limiter = SharedRateLimiter::mmap_shmem(
+                    &format!("{}.in", limiter_config.options.id),
+                    limit,
+                    limiter_config.burst_in.unwrap_or(limit),
+                    limiter_config.options.user.clone(),
+                    limiter_config.options.base_path.clone(),
+                )?;
+                https_connector.set_read_limiter(Some(Arc::new(limiter)));
+            }
+
+            if let Some(limit) = limiter_config.rate_out {
+                let limiter = SharedRateLimiter::mmap_shmem(
+                    &format!("{}.out", limiter_config.options.id),
+                    limit,
+                    limiter_config.burst_out.unwrap_or(limit),
+                    limiter_config.options.user.clone(),
+                    limiter_config.options.base_path.clone(),
+                )?;
+                https_connector.set_write_limiter(Some(Arc::new(limiter)));
+            }
+        }
+
         let client = Client::builder(TokioExecutor::new()).build::<_, Body>(https_connector);
 
         let authority_template = if let Some(port) = options.port {
diff --git a/proxmox-s3-client/src/lib.rs b/proxmox-s3-client/src/lib.rs
index 26e7032b..d02fd0dc 100644
--- a/proxmox-s3-client/src/lib.rs
+++ b/proxmox-s3-client/src/lib.rs
@@ -20,7 +20,9 @@ pub use aws_sign_v4::uri_decode;
 #[cfg(feature = "impl")]
 mod client;
 #[cfg(feature = "impl")]
-pub use client::{S3Client, S3ClientOptions, S3PathPrefix, S3_HTTP_REQUEST_TIMEOUT};
+pub use client::{
+    S3Client, S3ClientOptions, S3PathPrefix, S3RateLimiterOptions, S3_HTTP_REQUEST_TIMEOUT,
+};
 #[cfg(feature = "impl")]
 mod timestamps;
 #[cfg(feature = "impl")]
-- 
2.47.3



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v3 1/4] traffic control: use factored out shared rate limiter
  2025-11-12 11:53 [pbs-devel] [PATCH proxmox{, -backup} v3 0/9] shared rate limiter for s3 client instances Christian Ebner
                   ` (4 preceding siblings ...)
  2025-11-12 11:53 ` [pbs-devel] [PATCH proxmox v3 5/5] s3-client: add shared rate limiter via https connector Christian Ebner
@ 2025-11-12 11:53 ` Christian Ebner
  2025-11-12 11:53 ` [pbs-devel] [PATCH proxmox-backup v3 2/4] api: config: update s3 endpoint rate limits in config Christian Ebner
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Christian Ebner @ 2025-11-12 11:53 UTC (permalink / raw)
  To: pbs-devel

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 <c.ebner@proxmox.com>
Reviewed-by: Hannes Laimer <h.laimer@proxmox.com>
Tested-by: Hannes Laimer <h.laimer@proxmox.com>
---
Changes since version 2:
- no changes

 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 28fdbf546..c02991d2a 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 2149dd768..b996e66b7 100644
--- a/debian/control
+++ b/debian/control
@@ -81,7 +81,6 @@ Build-Depends: debhelper (>= 12~),
                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 4599bf226..af53f74e0 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 739086cf0..6a975bde2 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<Self>) {
-        // 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<WrapLimiter>,
-    padding: [u8; 4096 - 104],
-}
-
-impl Init for SharedRateLimiterData {
-    fn initialize(this: &mut MaybeUninit<Self>) {
-        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<Self>) -> 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<SharedRateLimiterData>,
-}
-
-impl SharedRateLimiter {
-    /// Creates a new mmap'ed instance.
-    ///
-    /// Data is mapped in `/var/run/proxmox-backup/shmem/tbf/<name>` using
-    /// `TMPFS`.
-    pub fn mmap_shmem(name: &str, rate: u64, burst: u64) -> Result<Self, Error> {
-        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<SharedRateLimiterData> = 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<dyn ShareableRateLimit>;
 
 /// Shared traffic control cache singleton.
@@ -110,7 +108,9 @@ fn create_limiter(
     burst: u64,
 ) -> Result<SharedRateLimit, Error> {
     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))))
-- 
2.47.3



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v3 2/4] api: config: update s3 endpoint rate limits in config
  2025-11-12 11:53 [pbs-devel] [PATCH proxmox{, -backup} v3 0/9] shared rate limiter for s3 client instances Christian Ebner
                   ` (5 preceding siblings ...)
  2025-11-12 11:53 ` [pbs-devel] [PATCH proxmox-backup v3 1/4] traffic control: use factored out shared rate limiter Christian Ebner
@ 2025-11-12 11:53 ` Christian Ebner
  2025-11-12 11:53 ` [pbs-devel] [PATCH proxmox-backup v3 3/4] datastore: s3: set rate limiter options for s3 client Christian Ebner
  2025-11-12 11:53 ` [pbs-devel] [PATCH proxmox-backup v3 4/4] ui: expose rate and burst limits for s3 endpoints Christian Ebner
  8 siblings, 0 replies; 10+ messages in thread
From: Christian Ebner @ 2025-11-12 11:53 UTC (permalink / raw)
  To: pbs-devel

Set the rate/burst limits in the s3 endpoint configuration whenever
the limits changed.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
Reviewed-by: Hannes Laimer <h.laimer@proxmox.com>
Tested-by: Hannes Laimer <h.laimer@proxmox.com>
---
Changes since version 2:
- no changes

 src/api2/config/s3.rs | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/src/api2/config/s3.rs b/src/api2/config/s3.rs
index 8270e8d52..1e421114c 100644
--- a/src/api2/config/s3.rs
+++ b/src/api2/config/s3.rs
@@ -139,6 +139,14 @@ pub enum DeletableProperty {
     Fingerprint,
     /// Delete the path-style property.
     PathStyle,
+    /// Delete the rate-in property.
+    RateIn,
+    /// Delete the burst-in property.
+    BurstIn,
+    /// Delete the rate-out property.
+    RateOut,
+    /// Delete the burst-out property.
+    BurstOut,
     /// Delete the provider quirks property.
     ProviderQuirks,
 }
@@ -213,6 +221,18 @@ pub fn update_s3_client_config(
                 DeletableProperty::PathStyle => {
                     data.config.path_style = None;
                 }
+                DeletableProperty::RateIn => {
+                    data.config.rate_in = None;
+                }
+                DeletableProperty::BurstIn => {
+                    data.config.burst_in = None;
+                }
+                DeletableProperty::RateOut => {
+                    data.config.rate_out = None;
+                }
+                DeletableProperty::BurstOut => {
+                    data.config.burst_out = None;
+                }
                 DeletableProperty::ProviderQuirks => {
                     data.config.provider_quirks = None;
                 }
@@ -238,6 +258,18 @@ pub fn update_s3_client_config(
     if let Some(path_style) = update.path_style {
         data.config.path_style = Some(path_style);
     }
+    if let Some(rate_in) = update.rate_in {
+        data.config.rate_in = Some(rate_in);
+    }
+    if let Some(burst_in) = update.burst_in {
+        data.config.burst_in = Some(burst_in);
+    }
+    if let Some(rate_out) = update.rate_out {
+        data.config.rate_out = Some(rate_out);
+    }
+    if let Some(burst_out) = update.burst_out {
+        data.config.burst_out = Some(burst_out);
+    }
     if let Some(provider_quirks) = update.provider_quirks {
         data.config.provider_quirks = Some(provider_quirks);
     }
-- 
2.47.3



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v3 3/4] datastore: s3: set rate limiter options for s3 client
  2025-11-12 11:53 [pbs-devel] [PATCH proxmox{, -backup} v3 0/9] shared rate limiter for s3 client instances Christian Ebner
                   ` (6 preceding siblings ...)
  2025-11-12 11:53 ` [pbs-devel] [PATCH proxmox-backup v3 2/4] api: config: update s3 endpoint rate limits in config Christian Ebner
@ 2025-11-12 11:53 ` Christian Ebner
  2025-11-12 11:53 ` [pbs-devel] [PATCH proxmox-backup v3 4/4] ui: expose rate and burst limits for s3 endpoints Christian Ebner
  8 siblings, 0 replies; 10+ messages in thread
From: Christian Ebner @ 2025-11-12 11:53 UTC (permalink / raw)
  To: pbs-devel

Set the shared rate limiter for each client instance based on the
endpoint configuration. The same limits are shared for each s3
endpoint. To avoid possibly id clashing with rate limits set via
traffic control, use the base directory `<rundir>/s3/shmem/tbf`
instead of the traffic control's `<rundir>/shmem/tbf`.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
Reviewed-by: Hannes Laimer <h.laimer@proxmox.com>
Tested-by: Hannes Laimer <h.laimer@proxmox.com>
---
Changes since version 2:
- no changes

 pbs-datastore/src/datastore.rs | 18 +++++++++++++++++-
 src/api2/admin/s3.rs           |  9 +++++++--
 src/api2/config/s3.rs          |  2 +-
 3 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index b9d4ed2d2..298d817a0 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -15,7 +15,9 @@ use tokio::io::AsyncWriteExt;
 use tracing::{info, warn};
 
 use proxmox_human_byte::HumanByte;
-use proxmox_s3_client::{S3Client, S3ClientConf, S3ClientOptions, S3ObjectKey, S3PathPrefix};
+use proxmox_s3_client::{
+    S3Client, S3ClientConf, S3ClientOptions, S3ObjectKey, S3PathPrefix, S3RateLimiterOptions,
+};
 use proxmox_schema::ApiType;
 
 use proxmox_sys::error::SysError;
@@ -55,6 +57,7 @@ pub const GROUP_NOTES_FILE_NAME: &str = "notes";
 pub const GROUP_OWNER_FILE_NAME: &str = "owner";
 /// Filename for in-use marker stored on S3 object store backend
 pub const S3_DATASTORE_IN_USE_MARKER: &str = ".in-use";
+const S3_CLIENT_RATE_LIMITER_BASE_PATH: &str = pbs_buildcfg::rundir!("/s3/shmem/tbf");
 const NAMESPACE_MARKER_FILENAME: &str = ".namespace";
 
 /// checks if auth_id is owner, or, if owner is a token, if
@@ -284,12 +287,18 @@ impl DataStore {
 
                 let (config, _config_digest) = pbs_config::s3::config()?;
                 let config: S3ClientConf = config.lookup(S3_CFG_TYPE_ID, s3_client_id)?;
+                let rate_limiter_options = S3RateLimiterOptions {
+                    id: s3_client_id.to_string(),
+                    user: pbs_config::backup_user()?,
+                    base_path: S3_CLIENT_RATE_LIMITER_BASE_PATH.into(),
+                };
 
                 let options = S3ClientOptions::from_config(
                     config.config,
                     config.secret_key,
                     Some(bucket),
                     self.name().to_owned(),
+                    Some(rate_limiter_options),
                 );
                 let s3_client = S3Client::new(options)?;
                 DatastoreBackend::S3(Arc::new(s3_client))
@@ -2511,11 +2520,18 @@ impl DataStore {
         let client_config: S3ClientConf = config
             .lookup(S3_CFG_TYPE_ID, s3_client_id)
             .with_context(|| format!("no '{s3_client_id}' in config"))?;
+        let rate_limiter_options = S3RateLimiterOptions {
+            id: s3_client_id.to_string(),
+            user: pbs_config::backup_user()?,
+            base_path: S3_CLIENT_RATE_LIMITER_BASE_PATH.into(),
+        };
+
         let options = S3ClientOptions::from_config(
             client_config.config,
             client_config.secret_key,
             Some(bucket),
             datastore_config.name.to_owned(),
+            Some(rate_limiter_options),
         );
         let s3_client = S3Client::new(options)
             .context("failed to create s3 client")
diff --git a/src/api2/admin/s3.rs b/src/api2/admin/s3.rs
index 73f3779a5..73388281b 100644
--- a/src/api2/admin/s3.rs
+++ b/src/api2/admin/s3.rs
@@ -49,8 +49,13 @@ pub async fn check(
         .context("config lookup failed")?;
 
     let store_prefix = store_prefix.unwrap_or_default();
-    let options =
-        S3ClientOptions::from_config(config.config, config.secret_key, Some(bucket), store_prefix);
+    let options = S3ClientOptions::from_config(
+        config.config,
+        config.secret_key,
+        Some(bucket),
+        store_prefix,
+        None,
+    );
 
     let test_object_key =
         S3ObjectKey::try_from(".s3-client-test").context("failed to generate s3 object key")?;
diff --git a/src/api2/config/s3.rs b/src/api2/config/s3.rs
index 1e421114c..27b3c4cc2 100644
--- a/src/api2/config/s3.rs
+++ b/src/api2/config/s3.rs
@@ -351,7 +351,7 @@ pub async fn list_buckets(
 
     let empty_prefix = String::new();
     let options =
-        S3ClientOptions::from_config(config.config, config.secret_key, None, empty_prefix);
+        S3ClientOptions::from_config(config.config, config.secret_key, None, empty_prefix, None);
     let client = S3Client::new(options).context("client creation failed")?;
     let list_buckets_response = client
         .list_buckets()
-- 
2.47.3



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [pbs-devel] [PATCH proxmox-backup v3 4/4] ui: expose rate and burst limits for s3 endpoints
  2025-11-12 11:53 [pbs-devel] [PATCH proxmox{, -backup} v3 0/9] shared rate limiter for s3 client instances Christian Ebner
                   ` (7 preceding siblings ...)
  2025-11-12 11:53 ` [pbs-devel] [PATCH proxmox-backup v3 3/4] datastore: s3: set rate limiter options for s3 client Christian Ebner
@ 2025-11-12 11:53 ` Christian Ebner
  8 siblings, 0 replies; 10+ messages in thread
From: Christian Ebner @ 2025-11-12 11:53 UTC (permalink / raw)
  To: pbs-devel

Allows to configure the s3 endpoints rate limits via the advanced
options.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
Reviewed-by: Hannes Laimer <h.laimer@proxmox.com>
Tested-by: Hannes Laimer <h.laimer@proxmox.com>
---
Changes since version 2:
- no changes

 www/window/S3ClientEdit.js | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/www/window/S3ClientEdit.js b/www/window/S3ClientEdit.js
index 269b35967..8862683b8 100644
--- a/www/window/S3ClientEdit.js
+++ b/www/window/S3ClientEdit.js
@@ -121,6 +121,20 @@ Ext.define('PBS.window.S3ClientEdit', {
         ],
 
         advancedColumn1: [
+            {
+                xtype: 'pmxBandwidthField',
+                name: 'rate-in',
+                fieldLabel: gettext('Rate In'),
+                emptyText: gettext('Unlimited'),
+                submitAutoScaledSizeUnit: true,
+            },
+            {
+                xtype: 'pmxBandwidthField',
+                name: 'rate-out',
+                fieldLabel: gettext('Rate Out'),
+                emptyText: gettext('Unlimited'),
+                submitAutoScaledSizeUnit: true,
+            },
             {
                 xtype: 'proxmoxKVComboBox',
                 name: 'provider-quirks',
@@ -136,6 +150,22 @@ Ext.define('PBS.window.S3ClientEdit', {
                 },
             },
         ],
+        advancedColumn2: [
+            {
+                xtype: 'pmxBandwidthField',
+                name: 'burst-in',
+                fieldLabel: gettext('Burst In'),
+                emptyText: gettext('Same as Rate'),
+                submitAutoScaledSizeUnit: true,
+            },
+            {
+                xtype: 'pmxBandwidthField',
+                name: 'burst-out',
+                fieldLabel: gettext('Burst Out'),
+                emptyText: gettext('Same as Rate'),
+                submitAutoScaledSizeUnit: true,
+            },
+        ],
     },
 
     getValues: function () {
@@ -146,6 +176,10 @@ Ext.define('PBS.window.S3ClientEdit', {
             values.delete = values.delete.split(',');
         }
         PBS.Utils.delete_if_default(values, 'path-style', false, me.isCreate);
+        PBS.Utils.delete_if_default(values, 'rate-in', undefined, me.isCreate);
+        PBS.Utils.delete_if_default(values, 'burst-in', undefined, me.isCreate);
+        PBS.Utils.delete_if_default(values, 'rate-out', undefined, me.isCreate);
+        PBS.Utils.delete_if_default(values, 'burst-out', undefined, me.isCreate);
 
         let https_scheme_prefix = 'https://';
         if (values.endpoint.startsWith(https_scheme_prefix)) {
-- 
2.47.3



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2025-11-12 11:54 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-11-12 11:53 [pbs-devel] [PATCH proxmox{, -backup} v3 0/9] shared rate limiter for s3 client instances Christian Ebner
2025-11-12 11:53 ` [pbs-devel] [PATCH proxmox v3 1/5] rate-limiter: add crate for traffic rate limiter implementations Christian Ebner
2025-11-12 11:53 ` [pbs-devel] [PATCH proxmox v3 2/5] rate-limiter: avoid division by zero in traffic updater Christian Ebner
2025-11-12 11:53 ` [pbs-devel] [PATCH proxmox v3 3/5] http: drop factored out rate limiter implementation Christian Ebner
2025-11-12 11:53 ` [pbs-devel] [PATCH proxmox v3 4/5] rest-server: optionally depend on factored out shared rate limiter Christian Ebner
2025-11-12 11:53 ` [pbs-devel] [PATCH proxmox v3 5/5] s3-client: add shared rate limiter via https connector Christian Ebner
2025-11-12 11:53 ` [pbs-devel] [PATCH proxmox-backup v3 1/4] traffic control: use factored out shared rate limiter Christian Ebner
2025-11-12 11:53 ` [pbs-devel] [PATCH proxmox-backup v3 2/4] api: config: update s3 endpoint rate limits in config Christian Ebner
2025-11-12 11:53 ` [pbs-devel] [PATCH proxmox-backup v3 3/4] datastore: s3: set rate limiter options for s3 client Christian Ebner
2025-11-12 11:53 ` [pbs-devel] [PATCH proxmox-backup v3 4/4] ui: expose rate and burst limits for s3 endpoints Christian Ebner

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal