[switchdev 5/x] Add skipVFconfiguration option for generic_plugin #614
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 7929101016Details
💛 - Coveralls |
ykulazhenkov
force-pushed
the
pr-phased-plugins
branch
from
e84e770 to
038c296
Compare
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
ykulazhenkov
force-pushed
the
pr-phased-plugins
branch
from
038c296 to
bcb1885
Compare
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
ykulazhenkov
force-pushed
the
pr-phased-plugins
branch
from
bcb1885 to
8af0d92
Compare
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
ykulazhenkov
changed the title
ykulazhenkov
force-pushed
the
pr-phased-plugins
branch
from
8af0d92 to
d4aaea7
Compare
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
zeeke
reviewed
Feb 13, 2024
pkg/host/internal/sriov/sriov.go
Outdated
| return nil | ||
| } | ||
|
|
||
| func (s *sriov) ConfigSriovDevice(iface *sriovnetworkv1.Interface, ifaceStatus *sriovnetworkv1.InterfaceExt, skipVFConfiguration bool) error { |
There was a problem hiding this comment.
can we make this function private and remove it from the interface?
There was a problem hiding this comment.
Sounds good. I made the function private.
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
ykulazhenkov
force-pushed
the
pr-phased-plugins
branch
from
8f2a378 to
83208d8
Compare
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
ykulazhenkov
force-pushed
the
pr-phased-plugins
branch
from
83208d8 to
cafbfef
Compare
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
|
@zeeke thx for the review, I addressed your comment. |
pkg/host/internal/sriov/sriov.go
Outdated
| return nil | ||
| } | ||
|
|
||
| func (s *sriov) configSriovDevice(iface *sriovnetworkv1.Interface, ifaceStatus *sriovnetworkv1.InterfaceExt, skipVFConfiguration bool) error { |
There was a problem hiding this comment.
still need to give this one a bit more thought ...
There was a problem hiding this comment.
We had an offline discussion about it. latest iteration is OK imo.
(had some concerns about how this plays out with Externally managed)
ykulazhenkov
force-pushed
the
pr-phased-plugins
branch
from
cafbfef to
0668e47
Compare
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
|
I updated PR to avoid usage of the information from the status field during the configuration. The status field may can contain outdated information because, for example, eswitch mode or VF count was changed by the previous step of the configuration. |
ykulazhenkov
force-pushed
the
pr-phased-plugins
branch
from
0668e47 to
96b29d7
Compare
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
ykulazhenkov
force-pushed
the
pr-phased-plugins
branch
from
96b29d7 to
8035f32
Compare
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
|
overall im LGTM. lets have a final pass after rebase. |
Signed-off-by: Yury Kulazhenkov <[email protected]>
Signed-off-by: Yury Kulazhenkov <[email protected]>
If skipVFConfiguration flag is set, the function will configure PF and create VFs on it, but will skip VFs configuration. This change is required to be able to use this function in the pre configuration phase in systemd mode. Signed-off-by: Yury Kulazhenkov <[email protected]>
Signed-off-by: Yury Kulazhenkov <[email protected]>
If WithSkipVFConfiguration() option is passed during the plugin initialization, then the plugin will configure PFs and create VFs, but will skip VF configuration phase. This is required to support pre configuration phase in systemd mode. Signed-off-by: Yury Kulazhenkov <[email protected]>
During the pre phase the plugin should be called with WithSkipVFConfiguration option to prevent VF configuration. In the post phase plugin should be executed as is. Signed-off-by: Yury Kulazhenkov <[email protected]>
ykulazhenkov
force-pushed
the
pr-phased-plugins
branch
from
8035f32 to
2fe88a3
Compare
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
| @@ -258,8 +258,8 @@ func (s *sriov) DiscoverSriovDevices(storeManager store.ManagerInterface) ([]sri | |||
| iface.Name = name | |||
| iface.Mac = s.networkHelper.GetNetDevMac(name) | |||
| iface.LinkSpeed = s.networkHelper.GetNetDevLinkSpeed(name) | |||
| iface.LinkType = s.GetLinkType(name) | |||
There was a problem hiding this comment.
we can get mtu, mac, link type from netlink Link obj and avoid a bunch of file operations.
not related to this PR. i will submit a small refactor to DiscoverSriovDevices.
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.
Add skipVFconfiguration option for generic_plugin
If
WithSkipVFConfiguration()option is passed during the plugin initialization, then the plugin will configure PFs and create VFs, but will skip VF configuration phase.This option is used by the systemd service during the
prephase. At this point we only need to create VFs but not configure them(e.g. bind to driver, create VDPA device, etc)In the
postphase the plugin will be called without this option and as a result will execute full plugin flow, meaning it will re-check that VFs are create and will continue configurations of VFs.cc @adrianchiris @zeeke @SchSeba