From 2fef4bc8441a5a273e9a7caa554a8624368c74c0 Mon Sep 17 00:00:00 2001 From: Archana Shinde Date: Wed, 6 Dec 2023 00:40:42 -0800 Subject: [PATCH 1/4] vfio: use driver_override field for device binding. The current implementation for device binding using driver bind/unbind and new_id fails in the scenario when the physical device is not bound to a driver before assigning it to vfio. There exists and updated mechanism to accomplish the same that does not have the same issue as above. The driver_override field for a device allows us to specify the driver for a device rather than relying on the bound driver to provide a positive match of the device. It also has other advantages referenced here: https://patchwork.kernel.org/project/linux-pci/patch/1396372540.476.160.camel@ul30vt.home/ So use the updated driver_override mechanism for binding/unbinding a physical device/virtual function to vfio-pci. Signed-off-by: liangxianlong Signed-off-by: Archana Shinde --- src/runtime/pkg/device/drivers/vfio.go | 101 ++++++++++-------- .../virtcontainers/physical_endpoint.go | 4 +- 2 files changed, 58 insertions(+), 47 deletions(-) diff --git a/src/runtime/pkg/device/drivers/vfio.go b/src/runtime/pkg/device/drivers/vfio.go index 394edd5848..e9c99b9221 100644 --- a/src/runtime/pkg/device/drivers/vfio.go +++ b/src/runtime/pkg/device/drivers/vfio.go @@ -22,13 +22,12 @@ import ( // bind/unbind paths to aid in SRIOV VF bring-up/restore const ( - pciDriverUnbindPath = "/sys/bus/pci/devices/%s/driver/unbind" - pciDriverBindPath = "/sys/bus/pci/drivers/%s/bind" - vfioNewIDPath = "/sys/bus/pci/drivers/vfio-pci/new_id" - vfioRemoveIDPath = "/sys/bus/pci/drivers/vfio-pci/remove_id" - iommuGroupPath = "/sys/bus/pci/devices/%s/iommu_group" - vfioDevPath = "/dev/vfio/%s" - vfioAPSysfsDir = "/sys/devices/vfio_ap" + pciDriverUnbindPath = "/sys/bus/pci/devices/%s/driver/unbind" + pciDriverOverridePath = "/sys/bus/pci/devices/%s/driver_override" + driversProbePath = "/sys/bus/pci/drivers_probe" + iommuGroupPath = "/sys/bus/pci/devices/%s/iommu_group" + vfioDevPath = "/dev/vfio/%s" + vfioAPSysfsDir = "/sys/devices/vfio_ap" ) // VFIODevice is a vfio device meant to be passed to the hypervisor @@ -269,42 +268,45 @@ func GetBDF(deviceSysStr string) string { return tokens[1] } -// BindDevicetoVFIO binds the device to vfio driver after unbinding from host. +// BindDevicetoVFIO binds the device to vfio driver after unbinding from host +// driver if present. // Will be called by a network interface or a generic pcie device. -func BindDevicetoVFIO(bdf, hostDriver, vendorDeviceID string) (string, error) { +func BindDevicetoVFIO(bdf, hostDriver string) (string, error) { + + overrideDriverPath := fmt.Sprintf(pciDriverOverridePath, bdf) + deviceLogger().WithFields(logrus.Fields{ + "device-bdf": bdf, + "driver-override-path": overrideDriverPath, + }).Info("Write vfio-pci to driver_override") + + // Write vfio-pci to driver_override file to allow the device to bind to vfio-pci + // Reference: https://www.kernel.org/doc/Documentation/ABI/testing/sysfs-bus-platform + if err := utils.WriteToFile(overrideDriverPath, []byte("vfio-pci")); err != nil { + return "", err + } - // Unbind from the host driver unbindDriverPath := fmt.Sprintf(pciDriverUnbindPath, bdf) deviceLogger().WithFields(logrus.Fields{ "device-bdf": bdf, "driver-path": unbindDriverPath, }).Info("Unbinding device from driver") - if err := utils.WriteToFile(unbindDriverPath, []byte(bdf)); err != nil { - return "", err - } + // Unbind device from the host driver. In some cases, a driver may not be bound + // to the device, in which case this step may fail. Hence ignore error for this step. + utils.WriteToFile(unbindDriverPath, []byte(bdf)) - // Add device id to vfio driver. deviceLogger().WithFields(logrus.Fields{ - "vendor-device-id": vendorDeviceID, - "vfio-new-id-path": vfioNewIDPath, - }).Info("Writing vendor-device-id to vfio new-id path") + "device-bdf": bdf, + "drivers-probe-path": driversProbePath, + }).Info("Writing bdf to drivers-probe-path") - if err := utils.WriteToFile(vfioNewIDPath, []byte(vendorDeviceID)); err != nil { + // Invoke drivers_probe so that the driver matching driver_override, in our case + // the vfio-pci driver will probe the device. + if err := utils.WriteToFile(driversProbePath, []byte(bdf)); err != nil { return "", err } - // Bind to vfio-pci driver. - bindDriverPath := fmt.Sprintf(pciDriverBindPath, "vfio-pci") - - api.DeviceLogger().WithFields(logrus.Fields{ - "device-bdf": bdf, - "driver-path": bindDriverPath, - }).Info("Binding device to vfio driver") - - // Device may be already bound at this time because of earlier write to new_id, ignore error - utils.WriteToFile(bindDriverPath, []byte(bdf)) - + // Determine the iommu group that the device belongs to. groupPath, err := os.Readlink(fmt.Sprintf(iommuGroupPath, bdf)) if err != nil { return "", err @@ -313,11 +315,25 @@ func BindDevicetoVFIO(bdf, hostDriver, vendorDeviceID string) (string, error) { return fmt.Sprintf(vfioDevPath, filepath.Base(groupPath)), nil } -// BindDevicetoHost binds the device to the host driver after unbinding from vfio-pci. -func BindDevicetoHost(bdf, hostDriver, vendorDeviceID string) error { - // Unbind from vfio-pci driver - unbindDriverPath := fmt.Sprintf(pciDriverUnbindPath, bdf) +// BindDevicetoHost unbinds the device from vfio-pci driver and binds it to the +// previously bound driver. +func BindDevicetoHost(bdf, hostDriver string) error { + overrideDriverPath := fmt.Sprintf(pciDriverOverridePath, bdf) api.DeviceLogger().WithFields(logrus.Fields{ + "device-bdf": bdf, + "driver-override-path": overrideDriverPath, + }).Infof("Write %s to driver_override", hostDriver) + + // write previously bound host driver to driver_override to allow the + // device to bind to it. This could be empty which means the device will not be + // bound to any driver later on. + if err := utils.WriteToFile(overrideDriverPath, []byte(hostDriver)); err != nil { + return err + } + + // Unbind device from vfio-pci driver. + unbindDriverPath := fmt.Sprintf(pciDriverUnbindPath, bdf) + deviceLogger().WithFields(logrus.Fields{ "device-bdf": bdf, "driver-path": unbindDriverPath, }).Info("Unbinding device from driver") @@ -326,17 +342,12 @@ func BindDevicetoHost(bdf, hostDriver, vendorDeviceID string) error { return err } - // To prevent new VFs from binding to VFIO-PCI, remove_id - if err := utils.WriteToFile(vfioRemoveIDPath, []byte(vendorDeviceID)); err != nil { - return err - } + deviceLogger().WithFields(logrus.Fields{ + "device-bdf": bdf, + "drivers-probe-path": driversProbePath, + }).Info("Writing bdf to drivers-probe-path") - // Bind back to host driver - bindDriverPath := fmt.Sprintf(pciDriverBindPath, hostDriver) - api.DeviceLogger().WithFields(logrus.Fields{ - "device-bdf": bdf, - "driver-path": bindDriverPath, - }).Info("Binding back device to host driver") - - return utils.WriteToFile(bindDriverPath, []byte(bdf)) + // Invoke drivers_probe so that the driver matching driver_override, in this case + // the previous host driver will probe the device. + return utils.WriteToFile(driversProbePath, []byte(bdf)) } diff --git a/src/runtime/virtcontainers/physical_endpoint.go b/src/runtime/virtcontainers/physical_endpoint.go index 615e256be5..7891e21ff4 100644 --- a/src/runtime/virtcontainers/physical_endpoint.go +++ b/src/runtime/virtcontainers/physical_endpoint.go @@ -218,11 +218,11 @@ func createPhysicalEndpoint(netInfo NetworkInfo) (*PhysicalEndpoint, error) { } func bindNICToVFIO(endpoint *PhysicalEndpoint) (string, error) { - return drivers.BindDevicetoVFIO(endpoint.BDF, endpoint.Driver, endpoint.VendorDeviceID) + return drivers.BindDevicetoVFIO(endpoint.BDF, endpoint.Driver) } func bindNICToHost(endpoint *PhysicalEndpoint) error { - return drivers.BindDevicetoHost(endpoint.BDF, endpoint.Driver, endpoint.VendorDeviceID) + return drivers.BindDevicetoHost(endpoint.BDF, endpoint.Driver) } func (endpoint *PhysicalEndpoint) save() persistapi.NetworkEndpoint { From 1e304e6307a7b45884f823b678d5659569efda09 Mon Sep 17 00:00:00 2001 From: Archana Shinde Date: Wed, 6 Dec 2023 12:09:56 -0800 Subject: [PATCH 2/4] network: Implement hotplug for physical endpoints Enable physical network interfaces to be hotplugged. For this, we need to change the signature of the HotAttach method to make use of Sandbox instead of Hypervisor. Similar approach was followed for Attach method, but this change was overlooked for HotAttach. The signature change is required in order to make use of device manager and receiver for physical network enpoints. Fixes: #8405 Signed-off-by: Archana Shinde --- src/runtime/virtcontainers/endpoint.go | 2 +- src/runtime/virtcontainers/ipvlan_endpoint.go | 3 +- .../virtcontainers/macvlan_endpoint.go | 3 +- .../virtcontainers/macvtap_endpoint.go | 2 +- src/runtime/virtcontainers/network_linux.go | 2 +- .../virtcontainers/physical_endpoint.go | 28 +++++++++++++++++-- .../virtcontainers/physical_endpoint_test.go | 6 ++-- src/runtime/virtcontainers/tap_endpoint.go | 3 +- src/runtime/virtcontainers/tuntap_endpoint.go | 3 +- src/runtime/virtcontainers/veth_endpoint.go | 3 +- .../virtcontainers/vhostuser_endpoint.go | 2 +- .../virtcontainers/vhostuser_endpoint_test.go | 6 ++-- 12 files changed, 48 insertions(+), 15 deletions(-) diff --git a/src/runtime/virtcontainers/endpoint.go b/src/runtime/virtcontainers/endpoint.go index 6c24b79ac4..cbf384f008 100644 --- a/src/runtime/virtcontainers/endpoint.go +++ b/src/runtime/virtcontainers/endpoint.go @@ -26,7 +26,7 @@ type Endpoint interface { SetPciPath(vcTypes.PciPath) Attach(context.Context, *Sandbox) error Detach(ctx context.Context, netNsCreated bool, netNsPath string) error - HotAttach(ctx context.Context, h Hypervisor) error + HotAttach(context.Context, *Sandbox) error HotDetach(ctx context.Context, h Hypervisor, netNsCreated bool, netNsPath string) error save() persistapi.NetworkEndpoint diff --git a/src/runtime/virtcontainers/ipvlan_endpoint.go b/src/runtime/virtcontainers/ipvlan_endpoint.go index 4c495ba67a..74c41576a8 100644 --- a/src/runtime/virtcontainers/ipvlan_endpoint.go +++ b/src/runtime/virtcontainers/ipvlan_endpoint.go @@ -125,10 +125,11 @@ func (endpoint *IPVlanEndpoint) Detach(ctx context.Context, netNsCreated bool, n }) } -func (endpoint *IPVlanEndpoint) HotAttach(ctx context.Context, h Hypervisor) error { +func (endpoint *IPVlanEndpoint) HotAttach(ctx context.Context, s *Sandbox) error { span, ctx := ipvlanTrace(ctx, "HotAttach", endpoint) defer span.End() + h := s.hypervisor if err := xConnectVMNetwork(ctx, endpoint, h); err != nil { networkLogger().WithError(err).Error("Error bridging ipvlan ep") return err diff --git a/src/runtime/virtcontainers/macvlan_endpoint.go b/src/runtime/virtcontainers/macvlan_endpoint.go index 974019fbb6..7ffcc582c8 100644 --- a/src/runtime/virtcontainers/macvlan_endpoint.go +++ b/src/runtime/virtcontainers/macvlan_endpoint.go @@ -122,10 +122,11 @@ func (endpoint *MacvlanEndpoint) Detach(ctx context.Context, netNsCreated bool, }) } -func (endpoint *MacvlanEndpoint) HotAttach(ctx context.Context, h Hypervisor) error { +func (endpoint *MacvlanEndpoint) HotAttach(ctx context.Context, s *Sandbox) error { span, ctx := macvlanTrace(ctx, "HotAttach", endpoint) defer span.End() + h := s.hypervisor if err := xConnectVMNetwork(ctx, endpoint, h); err != nil { networkLogger().WithError(err).Error("Error bridging macvlan ep") return err diff --git a/src/runtime/virtcontainers/macvtap_endpoint.go b/src/runtime/virtcontainers/macvtap_endpoint.go index b3fd9de7a1..c53d2603df 100644 --- a/src/runtime/virtcontainers/macvtap_endpoint.go +++ b/src/runtime/virtcontainers/macvtap_endpoint.go @@ -93,7 +93,7 @@ func (endpoint *MacvtapEndpoint) Detach(ctx context.Context, netNsCreated bool, } // HotAttach for macvtap endpoint not supported yet -func (endpoint *MacvtapEndpoint) HotAttach(ctx context.Context, h Hypervisor) error { +func (endpoint *MacvtapEndpoint) HotAttach(ctx context.Context, s *Sandbox) error { return fmt.Errorf("MacvtapEndpoint does not support Hot attach") } diff --git a/src/runtime/virtcontainers/network_linux.go b/src/runtime/virtcontainers/network_linux.go index f37f6f3de1..7160fbf678 100644 --- a/src/runtime/virtcontainers/network_linux.go +++ b/src/runtime/virtcontainers/network_linux.go @@ -202,7 +202,7 @@ func (n *LinuxNetwork) addSingleEndpoint(ctx context.Context, s *Sandbox, netInf networkLogger().WithField("endpoint-type", endpoint.Type()).WithField("hotplug", hotplug).Info("Attaching endpoint") if hotplug { - if err := endpoint.HotAttach(ctx, s.hypervisor); err != nil { + if err := endpoint.HotAttach(ctx, s); err != nil { return nil, err } } else { diff --git a/src/runtime/virtcontainers/physical_endpoint.go b/src/runtime/virtcontainers/physical_endpoint.go index 7891e21ff4..db098492b6 100644 --- a/src/runtime/virtcontainers/physical_endpoint.go +++ b/src/runtime/virtcontainers/physical_endpoint.go @@ -123,8 +123,32 @@ func (endpoint *PhysicalEndpoint) Detach(ctx context.Context, netNsCreated bool, } // HotAttach for physical endpoint not supported yet -func (endpoint *PhysicalEndpoint) HotAttach(ctx context.Context, h Hypervisor) error { - return fmt.Errorf("PhysicalEndpoint does not support Hot attach") +func (endpoint *PhysicalEndpoint) HotAttach(ctx context.Context, s *Sandbox) error { + span, ctx := physicalTrace(ctx, "HotAttach", endpoint) + defer span.End() + + // Unbind physical interface from host driver and bind to vfio + // so that it can be passed to the hypervisor. + vfioPath, err := bindNICToVFIO(endpoint) + if err != nil { + return err + } + + c, err := resCtrl.DeviceToCgroupDeviceRule(vfioPath) + if err != nil { + return err + } + + d := config.DeviceInfo{ + ContainerPath: vfioPath, + DevType: string(c.Type), + Major: c.Major, + Minor: c.Minor, + ColdPlug: false, + } + + _, err = s.AddDevice(ctx, d) + return err } // HotDetach for physical endpoint not supported yet diff --git a/src/runtime/virtcontainers/physical_endpoint_test.go b/src/runtime/virtcontainers/physical_endpoint_test.go index bba7f7d1c7..5aed350e8e 100644 --- a/src/runtime/virtcontainers/physical_endpoint_test.go +++ b/src/runtime/virtcontainers/physical_endpoint_test.go @@ -27,9 +27,11 @@ func TestPhysicalEndpoint_HotAttach(t *testing.T) { HardAddr: net.HardwareAddr{0x02, 0x00, 0xca, 0xfe, 0x00, 0x04}.String(), } - h := &mockHypervisor{} + s := &Sandbox{ + hypervisor: &mockHypervisor{}, + } - err := v.HotAttach(context.Background(), h) + err := v.HotAttach(context.Background(), s) assert.Error(err) } diff --git a/src/runtime/virtcontainers/tap_endpoint.go b/src/runtime/virtcontainers/tap_endpoint.go index 2f8ec33532..ff7dbe3af0 100644 --- a/src/runtime/virtcontainers/tap_endpoint.go +++ b/src/runtime/virtcontainers/tap_endpoint.go @@ -92,12 +92,13 @@ func (endpoint *TapEndpoint) Detach(ctx context.Context, netNsCreated bool, netN } // HotAttach for the tap endpoint uses hot plug device -func (endpoint *TapEndpoint) HotAttach(ctx context.Context, h Hypervisor) error { +func (endpoint *TapEndpoint) HotAttach(ctx context.Context, s *Sandbox) error { networkLogger().Info("Hot attaching tap endpoint") span, ctx := tapTrace(ctx, "HotAttach", endpoint) defer span.End() + h := s.hypervisor if err := tapNetwork(endpoint, h.HypervisorConfig().NumVCPUs(), h.HypervisorConfig().DisableVhostNet); err != nil { networkLogger().WithError(err).Error("Error bridging tap ep") return err diff --git a/src/runtime/virtcontainers/tuntap_endpoint.go b/src/runtime/virtcontainers/tuntap_endpoint.go index ddd371e9ea..3631d3d762 100644 --- a/src/runtime/virtcontainers/tuntap_endpoint.go +++ b/src/runtime/virtcontainers/tuntap_endpoint.go @@ -103,12 +103,13 @@ func (endpoint *TuntapEndpoint) Detach(ctx context.Context, netNsCreated bool, n } // HotAttach for the tun/tap endpoint uses hot plug device -func (endpoint *TuntapEndpoint) HotAttach(ctx context.Context, h Hypervisor) error { +func (endpoint *TuntapEndpoint) HotAttach(ctx context.Context, s *Sandbox) error { networkLogger().Info("Hot attaching tun/tap endpoint") span, ctx := tuntapTrace(ctx, "HotAttach", endpoint) defer span.End() + h := s.hypervisor if err := tuntapNetwork(endpoint, h.HypervisorConfig().NumVCPUs(), h.HypervisorConfig().DisableVhostNet); err != nil { networkLogger().WithError(err).Error("Error bridging tun/tap ep") return err diff --git a/src/runtime/virtcontainers/veth_endpoint.go b/src/runtime/virtcontainers/veth_endpoint.go index e3ed63d707..7102df8c5b 100644 --- a/src/runtime/virtcontainers/veth_endpoint.go +++ b/src/runtime/virtcontainers/veth_endpoint.go @@ -126,10 +126,11 @@ func (endpoint *VethEndpoint) Detach(ctx context.Context, netNsCreated bool, net } // HotAttach for the veth endpoint uses hot plug device -func (endpoint *VethEndpoint) HotAttach(ctx context.Context, h Hypervisor) error { +func (endpoint *VethEndpoint) HotAttach(ctx context.Context, s *Sandbox) error { span, ctx := vethTrace(ctx, "HotAttach", endpoint) defer span.End() + h := s.hypervisor if err := xConnectVMNetwork(ctx, endpoint, h); err != nil { networkLogger().WithError(err).Error("Error bridging virtual ep") return err diff --git a/src/runtime/virtcontainers/vhostuser_endpoint.go b/src/runtime/virtcontainers/vhostuser_endpoint.go index 6656f33dc2..ce884fe428 100644 --- a/src/runtime/virtcontainers/vhostuser_endpoint.go +++ b/src/runtime/virtcontainers/vhostuser_endpoint.go @@ -107,7 +107,7 @@ func (endpoint *VhostUserEndpoint) Detach(ctx context.Context, netNsCreated bool } // HotAttach for vhostuser endpoint not supported yet -func (endpoint *VhostUserEndpoint) HotAttach(ctx context.Context, h Hypervisor) error { +func (endpoint *VhostUserEndpoint) HotAttach(ctx context.Context, s *Sandbox) error { return fmt.Errorf("VhostUserEndpoint does not support Hot attach") } diff --git a/src/runtime/virtcontainers/vhostuser_endpoint_test.go b/src/runtime/virtcontainers/vhostuser_endpoint_test.go index a49a3e61e0..0a8448c32b 100644 --- a/src/runtime/virtcontainers/vhostuser_endpoint_test.go +++ b/src/runtime/virtcontainers/vhostuser_endpoint_test.go @@ -97,9 +97,11 @@ func TestVhostUserEndpoint_HotAttach(t *testing.T) { EndpointType: VhostUserEndpointType, } - h := &mockHypervisor{} + s := &Sandbox{ + hypervisor: &mockHypervisor{}, + } - err := v.HotAttach(context.Background(), h) + err := v.HotAttach(context.Background(), s) assert.Error(err) } From c6390f2a2af85c3493b014d8d5a1bda059fcbc4c Mon Sep 17 00:00:00 2001 From: Archana Shinde Date: Wed, 6 Dec 2023 21:29:20 -0800 Subject: [PATCH 3/4] vfio: Introduce function to get vfio dev path This function will be later used to get the vfio dev path. Signed-off-by: Archana Shinde --- src/runtime/pkg/device/drivers/vfio.go | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/runtime/pkg/device/drivers/vfio.go b/src/runtime/pkg/device/drivers/vfio.go index e9c99b9221..f315fddaf0 100644 --- a/src/runtime/pkg/device/drivers/vfio.go +++ b/src/runtime/pkg/device/drivers/vfio.go @@ -268,6 +268,16 @@ func GetBDF(deviceSysStr string) string { return tokens[1] } +func GetVFIODevPath(bdf string) (string, error) { + // Determine the iommu group that the device belongs to. + groupPath, err := os.Readlink(fmt.Sprintf(iommuGroupPath, bdf)) + if err != nil { + return "", err + } + + return fmt.Sprintf(vfioDevPath, filepath.Base(groupPath)), nil +} + // BindDevicetoVFIO binds the device to vfio driver after unbinding from host // driver if present. // Will be called by a network interface or a generic pcie device. @@ -306,13 +316,7 @@ func BindDevicetoVFIO(bdf, hostDriver string) (string, error) { return "", err } - // Determine the iommu group that the device belongs to. - groupPath, err := os.Readlink(fmt.Sprintf(iommuGroupPath, bdf)) - if err != nil { - return "", err - } - - return fmt.Sprintf(vfioDevPath, filepath.Base(groupPath)), nil + return GetVFIODevPath(bdf) } // BindDevicetoHost unbinds the device from vfio-pci driver and binds it to the From 1636c201f488eeb76a91ef25f8c8bbe2d945dd39 Mon Sep 17 00:00:00 2001 From: Archana Shinde Date: Wed, 6 Dec 2023 21:40:15 -0800 Subject: [PATCH 4/4] network: Implement network hotunplug for physical endpoints Similar to HotAttach, the HotDetach method signature for network endoints needs to be changed as well to allow for the method to make use of device manager to manage the hot unplug of physical network devices. Signed-off-by: Archana Shinde --- src/runtime/pkg/device/api/interface.go | 1 + src/runtime/pkg/device/manager/manager.go | 4 +-- src/runtime/virtcontainers/endpoint.go | 2 +- src/runtime/virtcontainers/ipvlan_endpoint.go | 3 +- .../virtcontainers/macvlan_endpoint.go | 3 +- .../virtcontainers/macvtap_endpoint.go | 2 +- src/runtime/virtcontainers/network_linux.go | 2 +- .../virtcontainers/physical_endpoint.go | 32 +++++++++++++++++-- .../virtcontainers/physical_endpoint_test.go | 6 ++-- src/runtime/virtcontainers/tap_endpoint.go | 3 +- src/runtime/virtcontainers/tuntap_endpoint.go | 3 +- src/runtime/virtcontainers/veth_endpoint.go | 3 +- .../virtcontainers/vhostuser_endpoint.go | 2 +- .../virtcontainers/vhostuser_endpoint_test.go | 6 ++-- 14 files changed, 55 insertions(+), 17 deletions(-) diff --git a/src/runtime/pkg/device/api/interface.go b/src/runtime/pkg/device/api/interface.go index 074980f44f..6ec7b2885e 100644 --- a/src/runtime/pkg/device/api/interface.go +++ b/src/runtime/pkg/device/api/interface.go @@ -94,4 +94,5 @@ type DeviceManager interface { GetDeviceByID(string) Device GetAllDevices() []Device LoadDevices([]config.DeviceState) + FindDevice(*config.DeviceInfo) Device } diff --git a/src/runtime/pkg/device/manager/manager.go b/src/runtime/pkg/device/manager/manager.go index 3e687cde37..667122f288 100644 --- a/src/runtime/pkg/device/manager/manager.go +++ b/src/runtime/pkg/device/manager/manager.go @@ -82,7 +82,7 @@ func NewDeviceManager(blockDriver string, vhostUserStoreEnabled bool, vhostUserS return dm } -func (dm *deviceManager) findDevice(devInfo *config.DeviceInfo) api.Device { +func (dm *deviceManager) FindDevice(devInfo *config.DeviceInfo) api.Device { // For devices with a major of -1, we use the host path to find existing instances. if devInfo.Major == -1 { for _, dev := range dm.devices { @@ -121,7 +121,7 @@ func (dm *deviceManager) createDevice(devInfo config.DeviceInfo) (dev api.Device } }() - if existingDev := dm.findDevice(&devInfo); existingDev != nil { + if existingDev := dm.FindDevice(&devInfo); existingDev != nil { return existingDev, nil } diff --git a/src/runtime/virtcontainers/endpoint.go b/src/runtime/virtcontainers/endpoint.go index cbf384f008..ef50a8eb7e 100644 --- a/src/runtime/virtcontainers/endpoint.go +++ b/src/runtime/virtcontainers/endpoint.go @@ -27,7 +27,7 @@ type Endpoint interface { Attach(context.Context, *Sandbox) error Detach(ctx context.Context, netNsCreated bool, netNsPath string) error HotAttach(context.Context, *Sandbox) error - HotDetach(ctx context.Context, h Hypervisor, netNsCreated bool, netNsPath string) error + HotDetach(ctx context.Context, s *Sandbox, netNsCreated bool, netNsPath string) error save() persistapi.NetworkEndpoint load(persistapi.NetworkEndpoint) diff --git a/src/runtime/virtcontainers/ipvlan_endpoint.go b/src/runtime/virtcontainers/ipvlan_endpoint.go index 74c41576a8..9ea66af9d1 100644 --- a/src/runtime/virtcontainers/ipvlan_endpoint.go +++ b/src/runtime/virtcontainers/ipvlan_endpoint.go @@ -143,7 +143,7 @@ func (endpoint *IPVlanEndpoint) HotAttach(ctx context.Context, s *Sandbox) error return nil } -func (endpoint *IPVlanEndpoint) HotDetach(ctx context.Context, h Hypervisor, netNsCreated bool, netNsPath string) error { +func (endpoint *IPVlanEndpoint) HotDetach(ctx context.Context, s *Sandbox, netNsCreated bool, netNsPath string) error { if !netNsCreated { return nil } @@ -157,6 +157,7 @@ func (endpoint *IPVlanEndpoint) HotDetach(ctx context.Context, h Hypervisor, net networkLogger().WithError(err).Warn("Error un-bridging ipvlan ep") } + h := s.hypervisor if _, err := h.HotplugRemoveDevice(ctx, endpoint, NetDev); err != nil { networkLogger().WithError(err).Error("Error detach ipvlan ep") return err diff --git a/src/runtime/virtcontainers/macvlan_endpoint.go b/src/runtime/virtcontainers/macvlan_endpoint.go index 7ffcc582c8..71a223e257 100644 --- a/src/runtime/virtcontainers/macvlan_endpoint.go +++ b/src/runtime/virtcontainers/macvlan_endpoint.go @@ -140,7 +140,7 @@ func (endpoint *MacvlanEndpoint) HotAttach(ctx context.Context, s *Sandbox) erro return nil } -func (endpoint *MacvlanEndpoint) HotDetach(ctx context.Context, h Hypervisor, netNsCreated bool, netNsPath string) error { +func (endpoint *MacvlanEndpoint) HotDetach(ctx context.Context, s *Sandbox, netNsCreated bool, netNsPath string) error { if !netNsCreated { return nil } @@ -154,6 +154,7 @@ func (endpoint *MacvlanEndpoint) HotDetach(ctx context.Context, h Hypervisor, ne networkLogger().WithError(err).Warn("Error un-bridging macvlan ep") } + h := s.hypervisor if _, err := h.HotplugRemoveDevice(ctx, endpoint, NetDev); err != nil { networkLogger().WithError(err).Error("Error detach macvlan ep") return err diff --git a/src/runtime/virtcontainers/macvtap_endpoint.go b/src/runtime/virtcontainers/macvtap_endpoint.go index c53d2603df..27fee59f0b 100644 --- a/src/runtime/virtcontainers/macvtap_endpoint.go +++ b/src/runtime/virtcontainers/macvtap_endpoint.go @@ -98,7 +98,7 @@ func (endpoint *MacvtapEndpoint) HotAttach(ctx context.Context, s *Sandbox) erro } // HotDetach for macvtap endpoint not supported yet -func (endpoint *MacvtapEndpoint) HotDetach(ctx context.Context, h Hypervisor, netNsCreated bool, netNsPath string) error { +func (endpoint *MacvtapEndpoint) HotDetach(ctx context.Context, s *Sandbox, netNsCreated bool, netNsPath string) error { return fmt.Errorf("MacvtapEndpoint does not support Hot detach") } diff --git a/src/runtime/virtcontainers/network_linux.go b/src/runtime/virtcontainers/network_linux.go index 7160fbf678..8346542e8f 100644 --- a/src/runtime/virtcontainers/network_linux.go +++ b/src/runtime/virtcontainers/network_linux.go @@ -265,7 +265,7 @@ func (n *LinuxNetwork) removeSingleEndpoint(ctx context.Context, s *Sandbox, end // if required. networkLogger().WithField("endpoint-type", endpoint.Type()).Info("Detaching endpoint") if hotplug && s != nil { - if err := endpoint.HotDetach(ctx, s.hypervisor, n.netNSCreated, n.netNSPath); err != nil { + if err := endpoint.HotDetach(ctx, s, n.netNSCreated, n.netNSPath); err != nil { return err } } else { diff --git a/src/runtime/virtcontainers/physical_endpoint.go b/src/runtime/virtcontainers/physical_endpoint.go index db098492b6..de6c3cc87a 100644 --- a/src/runtime/virtcontainers/physical_endpoint.go +++ b/src/runtime/virtcontainers/physical_endpoint.go @@ -152,8 +152,36 @@ func (endpoint *PhysicalEndpoint) HotAttach(ctx context.Context, s *Sandbox) err } // HotDetach for physical endpoint not supported yet -func (endpoint *PhysicalEndpoint) HotDetach(ctx context.Context, h Hypervisor, netNsCreated bool, netNsPath string) error { - return fmt.Errorf("PhysicalEndpoint does not support Hot detach") +func (endpoint *PhysicalEndpoint) HotDetach(ctx context.Context, s *Sandbox, netNsCreated bool, netNsPath string) error { + span, _ := physicalTrace(ctx, "HotDetach", endpoint) + defer span.End() + + var vfioPath string + var err error + + if vfioPath, err = drivers.GetVFIODevPath(endpoint.BDF); err != nil { + return err + } + + c, err := resCtrl.DeviceToCgroupDeviceRule(vfioPath) + if err != nil { + return err + } + + d := config.DeviceInfo{ + ContainerPath: vfioPath, + DevType: string(c.Type), + Major: c.Major, + Minor: c.Minor, + ColdPlug: false, + } + + device := s.devManager.FindDevice(&d) + s.devManager.RemoveDevice(device.DeviceID()) + + // We do not need to enter the network namespace to bind back the + // physical interface to host driver. + return bindNICToHost(endpoint) } // isPhysicalIface checks if an interface is a physical device. diff --git a/src/runtime/virtcontainers/physical_endpoint_test.go b/src/runtime/virtcontainers/physical_endpoint_test.go index 5aed350e8e..d3a0966079 100644 --- a/src/runtime/virtcontainers/physical_endpoint_test.go +++ b/src/runtime/virtcontainers/physical_endpoint_test.go @@ -42,9 +42,11 @@ func TestPhysicalEndpoint_HotDetach(t *testing.T) { HardAddr: net.HardwareAddr{0x02, 0x00, 0xca, 0xfe, 0x00, 0x04}.String(), } - h := &mockHypervisor{} + s := &Sandbox{ + hypervisor: &mockHypervisor{}, + } - err := v.HotDetach(context.Background(), h, true, "") + err := v.HotDetach(context.Background(), s, true, "") assert.Error(err) } diff --git a/src/runtime/virtcontainers/tap_endpoint.go b/src/runtime/virtcontainers/tap_endpoint.go index ff7dbe3af0..14ebebfece 100644 --- a/src/runtime/virtcontainers/tap_endpoint.go +++ b/src/runtime/virtcontainers/tap_endpoint.go @@ -112,7 +112,7 @@ func (endpoint *TapEndpoint) HotAttach(ctx context.Context, s *Sandbox) error { } // HotDetach for the tap endpoint uses hot pull device -func (endpoint *TapEndpoint) HotDetach(ctx context.Context, h Hypervisor, netNsCreated bool, netNsPath string) error { +func (endpoint *TapEndpoint) HotDetach(ctx context.Context, s *Sandbox, netNsCreated bool, netNsPath string) error { networkLogger().Info("Hot detaching tap endpoint") span, ctx := tapTrace(ctx, "HotDetach", endpoint) @@ -124,6 +124,7 @@ func (endpoint *TapEndpoint) HotDetach(ctx context.Context, h Hypervisor, netNsC networkLogger().WithError(err).Warn("Error un-bridging tap ep") } + h := s.hypervisor if _, err := h.HotplugRemoveDevice(ctx, endpoint, NetDev); err != nil { networkLogger().WithError(err).Error("Error detach tap ep") return err diff --git a/src/runtime/virtcontainers/tuntap_endpoint.go b/src/runtime/virtcontainers/tuntap_endpoint.go index 3631d3d762..6c67b02360 100644 --- a/src/runtime/virtcontainers/tuntap_endpoint.go +++ b/src/runtime/virtcontainers/tuntap_endpoint.go @@ -123,7 +123,7 @@ func (endpoint *TuntapEndpoint) HotAttach(ctx context.Context, s *Sandbox) error } // HotDetach for the tun/tap endpoint uses hot pull device -func (endpoint *TuntapEndpoint) HotDetach(ctx context.Context, h Hypervisor, netNsCreated bool, netNsPath string) error { +func (endpoint *TuntapEndpoint) HotDetach(ctx context.Context, s *Sandbox, netNsCreated bool, netNsPath string) error { networkLogger().Info("Hot detaching tun/tap endpoint") span, ctx := tuntapTrace(ctx, "HotDetach", endpoint) @@ -135,6 +135,7 @@ func (endpoint *TuntapEndpoint) HotDetach(ctx context.Context, h Hypervisor, net networkLogger().WithError(err).Warn("Error un-bridging tun/tap ep") } + h := s.hypervisor if _, err := h.HotplugRemoveDevice(ctx, endpoint, NetDev); err != nil { networkLogger().WithError(err).Error("Error detach tun/tap ep") return err diff --git a/src/runtime/virtcontainers/veth_endpoint.go b/src/runtime/virtcontainers/veth_endpoint.go index 7102df8c5b..566786d4cb 100644 --- a/src/runtime/virtcontainers/veth_endpoint.go +++ b/src/runtime/virtcontainers/veth_endpoint.go @@ -144,7 +144,7 @@ func (endpoint *VethEndpoint) HotAttach(ctx context.Context, s *Sandbox) error { } // HotDetach for the veth endpoint uses hot pull device -func (endpoint *VethEndpoint) HotDetach(ctx context.Context, h Hypervisor, netNsCreated bool, netNsPath string) error { +func (endpoint *VethEndpoint) HotDetach(ctx context.Context, s *Sandbox, netNsCreated bool, netNsPath string) error { if !netNsCreated { return nil } @@ -158,6 +158,7 @@ func (endpoint *VethEndpoint) HotDetach(ctx context.Context, h Hypervisor, netNs networkLogger().WithError(err).Warn("Error un-bridging virtual ep") } + h := s.hypervisor if _, err := h.HotplugRemoveDevice(ctx, endpoint, NetDev); err != nil { networkLogger().WithError(err).Error("Error detach virtual ep") return err diff --git a/src/runtime/virtcontainers/vhostuser_endpoint.go b/src/runtime/virtcontainers/vhostuser_endpoint.go index ce884fe428..5b079629db 100644 --- a/src/runtime/virtcontainers/vhostuser_endpoint.go +++ b/src/runtime/virtcontainers/vhostuser_endpoint.go @@ -112,7 +112,7 @@ func (endpoint *VhostUserEndpoint) HotAttach(ctx context.Context, s *Sandbox) er } // HotDetach for vhostuser endpoint not supported yet -func (endpoint *VhostUserEndpoint) HotDetach(ctx context.Context, h Hypervisor, netNsCreated bool, netNsPath string) error { +func (endpoint *VhostUserEndpoint) HotDetach(ctx context.Context, s *Sandbox, netNsCreated bool, netNsPath string) error { return fmt.Errorf("VhostUserEndpoint does not support Hot detach") } diff --git a/src/runtime/virtcontainers/vhostuser_endpoint_test.go b/src/runtime/virtcontainers/vhostuser_endpoint_test.go index 0a8448c32b..fe8c171884 100644 --- a/src/runtime/virtcontainers/vhostuser_endpoint_test.go +++ b/src/runtime/virtcontainers/vhostuser_endpoint_test.go @@ -113,9 +113,11 @@ func TestVhostUserEndpoint_HotDetach(t *testing.T) { EndpointType: VhostUserEndpointType, } - h := &mockHypervisor{} + s := &Sandbox{ + hypervisor: &mockHypervisor{}, + } - err := v.HotDetach(context.Background(), h, true, "") + err := v.HotDetach(context.Background(), s, true, "") assert.Error(err) }