public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: pbs-devel@lists.proxmox.com,
	Samuel Rufinatscha <s.rufinatscha@proxmox.com>
Subject: Re: [PATCH proxmox{-backup,,-datacenter-manager} v6 00/11] token-shadow: reduce api token verification overhead
Date: Wed, 11 Mar 2026 09:59:36 +0100	[thread overview]
Message-ID: <1773219511.dbh9fr76xn.astroid@yuna.none> (raw)
In-Reply-To: <20260303165005.373120-1-s.rufinatscha@proxmox.com>

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

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




  parent reply	other threads:[~2026-03-11  8:59 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-03 16:49 Samuel Rufinatscha
2026-03-03 16:49 ` [PATCH proxmox-backup v6 1/4] pbs-config: add token.shadow generation to ConfigVersionCache Samuel Rufinatscha
2026-03-03 16:49 ` [PATCH proxmox-backup v6 2/4] pbs-config: cache verified API token secrets Samuel Rufinatscha
2026-03-03 16:49 ` [PATCH proxmox-backup v6 3/4] pbs-config: invalidate token-secret cache on token.shadow changes Samuel Rufinatscha
2026-03-03 16:49 ` [PATCH proxmox-backup v6 4/4] pbs-config: add TTL window to token secret cache Samuel Rufinatscha
2026-03-03 16:49 ` [PATCH proxmox v6 1/4] proxmox-access-control: split AccessControlConfig and add token.shadow gen Samuel Rufinatscha
2026-03-03 16:49 ` [PATCH proxmox v6 2/4] proxmox-access-control: cache verified API token secrets Samuel Rufinatscha
2026-03-03 16:49 ` [PATCH proxmox v6 3/4] proxmox-access-control: invalidate token-secret cache on token.shadow changes Samuel Rufinatscha
2026-03-03 16:49 ` [PATCH proxmox v6 4/4] proxmox-access-control: add TTL window to token secret cache Samuel Rufinatscha
2026-03-03 16:49 ` [PATCH proxmox-datacenter-manager v6 1/3] pdm-config: implement token.shadow generation Samuel Rufinatscha
2026-03-03 16:49 ` [PATCH proxmox-datacenter-manager v6 2/3] docs: document API token-cache TTL effects Samuel Rufinatscha
2026-03-03 16:50 ` [PATCH proxmox-datacenter-manager v6 3/3] pdm-config: wire user+acl cache generation Samuel Rufinatscha
2026-03-11  8:59 ` Fabian Grünbichler [this message]
2026-03-11 16:26   ` [PATCH proxmox{-backup,,-datacenter-manager} v6 00/11] token-shadow: reduce api token verification overhead Samuel Rufinatscha
2026-03-12 10:38 ` [pbs-devel] superseded: " Samuel Rufinatscha

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1773219511.dbh9fr76xn.astroid@yuna.none \
    --to=f.gruenbichler@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    --cc=s.rufinatscha@proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal