From a5e93550c1d3b65fd74875cd20a41029b7e045d7 Mon Sep 17 00:00:00 2001 From: Ayush Ranjan Date: Fri, 10 Nov 2023 14:13:03 -0800 Subject: [PATCH] Move GPU device ownership to gofer process. Tested on a T4 GPU with driver version 525.60.13: ``` $ docker run --runtime=runsc --gpus=all --rm -it nvcr.io/nvidia/k8s/cuda-sample:vectoradd-cuda11.7.1-ubi8 [Vector addition of 50000 elements] Copy input data from the host memory to the CUDA device CUDA kernel launch with 196 blocks of 256 threads Copy output data from the CUDA device to the host memory Test PASSED Done ``` Also tested this on GKE with the same vectoradd workload. Checked that the device gofer connection is actually being closed when the container is deleted. Something to note is that the gofer logs for the GPU-container sometimes end abruptly (the "All lisafs servers exited." line does not print). This is because runsc/container/container.go:stop() SIGKILLs the gofer before it can cleanup naturally. The device gofer connection is only closed at the end of Loader.destroySubcontainer(), which gives little time before the gofer is SIGKILL-ed. PiperOrigin-RevId: 581365665 --- pkg/devutil/BUILD | 2 + pkg/devutil/devutil.go | 58 ++++++++++++++ pkg/sentry/devices/nvproxy/BUILD | 1 + pkg/sentry/devices/nvproxy/frontend.go | 20 +++-- pkg/sentry/devices/nvproxy/nvproxy.go | 8 +- pkg/sentry/devices/nvproxy/nvproxy_unsafe.go | 3 +- pkg/sentry/devices/nvproxy/seccomp_filters.go | 10 --- pkg/sentry/devices/nvproxy/uvm.go | 8 +- runsc/boot/BUILD | 2 +- runsc/boot/loader.go | 34 ++++---- runsc/boot/nvidia.go | 52 ------------ runsc/boot/vfs.go | 26 +++++- runsc/cmd/boot.go | 51 ++++++------ runsc/cmd/chroot.go | 28 +------ runsc/cmd/gofer.go | 36 ++++++++- runsc/container/container.go | 23 +----- runsc/sandbox/BUILD | 1 + runsc/sandbox/sandbox.go | 18 ++--- runsc/specutils/nvidia.go | 80 ++++++------------- 19 files changed, 222 insertions(+), 239 deletions(-) delete mode 100644 runsc/boot/nvidia.go diff --git a/pkg/devutil/BUILD b/pkg/devutil/BUILD index f8daac557..05909970e 100644 --- a/pkg/devutil/BUILD +++ b/pkg/devutil/BUILD @@ -13,7 +13,9 @@ go_library( visibility = ["//visibility:public"], deps = [ "//pkg/context", + "//pkg/fsutil", "//pkg/lisafs", + "//pkg/log", "//pkg/unet", "@org_golang_x_sys//unix:go_default_library", ], diff --git a/pkg/devutil/devutil.go b/pkg/devutil/devutil.go index b0d969783..85f1294fa 100644 --- a/pkg/devutil/devutil.go +++ b/pkg/devutil/devutil.go @@ -16,9 +16,13 @@ package devutil import ( + "fmt" + "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/context" + "gvisor.dev/gvisor/pkg/fsutil" "gvisor.dev/gvisor/pkg/lisafs" + "gvisor.dev/gvisor/pkg/log" "gvisor.dev/gvisor/pkg/unet" ) @@ -58,3 +62,57 @@ func (g *GoferClient) Close() { _ = unix.Close(g.hostFD) } } + +// DirentNames returns names of all the dirents for /dev on the gofer. +func (g *GoferClient) DirentNames(ctx context.Context) ([]string, error) { + if g.hostFD >= 0 { + return fsutil.DirentNames(g.hostFD) + } + client := g.clientFD.Client() + openFDID, _, err := g.clientFD.OpenAt(ctx, unix.O_RDONLY) + if err != nil { + return nil, fmt.Errorf("failed to open dev from gofer: %v", err) + } + defer client.CloseFD(ctx, openFDID, true /* flush */) + openFD := client.NewFD(openFDID) + const count = int32(64 * 1024) + var names []string + for { + dirents, err := openFD.Getdents64(ctx, count) + if err != nil { + return nil, fmt.Errorf("Getdents64 RPC failed: %v", err) + } + if len(dirents) == 0 { + break + } + for i := range dirents { + names = append(names, string(dirents[i].Name)) + } + } + return names, nil +} + +// OpenAt opens the device file at /dev/{name} on the gofer. +func (g *GoferClient) OpenAt(ctx context.Context, name string, flags uint32) (int, error) { + flags &= unix.O_ACCMODE + if g.hostFD >= 0 { + return unix.Openat(g.hostFD, name, int(flags|unix.O_NOFOLLOW), 0) + } + childInode, err := g.clientFD.Walk(ctx, name) + if err != nil { + log.Infof("failed to walk %q from dev gofer FD", name) + return 0, err + } + client := g.clientFD.Client() + childFD := client.NewFD(childInode.ControlFD) + + childOpenFD, childHostFD, err := childFD.OpenAt(ctx, flags) + if err != nil { + log.Infof("failed to open %q from child FD", name) + client.CloseFD(ctx, childFD.ID(), true /* flush */) + return 0, err + } + client.CloseFD(ctx, childFD.ID(), false /* flush */) + client.CloseFD(ctx, childOpenFD, true /* flush */) + return childHostFD, nil +} diff --git a/pkg/sentry/devices/nvproxy/BUILD b/pkg/sentry/devices/nvproxy/BUILD index 3a3e3ab22..e7324e538 100644 --- a/pkg/sentry/devices/nvproxy/BUILD +++ b/pkg/sentry/devices/nvproxy/BUILD @@ -37,6 +37,7 @@ go_library( "//pkg/abi/nvgpu", "//pkg/cleanup", "//pkg/context", + "//pkg/devutil", "//pkg/errors/linuxerr", "//pkg/fdnotifier", "//pkg/hostarch", diff --git a/pkg/sentry/devices/nvproxy/frontend.go b/pkg/sentry/devices/nvproxy/frontend.go index c52ac32ac..2bd4f06c4 100644 --- a/pkg/sentry/devices/nvproxy/frontend.go +++ b/pkg/sentry/devices/nvproxy/frontend.go @@ -23,6 +23,7 @@ import ( "gvisor.dev/gvisor/pkg/abi/nvgpu" "gvisor.dev/gvisor/pkg/cleanup" "gvisor.dev/gvisor/pkg/context" + "gvisor.dev/gvisor/pkg/devutil" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/fdnotifier" "gvisor.dev/gvisor/pkg/hostarch" @@ -46,15 +47,20 @@ type frontendDevice struct { // Open implements vfs.Device.Open. func (dev *frontendDevice) Open(ctx context.Context, mnt *vfs.Mount, vfsd *vfs.Dentry, opts vfs.OpenOptions) (*vfs.FileDescription, error) { - var hostPath string - if dev.minor == nvgpu.NV_CONTROL_DEVICE_MINOR { - hostPath = "/dev/nvidiactl" - } else { - hostPath = fmt.Sprintf("/dev/nvidia%d", dev.minor) + devClient := devutil.GoferClientFromContext(ctx) + if devClient == nil { + log.Warningf("devutil.CtxDevGoferClient is not set") + return nil, linuxerr.ENOENT } - hostFD, err := unix.Openat(-1, hostPath, int((opts.Flags&unix.O_ACCMODE)|unix.O_NOFOLLOW), 0) + var devName string + if dev.minor == nvgpu.NV_CONTROL_DEVICE_MINOR { + devName = "nvidiactl" + } else { + devName = fmt.Sprintf("nvidia%d", dev.minor) + } + hostFD, err := devClient.OpenAt(ctx, devName, opts.Flags) if err != nil { - ctx.Warningf("nvproxy: failed to open host %s: %v", hostPath, err) + ctx.Warningf("nvproxy: failed to open host %s: %v", devName, err) return nil, err } fd := &frontendFD{ diff --git a/pkg/sentry/devices/nvproxy/nvproxy.go b/pkg/sentry/devices/nvproxy/nvproxy.go index 7f7ad6dcb..e580c26b1 100644 --- a/pkg/sentry/devices/nvproxy/nvproxy.go +++ b/pkg/sentry/devices/nvproxy/nvproxy.go @@ -31,14 +31,10 @@ import ( ) // Register registers all devices implemented by this package in vfsObj. -func Register(vfsObj *vfs.VirtualFilesystem, uvmDevMajor uint32) error { +func Register(vfsObj *vfs.VirtualFilesystem, versionStr string, uvmDevMajor uint32) error { // The kernel driver's interface is unstable, so only allow versions of the // driver that are known to be supported. - versionStr, err := hostDriverVersion() - if err != nil { - return fmt.Errorf("failed to get Nvidia driver version: %w", err) - } - log.Debugf("NVIDIA driver version: %s", versionStr) + log.Infof("NVIDIA driver version: %s", versionStr) version, err := DriverVersionFrom(versionStr) if err != nil { return fmt.Errorf("failed to parse Nvidia driver version %s: %w", versionStr, err) diff --git a/pkg/sentry/devices/nvproxy/nvproxy_unsafe.go b/pkg/sentry/devices/nvproxy/nvproxy_unsafe.go index 18d00ace0..ba35bfff2 100644 --- a/pkg/sentry/devices/nvproxy/nvproxy_unsafe.go +++ b/pkg/sentry/devices/nvproxy/nvproxy_unsafe.go @@ -23,7 +23,8 @@ import ( "gvisor.dev/gvisor/pkg/abi/nvgpu" ) -func hostDriverVersion() (string, error) { +// HostDriverVersion returns the version of the host Nvidia driver. +func HostDriverVersion() (string, error) { ctlFD, err := unix.Openat(-1, "/dev/nvidiactl", unix.O_RDONLY|unix.O_NOFOLLOW, 0) if err != nil { return "", fmt.Errorf("failed to open /dev/nvidiactl: %w", err) diff --git a/pkg/sentry/devices/nvproxy/seccomp_filters.go b/pkg/sentry/devices/nvproxy/seccomp_filters.go index 88eb40dd9..0d5cfa880 100644 --- a/pkg/sentry/devices/nvproxy/seccomp_filters.go +++ b/pkg/sentry/devices/nvproxy/seccomp_filters.go @@ -25,16 +25,6 @@ import ( func Filters() seccomp.SyscallRules { notIocSizeMask := ^(((uintptr(1) << linux.IOC_SIZEBITS) - 1) << linux.IOC_SIZESHIFT) // for ioctls taking arbitrary size return seccomp.MakeSyscallRules(map[uintptr]seccomp.SyscallRule{ - unix.SYS_OPENAT: seccomp.PerArg{ - // All paths that we openat() are absolute, so we pass a dirfd - // of -1 (which is invalid for relative paths, but ignored for - // absolute paths) to hedge against bugs involving AT_FDCWD or - // real dirfds. - seccomp.EqualTo(^uintptr(0)), - seccomp.AnyValue{}, - seccomp.MaskedEqual(unix.O_NOFOLLOW|unix.O_CREAT, unix.O_NOFOLLOW), - seccomp.AnyValue{}, - }, unix.SYS_IOCTL: seccomp.Or{ seccomp.PerArg{ seccomp.NonNegativeFD{}, diff --git a/pkg/sentry/devices/nvproxy/uvm.go b/pkg/sentry/devices/nvproxy/uvm.go index 1b15fde9e..e46e2546d 100644 --- a/pkg/sentry/devices/nvproxy/uvm.go +++ b/pkg/sentry/devices/nvproxy/uvm.go @@ -20,6 +20,7 @@ import ( "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/abi/nvgpu" "gvisor.dev/gvisor/pkg/context" + "gvisor.dev/gvisor/pkg/devutil" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/fdnotifier" "gvisor.dev/gvisor/pkg/hostarch" @@ -41,7 +42,12 @@ type uvmDevice struct { // Open implements vfs.Device.Open. func (dev *uvmDevice) Open(ctx context.Context, mnt *vfs.Mount, vfsd *vfs.Dentry, opts vfs.OpenOptions) (*vfs.FileDescription, error) { - hostFD, err := unix.Openat(-1, "/dev/nvidia-uvm", int((opts.Flags&unix.O_ACCMODE)|unix.O_NOFOLLOW), 0) + devClient := devutil.GoferClientFromContext(ctx) + if devClient == nil { + log.Warningf("devutil.CtxDevGoferClient is not set") + return nil, linuxerr.ENOENT + } + hostFD, err := devClient.OpenAt(ctx, "nvidia-uvm", opts.Flags) if err != nil { ctx.Warningf("nvproxy: failed to open host /dev/nvidia-uvm: %v", err) return nil, err diff --git a/runsc/boot/BUILD b/runsc/boot/BUILD index 97eb48b39..162fa952b 100644 --- a/runsc/boot/BUILD +++ b/runsc/boot/BUILD @@ -19,7 +19,6 @@ go_library( "loader.go", "mount_hints.go", "network.go", - "nvidia.go", "seccheck.go", "strace.go", "vfs.go", @@ -40,6 +39,7 @@ go_library( "//pkg/control/server", "//pkg/coverage", "//pkg/cpuid", + "//pkg/devutil", "//pkg/errors/linuxerr", "//pkg/eventchannel", "//pkg/fd", diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go index d3c64b6f0..f61fa4eec 100644 --- a/runsc/boot/loader.go +++ b/runsc/boot/loader.go @@ -128,9 +128,8 @@ type containerInfo struct { // nvidiaUVMDevMajor is the device major number used for nvidia-uvm. nvidiaUVMDevMajor uint32 - // nvidiaDevMinors is a list of device minors for Nvidia GPU devices exposed - // to the sandbox. - nvidiaDevMinors NvidiaDevMinors + // nvidiaDriverVersion is the Nvidia driver version on the host. + nvidiaDriverVersion string } // Loader keeps state needed to start the kernel and run the container. @@ -290,9 +289,8 @@ type Args struct { // ProfileOpts contains the set of profiles to enable and the // corresponding FDs where profile data will be written. ProfileOpts profile.Opts - // NvidiaDevMinors is a list of device minors for Nvidia GPU devices exposed - // to the sandbox. - NvidiaDevMinors NvidiaDevMinors + // NvidiaDriverVersion is the Nvidia driver version on the host. + NvidiaDriverVersion string } // make sure stdioFDs are always the same on initial start and on restore @@ -323,10 +321,10 @@ func New(args Args) (*Loader, error) { kernel.IOUringEnabled = args.Conf.IOUring info := containerInfo{ - conf: args.Conf, - spec: args.Spec, - goferMountConfs: args.GoferMountConfs, - nvidiaDevMinors: args.NvidiaDevMinors, + conf: args.Conf, + spec: args.Spec, + goferMountConfs: args.GoferMountConfs, + nvidiaDriverVersion: args.NvidiaDriverVersion, } // Make host FDs stable between invocations. Host FDs must map to the exact @@ -881,14 +879,14 @@ func (l *Loader) startSubcontainer(spec *specs.Spec, conf *config.Config, cid st } info := &containerInfo{ - conf: conf, - spec: spec, - goferFDs: goferFDs, - devGoferFD: devGoferFD, - goferFilestoreFDs: goferFilestoreFDs, - goferMountConfs: goferMountConfs, - nvidiaUVMDevMajor: l.root.nvidiaUVMDevMajor, - nvidiaDevMinors: l.root.nvidiaDevMinors, + conf: conf, + spec: spec, + goferFDs: goferFDs, + devGoferFD: devGoferFD, + goferFilestoreFDs: goferFilestoreFDs, + goferMountConfs: goferMountConfs, + nvidiaUVMDevMajor: l.root.nvidiaUVMDevMajor, + nvidiaDriverVersion: l.root.nvidiaDriverVersion, } info.procArgs, err = createProcessArgs(cid, spec, creds, l.k, pidns) if err != nil { diff --git a/runsc/boot/nvidia.go b/runsc/boot/nvidia.go deleted file mode 100644 index 8021c07bf..000000000 --- a/runsc/boot/nvidia.go +++ /dev/null @@ -1,52 +0,0 @@ -// Copyright 2023 The gVisor Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package boot - -import ( - "fmt" - "strconv" - "strings" -) - -// NvidiaDevMinors can be used to pass nvidia device minors via flags. -type NvidiaDevMinors []uint32 - -// String implements flag.Value. -func (n *NvidiaDevMinors) String() string { - minors := make([]string, 0, len(*n)) - for _, minor := range *n { - minors = append(minors, strconv.Itoa(int(minor))) - } - return strings.Join(minors, ",") -} - -// Get implements flag.Value. -func (n *NvidiaDevMinors) Get() any { - return n -} - -// Set implements flag.Value and appends a device minor from the command -// line to the device minors array. Set(String()) should be idempotent. -func (n *NvidiaDevMinors) Set(s string) error { - minors := strings.Split(s, ",") - for _, minor := range minors { - minorVal, err := strconv.Atoi(minor) - if err != nil { - return fmt.Errorf("invalid device minor value (%d): %v", minorVal, err) - } - *n = append(*n, uint32(minorVal)) - } - return nil -} diff --git a/runsc/boot/vfs.go b/runsc/boot/vfs.go index e1db87cf5..62ecb2533 100644 --- a/runsc/boot/vfs.go +++ b/runsc/boot/vfs.go @@ -30,6 +30,7 @@ import ( "gvisor.dev/gvisor/pkg/abi/tpu" "gvisor.dev/gvisor/pkg/cleanup" "gvisor.dev/gvisor/pkg/context" + "gvisor.dev/gvisor/pkg/devutil" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/fd" "gvisor.dev/gvisor/pkg/fspath" @@ -1116,7 +1117,7 @@ func createDeviceFiles(ctx context.Context, creds *auth.Credentials, info *conta } } } - if specutils.GPUFunctionalityRequested(info.spec, info.conf) && info.conf.NVProxyDocker { + if info.conf.NVProxyDocker && specutils.GPUFunctionalityRequested(info.spec, info.conf) { // In Docker mode, devices are not injected into spec.Linux.Devices. So // manually create appropriate device files. mode := os.FileMode(0666) @@ -1124,7 +1125,24 @@ func createDeviceFiles(ctx context.Context, creds *auth.Credentials, info *conta specs.LinuxDevice{Path: "/dev/nvidiactl", Type: "c", Major: nvgpu.NV_MAJOR_DEVICE_NUMBER, Minor: nvgpu.NV_CONTROL_DEVICE_MINOR, FileMode: &mode}, specs.LinuxDevice{Path: "/dev/nvidia-uvm", Type: "c", Major: int64(info.nvidiaUVMDevMajor), Minor: nvgpu.NVIDIA_UVM_PRIMARY_MINOR_NUMBER, FileMode: &mode}, } - for _, minor := range info.nvidiaDevMinors { + devClient := devutil.GoferClientFromContext(ctx) + if devClient == nil { + return fmt.Errorf("dev gofer client not found in context") + } + names, err := devClient.DirentNames(ctx) + if err != nil { + return fmt.Errorf("failed to get names of dirents from dev gofer: %w", err) + } + nvidiaDeviceRegex := regexp.MustCompile(`^nvidia(\d+)$`) + for _, name := range names { + ms := nvidiaDeviceRegex.FindStringSubmatch(name) + if ms == nil { + continue + } + minor, err := strconv.ParseUint(ms[1], 10, 32) + if err != nil { + return fmt.Errorf("invalid nvidia device name %q: %w", name, err) + } nvidiaDevs = append(nvidiaDevs, specs.LinuxDevice{Path: fmt.Sprintf("/dev/nvidia%d", minor), Type: "c", Major: nvgpu.NV_MAJOR_DEVICE_NUMBER, Minor: int64(minor), FileMode: &mode}) } for _, nvidiaDev := range nvidiaDevs { @@ -1208,14 +1226,14 @@ func tpuProxyRegisterDevices(info *containerInfo, vfsObj *vfs.VirtualFilesystem) } func nvproxyRegisterDevices(info *containerInfo, vfsObj *vfs.VirtualFilesystem) error { - if !specutils.GPUFunctionalityRequested(info.spec, info.conf) { + if !specutils.NVProxyEnabled(info.spec, info.conf) { return nil } uvmDevMajor, err := vfsObj.GetDynamicCharDevMajor() if err != nil { return fmt.Errorf("reserving device major number for nvidia-uvm: %w", err) } - if err := nvproxy.Register(vfsObj, uvmDevMajor); err != nil { + if err := nvproxy.Register(vfsObj, info.nvidiaDriverVersion, uvmDevMajor); err != nil { return fmt.Errorf("registering nvproxy driver: %w", err) } info.nvidiaUVMDevMajor = uvmDevMajor diff --git a/runsc/cmd/boot.go b/runsc/cmd/boot.go index c36dccd86..d011aeb61 100644 --- a/runsc/cmd/boot.go +++ b/runsc/cmd/boot.go @@ -161,9 +161,8 @@ type Boot struct { // used to synchronize rootless user namespace initialization. syncUsernsFD int - // nvidiaDevMinors is a list of device minors for Nvidia GPU devices exposed - // to the sandbox. - nvidiaDevMinors boot.NvidiaDevMinors + // nvidiaDriverVersion is the Nvidia driver version on the host. + nvidiaDriverVersion string } // Name implements subcommands.Command.Name. @@ -194,6 +193,7 @@ func (b *Boot) SetFlags(f *flag.FlagSet) { f.Uint64Var(&b.totalHostMem, "total-host-memory", 0, "total memory reported by host /proc/meminfo") f.BoolVar(&b.attached, "attached", false, "if attached is true, kills the sandbox process when the parent process terminates") f.StringVar(&b.productName, "product-name", "", "value to show in /sys/devices/virtual/dmi/id/product_name") + f.StringVar(&b.nvidiaDriverVersion, "nvidia-driver-version", "", "Nvidia driver version on the host") // Open FDs that are donated to the sandbox. f.IntVar(&b.specFD, "spec-fd", -1, "required fd with the container spec") @@ -211,7 +211,6 @@ func (b *Boot) SetFlags(f *flag.FlagSet) { f.IntVar(&b.mountsFD, "mounts-fd", -1, "mountsFD is an optional file descriptor to read list of mounts after they have been resolved (direct paths, no symlinks).") f.IntVar(&b.podInitConfigFD, "pod-init-config-fd", -1, "file descriptor to the pod init configuration file.") f.Var(&b.sinkFDs, "sink-fds", "ordered list of file descriptors to be used by the sinks defined in --pod-init-config.") - f.Var(&b.nvidiaDevMinors, "nvidia-dev-minors", "list of device minors for Nvidia GPU devices exposed to the sandbox.") // Profiling flags. b.profileFDs.SetFromFlags(f) @@ -272,7 +271,7 @@ func (b *Boot) Execute(_ context.Context, f *flag.FlagSet, args ...any) subcomma } if b.setUpRoot { - if err := setUpChroot(b.pidns, spec, conf, b.nvidiaDevMinors); err != nil { + if err := setUpChroot(b.pidns, spec, conf); err != nil { util.Fatalf("error setting up chroot: %v", err) } argOverride["setup-root"] = "false" @@ -423,27 +422,27 @@ func (b *Boot) Execute(_ context.Context, f *flag.FlagSet, args ...any) subcomma // Create the loader. bootArgs := boot.Args{ - ID: f.Arg(0), - Spec: spec, - Conf: conf, - ControllerFD: b.controllerFD, - Device: os.NewFile(uintptr(b.deviceFD), "platform device"), - GoferFDs: b.ioFDs.GetArray(), - DevGoferFD: b.devIoFD, - StdioFDs: b.stdioFDs.GetArray(), - PassFDs: b.passFDs.GetArray(), - ExecFD: b.execFD, - GoferFilestoreFDs: b.goferFilestoreFDs.GetArray(), - GoferMountConfs: b.goferMountConfs.GetArray(), - NumCPU: b.cpuNum, - TotalMem: b.totalMem, - TotalHostMem: b.totalHostMem, - UserLogFD: b.userLogFD, - ProductName: b.productName, - PodInitConfigFD: b.podInitConfigFD, - SinkFDs: b.sinkFDs.GetArray(), - ProfileOpts: b.profileFDs.ToOpts(), - NvidiaDevMinors: b.nvidiaDevMinors, + ID: f.Arg(0), + Spec: spec, + Conf: conf, + ControllerFD: b.controllerFD, + Device: os.NewFile(uintptr(b.deviceFD), "platform device"), + GoferFDs: b.ioFDs.GetArray(), + DevGoferFD: b.devIoFD, + StdioFDs: b.stdioFDs.GetArray(), + PassFDs: b.passFDs.GetArray(), + ExecFD: b.execFD, + GoferFilestoreFDs: b.goferFilestoreFDs.GetArray(), + GoferMountConfs: b.goferMountConfs.GetArray(), + NumCPU: b.cpuNum, + TotalMem: b.totalMem, + TotalHostMem: b.totalHostMem, + UserLogFD: b.userLogFD, + ProductName: b.productName, + PodInitConfigFD: b.podInitConfigFD, + SinkFDs: b.sinkFDs.GetArray(), + ProfileOpts: b.profileFDs.ToOpts(), + NvidiaDriverVersion: b.nvidiaDriverVersion, } l, err := boot.New(bootArgs) if err != nil { diff --git a/runsc/cmd/chroot.go b/runsc/cmd/chroot.go index e78fa6930..df573d8ca 100644 --- a/runsc/cmd/chroot.go +++ b/runsc/cmd/chroot.go @@ -15,7 +15,6 @@ package cmd import ( - "errors" "fmt" "os" "path" @@ -84,7 +83,7 @@ func copyFile(dst, src string) error { // setUpChroot creates an empty directory with runsc mounted at /runsc and proc // mounted at /proc. -func setUpChroot(pidns bool, spec *specs.Spec, conf *config.Config, nvidiaDevMinors []uint32) error { +func setUpChroot(pidns bool, spec *specs.Spec, conf *config.Config) error { // We are a new mount namespace, so we can use /tmp as a directory to // construct a new root. chroot := os.TempDir() @@ -120,9 +119,6 @@ func setUpChroot(pidns bool, spec *specs.Spec, conf *config.Config, nvidiaDevMin } } - if err := nvproxyUpdateChroot(chroot, spec, conf, nvidiaDevMinors); err != nil { - return fmt.Errorf("error configuring chroot for Nvidia GPUs: %w", err) - } if err := tpuProxyUpdateChroot(chroot, spec, conf); err != nil { return fmt.Errorf("error configuring chroot for TPU devices: %w", err) } @@ -182,25 +178,3 @@ func tpuProxyUpdateChroot(chroot string, spec *specs.Spec, conf *config.Config) } return nil } - -func nvproxyUpdateChroot(chroot string, spec *specs.Spec, conf *config.Config, devMinors []uint32) error { - if !specutils.GPUFunctionalityRequested(spec, conf) { - return nil - } - if err := os.Mkdir(filepath.Join(chroot, "dev"), 0755); err != nil && !errors.Is(err, os.ErrExist) { - return fmt.Errorf("error creating /dev in chroot: %w", err) - } - if err := mountInChroot(chroot, "/dev/nvidiactl", "/dev/nvidiactl", "bind", unix.MS_BIND); err != nil { - return fmt.Errorf("error mounting /dev/nvidiactl in chroot: %w", err) - } - if err := mountInChroot(chroot, "/dev/nvidia-uvm", "/dev/nvidia-uvm", "bind", unix.MS_BIND); err != nil { - return fmt.Errorf("error mounting /dev/nvidia-uvm in chroot: %w", err) - } - for _, devMinor := range devMinors { - path := fmt.Sprintf("/dev/nvidia%d", devMinor) - if err := mountInChroot(chroot, path, path, "bind", unix.MS_BIND); err != nil { - return fmt.Errorf("error mounting %q in chroot: %v", path, err) - } - } - return nil -} diff --git a/runsc/cmd/gofer.go b/runsc/cmd/gofer.go index 11ada4e5c..0968c3928 100644 --- a/runsc/cmd/gofer.go +++ b/runsc/cmd/gofer.go @@ -21,6 +21,7 @@ import ( "io" "os" "path/filepath" + "regexp" "runtime" "runtime/debug" "strings" @@ -429,7 +430,7 @@ func (g *Gofer) setupRootFS(spec *specs.Spec, conf *config.Config) error { // Set up /dev directory is needed. if g.devIoFD >= 0 { - g.setupDev(root) + g.setupDev(spec, conf, root, procPath) } // Create working directory if needed. @@ -512,10 +513,41 @@ func (g *Gofer) setupMounts(conf *config.Config, mounts []specs.Mount, root, pro return nil } -func (g *Gofer) setupDev(root string) error { +// shouldExposeNvidiaDevice returns true if path refers to an Nvidia device +// which should be exposed to the container. +// +// Precondition: nvproxy is enabled. +func shouldExposeNvidiaDevice(path string) bool { + if !strings.HasPrefix(path, "/dev/nvidia") { + return false + } + if path == "/dev/nvidiactl" || path == "/dev/nvidia-uvm" { + return true + } + nvidiaDevPathReg := regexp.MustCompile(`^/dev/nvidia(\d+)$`) + return nvidiaDevPathReg.MatchString(path) +} + +func (g *Gofer) setupDev(spec *specs.Spec, conf *config.Config, root, procPath string) error { if err := os.MkdirAll(filepath.Join(root, "dev"), 0777); err != nil { return fmt.Errorf("creating dev directory: %v", err) } + // Mount any devices specified in the spec. + if spec.Linux == nil { + return nil + } + nvproxyEnabled := specutils.NVProxyEnabled(spec, conf) + for _, dev := range spec.Linux.Devices { + shouldMount := nvproxyEnabled && shouldExposeNvidiaDevice(dev.Path) + if !shouldMount { + continue + } + dst := filepath.Join(root, dev.Path) + log.Infof("Mounting device %q as bind mount at %q", dev.Path, dst) + if err := specutils.SafeSetupAndMount(dev.Path, dst, "bind", unix.MS_BIND, procPath); err != nil { + return fmt.Errorf("mounting %q: %v", dev.Path, err) + } + } return nil } diff --git a/runsc/container/container.go b/runsc/container/container.go index 76a4dc324..c799af944 100644 --- a/runsc/container/container.go +++ b/runsc/container/container.go @@ -329,23 +329,6 @@ func New(conf *config.Config, args Args) (*Container, error) { PassFiles: args.PassFiles, ExecFile: args.ExecFile, } - if specutils.GPUFunctionalityRequested(args.Spec, conf) { - // Expose all Nvidia devices in /dev/, because we don't know what - // devices future subcontainers will want. - searchDir := "/" - if conf.NVProxyDocker { - // For single-container use cases like Docker, the container rootfs - // is populated with the devices that need to be exposed. Scan that. - // This scan needs to happen outside the sandbox process because - // /rootfs/dev/nvidia* mounts made in gofer may not be propagated to - // sandbox's mount namespace. - searchDir = args.Spec.Root.Path - } - sandArgs.NvidiaDevMinors, err = specutils.FindAllGPUDevices(searchDir) - if err != nil { - return fmt.Errorf("FindAllGPUDevices: %w", err) - } - } sand, err := sandbox.New(conf, sandArgs) if err != nil { return fmt.Errorf("cannot create sandbox: %w", err) @@ -1844,7 +1827,7 @@ func logIDMappings(mappings []specs.LinuxIDMapping, idType string) { // This should only be necessary once on the host. It should be run during the // root container setup sequence to make sure it has run at least once. func nvProxyPreGoferHostSetup(spec *specs.Spec, conf *config.Config) error { - if !specutils.GPUFunctionalityRequested(spec, conf) || !conf.NVProxyDocker { + if !conf.NVProxyDocker || !specutils.GPUFunctionalityRequested(spec, conf) { return nil } @@ -1941,7 +1924,7 @@ func nvproxyLoadKernelModules() { // construction. For this reason, we don't need to parse // NVIDIA_VISIBLE_DEVICES or pass --device to nvidia-container-cli. func nvproxySetupAfterGoferUserns(spec *specs.Spec, conf *config.Config, goferCmd *exec.Cmd, goferDonations *donation.Agency) (func() error, error) { - if !specutils.GPUFunctionalityRequested(spec, conf) || !conf.NVProxyDocker { + if !conf.NVProxyDocker || !specutils.GPUFunctionalityRequested(spec, conf) { return func() error { return nil }, nil } @@ -1967,7 +1950,7 @@ func nvproxySetupAfterGoferUserns(spec *specs.Spec, conf *config.Config, goferCm ldconfigPath = "/sbin/ldconfig" } - devices, err := specutils.NvidiaDeviceList(spec, conf) + devices, err := specutils.ParseNvidiaVisibleDevices(spec) if err != nil { return nil, fmt.Errorf("failed to get nvidia device numbers: %w", err) } diff --git a/runsc/sandbox/BUILD b/runsc/sandbox/BUILD index f0d034c22..92c1ad6aa 100644 --- a/runsc/sandbox/BUILD +++ b/runsc/sandbox/BUILD @@ -28,6 +28,7 @@ go_library( "//pkg/metric:metric_go_proto", "//pkg/prometheus", "//pkg/sentry/control", + "//pkg/sentry/devices/nvproxy", "//pkg/sentry/fsimpl/erofs", "//pkg/sentry/platform", "//pkg/sentry/seccheck", diff --git a/runsc/sandbox/sandbox.go b/runsc/sandbox/sandbox.go index 1105f4eb5..7b857a2de 100644 --- a/runsc/sandbox/sandbox.go +++ b/runsc/sandbox/sandbox.go @@ -44,6 +44,7 @@ import ( metricpb "gvisor.dev/gvisor/pkg/metric/metric_go_proto" "gvisor.dev/gvisor/pkg/prometheus" "gvisor.dev/gvisor/pkg/sentry/control" + "gvisor.dev/gvisor/pkg/sentry/devices/nvproxy" "gvisor.dev/gvisor/pkg/sentry/fsimpl/erofs" "gvisor.dev/gvisor/pkg/sentry/platform" "gvisor.dev/gvisor/pkg/sentry/seccheck" @@ -266,10 +267,6 @@ type Args struct { // ExecFile is the file from the host used for program execution. ExecFile *os.File - - // NvidiaDevMinors is the list of device minors for Nvidia GPU devices - // exposed to the sandbox. - NvidiaDevMinors boot.NvidiaDevMinors } // New creates the sandbox process. The caller must call Destroy() on the @@ -765,11 +762,6 @@ func (s *Sandbox) createSandboxProcess(conf *config.Config, args *Args, startSyn return err } - // Pass nvidia device minors. - if len(args.NvidiaDevMinors) > 0 { - cmd.Args = append(cmd.Args, "--nvidia-dev-minors="+args.NvidiaDevMinors.String()) - } - // Pass gofer mount configs. cmd.Args = append(cmd.Args, "--gofer-mount-confs="+args.GoferMountConfs.String()) @@ -831,6 +823,14 @@ func (s *Sandbox) createSandboxProcess(conf *config.Config, args *Args, startSyn cmd.Args = append(cmd.Args, "--pidns=true") } + if specutils.NVProxyEnabled(args.Spec, conf) { + nvidiaDriverVersion, err := nvproxy.HostDriverVersion() + if err != nil { + return fmt.Errorf("failed to get Nvidia driver version: %w", err) + } + cmd.Args = append(cmd.Args, "--nvidia-driver-version="+nvidiaDriverVersion) + } + // Joins the network namespace if network is enabled. the sandbox talks // directly to the host network, which may have been configured in the // namespace. diff --git a/runsc/specutils/nvidia.go b/runsc/specutils/nvidia.go index 8a8b151a2..6dba7bf9f 100644 --- a/runsc/specutils/nvidia.go +++ b/runsc/specutils/nvidia.go @@ -16,9 +16,6 @@ package specutils import ( "fmt" - "path" - "path/filepath" - "regexp" "strconv" "strings" @@ -38,31 +35,35 @@ func NVProxyEnabled(spec *specs.Spec, conf *config.Config) bool { return true } val, ok := spec.Annotations[annotationNVProxy] - if ok { - ret, err := strconv.ParseBool(val) - if val != "" && err != nil { - log.Warningf("tpuproxy annotation set to invalid value %q. Skipping.", val) - } - return ret + if !ok { + return false } - return false + ret, err := strconv.ParseBool(val) + if err != nil { + log.Warningf("nvproxy annotation set to invalid value %q: %w. Skipping.", val, err) + } + return ret } -// GPUFunctionalityRequested returns true if the user intends for the sandbox -// to have access to GPU functionality (e.g. access to /dev/nvidiactl), -// irrespective of whether or not they want access to any specific GPU. +// GPUFunctionalityRequested returns true if the container should have access +// to GPU functionality. func GPUFunctionalityRequested(spec *specs.Spec, conf *config.Config) bool { if !NVProxyEnabled(spec, conf) { // nvproxy disabled. return false } - if !conf.NVProxyDocker { - // nvproxy enabled in non-Docker mode. - return true + if spec.Linux != nil { + for _, dev := range spec.Linux.Devices { + if dev.Path == "/dev/nvidiactl" { + return true + } + } } - // nvproxy enabled in Docker mode. - // GPU access is only requested if NVIDIA_VISIBLE_DEVICES is non-empty - // and set to a value that doesn't mean "no GPU". + if !conf.NVProxyDocker { + return false + } + // In Docker mode, GPU access is only requested if NVIDIA_VISIBLE_DEVICES is + // non-empty and set to a value that doesn't mean "no GPU". if spec.Process == nil { return false } @@ -72,42 +73,11 @@ func GPUFunctionalityRequested(spec *specs.Spec, conf *config.Config) bool { return nvd != "" && nvd != "void" } -// FindAllGPUDevices returns the Nvidia GPU device minor numbers of all GPUs -// mounted in the provided rootfs. -func FindAllGPUDevices(rootfs string) ([]uint32, error) { - devPathPrefix := path.Join(rootfs, "dev/nvidia") - nvidiaDeviceRegex := regexp.MustCompile(fmt.Sprintf(`^%s(\d+)$`, devPathPrefix)) - paths, err := filepath.Glob(devPathPrefix + "*") - if err != nil { - return nil, fmt.Errorf("enumerating Nvidia device files: %w", err) - } - var devMinors []uint32 - for _, path := range paths { - if ms := nvidiaDeviceRegex.FindStringSubmatch(path); ms != nil { - index, err := strconv.ParseUint(ms[1], 10, 32) - if err != nil { - return nil, fmt.Errorf("invalid host device file %q: %w", path, err) - } - devMinors = append(devMinors, uint32(index)) - } - } - return devMinors, nil -} - -// NvidiaDeviceList returns the list of devices that should be visible to the -// sandbox. In Docker mode, this is the set of devices specified in -// NVIDIA_VISIBLE_DEVICES. In non-Docker mode, this is all Nvidia devices, as -// we cannot know the set of usable GPUs until subcontainer creation. -func NvidiaDeviceList(spec *specs.Spec, conf *config.Config) (string, error) { - if !GPUFunctionalityRequested(spec, conf) { - return "", nil - } - if !conf.NVProxyDocker { - // nvproxy enabled in non-Docker mode. - // Return all GPUs on the machine. - return "all", nil - } - // nvproxy is enabled in Docker mode. +// ParseNvidiaVisibleDevices parses NVIDIA_VISIBLE_DEVICES env var and returns +// the devices specified in it. This can be passed to nvidia-container-cli. +// +// Precondition: conf.NVProxyDocker && GPUFunctionalityRequested(spec, conf). +func ParseNvidiaVisibleDevices(spec *specs.Spec) (string, error) { nvd, _ := EnvVar(spec.Process.Env, nvdEnvVar) if nvd == "none" { return "", nil