Merge pull request openshift#820 from liornoy/sync-05-09-2023

Downstream Sync 2023-09-05

.github/workflows/go.yml

@@ -9,10 +9,10 @@ jobs:
     runs-on: ubuntu-latest
     steps:
 
-    - name: Set up Go 1.19
+    - name: Set up Go 1.20
       uses: actions/setup-go@v3
       with:
-        go-version: 1.19.x
+        go-version: 1.20.x
 
     - name: Check out code into the Go module directory
       uses: actions/checkout@v2
@@ -28,10 +28,10 @@ jobs:
     runs-on: ubuntu-latest
     steps:
 
-    - name: Set up Go 1.19
+    - name: Set up Go 1.20
       uses: actions/setup-go@v2
       with:
-        go-version: 1.19.x
+        go-version: 1.20.x
 
     - name: Check out code into the Go module directory
       uses: actions/checkout@v2
@@ -49,27 +49,27 @@ jobs:
     name: Golangci-lint
     runs-on: ubuntu-latest
     steps:
-    - name: Set up Go 1.19
+    - name: Set up Go 1.20
       uses: actions/setup-go@v2
       with:
-        go-version: 1.19.x
+        go-version: 1.20.x
     - name: Check out code into the Go module directory
       uses: actions/checkout@v2
     - name: golangci-lint
       uses: golangci/golangci-lint-action@v3
       with:
         # Required: the version of golangci-lint is required and must be specified without patch version: we always use the latest patch version.
-        version: v1.46
+        version: v1.51.0
 
   test-coverage:
     name: test-coverage
     runs-on: ubuntu-latest
     steps:
 
-    - name: Set up Go 1.19
+    - name: Set up Go 1.20
       uses: actions/setup-go@v2
       with:
-        go-version: 1.19.x
+        go-version: 1.20.x
 
     - name: Check out code into the Go module directory
       uses: actions/checkout@v2

.golangci.yml

@@ -75,7 +75,6 @@ linters:
   disable-all: true
   enable:
     - bodyclose
-    - deadcode
     - depguard
     - dogsled
     # TODO fix issues- dupl
@@ -99,16 +98,13 @@ linters:
     - misspell
     # TODO fix issues- nakedret
     # TODO fix issues- prealloc
-    - rowserrcheck
       #- scopelint
     # TODO fix issues- staticcheck
-    - structcheck
     - stylecheck
     - typecheck
     - unconvert
     # TODO fix issues- unparam
     - unused
-    - varcheck
     - whitespace
 
 issues:

Dockerfile

@@ -1,4 +1,4 @@
-FROM golang:1.19 AS builder
+FROM golang:1.20 AS builder
 WORKDIR /go/src/github.com/k8snetworkplumbingwg/sriov-network-operator
 COPY . .
 RUN make _build-manager BIN_PATH=build/_output/cmd

Dockerfile.sriov-network-config-daemon

@@ -1,4 +1,4 @@
-FROM golang:1.19 AS builder
+FROM golang:1.20 AS builder
 WORKDIR /go/src/github.com/k8snetworkplumbingwg/sriov-network-operator
 COPY . .
 RUN make _build-sriov-network-config-daemon BIN_PATH=build/_output/cmd

Dockerfile.webhook

@@ -1,4 +1,4 @@
-FROM golang:1.19 AS builder
+FROM golang:1.20 AS builder
 WORKDIR /go/src/github.com/k8snetworkplumbingwg/sriov-network-operator
 COPY . .
 RUN make _build-webhook BIN_PATH=build/_output/cmd

Makefile

@@ -51,7 +51,7 @@ GOLANGCI_LINT = $(BIN_DIR)/golangci-lint
 # golangci-lint version should be updated periodically
 # we keep it fixed to avoid it from unexpectedly failing on the project
 # in case of a version bump
-GOLANGCI_LINT_VER = v1.46.1
+GOLANGCI_LINT_VER = v1.51.0
 
 GOLANGCI_LINT = $(BIN_DIR)/golangci-lint
 # golangci-lint version should be updated periodically

api/v1/helper_test.go

@@ -94,14 +94,37 @@ func newNodePolicy() *v1.SriovNetworkNodePolicy {
 	}
 }
 
