Merge pull request #20837 from dfr/freebsd-vnet

libpod: Allow using just one jail per container on FreeBSD

libpod/container_internal_freebsd.go

@@ -194,15 +194,18 @@ func openDirectory(path string) (fd int, err error) {
 
 func (c *Container) addNetworkNamespace(g *generate.Generator) error {
 	if c.config.CreateNetNS {
-		if c.state.NetNS == "" {
-			// This should not happen since network setup
-			// errors should be propagated correctly from
-			// (*Runtime).createNetNS. Check for it anyway
-			// since it caused nil pointer dereferences in
-			// the past (see #16333).
-			return fmt.Errorf("Inconsistent state: c.config.CreateNetNS is set but c.state.NetNS is nil")
+		// If PostConfigureNetNS is set (which is true on FreeBSD 13.3
+		// and later), we can manage a container's network settings
+		// without an extra parent jail to own the vnew.
+		//
+		// In this case, the OCI runtime creates a new vnet for the
+		// container jail, otherwise it creates the container jail as a
+		// child of the jail owning the vnet.
+		if c.config.PostConfigureNetNS {
+			g.AddAnnotation("org.freebsd.jail.vnet", "new")
+		} else {
+			g.AddAnnotation("org.freebsd.parentJail", c.state.NetNS)
 		}
-		g.AddAnnotation("org.freebsd.parentJail", c.state.NetNS)
 	}
 	return nil
 }

libpod/networking_freebsd.go

@@ -109,10 +109,14 @@ func getSlirp4netnsIP(subnet *net.IPNet) (*net.IP, error) {
 	return nil, errors.New("not implemented GetSlirp4netnsIP")
 }
 
-// While there is code in container_internal.go which calls this, in
-// my testing network creation always seems to go through createNetNS.
+// 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).
 func (r *Runtime) setupNetNS(ctr *Container) error {
-	return errors.New("not implemented (*Runtime) setupNetNS")
+	networkStatus, err := r.configureNetNS(ctr, ctr.ID())
+	ctr.state.NetNS = ctr.ID()
+	ctr.state.NetworkStatus = networkStatus
+	return err
 }
 
 // Create and configure a new network namespace for a container
@@ -197,22 +201,24 @@ func (r *Runtime) teardownNetNS(ctr *Container) error {
 	}
 
 	if ctr.state.NetNS != "" {
-		// Rather than destroying the jail immediately, reset the
-		// persist flag so that it will live until the container is
-		// done.
-		netjail, err := jail.FindByName(ctr.state.NetNS)
-		if err != nil {
-			return fmt.Errorf("finding network jail %s: %w", ctr.state.NetNS, err)
-		}
-		jconf := jail.NewConfig()
-		jconf.Set("persist", false)
-		if err := netjail.Set(jconf); err != nil {
-			return fmt.Errorf("releasing network jail %s: %w", ctr.state.NetNS, err)
+		// If PostConfigureNetNS is false, then we are running with a
+		// separate vnet jail so we need to clean that up now.
+		if !ctr.config.PostConfigureNetNS {
+			// Rather than destroying the jail immediately, reset the
+			// persist flag so that it will live until the container is
+			// done.
+			netjail, err := jail.FindByName(ctr.state.NetNS)
+			if err != nil {
+				return fmt.Errorf("finding network jail %s: %w", ctr.state.NetNS, err)
+			}
+			jconf := jail.NewConfig()
+			jconf.Set("persist", false)
+			if err := netjail.Set(jconf); err != nil {
+				return fmt.Errorf("releasing network jail %s: %w", ctr.state.NetNS, err)
+			}
 		}
-
 		ctr.state.NetNS = ""
 	}
-
 	return nil
 }
 
@@ -226,10 +232,18 @@ func getContainerNetIO(ctr *Container) (*LinkStatistics64, error) {
 		return nil, nil
 	}
 
-	cmd := exec.Command("jexec", ctr.state.NetNS, "netstat", "-bi", "--libxo", "json")
+	// First try running 'netstat -j' - this lets us retrieve stats from
+	// containers which don't have a separate vnet jail.
+	cmd := exec.Command("netstat", "-j", ctr.state.NetNS, "-bi", "--libxo", "json")
 	out, err := cmd.Output()
 	if err != nil {
-		return nil, err
+		// Fall back to using jexec so that this still works on 13.2
+		// which does not have the -j flag.
+		cmd := exec.Command("jexec", ctr.state.NetNS, "netstat", "-bi", "--libxo", "json")
+		out, err = cmd.Output()
+	}
+	if err != nil {
+		return nil, fmt.Errorf("failed to read network stats: %v", err)
 	}
 	stats := Netstat{}
 	if err := jdec.Unmarshal(out, &stats); err != nil {

pkg/specgen/generate/namespaces.go

@@ -296,7 +296,7 @@ func namespaceOptions(s *specgen.SpecGenerator, rt *libpod.Runtime, pod *libpod.
 		toReturn = append(toReturn, libpod.WithCgroupsMode(s.CgroupsMode))
 	}
 
-	postConfigureNetNS := !s.UserNS.IsHost()
+	postConfigureNetNS := needPostConfigureNetNS(s)
 
 	switch s.NetNS.NSMode {
 	case specgen.FromPod:

pkg/specgen/generate/namespaces_freebsd.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"os"
 
+	"github.com/containers/buildah/pkg/jail"
 	"github.com/containers/podman/v4/libpod"
 	"github.com/containers/podman/v4/pkg/specgen"
 	"github.com/opencontainers/runtime-tools/generate"
@@ -52,3 +53,10 @@ func specConfigureNamespaces(s *specgen.SpecGenerator, g *generate.Generator, rt
 
 	return nil
 }
+
+// On FreeBSD 13.3 and later, we can avoid creating a separate vnet jail but
+// only if we can initialise the network after the OCI container is created -
+// the OCI container will own the vnet in this case.
+func needPostConfigureNetNS(s *specgen.SpecGenerator) bool {
+	return jail.NeedVnetJail() == false
+}

pkg/specgen/generate/namespaces_linux.go

@@ -159,3 +159,7 @@ func specConfigureNamespaces(s *specgen.SpecGenerator, g *generate.Generator, rt
 
 	return nil
 }
+
+func needPostConfigureNetNS(s *specgen.SpecGenerator) bool {
+	return !s.UserNS.IsHost()
+}