Skip to content

Conversation

@c-po
Copy link
Member

@c-po c-po commented Dec 10, 2025

Change summary

The previous implementation of system login relied on Python's pwd.getpwall() to enumerate user accounts. This forces a full walk through the NSS stack, which is acceptable in general but problematic for our use-case. VyOS only needs information about locally created accounts and not remote accounts provided via AAA backends such as TACACS or RADIUS.

When TACACS servers are unreachable, NSS lookups become extremely slow due to repeated timeouts. As a result, any operation triggering pwd.getpwall() (including configuration commits) can stall for several minutes.

This change introduces a dedicated helper, get_local_passwd_entries(), which reads /etc/passwd directly and avoids NSS entirely. Since only local UIDs are relevant, this provides all required data with no external dependencies.

Performance improvement on VyOS 1.4.3 with two unreachable TACACS servers:

set system login tacacs server 192.168.1.50 key test123
set system login tacacs server 192.168.1.51 key test123
time commit

Before

real    3m29.825s
user    0m0.329s
sys     0m0.246s

After

real    0m1.464s
user    0m0.337s
sys     0m0.195s

This significantly improves commit performance and removes sensitivity to AAA server outages.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes)
  • Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
  • Other (please describe):

Related Task(s)

Related PR(s)

How to test / Smoketest result

All smoketests passed:

  • make test-interfaces
  • make test-no-interfaces-no-vpp
  • make testc

Checklist:

  • I have read the CONTRIBUTING document
  • I have linked this PR to one or more Phabricator Task(s)
  • I have run the components SMOKETESTS if applicable
  • My commit headlines contain a valid Task id
  • My change requires a change to the documentation
  • I have updated the documentation accordingly

c-po added 3 commits December 10, 2025 15:12
… timeouts

The previous implementation of "system login" relied on Python's pwd.getpwall()
to enumerate user accounts. This forces a full walk through the NSS stack,
which is acceptable in general but problematic for our use-case. VyOS only
needs information about locally created accounts and not remote accounts
provided via AAA backends such as TACACS or RADIUS.

When TACACS servers are unreachable, NSS lookups become extremely slow due to
repeated timeouts. As a result, any operation triggering pwd.getpwall()
(including configuration commits) can stall for several minutes.

This change introduces a dedicated helper, get_local_passwd_entries(), which
reads /etc/passwd directly and avoids NSS entirely. Since only local UIDs are
relevant, this provides all required data with no external dependencies.

Performance improvement on VyOS 1.4.3 with two unreachable TACACS servers:

  # set system login tacacs server 192.168.1.50 key test123
  # set system login tacacs server 192.168.1.51 key test123

  # time commit

  Before:
    real    3m29.825s
    user    0m0.329s
    sys     0m0.246s

  After:
    real    0m1.464s
    user    0m0.337s
    sys     0m0.195s

This significantly improves commit performance and removes sensitivity to AAA
server outages.
…ries()

Switch to our custom implementation to avoid NSS/TACACS timeouts as explained
in commit 4c9eaaa ("login: replace getpwall() user enumeration to avoid
NSS/TACACS timeouts").
…ries()

Switch to our custom implementation to avoid NSS/TACACS timeouts as explained
in commit 4c9eaaa ("login: replace getpwall() user enumeration to avoid
NSS/TACACS timeouts").
@c-po c-po added bp/sagitta Create automatic backport for sagitta LTS version bp/circinus Create automatic backport for circinus labels Dec 10, 2025
@github-actions
Copy link

👍
No issues in PR Title / Commit Title

@github-actions
Copy link

CI integration 👍 passed!

Details

CI logs

  • CLI Smoketests (no interfaces) 👍 passed
  • CLI Smoketests VPP 👍 passed
  • CLI Smoketests (interfaces only) 👍 passed
  • Config tests 👍 passed
  • Config tests VPP 👍 passed
  • RAID1 tests 👍 passed
  • TPM tests 👍 passed

@c-po c-po removed the bp/sagitta Create automatic backport for sagitta LTS version label Dec 10, 2025
Copy link
Member

@sarthurdev sarthurdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Migrates away from pwd module, uses a vyos.utils helper for reading /etc/passwd.

Smoketests pass, not tested locally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bp/circinus Create automatic backport for circinus current

Development

Successfully merging this pull request may close these issues.

2 participants