-func newVdpaNodePolicy() *v1.SriovNetworkNodePolicy {
+func newVirtioVdpaNodePolicy() *v1.SriovNetworkNodePolicy {
 	return &v1.SriovNetworkNodePolicy{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "p1",
 		},
 		Spec: v1.SriovNetworkNodePolicySpec{
 			DeviceType: consts.DeviceTypeNetDevice,
 			VdpaType:   consts.VdpaTypeVirtio,
+			NicSelector: v1.SriovNetworkNicSelector{
+				PfNames:     []string{"ens803f1#2-3"},
+				RootDevices: []string{"0000:86:00.1"},
+				Vendor:      "8086",
+			},
+			NodeSelector: map[string]string{
+				"feature.node.kubernetes.io/network-sriov.capable": "true",
+			},
+			NumVfs:       4,
+			Priority:     99,
+			ResourceName: "virtiovdpa",
+		},
+	}
+}
+
+func newVhostVdpaNodePolicy() *v1.SriovNetworkNodePolicy {
+	return &v1.SriovNetworkNodePolicy{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "p1",
+		},
+		Spec: v1.SriovNetworkNodePolicySpec{
+			DeviceType: consts.DeviceTypeNetDevice,
+			VdpaType:   consts.VdpaTypeVhost,
 			NicSelector: v1.SriovNetworkNicSelector{
 				PfNames:     []string{"ens803f1"},
 				RootDevices: []string{"0000:86:00.1"},
@@ -112,7 +135,7 @@ func newVdpaNodePolicy() *v1.SriovNetworkNodePolicy {
 			},
 			NumVfs:       2,
 			Priority:     99,
-			ResourceName: "p1res",
+			ResourceName: "vhostvdpa",
 		},
 	}
 }
@@ -437,6 +460,55 @@ func TestSriovNetworkNodePolicyApply(t *testing.T) {
 				},
 			},
 		},
+		{
+			// vdpa policy with same priority (both virtio and vhost), VfRange's do not overlap so all is merged
+			tname: "one vdpa policy present same pf same priority partitioning",
+			currentState: func() *v1.SriovNetworkNodeState {
+				st := newNodeState()
+				st.Spec.Interfaces = []v1.Interface{
+					{
+						Name:       "ens803f1",
+						NumVfs:     4,
+						PciAddress: "0000:86:00.1",
+						VfGroups: []v1.VfGroup{
+							{
+								DeviceType:   consts.DeviceTypeNetDevice,
+								VdpaType:     consts.VdpaTypeVhost,
+								ResourceName: "vhostvdpa",
+								VfRange:      "0-1",
+								PolicyName:   "p2",
+							},
+						},
+					},
+				}
+				return st
+			}(),
+			policy: newVirtioVdpaNodePolicy(),
+			equalP: true,
+			expectedInterfaces: []v1.Interface{
+				{
+					Name:       "ens803f1",
+					NumVfs:     4,
+					PciAddress: "0000:86:00.1",
+					VfGroups: []v1.VfGroup{
+						{
+							DeviceType:   consts.DeviceTypeNetDevice,
+							VdpaType:     consts.VdpaTypeVirtio,
+							ResourceName: "virtiovdpa",
+							VfRange:      "2-3",
+							PolicyName:   "p1",
+						},
+						{
+							DeviceType:   consts.DeviceTypeNetDevice,
+							VdpaType:     consts.VdpaTypeVhost,
+							ResourceName: "vhostvdpa",
+							VfRange:      "0-1",
+							PolicyName:   "p2",
+						},
+					},
+				},
+			},
+		},
 		{
 			// policy with same priority that overwrites the 2 present groups in
 			// SriovNetworkNodeState because they overlap VfRange
@@ -637,7 +709,7 @@ func TestSriovNetworkNodePolicyApply(t *testing.T) {
 	}
 }
 
