From 86ab1b22a34496bc8177452a215cd9278fac710e Mon Sep 17 00:00:00 2001 From: PiotrProkop Date: Thu, 2 Jul 2026 11:59:11 +0200 Subject: [PATCH] runtime: Hotplug VFIO network devices before any container starts A VFIO-backed network interface used to be hotplugged only as a side effect of creating the workload container that references the VFIO device in its OCI spec: setupNetworks() skipped VfioEndpoints when it ran at sandbox start and applied their configuration later, on the second invocation from createContainer(), guarded by the hotplugNetworkConfigApplied flag. Init containers run before that workload container, so they started with the VFIO interface absent and thus without pod networking. Attach the device at sandbox scope instead, from setupNetworks() at sandbox start, before any container is created: - Add Sandbox.hotplugVfioNetworkDevice(): resolve the VFIO device node backing the endpoint's host BDF, hotplug it and record the guest PCI path on the endpoint. When the workload container later references the same VFIO group through a device plugin, the device manager finds the device by major:minor and only bumps reference counts, so it is neither plugged twice nor unplugged when that container exits. - Configure all endpoints, VfioEndpoints included, in a single setupNetworks() pass at sandbox start. The Container parameter, the second invocation from createContainer() and the hotplugNetworkConfigApplied flag are no longer needed. Add unit tests for the hotplugVfioNetworkDevice() guard paths. Co-Authored-By: Claude Fable 5 Signed-off-by: PiotrProkop --- src/runtime/virtcontainers/kata_agent.go | 98 +++++++++------------- src/runtime/virtcontainers/sandbox.go | 65 ++++++++++++-- src/runtime/virtcontainers/sandbox_test.go | 77 +++++++++++++++++ 3 files changed, 175 insertions(+), 65 deletions(-) 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") +}