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

libct/cap: no need to load capabilities #4429

Merged
merged 1 commit into from
Oct 20, 2024

Conversation

kolyshkin
Copy link
Contributor

We are not really interested in the capabilities of the current process, so there is no need to load those.

This results in some performance improvement since now the capability package don't have to parse /proc/self/status.

@lifubang
Copy link
Member

lifubang commented Oct 9, 2024

Maybe we are interested in them in here:
https://github.com/moby/sys/blob/main/capability/capability_linux.go#L342-L346

@lifubang
Copy link
Member

lifubang commented Oct 9, 2024

Maybe we are interested in them in here: https://github.com/moby/sys/blob/main/capability/capability_linux.go#L342-L346

I have no strong objection to merge this PR, but maybe in future we need to add it back to reduce the prctl(PR_CAPBSET_DROP) syscall.

@kolyshkin
Copy link
Contributor Author

Maybe we are interested in them in here: https://github.com/moby/sys/blob/main/capability/capability_linux.go#L342-L346

In there it checks the bits that runc previously cleared and set itself. The values that are set by Load() are not used anywhere.

Verified

This commit was signed with the committer’s verified signature. The key has expired.
aladdin-add 唯然
We are not really interested in the capabilities of the current process,
so there is no need to load those.

This results in some performance improvement since now the capability
package don't have to parse /proc/self/status.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@kolyshkin
Copy link
Contributor Author

Maybe we are interested in them in here: https://github.com/moby/sys/blob/main/capability/capability_linux.go#L342-L346

In there it checks the bits that runc previously cleared and set itself. The values that are set by Load() are not used anywhere.

To reiterate, no values from Load are used anywhere. @lifubang please take another look.

Copy link
Member

@lifubang lifubang left a comment

Choose a reason for hiding this comment

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

LGTM, though I think it's useful to reduce prctl(PR_CAPBSET_DROP) syscall in future.
But let's leave future's things in the future.

@AkihiroSuda AkihiroSuda merged commit 08faf15 into opencontainers:main Oct 20, 2024
42 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants