From e424c64b471b79f67c4a0cf2deb9e7e64289fcab Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Fri, 11 Mar 2022 16:33:22 +0100 Subject: [PATCH] slirp: fix setup on ipv6 disabled systems When enable_ipv6=true is set for slirp4netns (default since podman v4), we will try to set the accept sysctl. This sysctl will not exist on systems that have ipv6 disabled. In this case we should not error and just ignore the extra ipv6 setup. Also the current logic to wait for the slirp4 setup was kinda broken, it did not actually wait until the sysctl was set before starting slirp. This should now be fixed by using two `sync.WaitGroup`s. [NO NEW TESTS NEEDED] Fixes #13388 Signed-off-by: Paul Holzinger --- libpod/networking_slirp4netns.go | 40 ++++++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/libpod/networking_slirp4netns.go b/libpod/networking_slirp4netns.go index 690f0c1fa5..cc44f78f70 100644 --- a/libpod/networking_slirp4netns.go +++ b/libpod/networking_slirp4netns.go @@ -13,6 +13,7 @@ import ( "path/filepath" "strconv" "strings" + "sync" "syscall" "time" @@ -302,11 +303,15 @@ func (r *Runtime) setupSlirp4netns(ctr *Container, netns ns.NetNS) error { cmd.Stdout = logFile cmd.Stderr = logFile - var slirpReadyChan (chan struct{}) - + var slirpReadyWg, netnsReadyWg *sync.WaitGroup if netOptions.enableIPv6 { - slirpReadyChan = make(chan struct{}) - defer close(slirpReadyChan) + // use two wait groups to make sure we set the sysctl before + // starting slirp and reset it only after slirp is ready + slirpReadyWg = &sync.WaitGroup{} + netnsReadyWg = &sync.WaitGroup{} + slirpReadyWg.Add(1) + netnsReadyWg.Add(1) + go func() { err := ns.WithNetNSPath(netnsPath, func(_ ns.NetNS) error { // Duplicate Address Detection slows the ipv6 setup down for 1-2 seconds. @@ -318,23 +323,37 @@ func (r *Runtime) setupSlirp4netns(ctr *Container, netns ns.NetNS) error { // is ready in case users rely on this sysctl. orgValue, err := ioutil.ReadFile(ipv6ConfDefaultAcceptDadSysctl) if err != nil { + netnsReadyWg.Done() + // on ipv6 disabled systems the sysctl does not exists + // so we should not error + if errors.Is(err, os.ErrNotExist) { + return nil + } return err } err = ioutil.WriteFile(ipv6ConfDefaultAcceptDadSysctl, []byte("0"), 0644) + netnsReadyWg.Done() if err != nil { return err } - // wait for slirp to finish setup - <-slirpReadyChan + + // wait until slirp4nets is ready before reseting this value + slirpReadyWg.Wait() return ioutil.WriteFile(ipv6ConfDefaultAcceptDadSysctl, orgValue, 0644) }) if err != nil { logrus.Warnf("failed to set net.ipv6.conf.default.accept_dad sysctl: %v", err) } }() + + // wait until we set the sysctl + netnsReadyWg.Wait() } if err := cmd.Start(); err != nil { + if netOptions.enableIPv6 { + slirpReadyWg.Done() + } return errors.Wrapf(err, "failed to start slirp4netns process") } defer func() { @@ -344,11 +363,12 @@ func (r *Runtime) setupSlirp4netns(ctr *Container, netns ns.NetNS) error { } }() - if err := waitForSync(syncR, cmd, logFile, 1*time.Second); err != nil { - return err + err = waitForSync(syncR, cmd, logFile, 1*time.Second) + if netOptions.enableIPv6 { + slirpReadyWg.Done() } - if slirpReadyChan != nil { - slirpReadyChan <- struct{}{} + if err != nil { + return err } // Set a default slirp subnet. Parsing a string with the net helper is easier than building the struct myself