Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

Is there any value in introducing dynamically loadable kernel modules? #223

Closed
jodh-intel opened this issue Oct 12, 2018 · 14 comments
Closed

Comments

@jodh-intel
Copy link
Contributor

Currently, the kernel config files are supposed to set all CONFIG_* options to y (to make the features "builtins").

However, that doesn't seem to be the case exactly:

$ cd $GOPATH/src/github.com/kata-containers/packaging/kernel/configs
$ grep -c '^CONFIG_.*=m' *4.14* | grep -v ':0$'
arm64_kata_kvm_4.14.x:11
s390_kata_kvm_4.14.x:177
x86_64_kata_kvm_4.14.x:10

We need to decide what to do here:

Before we can decide this, we'd need to know the impact of all of the options above:

  • Performance
    • boot speed
    • in-memory kernel size (memory density)
  • on-disk kernel size
  • security issues?
  • administrative overhead
    • test / CI overhead
    • development overhead

/cc @bergwolf, @egernst, @grahamwhaley, @jcvenegas, @sboeuf.


[1] - 🎸 👨‍🎤 🎤 😄

@jodh-intel
Copy link
Contributor Author

Related: #222.

@grahamwhaley
Copy link
Contributor

I suspect currently the =m options are having zero impact, and can all be changed to =n. My theory is that the modules get built into a separate (install) dir in the kernel tree, and we don't currently (afaik) copy them over into the image - so, they cost us build time, but no actual image space or impact on runtime at all.

That is, unless somebody knows differently :-)

@jodh-intel
Copy link
Contributor Author

Oooh - so all the =m options are essentially ignored in our scenario.

/cc @alicefr, @Weichen81 as s390 in particular looks to have a lot of potentially missing functionality then.

@sboeuf
Copy link

sboeuf commented Oct 12, 2018

@jodh-intel if we don't include the kernel modules into the image, all the config options =m will be ignored. I don't think that by default our kernel configs should include any module.

@jodh-intel
Copy link
Contributor Author

Right, so we need to remove those =m options or convert them to =y's. ftr, here's what we have for x86_64 that isn't active:

$ grep '^CONFIG_.*=m' x86_64_kata_kvm_4.14.x | sort
CONFIG_DNS_RESOLVER=m
CONFIG_GRACE_PERIOD=m
CONFIG_LOCKD=m
CONFIG_NFS_FS=m
CONFIG_NFS_V2=m
CONFIG_NFS_V3=m
CONFIG_NFS_V4=m
CONFIG_OID_REGISTRY=m
CONFIG_SUNRPC_GSS=m
CONFIG_SUNRPC=m

@grahamwhaley
Copy link
Contributor

I think convert to n is the right thing - as afaik, nobody is (or can unless they are doing something smart) use them right now.... but, we need to check with they Hyper and Huawei folks, in case they are doing some magic 'map the module dir into the container' trick for some reason.
So, before we merge anything, let's check with @bergwolf and @WeiZhang555

@sboeuf
Copy link

sboeuf commented Oct 12, 2018

I agree @grahamwhaley
I don't think they're doing anything with modules but we never know, and by default we should switch those configs to n

@bergwolf
Copy link
Member

osbuilder does support kernel module dir when building rootfs, see https://github.com/kata-containers/osbuilder/tree/master/rootfs-builder#creating-a-rootfs-with-kernel-modules

@Weichen81
Copy link
Contributor

@jodh-intel @grahamwhaley @sboeuf I agree to switch these =m to =n except the NFS options. Because I remember NFS is or would be one option of the Guest rootfs. Although I know few people is using NFS.

@bergwolf
Copy link
Member

@Weichen81 NFS are configured as modules to follow runv to support NFS storage types -- both rootfs and volume can be NFS based. It is a useful feature when users want to share volumes in a cloud CaaS deployment. Although we haven't got that far in kata containers yet, I think we still want to enable NFS volumes in future.

@Weichen81
Copy link
Contributor

@bergwolf So, we should set NFS =n in current stage?

@bergwolf
Copy link
Member

@Weichen81 yes, I agree. But that is not because kernel modules are not supported in kata containers osbuilder. Each kernel module takes some time and resources to initialize. If we make them all builtin, it might have unnecessary performance impact, -- to answer the very initial question from @jodh-intel in the top message, I'm voting Make a well-defined subset of options builtins and the remainder modules.

@Weichen81
Copy link
Contributor

Ok, I understand and agree with you : )

@jcvenegas
Copy link
Member

agree to mark as =n all the current modules, it is nice that osbuilder support modules dir for the current kernel config this is not used.

For the rest of build in modules we should see if mark them as modules inproves the memory and boot time. If yes we should work in a solution to add not boot needed modules to the VM as a share directory.

qemu -9pfs /usr/share/katacontainers/kernel-${version}/modules

Then when the agent start mount this share directory to load modules as needed

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

No branches or pull requests

6 participants