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

Run API check to assert xfrm modules #1502

Merged
merged 1 commit into from
Oct 14, 2016
Merged

Run API check to assert xfrm modules #1502

merged 1 commit into from
Oct 14, 2016

Conversation

aboch
Copy link
Contributor

@aboch aboch commented Oct 10, 2016

  • When docker is run inside a container, the infrastructure
    needed by modprobe is not always available, causing the
    xfrm module load to fail even when these modules are already
    loaded or builtin in the kernel.
  • In case of probe failure, before declaring the failure,
    run an API check by attempting the creation of
    a NETLINK_XFRM socket.

Fixes #1439
Related to moby/moby#27188

Signed-off-by: Alessandro Boch [email protected]

}
return nil
}

func loadModule(module string) error {
if !isModuleAlive(module) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this based on the assumption that the module is already loaded in the host before you started the daemon inside dind? Because of the module is not loaded you for sure cannot try to load it inside dind unless you bind mount certain host directories.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is for avoiding to modprobe when we know from /proc/modules that the module is already there. Given we know there are cases when running inside a container where modprobe would fail for reasons not related to the bring up of the module.
This is expecially true when running docker daemon inside a container.

@aboch
Copy link
Contributor Author

aboch commented Oct 10, 2016

Discussed this offline with @mrjana.
He is suggesting to instead run an API check, because this is the most future proof approach.
I agree with that and will make the change.

@aboch aboch changed the title Look for xfrm module in proc/modules before attempting modprobe Run API check to assert xfrm modules Oct 10, 2016
if err := loadXfrmModules(); err != nil {
log.Warnf("Could not load necessary modules for IPSEC rules: %v", err)
if err := checkXfrmSocket(); err != nil {
log.Warnf("Required socket to program IPSEC rules cannot be created: %v. (Requires xfrm_user and xfrm_ago modules)", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Uhm.. You still need to try loading xfrm modules in case the checkXfrmSocket fails. This is for cases where the module doesn't get autoloaded.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. Thanks.

- When docker is run inside a container, the infrastructure
  needed by modprobe is not always available, causing the
  xfrm module load to fail even when these modules are already
  loaded or builtin in the kernel.
- In case of probe failure, before declaring the failure,
  run an API check by attempting the creation of
  a NETLINK_XFRM socket.

Signed-off-by: Alessandro Boch <[email protected]>
@aboch
Copy link
Contributor Author

aboch commented Oct 10, 2016

@mrjana Took care of your comments.
PTAL when you get a chance

Copy link
Contributor

@mrjana mrjana left a comment

Choose a reason for hiding this comment

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

LGTM

@mavenugo
Copy link
Contributor

LGTM

@mavenugo mavenugo merged commit 7768939 into moby:master Oct 14, 2016
@justincormack
Copy link
Contributor

Normally modules autoload anyway when you try to use the symbols, so I don't think it is even necessary to try to modprobe on fallback, but this is certainly an improvement.

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.

5 participants