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

totemknet: retry knet_handle_new without privileged operations if it … #623

Closed
wants to merge 1 commit into from
Closed

totemknet: retry knet_handle_new without privileged operations if it … #623

wants to merge 1 commit into from

Conversation

ddstreet
Copy link
Contributor

…fails

knet_handle_new can fail with ENAMETOOLONG if its privileged operations
fail, which can happen if we're running as a user process or in an
unprivileged container. Instead of failing to start, let's retry
without the privileged operations, which may result in a reduction in
performance, but at least it doesn't completely prevent us from starting.

Signed-off-by: Dan Streetman ddstreet@canonical.com

@knet-ci-bot
Copy link

Can one of the admins verify this patch?

@jfriesse
Copy link
Member

@ddstreet Thank you for the PR. It is generally good but after brief talk with knet maintainer there was a bit worry about automatic fallback because of possible performance problems.

Would you mind to improve patch so it is using new key in cmap (something like system.allow_unpriviledged_knet_handle_fallback, ... you got the idea) which would be by default false/no but one can set it to yes (similarly as (for example) system.move_to_root_cgroup - you can just copy/paste the code) to get automatic fallback?

@jfriesse
Copy link
Member

@knet-ci-bot ok to test

…fails

knet_handle_new can fail with ENAMETOOLONG if its privileged operations
fail, which can happen if we're running as a user process or in an
unprivileged container.

This adds a cmap key 'allow_knet_handle_fallback' that defaults to no,
which is the current behavior of exiting with error if the knet_handle
can't be created with privileged operations. If the new cmap key is set
to 'yes' and the knet_handle creation fails, fallback to creating the
handle using unprivileged operations is tried.

Signed-off-by: Dan Streetman <ddstreet@canonical.com>
@ddstreet
Copy link
Contributor Author

@ddstreet Thank you for the PR. It is generally good but after brief talk with knet maintainer there was a bit worry about automatic fallback because of possible performance problems.

Would you mind to improve patch so it is using new key in cmap (something like system.allow_unpriviledged_knet_handle_fallback, ... you got the idea) which would be by default false/no but one can set it to yes (similarly as (for example) system.move_to_root_cgroup - you can just copy/paste the code) to get automatic fallback?

I updated the commit, hopefully I got the cmap key addition correct; it did seem to test correctly for me.

I named it 'allow_knet_handle_fallback' as it's very easy to accidentally misspell 'unprivileged' so might cause confusion if it's in the param name.

Let me know if I can adjust anything else. Thanks!

@jfriesse
Copy link
Member

@ddstreet Thank you for the update. I really like the name of the key, big thanks for the man page update (this is easily forgotten), loading of key is also correct. In general it looks really nice and I was unable find anything wrong during testing, so ACK and merged as a 4f171ea.

Regards,
Honza

@jfriesse jfriesse closed this Mar 18, 2021
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.

None yet

3 participants