From 261389741672eb4a8114cdff4bcecbad76cd9f97 Mon Sep 17 00:00:00 2001
From: Andrea Panattoni <apanatto@redhat.com>
Date: Mon, 10 Jun 2024 14:00:35 +0200
Subject: [PATCH] Avoid reconfiguring unmentioned DPDK VFs

If a Virtual Function is configured with a DPDK driver (e.g. `vfio-pci`) and it is not
referred by any SriovNetworkNodePolicy, `NeedToUpdateSriov` function must not
trigger a  reconfiguration. This may happen if a PF is configured by multiple policies
(via PF partitioning) and a policy is deleted by the user. In these cases, the VF is not
reconfigured [1] and a drain loop is started

The same logic applies to VDPA devices.

refs:
[1] https://github.com/k8snetworkplumbingwg/sriov-network-operator/blob/5f3c4e903f789aa177fe54686efd6c18576b7ab1/pkg/host/internal/sriov/sriov.go#L457

Signed-off-by: Andrea Panattoni <apanatto@redhat.com>
---
 api/v1/helper.go      |  8 -----
 api/v1/helper_test.go | 68 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 68 insertions(+), 8 deletions(-)

diff --git a/api/v1/helper.go b/api/v1/helper.go
index 782240ba8..8c51d4530 100644
--- a/api/v1/helper.go
+++ b/api/v1/helper.go
@@ -280,10 +280,8 @@ func NeedToUpdateSriov(ifaceSpec *Interface, ifaceStatus *InterfaceExt) bool {
 
 	if ifaceSpec.NumVfs > 0 {
 		for _, vfStatus := range ifaceStatus.VFs {
-			ingroup := false
 			for _, groupSpec := range ifaceSpec.VfGroups {
 				if IndexInRange(vfStatus.VfID, groupSpec.VfRange) {
-					ingroup = true
 					if vfStatus.Driver == "" {
 						log.V(2).Info("NeedToUpdateSriov(): Driver needs update - has no driver",
 							"desired", groupSpec.DeviceType)
@@ -332,12 +330,6 @@ func NeedToUpdateSriov(ifaceSpec *Interface, ifaceStatus *InterfaceExt) bool {
 					break
 				}
 			}
-			if !ingroup && (StringInArray(vfStatus.Driver, vars.DpdkDrivers) || vfStatus.VdpaType != "") {
-				// need to reset VF if it is not a part of a group and:
-				// a. has DPDK driver loaded
-				// b. has VDPA device
-				return true
-			}
 		}
 	}
 	return false
diff --git a/api/v1/helper_test.go b/api/v1/helper_test.go
index cae4e2a00..4c899bcc9 100644
--- a/api/v1/helper_test.go
+++ b/api/v1/helper_test.go
@@ -1054,3 +1054,71 @@ func TestSriovNetworkPoolConfig_MaxUnavailable(t *testing.T) {
 		})
 	}
 }
+
+func TestNeedToUpdateSriov(t *testing.T) {
+	type args struct {
+		ifaceSpec   *v1.Interface
+		ifaceStatus *v1.InterfaceExt
+	}
+	tests := []struct {
+		name string
+		args args
+		want bool
+	}{
+		{
+			name: "number of VFs changed",
+			args: args{
+				ifaceSpec:   &v1.Interface{NumVfs: 1},
+				ifaceStatus: &v1.InterfaceExt{NumVfs: 0},
+			},
+			want: true,
+		},
+		{
+			name: "no update",
+			args: args{
+				ifaceSpec:   &v1.Interface{NumVfs: 1},
+				ifaceStatus: &v1.InterfaceExt{NumVfs: 1},
+			},
+			want: false,
+		},
+		{
+			name: "vfio-pci VF is not configured for any group",
+			args: args{
+				ifaceSpec: &v1.Interface{
+					NumVfs: 3,
+					VfGroups: []v1.VfGroup{
+						{
+							VfRange:    "1-2",
+							DeviceType: consts.DeviceTypeNetDevice,
+						},
+					},
+				},
+				ifaceStatus: &v1.InterfaceExt{
+					NumVfs: 3,
+					VFs: []v1.VirtualFunction{
+						{
+							VfID:   0,
+							Driver: "vfio-pci",
+						},
+						{
+							VfID:   1,
+							Driver: "iavf",
+						},
+						{
+							VfID:   2,
+							Driver: "iavf",
+						},
+					},
+				},
+			},
+			want: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := v1.NeedToUpdateSriov(tt.args.ifaceSpec, tt.args.ifaceStatus); got != tt.want {
+				t.Errorf("NeedToUpdateSriov() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}