Squashed Together fork.

clarkzinzow · clarkzinzow · commit 963fed649360 · 2025-08-27T18:58:46.000-07:00
Upgrade golangci-lint to work with Go 1.23 Add platform build arg. Comment out Mellanox plugin's draining + rebooting for totalVfs + SRIOV_EN configs, which is buggy. Scan GUIDs (#7) * Squash commits into one * Squash commits into one * Cherry-picked types (build fix) * merge issues fix * read GUID from sysfs * node -> port * args mismatch :( * NAD config fix to include guid * port -> node back * rollback partially * rollback partially * rollback partially * rollback partially * rollback partially * rollback partially * rollback partially * fix * bring back pKey to netAttDef definition * pkey proper location * removed excessive log lines * quotes fix ENG-21048 - KernelArgIommuOn instead of KernelArgIommuPt (to enable ATS & ACS) (#9) KernelArgIommuOn instead of KernelArgIommuPt ENG-19808 mlxfwreset before reboot in SR-IOV operator (#8) * bring reboots back * bring reboots back GUIDSavedInUFM config parameter added camelcase -> snake case
diff --git a/Makefile b/Makefile
@@ -11,7 +11,7 @@ export OPERATOR_EXEC?=oc
 
 BUILD_GOPATH=$(TARGET_DIR):$(TARGET_DIR)/vendor:$(CURPATH)/cmd
 IMAGE_BUILDER?=docker
-IMAGE_BUILD_OPTS?=
+IMAGE_BUILD_OPTS?=--platform linux/amd64
 DOCKERFILE?=Dockerfile
 DOCKERFILE_CONFIG_DAEMON?=Dockerfile.sriov-network-config-daemon
 DOCKERFILE_WEBHOOK?=Dockerfile.webhook
diff --git a/api/v1/helper.go b/api/v1/helper.go
@@ -671,7 +671,13 @@ func (cr *SriovIBNetwork) RenderNetAttDef() (*uns.Unstructured, error) {
 		data.Data["CapabilitiesConfigured"] = true
 		data.Data["SriovCniCapabilities"] = cr.Spec.Capabilities
 	}