-func TestVdpaNodePolicyApply(t *testing.T) {
+func TestVirtioVdpaNodePolicyApply(t *testing.T) {
 	testtable := []struct {
 		tname              string
 		currentState       *v1.SriovNetworkNodeState
@@ -647,20 +719,67 @@ func TestVdpaNodePolicyApply(t *testing.T) {
 		expectedErr        bool
 	}{
 		{
-			tname:        "vdpa configuration",
+			tname:        "virtio/vdpa configuration",
 			currentState: newNodeState(),
-			policy:       newVdpaNodePolicy(),
+			policy:       newVirtioVdpaNodePolicy(),
 			equalP:       false,
 			expectedInterfaces: []v1.Interface{
 				{
 					Name:       "ens803f1",
-					NumVfs:     2,
+					NumVfs:     4,
 					PciAddress: "0000:86:00.1",
 					VfGroups: []v1.VfGroup{
 						{
 							DeviceType:   consts.DeviceTypeNetDevice,
 							VdpaType:     consts.VdpaTypeVirtio,
-							ResourceName: "p1res",
+							ResourceName: "virtiovdpa",
+							VfRange:      "2-3",
+							PolicyName:   "p1",
+						},
+					},
+				},
+			},
+		},
+	}
+	for _, tc := range testtable {
+		t.Run(tc.tname, func(t *testing.T) {
+			err := tc.policy.Apply(tc.currentState, tc.equalP)
+			if tc.expectedErr && err == nil {
+				t.Errorf("Apply expecting error.")
+			} else if !tc.expectedErr && err != nil {
+				t.Errorf("Apply error:\n%s", err)
+			}
+			if diff := cmp.Diff(tc.expectedInterfaces, tc.currentState.Spec.Interfaces); diff != "" {
+				t.Errorf("SriovNetworkNodeState spec diff (-want +got):\n%s", diff)
+			}
+		})
+	}
+}
+
+func TestVhostVdpaNodePolicyApply(t *testing.T) {
+	testtable := []struct {
+		tname              string
+		currentState       *v1.SriovNetworkNodeState
+		policy             *v1.SriovNetworkNodePolicy
+		expectedInterfaces v1.Interfaces
+		equalP             bool
+		expectedErr        bool
+	}{
+		{
+			tname:        "vhost/vdpa configuration",
+			currentState: newNodeState(),
+			policy:       newVhostVdpaNodePolicy(),
+			equalP:       false,
+			expectedInterfaces: []v1.Interface{
+				{
+					Name:       "ens803f1",
+					NumVfs:     2,
+					PciAddress: "0000:86:00.1",
+					VfGroups: []v1.VfGroup{
+						{
+							DeviceType:   consts.DeviceTypeNetDevice,
+							VdpaType:     consts.VdpaTypeVhost,
+							ResourceName: "vhostvdpa",
 							VfRange:      "0-1",
 							PolicyName:   "p1",
 						},

api/v1/sriovnetworknodepolicy_types.go

@@ -54,8 +54,8 @@ type SriovNetworkNodePolicySpec struct {
 	// +kubebuilder:validation:Enum=legacy;switchdev
 	// NIC Device Mode. Allowed value "legacy","switchdev".
 	EswitchMode string `json:"eSwitchMode,omitempty"`
-	// +kubebuilder:validation:Enum=virtio
-	// VDPA device type. Allowed value "virtio"
+	// +kubebuilder:validation:Enum=virtio;vhost
+	// VDPA device type. Allowed value "virtio", "vhost"
 	VdpaType string `json:"vdpaType,omitempty"`
 	// Exclude device's NUMA node when advertising this resource by SRIOV network device plugin. Default to false.
 	ExcludeTopology bool `json:"excludeTopology,omitempty"`

bindata/manifests/switchdev-config/files/switchdev-configuration-after-nm.sh.yaml

@@ -58,9 +58,17 @@ contents:
           do
             extract_min_max_ids
             vdpaType=$(jq -c '.vdpaType' -r <<< $group)
-            if [ $vfid -le $maxId ] && [ $vfid -ge $minId ] && [ $vdpaType == "virtio" ]; then
-              vdpa_cmd="vdpa dev add name vdpa:"${VfPciAddr}" mgmtdev pci/"${VfPciAddr}
+            if [ $vfid -le $maxId ] && [ $vfid -ge $minId ] && ([ $vdpaType == "virtio" ] || [ $vdpaType == "vhost" ]); then
+              vdpa_name=vdpa:${VfPciAddr}
+              vdpa_cmd="vdpa dev add name "${vdpa_name}" mgmtdev pci/"${VfPciAddr}" max_vqp 32"
               eval $vdpa_cmd
+              # set the driver_override. When the specified driver will be loaded in the kernel
+              # it will be automatically bound to the vdpa device
+              driver_override=virtio_vdpa
+              if [ $vdpaType == "vhost" ]; then
+                driver_override=vhost_vdpa
+              fi
+              echo $driver_override > /sys/bus/vdpa/devices/$vdpa_name/driver_override
             fi
           done
         done

config/crd/bases/sriovnetwork.openshift.io_sriovnetworknodepolicies.yaml

@@ -117,9 +117,10 @@ spec:
                 description: SRIOV Network device plugin endpoint resource name
                 type: string
               vdpaType:
-                description: VDPA device type. Allowed value "virtio"
+                description: VDPA device type. Allowed value "virtio", "vhost"
                 enum:
                 - virtio
+                - vhost
                 type: string
             required:
             - nicSelector

controllers/sriovibnetwork_controller.go

@@ -30,7 +30,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
-	"sigs.k8s.io/controller-runtime/pkg/source"
 
 	sriovnetworkv1 "github.com/k8snetworkplumbingwg/sriov-network-operator/api/v1"
 )
@@ -175,6 +174,6 @@ func (r *SriovIBNetworkReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 func (r *SriovIBNetworkReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&sriovnetworkv1.SriovIBNetwork{}).
-		Watches(&source.Kind{Type: &netattdefv1.NetworkAttachmentDefinition{}}, &handler.EnqueueRequestForObject{}).
+		Watches(&netattdefv1.NetworkAttachmentDefinition{}, &handler.EnqueueRequestForObject{}).
 		Complete(r)
 }

controllers/sriovnetwork_controller.go

@@ -31,7 +31,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
-	"sigs.k8s.io/controller-runtime/pkg/source"
 
 	sriovnetworkv1 "github.com/k8snetworkplumbingwg/sriov-network-operator/api/v1"
 )
@@ -176,6 +175,6 @@ func (r *SriovNetworkReconciler) Reconcile(ctx context.Context, req ctrl.Request
 func (r *SriovNetworkReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&sriovnetworkv1.SriovNetwork{}).
-		Watches(&source.Kind{Type: &netattdefv1.NetworkAttachmentDefinition{}}, &handler.EnqueueRequestForObject{}).
+		Watches(&netattdefv1.NetworkAttachmentDefinition{}, &handler.EnqueueRequestForObject{}).
 		Complete(r)
 }