Avoid reconfiguring unmentioned DPDK VFs #710
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
Pull Request Test Coverage Report for Build 9447901897Details
💛 - Coveralls |
zeeke
force-pushed
the
drain-loop-vfio-pci
branch
from
3406e9b
to
349b8ef
Compare
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
Pull Request Test Coverage Report for Build 9449952944Details
💛 - Coveralls |
zeeke
force-pushed
the
drain-loop-vfio-pci
branch
from
349b8ef
to
a02d734
Compare
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
Pull Request Test Coverage Report for Build 9450305059Details
💛 - Coveralls |
Eoghan1232
approved these changes
Jun 11, 2024
zeeke
commented
Jun 13, 2024
api/v1/helper.go
Outdated
| // need to reset VF if it is not a part of a group and: | ||
| // a. has DPDK driver loaded | ||
| // b. has VDPA device | ||
| if !ingroup && vfStatus.VdpaType != "" { |
There was a problem hiding this comment.
@lmilleri @wizhaoredhat I'm thinking about removing also this case. Are you aware of any case in which a VF with VdpaType that is not referenced by any policy should be configured/reset by the operator?
There was a problem hiding this comment.
@zeeke Vdpa devices should be created/delete with policies, they should not be on their own. IIUC with this change, we won't trigger a reconfiguration if vdpa device is not in a policy? If yes, it looks ok to me
There was a problem hiding this comment.
Correct, this if statement checks if a VF has Vdpa but it is not referenced by any policy. The current behavior is to try to reconcile it (not sure which changes are going to be done). I'm proposing to make the operator leave the device as is, as there is no policy that says it should be managed.
There was a problem hiding this comment.
Looks good to me too.
zeeke
force-pushed
the
drain-loop-vfio-pci
branch
from
a02d734
to
2613897
Compare
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
zeeke
force-pushed
the
drain-loop-vfio-pci
branch
from
2613897
to
a5c080d
Compare
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
Pull Request Test Coverage Report for Build 9547721609Details
💛 - Coveralls |
zeeke
added a commit
to zeeke/sriov-network-operator-1
that referenced
this pull request
Jun 18, 2024
Add an end-to-end test to verify removing a partial SriovNetworkNodePolicy does not trigger any issue in the operator's deployment. Refs: - https://issues.redhat.com/browse/OCPBUGS-34934 - k8snetworkplumbingwg#710 Signed-off-by: Andrea Panattoni <[email protected]>
zeeke
force-pushed
the
drain-loop-vfio-pci
branch
from
a5c080d
to
ebc9719
Compare
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
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 <[email protected]>
zeeke
force-pushed
the
drain-loop-vfio-pci
branch
from
ebc9719
to
8b7f22a
Compare
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
Pull Request Test Coverage Report for Build 9677744999Details
💛 - Coveralls |
zeeke
added a commit
to zeeke/sriov-network-operator-1
that referenced
this pull request
Jul 8, 2024
Add an end-to-end test to verify removing a partial SriovNetworkNodePolicy does not trigger any issue in the operator's deployment. Refs: - https://issues.redhat.com/browse/OCPBUGS-34934 - k8snetworkplumbingwg#710 Signed-off-by: Andrea Panattoni <[email protected]>
zeeke
added a commit
to zeeke/sriov-network-operator-1
that referenced
this pull request
Jul 8, 2024
Add an end-to-end test to verify removing a partial SriovNetworkNodePolicy does not trigger any issue in the operator's deployment. Refs: - https://issues.redhat.com/browse/OCPBUGS-34934 - k8snetworkplumbingwg#710 Signed-off-by: Andrea Panattoni <[email protected]>
zeeke
added a commit
to zeeke/sriov-network-operator-1
that referenced
this pull request
Jul 9, 2024
Add an end-to-end test to verify removing a partial SriovNetworkNodePolicy does not trigger any issue in the operator's deployment. Refs: - https://issues.redhat.com/browse/OCPBUGS-34934 - k8snetworkplumbingwg#710 Signed-off-by: Andrea Panattoni <[email protected]>
zeeke
added a commit
to zeeke/sriov-network-operator-1
that referenced
this pull request
Jul 19, 2024
Add an end-to-end test to verify removing a partial SriovNetworkNodePolicy does not trigger any issue in the operator's deployment. Refs: - https://issues.redhat.com/browse/OCPBUGS-34934 - k8snetworkplumbingwg#710 Signed-off-by: Andrea Panattoni <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If a Virtual Function is configured with a DPDK driver (e.g.
vfio-pci) and it is not referred by any SriovNetworkNodePolicy,NeedToUpdateSriovfunction 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 startedrefs:
[1]
sriov-network-operator/pkg/host/internal/sriov/sriov.go
Line 457 in 5f3c4e9