From cefb9446479cf5ff9801eba5111c4b661d38597a Mon Sep 17 00:00:00 2001 From: Lokesh Mandvekar Date: Thu, 25 Dec 2025 15:56:29 -0500 Subject: [PATCH] Remove slirp4netns setup functions (preserve RLK) Remove all slirp4netns-specific setup and helper functions while preserving the RootlessRLK port mapping functions that are still used by pasta and bridge networking. Rename networking_slirp4netns.go to networking_rootlessport.go. Convert the slirp4netns compose test to use pasta instead. Remove rootlessSlirpSyncR/W fields from container struct and getSlirp4netnsIP stub from FreeBSD networking. Signed-off-by: Lokesh Mandvekar --- libpod/container.go | 6 +- libpod/container_internal_common.go | 18 ++---- libpod/container_internal_linux.go | 28 ---------- libpod/networking_common.go | 3 +- libpod/networking_freebsd.go | 5 -- libpod/networking_linux.go | 1 - ...rp4netns.go => networking_rootlessport.go} | 55 +++---------------- libpod/oci_conmon_common.go | 36 +++++------- pkg/namespaces/namespaces.go | 9 +-- test/compose/pasta_opts/SKIP_ROOT | 1 + test/compose/pasta_opts/docker-compose.yml | 5 ++ .../{slirp4netns_opts => pasta_opts}/setup.sh | 0 .../teardown.sh | 0 .../{slirp4netns_opts => pasta_opts}/tests.sh | 0 .../slirp4netns_opts/docker-compose.yml | 5 -- test/compose/test-compose | 9 +++ 16 files changed, 43 insertions(+), 138 deletions(-) rename libpod/{networking_slirp4netns.go => networking_rootlessport.go} (59%) create mode 100644 test/compose/pasta_opts/SKIP_ROOT create mode 100644 test/compose/pasta_opts/docker-compose.yml rename test/compose/{slirp4netns_opts => pasta_opts}/setup.sh (100%) rename test/compose/{slirp4netns_opts => pasta_opts}/teardown.sh (100%) rename test/compose/{slirp4netns_opts => pasta_opts}/tests.sh (100%) delete mode 100644 test/compose/slirp4netns_opts/docker-compose.yml diff --git a/libpod/container.go b/libpod/container.go index 6e5894c983..19effa5bf3 100644 --- a/libpod/container.go +++ b/libpod/container.go @@ -108,9 +108,6 @@ type Container struct { runtime *Runtime ociRuntime OCIRuntime - rootlessSlirpSyncR *os.File - rootlessSlirpSyncW *os.File - rootlessPortSyncR *os.File rootlessPortSyncW *os.File @@ -127,8 +124,7 @@ type Container struct { // This is true if a container is restored from a checkpoint. restoreFromCheckpoint bool - slirp4netnsSubnet *net.IPNet - pastaResult *pasta.SetupResult + pastaResult *pasta.SetupResult } // ContainerState contains the current state of the container diff --git a/libpod/container_internal_common.go b/libpod/container_internal_common.go index 17d7d7d8ce..0d7da7e9e0 100644 --- a/libpod/container_internal_common.go +++ b/libpod/container_internal_common.go @@ -2287,7 +2287,7 @@ func (c *Container) checkForIPv6(netStatus map[string]types.StatusBlock) bool { return c.pastaResult.IPv6 } - return c.isSlirp4netnsIPv6() + return false } // Add a new nameserver to the container's resolv.conf, ensuring that it is the @@ -2338,29 +2338,22 @@ func getLocalhostHostEntry(c *Container) etchosts.HostEntries { } // getHostsEntries returns the container ip host entries for the correct netmode -func (c *Container) getHostsEntries() (etchosts.HostEntries, error) { +func (c *Container) getHostsEntries() etchosts.HostEntries { var entries etchosts.HostEntries names := []string{c.Hostname(), c.config.Name} switch { case c.config.NetMode.IsBridge(): entries = etchosts.GetNetworkHostEntries(c.state.NetworkStatus, names...) case c.config.NetMode.IsPasta(): - // this should never be the case but check just to be sure and not panic if len(c.pastaResult.IPAddresses) > 0 { entries = etchosts.HostEntries{{IP: c.pastaResult.IPAddresses[0].String(), Names: names}} } - case c.config.NetMode.IsSlirp4netns(): - ip, err := getSlirp4netnsIP(c.slirp4netnsSubnet) - if err != nil { - return nil, err - } - entries = etchosts.HostEntries{{IP: ip.String(), Names: names}} default: if c.hasNetNone() { entries = etchosts.HostEntries{{IP: "127.0.0.1", Names: names}} } } - return entries, nil + return entries } func (c *Container) createHostsFile() error { @@ -2379,10 +2372,7 @@ func (c *Container) addHosts() error { // no host file nothing to do return nil } - containerIPsEntries, err := c.getHostsEntries() - if err != nil { - return fmt.Errorf("failed to get container ip host entries: %w", err) - } + containerIPsEntries := c.getHostsEntries() // Consider container level BaseHostsFile configuration first. // If it is empty, fallback to containers.conf level configuration. diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index f4bd473483..a49986afbc 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -22,7 +22,6 @@ import ( "github.com/opencontainers/runtime-tools/generate" "github.com/opencontainers/selinux/go-selinux/label" "github.com/sirupsen/logrus" - "go.podman.io/common/libnetwork/slirp4netns" "go.podman.io/common/libnetwork/types" "go.podman.io/common/pkg/cgroups" "go.podman.io/common/pkg/config" @@ -567,38 +566,11 @@ func (c *Container) addSpecialDNS(nameservers []string) []string { } case c.pastaResult != nil: nameservers = append(nameservers, c.pastaResult.DNSForwardIPs...) - case c.config.NetMode.IsSlirp4netns(): - // slirp4netns has a built in DNS forwarder. - slirp4netnsDNS, err := slirp4netns.GetDNS(c.slirp4netnsSubnet) - if err != nil { - logrus.Warn("Failed to determine Slirp4netns DNS: ", err.Error()) - } else { - nameservers = append(nameservers, slirp4netnsDNS.String()) - } } return nameservers } func (c *Container) isSlirp4netnsIPv6() bool { - if c.config.NetMode.IsSlirp4netns() { - extraOptions := c.config.NetworkOptions[slirp4netns.BinaryName] - options := make([]string, 0, len(c.runtime.config.Engine.NetworkCmdOptions.Get())+len(extraOptions)) - options = append(options, c.runtime.config.Engine.NetworkCmdOptions.Get()...) - options = append(options, extraOptions...) - - // loop backwards as the last argument wins and we can exit early - // This should be kept in sync with c/common/libnetwork/slirp4netns. - for i := len(options) - 1; i >= 0; i-- { - switch options[i] { - case "enable_ipv6=true": - return true - case "enable_ipv6=false": - return false - } - } - // default is true - return true - } return false } diff --git a/libpod/networking_common.go b/libpod/networking_common.go index 7ce388b74c..f77cefc5aa 100644 --- a/libpod/networking_common.go +++ b/libpod/networking_common.go @@ -110,8 +110,7 @@ func (r *Runtime) teardownNetwork(ctr *Container) error { return err } - if !ctr.config.NetMode.IsSlirp4netns() && - !ctr.config.NetMode.IsPasta() && len(networks) > 0 { + if !ctr.config.NetMode.IsPasta() && len(networks) > 0 { netOpts := ctr.getNetworkOptions(networks) return r.teardownNetworkBackend(ctr.state.NetNS, netOpts) } diff --git a/libpod/networking_freebsd.go b/libpod/networking_freebsd.go index 3000ff25ce..ff4ce8f811 100644 --- a/libpod/networking_freebsd.go +++ b/libpod/networking_freebsd.go @@ -7,7 +7,6 @@ import ( jdec "encoding/json" "errors" "fmt" - "net" "os/exec" "github.com/containers/buildah/pkg/jail" @@ -44,10 +43,6 @@ type NetstatAddress struct { Collisions uint64 `json:"collisions"` } -func getSlirp4netnsIP(_ *net.IPNet) (*net.IP, error) { - return nil, errors.New("not implemented GetSlirp4netnsIP") -} - // This is called after the container's jail is created but before its // started. We can use this to initialise the container's vnet when we don't // have a separate vnet jail (which is the case in FreeBSD 13.3 and later). diff --git a/libpod/networking_linux.go b/libpod/networking_linux.go index 71b9933bcf..b8f59aca32 100644 --- a/libpod/networking_linux.go +++ b/libpod/networking_linux.go @@ -64,7 +64,6 @@ func (r *Runtime) configureNetNS(ctr *Container, ctrNS string) (status map[strin // not set up port because they are still active if rootless.IsRootless() && len(ctr.config.PortMappings) > 0 && ctr.getNetworkStatus() == nil { // set up port forwarder for rootless netns - // TODO: support slirp4netns port forwarder as well // make sure to fix this in container.handleRestartPolicy() as well // Important we have to call this after r.setUpNetwork() so that // we can use the proper netStatus diff --git a/libpod/networking_slirp4netns.go b/libpod/networking_rootlessport.go similarity index 59% rename from libpod/networking_slirp4netns.go rename to libpod/networking_rootlessport.go index 0b2243f6a5..9a4abd1507 100644 --- a/libpod/networking_slirp4netns.go +++ b/libpod/networking_rootlessport.go @@ -5,63 +5,28 @@ package libpod import ( "fmt" "io" - "net" "os" "path/filepath" "github.com/containers/podman/v6/pkg/errorhandling" "github.com/sirupsen/logrus" - "go.podman.io/common/libnetwork/slirp4netns" + "go.podman.io/common/libnetwork/slirp4netns" // RootlessKit port mapping only, not the removed slirp4netns backend "go.podman.io/common/libnetwork/types" ) -// setupSlirp4netns can be called in rootful as well as in rootless -func (r *Runtime) setupSlirp4netns(ctr *Container, netns string) error { - ports := ctr.convertPortMappings() - - if !ctr.config.PostConfigureNetNS { - var err error - ctr.rootlessSlirpSyncR, ctr.rootlessSlirpSyncW, err = os.Pipe() - if err != nil { - return fmt.Errorf("failed to create rootless network sync pipe: %w", err) - } - if len(ports) > 0 { - ctr.rootlessPortSyncR, ctr.rootlessPortSyncW, err = os.Pipe() - if err != nil { - return fmt.Errorf("failed to create rootless port sync pipe: %w", err) - } - } - } - defer errorhandling.CloseQuiet(ctr.rootlessSlirpSyncR) - if ctr.rootlessPortSyncR != nil { - defer errorhandling.CloseQuiet(ctr.rootlessPortSyncR) - } - - res, err := slirp4netns.Setup(&slirp4netns.SetupOptions{ - Config: r.config, - ContainerID: ctr.ID(), - Netns: netns, - Ports: ports, - ExtraOptions: ctr.config.NetworkOptions[slirp4netns.BinaryName], - Slirp4netnsExitPipeR: ctr.rootlessSlirpSyncR, - RootlessPortExitPipeR: ctr.rootlessPortSyncR, - }) - if err != nil { - return err - } - ctr.slirp4netnsSubnet = res.Subnet - return nil -} - func (r *Runtime) setupRootlessPortMappingViaRLK(ctr *Container, netnsPath string, netStatus map[string]types.StatusBlock) error { - var err error - if !ctr.config.PostConfigureNetNS { + // Only create pipes if they don't exist yet + if ctr.rootlessPortSyncR == nil { + var err error ctr.rootlessPortSyncR, ctr.rootlessPortSyncW, err = os.Pipe() if err != nil { return fmt.Errorf("failed to create rootless port sync pipe: %w", err) } } - defer errorhandling.CloseQuiet(ctr.rootlessPortSyncR) + // Only defer close if not in PostConfigureNetNS mode to avoid double-close + if !ctr.config.PostConfigureNetNS { + defer errorhandling.CloseQuiet(ctr.rootlessPortSyncR) + } return slirp4netns.SetupRootlessPortMappingViaRLK(&slirp4netns.SetupOptions{ Config: r.config, ContainerID: ctr.ID(), @@ -100,7 +65,3 @@ func (c *Container) reloadRootlessRLKPortMapping() error { } return nil } - -func getSlirp4netnsIP(subnet *net.IPNet) (*net.IP, error) { - return slirp4netns.GetIP(subnet) -} diff --git a/libpod/oci_conmon_common.go b/libpod/oci_conmon_common.go index 1599190fe1..4b4b680a2e 100644 --- a/libpod/oci_conmon_common.go +++ b/libpod/oci_conmon_common.go @@ -1187,37 +1187,27 @@ func (r *ConmonOCIRuntime) createOCIContainer(ctr *Container, restoreOptions *Co // by the container and conmon will keep the ports busy so that another // process cannot use them. cmd.ExtraFiles = append(cmd.ExtraFiles, ports...) + + // For rootless port forwarding, create sync pipe and leak write end to conmon + if rootless.IsRootless() && len(ctr.config.PortMappings) > 0 { + ctr.rootlessPortSyncR, ctr.rootlessPortSyncW, err = os.Pipe() + if err != nil { + return 0, fmt.Errorf("failed to create rootless port sync pipe: %w", err) + } + defer errorhandling.CloseQuiet(ctr.rootlessPortSyncW) + // Leak one end in conmon, the other one will be used by rootlessport + cmd.ExtraFiles = append(cmd.ExtraFiles, ctr.rootlessPortSyncW) + } } else { // ports were bound in ctr.prepare() as we must do it before the netns setup filesToClose = append(filesToClose, ctr.reservedPorts...) cmd.ExtraFiles = append(cmd.ExtraFiles, ctr.reservedPorts...) ctr.reservedPorts = nil - } - - if ctr.config.NetMode.IsSlirp4netns() || rootless.IsRootless() { - if ctr.config.PostConfigureNetNS { - havePortMapping := len(ctr.config.PortMappings) > 0 - if havePortMapping { - ctr.rootlessPortSyncR, ctr.rootlessPortSyncW, err = os.Pipe() - if err != nil { - return 0, fmt.Errorf("failed to create rootless port sync pipe: %w", err) - } - } - ctr.rootlessSlirpSyncR, ctr.rootlessSlirpSyncW, err = os.Pipe() - if err != nil { - return 0, fmt.Errorf("failed to create rootless network sync pipe: %w", err) - } - } - - if ctr.rootlessSlirpSyncW != nil { - defer errorhandling.CloseQuiet(ctr.rootlessSlirpSyncW) - // Leak one end in conmon, the other one will be leaked into slirp4netns - cmd.ExtraFiles = append(cmd.ExtraFiles, ctr.rootlessSlirpSyncW) - } + // For rootless port forwarding, leak write end to conmon + // The pipes were created in setupRootlessPortMappingViaRLK() during network setup if ctr.rootlessPortSyncW != nil { defer errorhandling.CloseQuiet(ctr.rootlessPortSyncW) - // Leak one end in conmon, the other one will be leaked into rootlessport cmd.ExtraFiles = append(cmd.ExtraFiles, ctr.rootlessPortSyncW) } } diff --git a/pkg/namespaces/namespaces.go b/pkg/namespaces/namespaces.go index 0c1efd7c68..8407218e68 100644 --- a/pkg/namespaces/namespaces.go +++ b/pkg/namespaces/namespaces.go @@ -201,13 +201,6 @@ func (n NetworkMode) IsBridge() bool { return n == bridgeType } -// IsSlirp4netns indicates if we are running a rootless network stack -// Deprecated: slirp4netns is no longer supported, use IsPasta instead -func (n NetworkMode) IsSlirp4netns() bool { - // Check for slirp4netns mode (deprecated, will be auto-migrated to pasta) - return n == "slirp4netns" || strings.HasPrefix(string(n), "slirp4netns:") -} - // IsPasta indicates if we are running a rootless network stack using pasta func (n NetworkMode) IsPasta() bool { return n == pastaType || strings.HasPrefix(string(n), pastaType+":") @@ -231,5 +224,5 @@ func (n NetworkMode) IsPod() bool { // IsUserDefined indicates user-created network func (n NetworkMode) IsUserDefined() bool { - return !n.IsDefault() && !n.IsBridge() && !n.IsHost() && !n.IsNone() && !n.IsContainer() && !n.IsSlirp4netns() && !n.IsPasta() && !n.IsNS() + return !n.IsDefault() && !n.IsBridge() && !n.IsHost() && !n.IsNone() && !n.IsContainer() && !n.IsPasta() && !n.IsNS() } diff --git a/test/compose/pasta_opts/SKIP_ROOT b/test/compose/pasta_opts/SKIP_ROOT new file mode 100644 index 0000000000..994bf93254 --- /dev/null +++ b/test/compose/pasta_opts/SKIP_ROOT @@ -0,0 +1 @@ +pasta networking is only supported for rootless mode diff --git a/test/compose/pasta_opts/docker-compose.yml b/test/compose/pasta_opts/docker-compose.yml new file mode 100644 index 0000000000..9af3fafdeb --- /dev/null +++ b/test/compose/pasta_opts/docker-compose.yml @@ -0,0 +1,5 @@ +services: + alpine: + image: alpine + network_mode: "pasta:--ipv4-only,-a,10.0.2.0,-n,24,-g,10.0.2.2,--dns-forward,10.0.2.3,--mtu,1280,--no-ndp,--no-dhcpv6,--no-dhcp" + command: sh -c "echo teststring | nc 10.0.2.2 5001" diff --git a/test/compose/slirp4netns_opts/setup.sh b/test/compose/pasta_opts/setup.sh similarity index 100% rename from test/compose/slirp4netns_opts/setup.sh rename to test/compose/pasta_opts/setup.sh diff --git a/test/compose/slirp4netns_opts/teardown.sh b/test/compose/pasta_opts/teardown.sh similarity index 100% rename from test/compose/slirp4netns_opts/teardown.sh rename to test/compose/pasta_opts/teardown.sh diff --git a/test/compose/slirp4netns_opts/tests.sh b/test/compose/pasta_opts/tests.sh similarity index 100% rename from test/compose/slirp4netns_opts/tests.sh rename to test/compose/pasta_opts/tests.sh diff --git a/test/compose/slirp4netns_opts/docker-compose.yml b/test/compose/slirp4netns_opts/docker-compose.yml deleted file mode 100644 index dcdcae04c3..0000000000 --- a/test/compose/slirp4netns_opts/docker-compose.yml +++ /dev/null @@ -1,5 +0,0 @@ -services: - alpine: - image: alpine - network_mode: "slirp4netns:allow_host_loopback=true" - command: sh -c "echo teststring | nc 10.0.2.2 5001" diff --git a/test/compose/test-compose b/test/compose/test-compose index 07e28d4de6..ba956e0096 100755 --- a/test/compose/test-compose +++ b/test/compose/test-compose @@ -334,6 +334,15 @@ for t in "${tests_to_run[@]}"; do continue fi + if ! is_rootless && [ -e $testdir/SKIP_ROOT ]; then + reason="$(<$testdir/SKIP_ROOT)" + if [ -n "$reason" ]; then + reason=" - $reason" + fi + _show_ok skip "$testname # skip$reason" + continue + fi + start_service logfile=$WORKDIR/$testname.log