Re: [PATCH proxmox{-backup,,-datacenter-manager} v6 00/11] token-shadow: reduce api token verification overhead

[Samuel Rufinatscha <s.rufinatscha@proxmox.com>]

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
>>
>>
>>
>>
>>