-
Notifications
You must be signed in to change notification settings - Fork 880
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
Conversation
} | ||
return nil | ||
} | ||
|
||
func loadModule(module string) error { | ||
if !isModuleAlive(module) { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Discussed this offline with @mrjana. |
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) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 <aboch@docker.com>
@mrjana Took care of your comments. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
LGTM |
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. |
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.
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 aboch@docker.com