From d4ac2f3dd52b8962f238fcb48ab5b00d8dd3aab1 Mon Sep 17 00:00:00 2001 From: Doug Rabson Date: Wed, 29 Nov 2023 16:07:03 +0000 Subject: [PATCH] libpod: Allow using just one jail per container on FreeBSD In FreeBSD-14.0, it is possible to configure a jail's network settings from outside the jail using ifconfig and route's new '-j' option. This removes the need for a separate jail to own the container's vnet. [NO NEW TESTS NEEDED] Signed-off-by: Doug Rabson --- libpod/container_internal_freebsd.go | 19 ++++---- libpod/networking_freebsd.go | 50 ++++++++++++++-------- pkg/specgen/generate/namespaces.go | 2 +- pkg/specgen/generate/namespaces_freebsd.go | 8 ++++ pkg/specgen/generate/namespaces_linux.go | 4 ++ 5 files changed, 56 insertions(+), 27 deletions(-) diff --git a/libpod/container_internal_freebsd.go b/libpod/container_internal_freebsd.go index 2ecb931f2d..6ad8dd853e 100644 --- a/libpod/container_internal_freebsd.go +++ b/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 } diff --git a/libpod/networking_freebsd.go b/libpod/networking_freebsd.go index 75cb4fc80d..a81551f6ec 100644 --- a/libpod/networking_freebsd.go +++ b/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 { diff --git a/pkg/specgen/generate/namespaces.go b/pkg/specgen/generate/namespaces.go index f8c827c626..c151738d1c 100644 --- a/pkg/specgen/generate/namespaces.go +++ b/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: diff --git a/pkg/specgen/generate/namespaces_freebsd.go b/pkg/specgen/generate/namespaces_freebsd.go index 4fb6a4c51d..40ac22964d 100644 --- a/pkg/specgen/generate/namespaces_freebsd.go +++ b/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 +} diff --git a/pkg/specgen/generate/namespaces_linux.go b/pkg/specgen/generate/namespaces_linux.go index 1ff539ac4a..265937e0ba 100644 --- a/pkg/specgen/generate/namespaces_linux.go +++ b/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() +}