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