rootless: use RootlessKit port forwarder

[AkihiroSuda]

RootlessKit port forwarder has a lot of advantages over the slirp4netns port forwarder:

• Very high throughput.
  Benchmark result on Travis: socat: 5.2 Gbps, slirp4netns: 8.3 Gbps, RootlessKit: 27.3 Gbps
  (https://travis-ci.org/rootless-containers/rootlesskit/builds/597056377)

• Connections from the host are treated as 127.0.0.1 rather than 10.0.2.2 in the namespace.
  No UDP issue ([rootless] slirp4netns w/ --disable-host-loopback: host initiated udp communication container <-> host impossible #4586)

• No tcp_rmem issue (Rootless: slirp4netns slow initial input to container #4537)

• Probably works with IPv6. Even if not, it is trivial to support IPv6. (podman does not forward ipv6 ports #4311)

• Easily extensible for future support of SCTP

• Easily extensible for future support of lxc-user-nic SUID network

• Human-readable error messages (--publish error message is not clear #4804)

RootlessKit port forwarder has been already adopted as the default port forwarder by Rootless Docker/Moby, and no issue has been reported AFAIK w.r.t. TCP forwarding.

As the port forwarder is imported as a Go package, no rootlesskit binary is required for Podman.

Fix #4586
May-fix #4559
Fix #4537
May-fix #4311
Fix #4804

See https://github.com/rootless-containers/rootlesskit/tree/v0.7.1/pkg/port/builtin

Signed-off-by: Akihiro Suda [email protected]

[AkihiroSuda]

Is CI failure related?

[mheon]

Search tests? Probably a registry flake, I'll restart

[AkihiroSuda]

Reverted to WIP due to an UDP issue: rootless-containers/rootlesskit#86

[giuseppe]

I am impressed by the results, good work!

Is RootlessKit copying the connection from the listen socket on the host to the socket in the network namespace?

Is that something slirp4netns should also handle so that every slirp4netns user can benefit from?

[AkihiroSuda]

Is RootlessKit copying the connection from the listen socket on the host to the socket in the network namespace?

Only the FD of accept-ed socket in the parent is transferred across the namespaces.
Inside the child namespace, bidirectional io.Copy occurs for copying data across the FD and the actual port in the namespace.

Is that something slirp4netns should also handle so that every slirp4netns user can benefit from?

Not easy

[TomSweeneyRedHat]

I just restarted the failing test in hopes that it's a flake....

[AkihiroSuda]

UDP issue (rootless-containers/rootlesskit#86) was resolved in RootlessKit v0.7.1 (rootless-containers/rootlesskit#87).

Updated PR.

[AkihiroSuda]

Is CI passing or not?

[mheon]

Seems green to me

[mheon]

Hm. SPECIAL_TESTING_ROOTLESS failed, according to Cirrus

[AkihiroSuda]

Where is the failure log?

[mheon]

It should show up in Github automatically. I re-triggered the tests to hopefully fix things. If that doesn't work you might want to make a trivial change and force-push to retrigger CI

[rh-atomic-bot]

☔ The latest upstream changes (presumably #4730) made this pull request unmergeable. Please resolve the merge conflicts.

[AkihiroSuda]

reexec stuff doesn't seem working with podman run -d and hence CI is failing.
Anyone can help me?

[rhatdan]

@mheon @giuseppe PTAL

[giuseppe]

one thing to do is to inject the childQuit pipe into conmon, so that the process is not killed when podman exits:

diff --git a/libpod/container.go b/libpod/container.go
index 4f7fc067..d65fe4d1 100644
--- a/libpod/container.go
+++ b/libpod/container.go
@@ -133,6 +133,7 @@ type Container struct {
 
        rootlessSlirpSyncR *os.File
        rootlessSlirpSyncW *os.File
+       injectFDs []*os.File
 
        // A restored container should have the same IP address as before
        // being checkpointed. If requestedIP is set it will be used instead
diff --git a/libpod/networking_linux.go b/libpod/networking_linux.go
index df554d9b..83ba2f7b 100644
--- a/libpod/networking_linux.go
+++ b/libpod/networking_linux.go
@@ -342,9 +342,6 @@ func (r *Runtime) setupRootlessPortMapping(ctr *Container, netnsPath string) (er
        if err != nil {
                return err
        }
-       r.rootlessTeardown = append(r.rootlessTeardown, func() error {
-               return childQuitW.Close()
-       })
        // reexec the child process in the child netns
        cmd := exec.Command(fmt.Sprintf("/proc/%d/exe", os.Getpid()))
        cmd.Args = []string{reexecKeyRootlessPortChild}
@@ -362,6 +359,7 @@ func (r *Runtime) setupRootlessPortMapping(ctr *Container, netnsPath string) (er
        }); err != nil {
                return err
        }
        logrus.Debugf("rootless port: waiting for initComplete")
        // wait for the child to connect to the parent
        select {
@@ -374,6 +372,7 @@ func (r *Runtime) setupRootlessPortMapping(ctr *Container, netnsPath string) (er
                quit <- struct{}{}
                return nil
        })
+       ctr.injectFDs = append(ctr.injectFDs, childQuitW)
        // add ports
        ctx := context.TODO()
        for _, i := range ctr.config.PortMappings {
diff --git a/libpod/oci_conmon_linux.go b/libpod/oci_conmon_linux.go
index 026b1312..2a5dc65f 100644
--- a/libpod/oci_conmon_linux.go
+++ b/libpod/oci_conmon_linux.go
@@ -51,6 +51,7 @@ type ConmonOCIRuntime struct {
        supportsJSON      bool
        supportsNoCgroups bool
        sdNotify          bool
+       injectFiles       []*os.File
 }
 
 // Make a new Conmon-based OCI runtime with the given options.
@@ -949,6 +950,7 @@ func (r *ConmonOCIRuntime) createOCIContainer(ctr *Container, restoreOptions *Co
        cmd.Env = append(cmd.Env, conmonEnv...)
        cmd.ExtraFiles = append(cmd.ExtraFiles, childSyncPipe, childStartPipe)
        cmd.ExtraFiles = append(cmd.ExtraFiles, envFiles...)
+       cmd.ExtraFiles = append(cmd.ExtraFiles, ctr.injectFDs...)
 
        if r.reservePorts && !ctr.config.NetMode.IsSlirp4netns() {
                ports, err := bindPorts(ctr.config.PortMappings)

that is not enough though, as the RunParentDriver won't be running.

@AkihiroSuda could we move the RunParentDriver inside the new process? Something like passing down the open fd and let the new process do the handling?

[AkihiroSuda]

thanks, I'll look into that.

so that the process is not killed when podman exits:

How is this implemented to keep the process alive?
Couldn't find corresponding code in conmon.c .

[giuseppe]

How is this implemented to keep the process alive?
Couldn't find corresponding code in conmon.c .

the patch I've provided above does that. It passes a FD to conmon that will be kept open until conmon runs. When it exits, the FD passed down to conmon will be closed as well:

https://github.com/containers/conmon/blob/master/src/conmon.c#L1763-L1780

[AkihiroSuda]

Thanks, now it works with podman run -d

[giuseppe]

LGTM

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: AkihiroSuda, giuseppe

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [giuseppe]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[vrothberg]

/lgtm

[vrothberg]

@baude @mheon do we need to do a manual merge?