From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id EA0951FF13B for ; Wed, 11 Mar 2026 17:26:50 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id A3D0A32F4C; Wed, 11 Mar 2026 17:26:45 +0100 (CET) Message-ID: <725687dd-5a35-41ed-af62-6dc9f062cbd4@proxmox.com> Date: Wed, 11 Mar 2026 17:26:08 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH proxmox{-backup,,-datacenter-manager} v6 00/11] token-shadow: reduce api token verification overhead To: =?UTF-8?Q?Fabian_Gr=C3=BCnbichler?= , pbs-devel@lists.proxmox.com References: <20260303165005.373120-1-s.rufinatscha@proxmox.com> <1773219511.dbh9fr76xn.astroid@yuna.none> Content-Language: en-US From: Samuel Rufinatscha In-Reply-To: <1773219511.dbh9fr76xn.astroid@yuna.none> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.974 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment POISEN_SPAM_PILL 0.1 Meta: its spam POISEN_SPAM_PILL_1 0.1 random spam to be learned in bayes POISEN_SPAM_PILL_3 0.1 random spam to be learned in bayes RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.408 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.819 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.903 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: HBMOHKHIP5TS4KCCVJYAYLUNUGTAWLOC X-Message-ID-Hash: HBMOHKHIP5TS4KCCVJYAYLUNUGTAWLOC X-MailFrom: s.rufinatscha@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Backup Server development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On 3/11/26 9:59 AM, Fabian Grünbichler wrote: > On March 3, 2026 5:49 pm, Samuel Rufinatscha wrote: >> Hi, >> >> this series improves the performance of token-based API authentication >> in PBS (pbs-config) and in PDM (underlying proxmox-access-control >> crate), addressing the API token verification hotspot reported in our >> bugtracker #7017 [1]. > > could you re-spin this so the proxmox/pdm parts compile (the proxmox > workspace got switched to edition 2024, which clashes with the `gen` > variable/binding name used in the patches here). > > please also adapt the PBS patches to the new compatible naming - at some > point we probably want to switch that over as well ;) > Sure, will take care of this! :) Thanks! >> >> When profiling PBS /status endpoint with cargo flamegraph [2], >> token-based authentication showed up as a dominant hotspot via >> proxmox_sys::crypt::verify_crypt_pw. Applying this series removes that >> path from the hot section of the flamegraph. The same performance issue >> was measured [2] for PDM. PDM uses the underlying shared >> proxmox-access-control library for token handling, which is a >> factored out version of the token.shadow handling code from PBS. >> >> While this series fixes the immediate performance issue both in PBS >> (pbs-config) and in the shared proxmox-access-control crate used by >> PDM, PBS should eventually, ideally be refactored, in a separate >> effort, to use proxmox-access-control for token handling instead of its >> local implementation. >> >> Approach >> >> The goal is to reduce the cost of token-based authentication preserving >> the existing token handling semantics (including detecting manual edits >> to token.shadow) and be consistent between PBS (pbs-config) and >> PDM (proxmox-access-control). For both sites, this series proposes to: >> >> 1. Introduce an in-memory cache for verified token secrets and >> invalidate it through a shared ConfigVersionCache generation. Note, a >> shared generation is required to keep privileged and unprivileged >> daemon in sync to avoid caching inconsistencies across processes. >> 2. Invalidate on token.shadow API changes (set_secret, >> delete_secret) >> 3. Invalidate on direct/manual token.shadow file changes (mtime + >> length) >> 4. Avoid per-request file stat calls using a TTL window >> >> Testing >> >> To verify the effect in PBS (pbs-config changes), I: >> 1. Set up test environment based on latest PBS ISO, installed Rust >> toolchain, cloned proxmox-backup repository to use with cargo >> flamegraph. Reproduced bug #7017 [1] by profiling the /status >> endpoint with token-based authentication using cargo flamegraph [2]. >> 2. Built PBS with pbs-config patches and re-ran the same workload and >> profiling setup. Confirmed that >> proxmox_sys::crypt::verify_crypt_pw path no longer appears in the >> hot section of the flamegraph. CPU usage is now dominated by TLS >> overhead. >> 3. Functionally-wise, I verified that: >> * valid tokens authenticate correctly when used in API requests >> * invalid secrets are rejected as before >> * generating a new token secret via dashboard (create token for >> user, regenerate existing secret) works and authenticates correctly >> >> To verify the effect in PDM (proxmox-access-control changes), instead >> of PBS’ /status, I profiled the /version endpoint with cargo flamegraph >> [2] and verified that the expensive hashing path disappears from the >> hot section after introducing caching. Functionally-wise, I verified >> that: >> * valid tokens authenticate correctly when used in API requests >> * invalid secrets are rejected as before >> * generating a new token secret via dashboard (create token for user, >> regenerate existing secret) works and authenticates correctly >> >> Results >> >> To measure caching effect I benchmarked parallel token auth requests >> for /status?verbose=0 on top of the datastore lookup cache series [3] >> to check throughput impact. With datastores=1, repeat=5000, parallel=16 >> this series gives ~172 req/s compared to ~65 req/s without it. >> This is a ~2.6x improvement (and aligns with the ~179 req/s from the >> previous series, which used per-process cache invalidation). >> >> Patch summary >> >> pbs-config: >> 0001 – pbs-config: add token.shadow generation to ConfigVersionCache >> 0002 – pbs-config: cache verified API token secrets >> 0003 – pbs-config: invalidate token-secret cache on token.shadow >> changes >> 0004 – pbs-config: add TTL window to token-secret cache >> >> proxmox-access-control: >> 0005 – access-control: extend AccessControlConfig for token.shadow invalidation >> 0006 – access-control: cache verified API token secrets >> 0007 – access-control: invalidate token-secret cache on token.shadow changes >> 0008 – access-control: add TTL window to token-secret cache >> >> proxmox-datacenter-manager: >> 0009 – pdm-config: add token.shadow generation to ConfigVersionCache >> 0010 – docs: document API token-cache TTL effects >> 0011 – pdm-config: wire user+acl cache generation >> >> Maintainer Notes: >> * proxmox-access-control trait split: permissions now live in >> AccessControlPermissions, and AccessControlConfig now requires >> fn permissions(&self) -> &dyn AccessControlPermissions -> >> version bump >> * Renames ConfigVersionCache`s pub user_cache_generation and >> increase_user_cache_generation -> version bump >> * Adds parking_lot::RwLock dependency in PBS and proxmox-access-control >> >> This version and the version before only incorporate the reviewers' >> feedback [4][5], also please consider Christian's R-b tag [4]. >> >> [1] https://bugzilla.proxmox.com/show_bug.cgi?id=7017 >> [2] attachment 1767 [1]: Flamegraph showing the proxmox_sys::crypt::verify_crypt_pw stack >> [3] https://bugzilla.proxmox.com/show_bug.cgi?id=6049 >> [4] https://lore.proxmox.com/pbs-devel/20260121151408.731516-1-s.rufinatscha@proxmox.com/T/#t >> [5] https://lore.proxmox.com/pbs-devel/20260217111229.78661-1-s.rufinatscha@proxmox.com/T/#t >> >> proxmox-backup: >> >> Samuel Rufinatscha (4): >> pbs-config: add token.shadow generation to ConfigVersionCache >> pbs-config: cache verified API token secrets >> pbs-config: invalidate token-secret cache on token.shadow changes >> pbs-config: add TTL window to token secret cache >> >> Cargo.toml | 1 + >> docs/user-management.rst | 4 + >> pbs-config/Cargo.toml | 1 + >> pbs-config/src/config_version_cache.rs | 18 ++ >> pbs-config/src/token_shadow.rs | 314 ++++++++++++++++++++++++- >> 5 files changed, 335 insertions(+), 3 deletions(-) >> >> >> proxmox: >> >> Samuel Rufinatscha (4): >> proxmox-access-control: split AccessControlConfig and add token.shadow >> gen >> proxmox-access-control: cache verified API token secrets >> proxmox-access-control: invalidate token-secret cache on token.shadow >> changes >> proxmox-access-control: add TTL window to token secret cache >> >> Cargo.toml | 1 + >> proxmox-access-control/Cargo.toml | 1 + >> proxmox-access-control/src/acl.rs | 10 +- >> proxmox-access-control/src/init.rs | 113 ++++++-- >> proxmox-access-control/src/token_shadow.rs | 315 ++++++++++++++++++++- >> 5 files changed, 413 insertions(+), 27 deletions(-) >> >> >> proxmox-datacenter-manager: >> >> Samuel Rufinatscha (3): >> pdm-config: implement token.shadow generation >> docs: document API token-cache TTL effects >> pdm-config: wire user+acl cache generation >> >> cli/admin/src/main.rs | 2 +- >> docs/access-control.rst | 4 +++ >> lib/pdm-api-types/src/acl.rs | 4 +-- >> lib/pdm-config/Cargo.toml | 1 + >> lib/pdm-config/src/access_control.rs | 31 ++++++++++++++++++++ >> lib/pdm-config/src/config_version_cache.rs | 34 +++++++++++++++++----- >> lib/pdm-config/src/lib.rs | 2 ++ >> server/src/acl.rs | 3 +- >> ui/src/main.rs | 10 ++++++- >> 9 files changed, 77 insertions(+), 14 deletions(-) >> create mode 100644 lib/pdm-config/src/access_control.rs >> >> >> Summary over all repositories: >> 19 files changed, 825 insertions(+), 44 deletions(-) >> >> -- >> Generated by git-murpp 0.8.1 >> >> >> >> >>