Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport CP-43755 - rate limits for PAM logins #5484

Merged
merged 5 commits into from
Mar 4, 2024

Conversation

lindig
Copy link
Contributor

@lindig lindig commented Mar 4, 2024

We are backporting a list of commits from master but drop the changes to the datamodel and instead use a hard-coded value.

edwintorok and others added 4 commits March 1, 2024 11:17
Backport 0dc9939

The codepaths are completely independent.
Rename `serialize` to `throttle`.

This prepares the code for parallel authentication, with independently tunable thresholds.

No functional change.

Signed-off-by: Edwin Török <[email protected]>
Signed-off-by: Christian Lindig <[email protected]>
Backport of 8aa69bb. This required to
introduce a Semaphore_vendored to have access to the Counting
sub-module.

Small wrapper around OCaml's Semaphore module.
It doesn't yet use Thread_state to avoid potential performance issues.

The semaphore is initialized with a count of 1, and its maximum can be
changed at runtime using [set_max] in a safe way that takes into account
threads that already use the semaphore: we store the current max in the
record and call [acquire] or [release] the appropriate number of times
to make it match the new desired max.  This always works, even if there
are threads currently using the semaphore: if we decrease it below max
then 'set_max' will block until sufficient number of threads have
released the semaphore.

[set_max] will be useful for implementing the semaphore maximum value as
a datamodel field, and won't require restarting XAPI to change it.  Even
reading this value from a config file would be difficult otherwise
because the semaphore needs to be initialized with a count at the time
it is created, which is before any configuration files are read.

No functional change.

Signed-off-by: Edwin Török <[email protected]>
Signed-off-by: Christian Lindig <[email protected]>
Backport of 4903400

The default count is still 1, so the behaviour is equivalent to Mutex.
Internally Semaphore uses a Mutex + Condition and its performance is
very similar to Mutex.

No functional change.

Signed-off-by: Edwin Török <[email protected]>
Signed-off-by: Christian Lindig <[email protected]>
We are backporting this from the master branch but are taking the
changes to the datamodel that allows to adjust thread counts at runtime.
Instead, we create a fixed number upon creation.

Signed-off-by: Christian Lindig <[email protected]>
@lindig lindig requested a review from edwintorok March 4, 2024 15:39
Signed-off-by: Christian Lindig <[email protected]>
Comment on lines +29 to +30
let with_lock = Xapi_stdext_threads.Threadext.Mutex.execute

Copy link
Member

Choose a reason for hiding this comment

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

This binding isn't needed, all the users were changed to semaphores

Copy link
Member

@psafont psafont left a comment

Choose a reason for hiding this comment

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

I'm surprised that the location of the vendored semaphore location works, but the backport seems solid

@lindig lindig merged commit 4537f85 into xapi-project:1.249-lcm Mar 4, 2024
4 of 5 checks passed
@psafont
Copy link
Member

psafont commented Mar 4, 2024

The vendored module can't be located, easiest thing is probably to move it to ocaml/xapi

@lindig
Copy link
Contributor Author

lindig commented Mar 4, 2024

Sorry - I pushed to the wrong branch. This needs still reviewing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants