net: replace the ethtool external program calls with the safchain/ethtool package
#321
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,21 +6,14 @@ | |
| package net | ||
|
|
||
| import ( | ||
| "os" | ||
| "path/filepath" | ||
| "strings" | ||
|
|
||
| "github.com/safchain/ethtool" | ||
|
|
||
| "github.com/jaypipes/ghw/pkg/context" | ||
| "github.com/jaypipes/ghw/pkg/linuxpath" | ||
| ) | ||
|
|
||
| func (i *Info) load() error { | ||
|
|
@@ -37,14 +30,6 @@ func nics(ctx *context.Context) []*NIC { | |
| return nics | ||
| } | ||
|
|
||
| for _, file := range files { | ||
| filename := file.Name() | ||
| // Ignore loopback... | ||
|
|
@@ -65,16 +50,12 @@ func nics(ctx *context.Context) []*NIC { | |
| } | ||
|
|
||
| mac := netDeviceMacAddress(paths, filename) | ||
| nic.MacAddress = mac // keep this for backward compatibility | ||
| nic.MACAddress = mac | ||
| // Get speed and duplex from /sys/class/net/$DEVICE/ directory | ||
| nic.Speed = readFile(filepath.Join(paths.SysClassNet, filename, "speed")) | ||
| nic.Duplex = readFile(filepath.Join(paths.SysClassNet, filename, "duplex")) | ||
| nic.Capabilities = netDeviceCapabilities(ctx, filename) | ||
| nic.PCIAddress = netDevicePCIAddress(paths.SysClassNet, filename) | ||
|
|
||
| nics = append(nics, nic) | ||
|
|
@@ -103,99 +84,41 @@ func netDeviceMacAddress(paths *linuxpath.Paths, dev string) string { | |
| return strings.TrimSpace(string(contents)) | ||
| } | ||
|
|
||
| func netDeviceCapabilities(ctx *context.Context, dev string) []*NICCapability { | ||
| ethHandle, err := ethtool.NewEthtool() | ||
| if err != nil { | ||
| ctx.Warn("failed to create ethtool instance: %v", err) | ||
| return []*NICCapability{} | ||
| } | ||
|
Comment on lines
-147
to
-162
Owner
There was a problem hiding this comment. Are the capability strings exactly the same with the ethtool library as they were with the raw parsing of the
Collaborator
Author
There was a problem hiding this comment. right, pretty bad oversight on my side. Let me add a test which ensure this. On the code side, if this is not already the case, we can add a thin translation layer.
Owner
There was a problem hiding this comment. @ffromani I was only making sure :) it may be that the ethtool library does use the exact same strings. but if not, we'll need to add a translation table to ensure backwards compat.
Collaborator
Author
There was a problem hiding this comment. OK, so the ethtool program gets the identifiers from the kernel using the ioctl interface: https://kernel.googlesource.com/pub/scm/network/ethtool/ethtool/+/refs/heads/ethtool-3.4.y/ethtool.c#1310 https://docs.kernel.org/networking/ethtool-netlink.html#strset-get And so it does the ethtool package: https://github.com/safchain/ethtool/blob/master/ethtool.go#L733 Hence the names should be exactly the same, unless it is the |
||
| defer ethHandle.Close() | ||
| feats, err := netDeviceCapabilitiesFromEthHandle(ethHandle, dev) | ||
| if err != nil { | ||
| ctx.Warn(err.Error()) | ||
| return []*NICCapability{} | ||
| } | ||
| return feats | ||
| } | ||
|
|
||
| type ethtoolCollector interface { | ||
| FeaturesWithState(intf string) (map[string]ethtool.FeatureState, error) | ||
| } | ||
|
|
||
| // make it mockable for test purposes | ||
| func netDeviceCapabilitiesFromEthHandle(collector ethtoolCollector, dev string) ([]*NICCapability, error) { | ||
| feats, err := collector.FeaturesWithState(dev) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| caps := []*NICCapability{} | ||
| for key, state := range feats { | ||
| caps = append(caps, &NICCapability{ | ||
| Name: key, | ||
| IsEnabled: state.Active, | ||
| CanEnable: state.Available, | ||
| }) | ||
| } | ||
| return caps, err | ||
| } | ||
|
|
||
| func netDevicePCIAddress(netDevDir, netDevName string) *string { | ||
|
|
@@ -249,110 +172,10 @@ func netDevicePCIAddress(netDevDir, netDevName string) *string { | |
| return &pciAddr | ||
| } | ||
|
|
||
| func readFile(path string) string { | ||
| contents, err := os.ReadFile(path) | ||
| if err != nil { | ||
| return "" | ||
| } | ||
| return strings.TrimSpace(string(contents)) | ||
| } | ||
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.