[pbs-devel] [PATCH proxmox{, -backup} 0/6] shared rate limiter for s3 client instances

[pbs-devel] [PATCH proxmox{, -backup} 0/6] shared rate limiter for s3 client instances

[Christian Ebner]

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 proxmox-http as this already contains the non-shared rate
limiter implementation.

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.

proxmox:

Christian Ebner (2):
  http: factor out PBS shared rate limiter implementation
  s3-client: add shared rate limiter via https connector

 proxmox-http/Cargo.toml                 |   7 ++
 proxmox-http/debian/control             |  18 ++++
 proxmox-http/src/lib.rs                 |   5 +
 proxmox-http/src/shared_rate_limiter.rs | 129 ++++++++++++++++++++++++
 proxmox-s3-client/Cargo.toml            |   5 +-
 proxmox-s3-client/debian/control        |   4 +
 proxmox-s3-client/examples/s3_client.rs |   1 +
 proxmox-s3-client/src/api_types.rs      |  29 ++++++
 proxmox-s3-client/src/client.rs         |  62 +++++++++++-
 proxmox-s3-client/src/lib.rs            |   4 +-
 10 files changed, 259 insertions(+), 5 deletions(-)
 create mode 100644 proxmox-http/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                       |   2 +-
 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 +++++++++
 8 files changed, 96 insertions(+), 135 deletions(-)
 delete mode 100644 src/tools/shared_rate_limiter.rs


Summary over all repositories:
  18 files changed, 355 insertions(+), 140 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

[pbs-devel] [PATCH proxmox 1/2] http: factor out PBS shared rate limiter implementation

[Christian Ebner]

Moves the current shared rate limiter implementation from the Proxmox
Backup Server into proxmox-http for it to be reusable, e.g. for s3
client rate limiting.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
Note: Unsure if also the magic number for the mmapped files should be
adapted, leaving it as is for full backwards compat for now.

 proxmox-http/Cargo.toml                 |   7 ++
 proxmox-http/debian/control             |  18 ++++
 proxmox-http/src/lib.rs                 |   5 +
 proxmox-http/src/shared_rate_limiter.rs | 129 ++++++++++++++++++++++++
 4 files changed, 159 insertions(+)
 create mode 100644 proxmox-http/src/shared_rate_limiter.rs

diff --git a/proxmox-http/Cargo.toml b/proxmox-http/Cargo.toml
index 1717673c..e70b1fc3 100644
--- a/proxmox-http/Cargo.toml
+++ b/proxmox-http/Cargo.toml
@@ -21,6 +21,7 @@ http-body-util = { workspace = true, optional = true }
 hyper = { workspace = true, optional = true }
 hyper-util = { workspace = true, optional = true, features = ["http2"] }
 native-tls = { workspace = true, optional = true }
+nix = { workspace = true, optional = true }
 openssl =  { version = "0.10", optional = true }
 serde_json = { workspace = true, optional = true }
 sync_wrapper = { workspace = true, optional = true }
@@ -32,6 +33,7 @@ url = { workspace = true, optional = true }
 
 proxmox-async = { workspace = true, optional = true }
 proxmox-base64 = { workspace = true, optional = true }
+proxmox-shared-memory = { workspace = true, optional = true }
 proxmox-sys = { workspace = true, optional = true }
 proxmox-io = { workspace = true, optional = true }
 proxmox-lang = { workspace = true, optional = true }
@@ -54,6 +56,11 @@ body = [
     "sync_wrapper?/futures",
 ]
 rate-limiter = ["dep:hyper"]
+shared-rate-limiter = [
+    "dep:nix",
+    "dep:proxmox-shared-memory",
+    "dep:proxmox-sys",
+]
 rate-limited-stream = [
     "dep:tokio",
     "dep:hyper-util",
diff --git a/proxmox-http/debian/control b/proxmox-http/debian/control
index f50f8ea7..e3996f6c 100644
--- a/proxmox-http/debian/control
+++ b/proxmox-http/debian/control
@@ -30,6 +30,7 @@ Suggests:
  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+shared-rate-limiter-dev (= ${binary:Version}),
  librust-proxmox-http+websocket-dev (= ${binary:Version})
 Provides:
  librust-proxmox-http+default-dev (= ${binary:Version}),
@@ -203,6 +204,23 @@ 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+shared-rate-limiter-dev
+Architecture: any
+Multi-Arch: same
+Depends:
+ ${misc:Depends},
+ librust-proxmox-http-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-http-1+shared-rate-limiter-dev (= ${binary:Version}),
+ librust-proxmox-http-1.0+shared-rate-limiter-dev (= ${binary:Version}),
+ librust-proxmox-http-1.0.2+shared-rate-limiter-dev (= ${binary:Version})
+Description: Proxmox HTTP library - feature "shared-rate-limiter"
+ This metapackage enables feature "shared-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/lib.rs b/proxmox-http/src/lib.rs
index 8b6953b0..ce6a77a1 100644
--- a/proxmox-http/src/lib.rs
+++ b/proxmox-http/src/lib.rs
@@ -31,6 +31,11 @@ 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;
+
 #[cfg(feature = "rate-limited-stream")]
 mod rate_limited_stream;
 #[cfg(feature = "rate-limited-stream")]
diff --git a/proxmox-http/src/shared_rate_limiter.rs b/proxmox-http/src/shared_rate_limiter.rs
new file mode 100644
index 00000000..7be321a5
--- /dev/null
+++ b/proxmox-http/src/shared_rate_limiter.rs
@@ -0,0 +1,129 @@
+//! 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_sys::fs::{create_path, CreateOptions};
+use proxmox_shared_memory::{check_subtype, initialize_subtype};
+use proxmox_shared_memory::{Init, SharedMemory, SharedMutex};
+
+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];`
+pub const PROXMOX_BACKUP_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_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 `<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.2



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

