Use fewer modprobes

[daym]

It's not customary for user space programs to modprobe stuff in general. Modern distributions will autoload modules (if it is indeed a module and not built-in) on access anyway, so it's also not necessary to manually modprobe stuff.

Therefore, this replaces modprobe by actual accesses of the devices. See also moby/moby#38930

[thaJeztah]

Should this still print a warning perhaps, if iptables cannot be found?

[daym]

Sure, why not.

Note: detectIptables, where I copied that chunk from, doesn't either.

[thaJeztah]

Ah, hm, yes, if it wasn't there (not sure why though 🤔 😅) (I'm not a maintainer in this repository, so perhaps there was a reason for that 😅)

[thaJeztah]

ping @euanh @arkodg PTAL

[thaJeztah]

ping @euanh @arkodg @mavenugo PTAL

[thaJeztah]

This function is not exported, so perhaps it should be removed altogether; looks like checkXfrmSocket() is also no longer called now, so could be removed (do we need to try that socket to force the modules to load?)

perhaps add a comment in the calling function about these being automatically loaded, e.g.;

diff --git a/ns/init_linux.go b/ns/init_linux.go
index 6a56b837..ed78bfa6 100644
--- a/ns/init_linux.go
+++ b/ns/init_linux.go
@@ -74,17 +74,10 @@ func NlHandle() *netlink.Handle {
 }
 
 func getSupportedNlFamilies() []int {
-       fams := []int{syscall.NETLINK_ROUTE}
-       // NETLINK_XFRM test
-       if err := loadXfrmModules(); err != nil {
-               if checkXfrmSocket() != nil {
-                       logrus.Warnf("Could not load necessary modules for IPSEC rules: %v", err)
-               } else {
-                       fams = append(fams, syscall.NETLINK_XFRM)
-               }
-       } else {
-               fams = append(fams, syscall.NETLINK_XFRM)
-       }
+       // No need to "modprobe" xfrm_user and xfrm_algo, as they are loaded
+       // automatically when a socket is opened
+       fams := []int{syscall.NETLINK_ROUTE, syscall.NETLINK_XFRM}
+
        // NETLINK_NETFILTER test
        if err := loadNfConntrackModules(); err != nil {
                if checkNfSocket() != nil {
@@ -99,21 +92,6 @@ func getSupportedNlFamilies() []int {
        return fams
 }
 
-func loadXfrmModules() error {
-       // Those are automatically loaded when someone opens the socket anyway.
-       return nil
-}
-
-// API check on required xfrm modules (xfrm_user, xfrm_algo)
-func checkXfrmSocket() error {
-       fd, err := syscall.Socket(syscall.AF_NETLINK, syscall.SOCK_RAW, syscall.NETLINK_XFRM)
-       if err != nil {
-               return err
-       }
-       syscall.Close(fd)
-       return nil
-}
-
 func loadNfConntrackModules() error {
        if out, err := exec.Command("modprobe", "-va", "nf_conntrack").CombinedOutput(); err != nil {
                return fmt.Errorf("Running modprobe nf_conntrack failed with message: `%s`, error: %v", strings.TrimSpace(string(out)), err)

[daym]

Right. But is XFRM supposed to be optional? maybe checkXfrmSocket was supposed to be called still.

Frankly, I hadn't seen the part

        if err := loadXfrmModules(); err != nil {

Maybe it would be better to call checkXfrmSocket anyway--and warn on error.

If the XFRM modules are mandatory for this module anyway, I agree with your approach.

[thaJeztah]

Yes, was suspecting the same, hence my question above;

do we need to try that socket to force the modules to load?

So probably call it unconditionally, and indeed produce an error on failure

[daym]

I've updated the branch to do that.

[thaJeztah]

ping @arkodg @kolyshkin PTAL

[kolyshkin]

LGTM

[selansen]

LGTM

[selansen]

Thanks @daym for working on this PR.
Thanks @thaJeztah @kolyshkin for reviews