-
+	if cr.Spec.PKey == "" {
+		data.Data["pKeyConfigured"] = false
+	} else {
+		data.Data["pKeyConfigured"] = true
+		data.Data["pKey"] = cr.Spec.PKey
+		data.Data["GUIDSavedInUFM"] = false
+	}
 	if cr.Spec.IPAM != "" {
 		data.Data["SriovCniIpam"] = SriovCniIpam + ":" + strings.Join(strings.Fields(cr.Spec.IPAM), "")
 	} else {
@@ -714,6 +720,7 @@ func (cr *SriovNetwork) RenderNetAttDef() (*uns.Unstructured, error) {
 	data := render.MakeRenderData()
 	data.Data["CniType"] = "sriov"
 	data.Data["SriovNetworkName"] = cr.Name
+	data.Data["pKeyConfigured"] = false
 	if cr.Spec.NetworkNamespace == "" {
 		data.Data["SriovNetworkNamespace"] = cr.Namespace
 	} else {
@@ -734,7 +741,6 @@ func (cr *SriovNetwork) RenderNetAttDef() (*uns.Unstructured, error) {
 		data.Data["VlanProtoConfigured"] = true
 		data.Data["SriovCniVlanProto"] = cr.Spec.VlanProto
 	}
-
 	if cr.Spec.Capabilities == "" {
 		data.Data["CapabilitiesConfigured"] = false
 	} else {
@@ -832,6 +838,7 @@ func (cr *OVSNetwork) RenderNetAttDef() (*uns.Unstructured, error) {
 	data := render.MakeRenderData()
 	data.Data["CniType"] = "ovs"
 	data.Data["NetworkName"] = cr.Name
+	data.Data["pKeyConfigured"] = false
 	if cr.Spec.NetworkNamespace == "" {
 		data.Data["NetworkNamespace"] = cr.Namespace
 	} else {
diff --git a/api/v1/sriovibnetwork_types.go b/api/v1/sriovibnetwork_types.go
@@ -43,6 +43,7 @@ type SriovIBNetworkSpec struct {
 	// MetaPluginsConfig configuration to be used in order to chain metaplugins to the sriov interface returned
 	// by the operator.
 	MetaPluginsConfig string `json:"metaPlugins,omitempty"`
+	PKey              string `json:"pKey,omitempty"`
 }
 
 // SriovIBNetworkStatus defines the observed state of SriovIBNetwork
diff --git a/bindata/manifests/cni-config/sriov/sriov-cni-config.yaml b/bindata/manifests/cni-config/sriov/sriov-cni-config.yaml
@@ -38,6 +38,10 @@ spec:
 {{- if .CapabilitiesConfigured -}}
   "capabilities":{{.SriovCniCapabilities}},
 {{- end -}}
+{{- if .pKeyConfigured -}}
+  "pkey":"{{.pKey}}",
+  "guid_saved_in_ufm": "{{.GUIDSavedInUFM}}",
+{{- end -}}
 {{- if .StateConfigured -}}
   "link_state":"{{.SriovCniState}}",
 {{- end -}}
diff --git a/config/crd/bases/sriovnetwork.openshift.io_sriovibnetworks.yaml b/config/crd/bases/sriovnetwork.openshift.io_sriovibnetworks.yaml
@@ -62,6 +62,8 @@ spec:
               networkNamespace:
                 description: Namespace of the NetworkAttachmentDefinition custom resource
                 type: string
+              pKey:
+                type: string
               resourceName:
                 description: SRIOV Network device plugin endpoint resource name
                 type: string
diff --git a/controllers/generic_network_controller.go b/controllers/generic_network_controller.go
@@ -19,6 +19,7 @@ package controllers
 import (
 	"context"
 	"fmt"
+	"strings"
 
 	netattdefv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -278,8 +279,32 @@ func (r *genericNetworkReconciler) deleteNetAttDef(ctx context.Context, cr Netwo
 	if namespace == "" {
 		namespace = cr.GetNamespace()
 	}
-	instance := &netattdefv1.NetworkAttachmentDefinition{ObjectMeta: metav1.ObjectMeta{Name: cr.GetName(), Namespace: namespace}}
-	err := r.Delete(ctx, instance)
+
+	// First get the NetworkAttachmentDefinition to check if GUIDSavedInUFM is True
+	instance := &netattdefv1.NetworkAttachmentDefinition{}
+	err := r.Get(ctx, types.NamespacedName{Name: cr.GetName(), Namespace: namespace}, instance)
+	if err != nil {
+		if errors.IsNotFound(err) {
+			return nil
+		}
+		return err
+	}
+
+	// Check the GUIDSavedInUFM in the spec's config JSON
+	// The CNI config is a JSON string in the spec
+	if instance.Spec.Config != "" {
+		// Check if the config contains "GUIDSavedInUFM": "true"
+		if strings.Contains(instance.Spec.Config, `"GUIDSavedInUFM": "true"`) {
+			// Skip deletion because GUIDSavedInUFM is True
+			logger := log.Log.WithName("deleteNetAttDef")
+			logger.Info("Skipping NetworkAttachmentDefinition deletion because GUIDSavedInUFM is true in spec",
+				"Namespace", namespace, "Name", cr.GetName())
+			return nil
+		}
+	}
+
+	// Proceed with deletion since GUIDSavedInUFM is not True
+	err = r.Delete(ctx, instance)
 	if err != nil {
 		if errors.IsNotFound(err) {
 			return nil
diff --git a/deployment/sriov-network-operator-chart/crds/sriovnetwork.openshift.io_sriovibnetworks.yaml b/deployment/sriov-network-operator-chart/crds/sriovnetwork.openshift.io_sriovibnetworks.yaml
@@ -62,6 +62,8 @@ spec:
               networkNamespace:
                 description: Namespace of the NetworkAttachmentDefinition custom resource
                 type: string
+              pKey:
+                type: string
               resourceName:
                 description: SRIOV Network device plugin endpoint resource name
                 type: string
diff --git a/pkg/consts/constants.go b/pkg/consts/constants.go
@@ -143,6 +143,7 @@ const (
 	KernelArgPciRealloc    = "pci=realloc"
 	KernelArgIntelIommu    = "intel_iommu=on"
 	KernelArgIommuPt       = "iommu=pt"
+	KernelArgIommuOn       = "iommu=on"
 	KernelArgRdmaShared    = "ib_core.netns_mode=1"
 	KernelArgRdmaExclusive = "ib_core.netns_mode=0"
 
diff --git a/pkg/helper/mock/mock_helper.go b/pkg/helper/mock/mock_helper.go
diff --git a/pkg/host/internal/infiniband/infiniband.go b/pkg/host/internal/infiniband/infiniband.go
@@ -5,6 +5,10 @@ import (
 	"fmt"
 	"io/fs"
 	"net"
+	"os"
+	"path/filepath"
+	"strconv"
+	"strings"
 
 	"github.com/vishvananda/netlink"
 	"sigs.k8s.io/controller-runtime/pkg/log"
@@ -55,6 +59,29 @@ func (i *infiniband) ConfigureVfGUID(vfAddr string, pfAddr string, vfID int, pfL
 	return i.applyVfGUIDToInterface(guid, vfAddr, vfID, pfLink)
 }
 
+// GetVfGUID gets a GUID from sysfs for an IB VF device
+func (i *infiniband) GetVfGUID(vfAddr string, pfAddr string, vfID int) (net.HardwareAddr, error) {
+	guidPath := filepath.Join(consts.SysBusPciDevices, pfAddr, "sriov", strconv.Itoa(vfID), "node")
+	data, err := os.ReadFile(guidPath)
+	if err != nil {
+		if os.IsNotExist(err) {
+			log.Log.Info("GetVfGUID(): GUID file doesn't exist", "path", guidPath)
+			return nil, nil
+		}
+		return nil, fmt.Errorf("failed to read GUID file %s: %v", guidPath, err)
+	}
+	guidStr := strings.TrimSpace(string(data))
+	if guidStr == "" {
+		return nil, nil
+	}
+	guid, err := net.ParseMAC(guidStr)
+	if err != nil {
+		return nil, fmt.Errorf("failed to parse GUID %s: %v", guidStr, err)
+	}
+	log.Log.Info("GetVfGUID(): found guid", "guid", guid)
+	return guid, nil
+}
+
 func (i *infiniband) applyVfGUIDToInterface(guid net.HardwareAddr, vfAddr string, vfID int, pfLink netlink.Link) error {
 	if err := i.netlinkLib.LinkSetVfNodeGUID(pfLink, vfID, guid); err != nil {
 		return err
diff --git a/pkg/host/internal/sriov/sriov.go b/pkg/host/internal/sriov/sriov.go
@@ -122,24 +122,30 @@ func (s *sriov) ResetSriovDevice(ifaceStatus sriovnetworkv1.InterfaceExt) error
 	return nil
 }
 
-func (s *sriov) getVfInfo(vfAddr string, pfName string, eswitchMode string, devices []*ghw.PCIDevice) sriovnetworkv1.VirtualFunction {
+func (s *sriov) getVfInfo(vfAddr string, pfAddr string, pfName string, eswitchMode string, devices []*ghw.PCIDevice) sriovnetworkv1.VirtualFunction {
 	driver, err := s.dputilsLib.GetDriverName(vfAddr)
 	if err != nil {
 		log.Log.Error(err, "getVfInfo(): unable to parse device driver", "device", vfAddr)
 	}
-	id, err := s.dputilsLib.GetVFID(vfAddr)
+	vfid, err := s.dputilsLib.GetVFID(vfAddr)
 	if err != nil {
 		log.Log.Error(err, "getVfInfo(): unable to get VF index", "device", vfAddr)
 	}
+	guid, err := s.infinibandHelper.GetVfGUID(vfAddr, pfAddr, vfid)
+	if err != nil {
+		log.Log.Error(err, "GetVfGUID(): unable to get VF GUID", "device", vfAddr)
+	}
+
 	vf := sriovnetworkv1.VirtualFunction{
 		PciAddress: vfAddr,
 		Driver:     driver,
-		VfID:       id,
+		VfID:       vfid,
 		VdpaType:   s.vdpaHelper.DiscoverVDPAType(vfAddr),
+		GUID:       guid.String(),
 	}
 
 	if eswitchMode == sriovnetworkv1.ESwithModeSwitchDev {
-		repName, err := s.sriovnetLib.GetVfRepresentor(pfName, id)
+		repName, err := s.sriovnetLib.GetVfRepresentor(pfName, vfid)
 		if err != nil {
 			log.Log.Error(err, "getVfInfo(): failed to get VF representor name", "device", vfAddr)
 		} else {
@@ -157,7 +163,6 @@ func (s *sriov) getVfInfo(vfAddr string, pfName string, eswitchMode string, devi
 			vf.Mac = link.Attrs().HardwareAddr.String()
 		}
 	}
-	vf.GUID = s.networkHelper.GetNetDevNodeGUID(vfAddr)
 
 	for _, device := range devices {
 		if vfAddr == device.Address {
@@ -300,7 +305,7 @@ func (s *sriov) DiscoverSriovDevices(storeManager store.ManagerInterface) ([]sri
 					continue
 				}
 				for _, vf := range vfs {
-					instance := s.getVfInfo(vf, pfNetName, iface.EswitchMode, devices)
+					instance := s.getVfInfo(vf, iface.PciAddress, pfNetName, iface.EswitchMode, devices)
 					iface.VFs = append(iface.VFs, instance)
 				}
 			}
@@ -488,6 +493,7 @@ func (s *sriov) configSriovVFDevices(iface *sriovnetworkv1.Interface) error {
 					if err := s.infinibandHelper.ConfigureVfGUID(addr, iface.PciAddress, vfID, pfLink); err != nil {
 						return err
 					}
+
 					if err := s.kernelHelper.Unbind(addr); err != nil {
 						return err
 					}
diff --git a/pkg/host/internal/sriov/sriov_test.go b/pkg/host/internal/sriov/sriov_test.go
@@ -88,6 +88,7 @@ var _ = Describe("SRIOV", func() {
 			hostMock.EXPECT().GetNetDevLinkSpeed("enp216s0f0np0").Return("100000 Mb/s")
 			hostMock.EXPECT().GetNetDevLinkAdminState("enp216s0f0np0").Return("up")
 			hostMock.EXPECT().GetNetDevNodeGUID("0000:d8:00.2").Return("guid1")
+			hostMock.EXPECT().GetVfGUID("enp216s0f0np0", "0000:d8:00.0", 0).Return(net.HardwareAddr{}, nil)
 			storeManagerMode.EXPECT().LoadPfsStatus("0000:d8:00.0").Return(nil, false, nil)
 
 			dputilsLibMock.EXPECT().IsSriovPF("0000:d8:00.0").Return(true)
diff --git a/pkg/host/mock/mock_host.go b/pkg/host/mock/mock_host.go
diff --git a/pkg/host/types/interfaces.go b/pkg/host/types/interfaces.go
@@ -1,6 +1,8 @@
 package types
 
 import (
+	"net"
+
 	"github.com/vishvananda/netlink"
 
 	sriovnetworkv1 "github.com/k8snetworkplumbingwg/sriov-network-operator/api/v1"
@@ -193,6 +195,7 @@ type BridgeInterface interface {
 type InfinibandInterface interface {
 	// ConfigureVfGUID configures and sets a GUID for an IB VF device
 	ConfigureVfGUID(vfAddr string, pfAddr string, vfID int, pfLink netlink.Link) error
+	GetVfGUID(vfAddr string, pfAddr string, vfID int) (net.HardwareAddr, error)
 }
 
 type CPUVendor int
diff --git a/pkg/plugins/generic/generic_plugin.go b/pkg/plugins/generic/generic_plugin.go
@@ -120,6 +120,7 @@ func NewGenericPlugin(helpers helper.HostHelpersInterface, options ...Option) (p
 	desiredKernelArgs := KargStateMapType{
 		consts.KernelArgPciRealloc:    helpers.IsKernelArgsSet(kargs, consts.KernelArgPciRealloc),
 		consts.KernelArgIntelIommu:    helpers.IsKernelArgsSet(kargs, consts.KernelArgIntelIommu),
+		consts.KernelArgIommuOn:       helpers.IsKernelArgsSet(kargs, consts.KernelArgIommuOn),
 		consts.KernelArgIommuPt:       helpers.IsKernelArgsSet(kargs, consts.KernelArgIommuPt),
 		consts.KernelArgRdmaShared:    false,
 		consts.KernelArgRdmaExclusive: false,
@@ -425,10 +426,10 @@ func (p *GenericPlugin) addVfioDesiredKernelArg(state *sriovnetworkv1.SriovNetwo
 	kernelArgFnByCPUVendor := map[hostTypes.CPUVendor]func(){
 		hostTypes.CPUVendorIntel: func() {
 			p.enableDesiredKernelArgs(consts.KernelArgIntelIommu)
-			p.enableDesiredKernelArgs(consts.KernelArgIommuPt)
+			p.enableDesiredKernelArgs(consts.KernelArgIommuOn)
 		},
 		hostTypes.CPUVendorAMD: func() {
-			p.enableDesiredKernelArgs(consts.KernelArgIommuPt)
+			p.enableDesiredKernelArgs(consts.KernelArgIommuOn)
 		},
 	}
 

Original file line number	Diff line number	Diff line change
`@@ -43,6 +43,7 @@ type SriovIBNetworkSpec struct {`
`43`	`43`	`// MetaPluginsConfig configuration to be used in order to chain metaplugins to the sriov interface returned`
`44`	`44`	`// by the operator.`
`45`	`45`	MetaPluginsConfig string `json:"metaPlugins,omitempty"`
	`46`	+ PKey string `json:"pKey,omitempty"`
`46`	`47`	`}`
`47`	`48`
`48`	`49`	`// SriovIBNetworkStatus defines the observed state of SriovIBNetwork`