[pbs-devel] [PATCH proxmox 2/2] s3-client: add shared rate limiter via https connector

[Christian Ebner]

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>
---
 proxmox-s3-client/Cargo.toml            |  5 +-
 proxmox-s3-client/debian/control        |  4 ++
 proxmox-s3-client/examples/s3_client.rs |  1 +
 proxmox-s3-client/src/api_types.rs      | 29 ++++++++++++
 proxmox-s3-client/src/client.rs         | 62 +++++++++++++++++++++++--
 proxmox-s3-client/src/lib.rs            |  4 +-
 6 files changed, 100 insertions(+), 5 deletions(-)

diff --git a/proxmox-s3-client/Cargo.toml b/proxmox-s3-client/Cargo.toml
index a4a947c1..371921f1 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,8 @@ 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", "rate-limiter", "shared-rate-limiter" ], optional = true }
+proxmox-human-byte.workspace = true
 proxmox-schema = { workspace = true, features = [ "api-macro", "api-types" ] }
 proxmox-serde.workspace = true
 proxmox-time = {workspace = true, optional = true }
@@ -51,6 +53,7 @@ impl = [
     "dep:hyper",
     "dep:iso8601",
     "dep:md5",
+    "dep:nix",
     "dep:openssl",
     "dep:quick-xml",
     "dep:serde-xml-rs",
diff --git a/proxmox-s3-client/debian/control b/proxmox-s3-client/debian/control
index b9a25246..817c248b 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-4+api-macro-dev (>= 4.1.0-~~) <!nocheck>,
  librust-proxmox-schema-4+api-types-dev (>= 4.1.0-~~) <!nocheck>,
  librust-proxmox-schema-4+default-dev (>= 4.1.0-~~) <!nocheck>,
@@ -31,6 +32,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-4+api-macro-dev (>= 4.1.0-~~),
  librust-proxmox-schema-4+api-types-dev (>= 4.1.0-~~),
  librust-proxmox-schema-4+default-dev (>= 4.1.0-~~),
@@ -74,6 +76,7 @@ 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.2-~~),
@@ -81,6 +84,7 @@ Depends:
  librust-proxmox-http-1+client-trait-dev (>= 1.0.2-~~),
  librust-proxmox-http-1+default-dev (>= 1.0.2-~~),
  librust-proxmox-http-1+rate-limiter-dev (>= 1.0.2-~~),
+ librust-proxmox-http-1+shared-rate-limiter-dev (>= 1.0.2-~~),
  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 67baf467..dd3885d7 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 115b1d2d..9071868a 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 ec29d95a..4e3872a0 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,7 @@ use openssl::x509::X509StoreContextRef;
 use tracing::error;
 
 use proxmox_http::client::HttpsConnector;
-use proxmox_http::{Body, RateLimit, RateLimiter};
+use proxmox_http::{Body, RateLimit, RateLimiter, SharedRateLimiter};
 use proxmox_schema::api_types::CERT_FINGERPRINT_SHA256_SCHEMA;
 
 use crate::api_types::{ProviderQuirks, S3ClientConfig};
@@ -51,6 +52,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.
@@ -75,6 +95,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 {
@@ -84,7 +106,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,
@@ -97,6 +127,7 @@ impl S3ClientOptions {
             secret_key,
             put_rate_limit: config.put_rate_limit,
             provider_quirks: config.provider_quirks.unwrap_or_default(),
+            rate_limiter_config,
         }
     }
 }
@@ -149,11 +180,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_KEEPALIVE_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.2



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

[pbs-devel] [PATCH proxmox-backup 1/4] traffic control: use factored out shared rate limiter

[Christian Ebner]

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>
---
 Cargo.toml                       |   2 +-
 src/tools/mod.rs                 |   4 -
 src/tools/shared_rate_limiter.rs | 122 -------------------------------
 src/traffic_control_cache.rs     |   8 +-
 4 files changed, 5 insertions(+), 131 deletions(-)
 delete mode 100644 src/tools/shared_rate_limiter.rs

diff --git a/Cargo.toml b/Cargo.toml
index 19974da2b..82e3b33f3 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -216,7 +216,7 @@ proxmox-base64.workspace = true
 proxmox-compression.workspace = true
 proxmox-config-digest.workspace = true
 proxmox-daemon.workspace = true
-proxmox-http = { workspace = true, features = [ "body", "client-trait", "proxmox-async", "rate-limited-stream" ] } # pbs-client doesn't use these
+proxmox-http = { workspace = true, features = [ "body", "client-trait", "proxmox-async", "rate-limited-stream", "shared-rate-limiter" ] } # pbs-client doesn't use these
 proxmox-human-byte.workspace = true
 proxmox-io.workspace = true
 proxmox-lang.workspace = true
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<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..de5c9dc77 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_http::{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.2



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

[pbs-devel] [PATCH proxmox-backup 2/4] api: config: update s3 endpoint rate limits in config

[Christian Ebner]

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

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 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.2



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

[pbs-devel] [PATCH proxmox-backup 3/4] datastore: s3: set rate limiter options for s3 client

[Christian Ebner]

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>
---
 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 7cf020fc0..e7cc76dc7 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -14,7 +14,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
@@ -254,12 +257,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))
@@ -2432,11 +2441,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.2



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

[pbs-devel] [PATCH proxmox-backup 4/4] ui: expose rate and burst limits for s3 endpoints

[Christian Ebner]

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

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 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.2



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