-
Notifications
You must be signed in to change notification settings - Fork 114
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[switchdev 7/x] Add function EnableHwTcOffload to the network pkg #628
[switchdev 7/x] Add function EnableHwTcOffload to the network pkg #628
Conversation
Thanks for your PR,
To skip the vendors CIs use one of:
|
Pull Request Test Coverage Report for Build 8051307951Details
💛 - Coveralls |
c87a5f9
to
5c9c6fa
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
5c9c6fa
to
7861d5b
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
7861d5b
to
a20d5e5
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
will require rebase once [switchdev 6/x] is merged
a20d5e5
to
eb2cc96
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
eb2cc96
to
d00c856
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for addressing my comments
pkg/host/internal/network/network.go
Outdated
return err | ||
} | ||
if _, isKnown := knownFeatures[hwTcOffloadFeatureName]; !isKnown { | ||
log.Log.V(2).Info("EnableHwTcOffload(): can't enable feature, feature is not supported", "device", ifaceName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can log with V(0)
, or it can even return an error.
It really depends on how you're going to use this function, but if the user set a flag somewhere to enable HwTcOffload, it is expected to work or to get an error somewhere. WDYT?
Same for L329
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agree, I fixed the log level
This function ensures that hw-tc-offload feature is enabled for the interface if the interface supports it. relies on github.com/safchain/ethtool lib Signed-off-by: Yury Kulazhenkov <[email protected]>
d00c856
to
154a9ed
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This function ensures that hw-tc-offload feature is enabled for the interface if the interface supports it.
required to be compatible with the current switchdev implementation
uses
github.com/safchain/ethtool
libcc @adrianchiris @SchSeba @zeeke
depends on #633
only last commit to review