diff --git a/src/runtime/virtcontainers/kata_agent.go b/src/runtime/virtcontainers/kata_agent.go index d6139577da..cc47c2f40d 100644 --- a/src/runtime/virtcontainers/kata_agent.go +++ b/src/runtime/virtcontainers/kata_agent.go @@ -861,7 +861,7 @@ func (k *kataAgent) startSandbox(ctx context.Context, sandbox *Sandbox) error { if sandbox.config.HypervisorType != RemoteHypervisor { // Setup network interfaces and routes - err = k.setupNetworks(ctx, sandbox, nil) + err = k.setupNetworks(ctx, sandbox) if err != nil { return err } @@ -1473,7 +1473,7 @@ func (k *kataAgent) rollbackFailingContainerCreation(ctx context.Context, c *Con } } -func (k *kataAgent) setupNetworks(ctx context.Context, sandbox *Sandbox, c *Container) error { +func (k *kataAgent) setupNetworks(ctx context.Context, sandbox *Sandbox) error { if sandbox.network.NetworkID() == "" { return nil } @@ -1503,64 +1503,46 @@ func (k *kataAgent) setupNetworks(ctx context.Context, sandbox *Sandbox, c *Cont } } - var err error - var endpoints []Endpoint - if c == nil || c.id == sandbox.id { - // TODO: VFIO network device has not been hotplugged when creating the Sandbox, - // so need to skip VFIO endpoint here. - // After KEP #4113(https://github.com/kubernetes/enhancements/pull/4113) - // is implemented, the VFIO network devices will be attached before container - // creation, so no need to skip them here anymore. - for _, ep := range sandbox.network.Endpoints() { - if ep.Type() != VfioEndpointType { - // For cold-plugged SR-IOV VFs that appear as PhysicalEndpoints, - // the guest PCI path is known after resolveColdPlugVFIOGuestPciPaths - // has run (during createContainers). Look it up and stamp it on the - // endpoint so that generateVCNetworkStructures emits a non-empty - // devicePath in the agent Interface proto. Without this the agent - // receives devicePath="" and falls back to a by-MAC link lookup, - // which fails when the VF firmware MAC differs from the OVN MAC. - if ep.Type() == PhysicalEndpointType && ep.PciPath().IsNil() { - if pe, ok := ep.(*PhysicalEndpoint); ok && pe.BDF != "" { - guestPath := sandbox.GetVfioDeviceGuestPciPath(pe.BDF) - if !guestPath.IsNil() { - ep.SetPciPath(guestPath) - k.Logger().WithFields(logrus.Fields{ - "endpoint-name": ep.Name(), - "host-bdf": pe.BDF, - "guest-pci-path": guestPath.String(), - }).Info("setupNetworks: filled guest PCI path for PhysicalEndpoint cold-plug") - } - } - } - endpoints = append(endpoints, ep) - } - } - } else if !sandbox.hotplugNetworkConfigApplied { - // Apply VFIO network devices' configuration after they are hot-plugged. - for _, ep := range sandbox.network.Endpoints() { - if ep.Type() == VfioEndpointType { - hostBDF := ep.(*VfioEndpoint).HostBDF - pciPath := sandbox.GetVfioDeviceGuestPciPath(hostBDF) - if pciPath.IsNil() { - return fmt.Errorf("PCI path for VFIO interface '%s' not found", ep.Name()) - } - ep.SetPciPath(pciPath) - endpoints = append(endpoints, ep) - } - } - - defer func() { - if err == nil { - sandbox.hotplugNetworkConfigApplied = true - } - }() - } - + endpoints := sandbox.network.Endpoints() if len(endpoints) == 0 { return nil } + // VFIO network devices (DAN) are not attached by the generic endpoint + // attach path. Hotplug them now, before any container is created, so + // that init containers that do not reference the device in their spec + // get working networking as well. + for _, ep := range endpoints { + if ep.Type() == VfioEndpointType { + if err := sandbox.hotplugVfioNetworkDevice(ctx, ep.(*VfioEndpoint)); err != nil { + return err + } + } + } + + // For cold-plugged SR-IOV VFs that appear as PhysicalEndpoints, the + // guest PCI path is known after the ResolveColdPlugVFIOGuestPciPaths + // call above. Look it up and stamp it on the endpoint so that + // generateVCNetworkStructures emits a non-empty devicePath in the agent + // Interface proto. Without this the agent receives devicePath="" and + // falls back to a by-MAC link lookup, which fails when the VF firmware + // MAC differs from the OVN MAC. + for _, ep := range endpoints { + if ep.Type() == PhysicalEndpointType && ep.PciPath().IsNil() { + if pe, ok := ep.(*PhysicalEndpoint); ok && pe.BDF != "" { + guestPath := sandbox.GetVfioDeviceGuestPciPath(pe.BDF) + if !guestPath.IsNil() { + ep.SetPciPath(guestPath) + k.Logger().WithFields(logrus.Fields{ + "endpoint-name": ep.Name(), + "host-bdf": pe.BDF, + "guest-pci-path": guestPath.String(), + }).Info("setupNetworks: filled guest PCI path for PhysicalEndpoint cold-plug") + } + } + } + } + interfaces, routes, neighs, err := generateVCNetworkStructures(ctx, endpoints) if err != nil { return err @@ -1745,10 +1727,6 @@ func (k *kataAgent) createContainer(ctx context.Context, sandbox *Sandbox, c *Co return nil, err } - if err = k.setupNetworks(ctx, sandbox, c); err != nil { - return nil, err - } - return buildProcessFromExecID(req.ExecId) } diff --git a/src/runtime/virtcontainers/sandbox.go b/src/runtime/virtcontainers/sandbox.go index 1e7b400d0b..f64ec81118 100644 --- a/src/runtime/virtcontainers/sandbox.go +++ b/src/runtime/virtcontainers/sandbox.go @@ -50,6 +50,7 @@ import ( "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/types" "github.com/kata-containers/kata-containers/src/runtime/virtcontainers/utils" + "golang.org/x/sys/unix" "google.golang.org/grpc/codes" grpcStatus "google.golang.org/grpc/status" ) @@ -264,11 +265,6 @@ type Sandbox struct { seccompSupported bool disableVMShutdown bool isVCPUsPinningOn bool - - // hotplugNetworkConfigApplied prevents network config API being called - // multiple times for hot-plugged network device when Sandbox has multiple - // containers. - hotplugNetworkConfigApplied bool } // ID returns the sandbox identifier string. @@ -2384,6 +2380,65 @@ func (s *Sandbox) GetVfioDeviceGuestPciPath(hostBDF string) types.PciPath { return types.PciPath{} } +// hotplugVfioNetworkDevice hotplugs the VFIO device backing a DAN network +// endpoint into the VM and records its guest PCI path on the endpoint. +// +// The device is attached at sandbox scope so that it is available before any +// container is created -- in particular before init containers, which do not +// reference the device in their spec. When a workload container later +// references the same VFIO group through a device plugin, the device manager +// finds the existing device by major:minor and only bumps its reference +// counts, so the device is neither plugged twice nor unplugged when that +// container exits. +func (s *Sandbox) hotplugVfioNetworkDevice(ctx context.Context, ep *VfioEndpoint) error { + if !ep.PciPath().IsNil() { + // The device is already attached and configured. + return nil + } + + if s.config.HypervisorConfig.HotPlugVFIO == config.NoPort { + return fmt.Errorf("cannot attach VFIO network interface %q (BDF %s): hot_plug_vfio port is not configured", ep.Name(), ep.HostBDF) + } + + devPath, err := drivers.GetVFIODevPath(ep.HostBDF) + if err != nil { + return fmt.Errorf("failed to resolve VFIO device path for network interface %q (BDF %s): %v", ep.Name(), ep.HostBDF, err) + } + + var stat unix.Stat_t + if err := unix.Stat(devPath, &stat); err != nil { + return fmt.Errorf("stat %q failed for network interface %q: %v", devPath, ep.Name(), err) + } + + devInfo := config.DeviceInfo{ + HostPath: devPath, + ContainerPath: devPath, + DevType: "c", + Major: int64(unix.Major(uint64(stat.Rdev))), + Minor: int64(unix.Minor(uint64(stat.Rdev))), + Port: s.config.HypervisorConfig.HotPlugVFIO, + } + + if _, err := s.AddDevice(ctx, devInfo); err != nil { + return fmt.Errorf("failed to hotplug VFIO device %q for network interface %q: %v", devPath, ep.Name(), err) + } + + pciPath := s.GetVfioDeviceGuestPciPath(ep.HostBDF) + if pciPath.IsNil() { + return fmt.Errorf("PCI path for VFIO interface %q (BDF %s) not found after hotplug", ep.Name(), ep.HostBDF) + } + ep.SetPciPath(pciPath) + + s.Logger().WithFields(logrus.Fields{ + "interface": ep.Name(), + "host-bdf": ep.HostBDF, + "device": devPath, + "pci-path": pciPath.String(), + }).Info("VFIO network device hotplugged") + + return nil +} + // updateResources will: // - calculate the resources required for the virtual machine, and adjust the virtual machine // sizing accordingly. For a given sandbox, it will calculate the number of vCPUs required based diff --git a/src/runtime/virtcontainers/sandbox_test.go b/src/runtime/virtcontainers/sandbox_test.go index 62675e3d07..a3bbde9d14 100644 --- a/src/runtime/virtcontainers/sandbox_test.go +++ b/src/runtime/virtcontainers/sandbox_test.go @@ -1719,3 +1719,80 @@ func TestCheckVCPUsPinningNUMABadHostCPUs(t *testing.T) { assert.Error(err) assert.Contains(err.Error(), "failed to parse HostCPUs") } + +func TestHotplugVfioNetworkDeviceAlreadyAttached(t *testing.T) { + assert := assert.New(t) + + pciPath, err := types.PciPathFromString("02") + assert.NoError(err) + + s := &Sandbox{ + config: &SandboxConfig{ + HypervisorConfig: HypervisorConfig{ + HotPlugVFIO: config.NoPort, + }, + }, + } + + // An endpoint whose guest PCI path is already known has been hotplugged + // before: the call must be a no-op, even when no hotplug port is + // configured. + ep := &VfioEndpoint{ + EndpointType: VfioEndpointType, + HostBDF: "0000:ff:1f.7", + PCIPath: pciPath, + Iface: NetworkInterface{Name: "eth1"}, + } + + assert.NoError(s.hotplugVfioNetworkDevice(context.Background(), ep)) + assert.Equal(pciPath, ep.PciPath()) +} + +func TestHotplugVfioNetworkDeviceNoPort(t *testing.T) { + assert := assert.New(t) + + s := &Sandbox{ + config: &SandboxConfig{ + HypervisorConfig: HypervisorConfig{ + HotPlugVFIO: config.NoPort, + }, + }, + } + + ep := &VfioEndpoint{ + EndpointType: VfioEndpointType, + HostBDF: "0000:ff:1f.7", + Iface: NetworkInterface{Name: "eth1"}, + } + + err := s.hotplugVfioNetworkDevice(context.Background(), ep) + assert.Error(err) + assert.Contains(err.Error(), "hot_plug_vfio") + assert.True(ep.PciPath().IsNil()) +} + +func TestHotplugVfioNetworkDevicePortConfigured(t *testing.T) { + assert := assert.New(t) + + s := &Sandbox{ + config: &SandboxConfig{ + HypervisorConfig: HypervisorConfig{ + HotPlugVFIO: config.RootPort, + }, + }, + } + + // With a hotplug port configured the guards are passed and the VFIO + // device path resolution is attempted. The malformed BDF cannot match + // any sysfs PCI device directory, so the resolution fails + // deterministically on every host. + ep := &VfioEndpoint{ + EndpointType: VfioEndpointType, + HostBDF: "not-a-bdf", + Iface: NetworkInterface{Name: "eth1"}, + } + + err := s.hotplugVfioNetworkDevice(context.Background(), ep) + assert.Error(err) + assert.Contains(err.Error(), "failed to resolve VFIO device path") +}