diff --git a/Dockerfile b/Dockerfile index ee5d2761..d7019137 100644 --- a/Dockerfile +++ b/Dockerfile @@ -15,7 +15,6 @@ COPY . . RUN CGO_ENABLED=0 go build -o ghwc ./cmd/ghwc/ FROM alpine:3.7@sha256:8421d9a84432575381bfabd248f1eb56f3aa21d9d7cd2511583c68c9b7511d10 -RUN apk add --no-cache ethtool WORKDIR /bin diff --git a/README.md b/README.md index c1cdb106..3819815d 100644 --- a/README.md +++ b/README.md @@ -1379,10 +1379,9 @@ if err := snapshot.PackFrom("my-snapshot.tgz", scratchDir); err != nil { ## Calling external programs -By default `ghw` may call external programs, for example `ethtool`, to learn -about hardware capabilities. In some rare circumstances it may be useful to -opt out from this behaviour and rely only on the data provided by -pseudo-filesystems, like sysfs. +By default ghw may call external programs to learn about hardware capabilities. +We gather information using Go packages and system libraries, but in some cases ghw +may call external tools/binaries, particularly on non-Linux platforms. The most common use case is when we want to read a snapshot from `ghw`. In these cases the information provided by tools will be inconsistent with the diff --git a/go.mod b/go.mod index f52637a5..a415df5d 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,10 @@ require ( github.com/StackExchange/wmi v1.2.1 github.com/jaypipes/pcidb v1.0.1 github.com/pkg/errors v0.9.1 + github.com/safchain/ethtool v0.3.1-0.20240611095507-191590141ec6 github.com/spf13/cobra v1.8.0 + github.com/spf13/pflag v1.0.5 // indirect + gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 // indirect gopkg.in/yaml.v3 v3.0.1 howett.net/plist v1.0.0 ) @@ -16,7 +19,5 @@ require ( github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/kr/pretty v0.1.0 // indirect github.com/mitchellh/go-homedir v1.1.0 // indirect - github.com/spf13/pflag v1.0.5 // indirect - golang.org/x/sys v0.1.0 // indirect - gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 // indirect + golang.org/x/sys v0.21.0 // indirect ) diff --git a/go.sum b/go.sum index cc122227..f5907f26 100644 --- a/go.sum +++ b/go.sum @@ -19,13 +19,15 @@ github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrk github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= +github.com/safchain/ethtool v0.3.1-0.20240611095507-191590141ec6 h1:EDGd3d1JQDq5BFMZOp4ePK1M6Om9ZGhfh/LJfrjiyEQ= +github.com/safchain/ethtool v0.3.1-0.20240611095507-191590141ec6/go.mod h1:XLLnZmy4OCRTkksP/UiMjij96YmIsBfmBQcs7H6tA48= github.com/spf13/cobra v1.8.0 h1:7aJaZx1B85qltLMc546zn58BxxfZdR/W22ej9CFoEf0= github.com/spf13/cobra v1.8.0/go.mod h1:WXLWApfZ71AjXPya3WOlMsY9yMs7YeiHhFVlvLyhcho= github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= golang.org/x/sys v0.0.0-20190916202348-b4ddaad3f8a3/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.1.0 h1:kunALQeHf1/185U1i0GOB/fy1IPRDDpuoOOqRReG57U= -golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.21.0 h1:rF+pYz3DAGSQAxAu1CbC7catZg4ebC4UIeIhKxBZvws= +golang.org/x/sys v0.21.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 h1:qIbj1fsPNlZgppZ+VLlY7N33q108Sa+fhmuc+sWQYwY= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/pkg/net/net_linux.go b/pkg/net/net_linux.go index d7d7e8ca..7e028189 100644 --- a/pkg/net/net_linux.go +++ b/pkg/net/net_linux.go @@ -6,21 +6,14 @@ package net import ( - "bufio" - "bytes" - "fmt" "os" - "os/exec" "path/filepath" "strings" + "github.com/safchain/ethtool" + "github.com/jaypipes/ghw/pkg/context" "github.com/jaypipes/ghw/pkg/linuxpath" - "github.com/jaypipes/ghw/pkg/util" -) - -const ( - warnEthtoolNotInstalled = `ethtool not installed. Cannot grab NIC capabilities` ) func (i *Info) load() error { @@ -37,14 +30,6 @@ func nics(ctx *context.Context) []*NIC { return nics } - etAvailable := ctx.EnableTools - if etAvailable { - if etInstalled := ethtoolInstalled(); !etInstalled { - ctx.Warn(warnEthtoolNotInstalled) - etAvailable = false - } - } - 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 + nic.MacAddress = mac // keep this for backward compatibility nic.MACAddress = mac - if etAvailable { - nic.netDeviceParseEthtool(ctx, filename) - } else { - nic.Capabilities = []*NICCapability{} - // Sets NIC struct fields from data in SysFs - nic.setNicAttrSysFs(paths, filename) - } - + // 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 ethtoolInstalled() bool { - _, err := exec.LookPath("ethtool") - return err == nil -} - -func (n *NIC) netDeviceParseEthtool(ctx *context.Context, dev string) { - var out bytes.Buffer - path, _ := exec.LookPath("ethtool") - - // Get auto-negotiation and pause-frame-use capabilities from "ethtool" (with no options) - // Populate Speed, Duplex, SupportedLinkModes, SupportedPorts, SupportedFECModes, - // AdvertisedLinkModes, and AdvertisedFECModes attributes from "ethtool" output. - cmd := exec.Command(path, dev) - cmd.Stdout = &out - err := cmd.Run() - if err == nil { - m := parseNicAttrEthtool(&out) - n.Capabilities = append(n.Capabilities, autoNegCap(m)) - n.Capabilities = append(n.Capabilities, pauseFrameUseCap(m)) - - // Update NIC Attributes with ethtool output - n.Speed = strings.Join(m["Speed"], "") - n.Duplex = strings.Join(m["Duplex"], "") - n.SupportedLinkModes = m["Supported link modes"] - n.SupportedPorts = m["Supported ports"] - n.SupportedFECModes = m["Supported FEC modes"] - n.AdvertisedLinkModes = m["Advertised link modes"] - n.AdvertisedFECModes = m["Advertised FEC modes"] - } else { - msg := fmt.Sprintf("could not grab NIC link info for %s: %s", dev, err) - ctx.Warn(msg) +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{} } - - // Get all other capabilities from "ethtool -k" - cmd = exec.Command(path, "-k", dev) - cmd.Stdout = &out - err = cmd.Run() - if err == nil { - // The out variable will now contain something that looks like the - // following. - // - // Features for enp58s0f1: - // rx-checksumming: on - // tx-checksumming: off - // tx-checksum-ipv4: off - // tx-checksum-ip-generic: off [fixed] - // tx-checksum-ipv6: off - // tx-checksum-fcoe-crc: off [fixed] - // tx-checksum-sctp: off [fixed] - // scatter-gather: off - // tx-scatter-gather: off - // tx-scatter-gather-fraglist: off [fixed] - // tcp-segmentation-offload: off - // tx-tcp-segmentation: off - // tx-tcp-ecn-segmentation: off [fixed] - // tx-tcp-mangleid-segmentation: off - // tx-tcp6-segmentation: off - // < snipped > - scanner := bufio.NewScanner(&out) - // Skip the first line... - scanner.Scan() - for scanner.Scan() { - line := strings.TrimPrefix(scanner.Text(), "\t") - n.Capabilities = append(n.Capabilities, netParseEthtoolFeature(line)) - } - - } else { - msg := fmt.Sprintf("could not grab NIC capabilities for %s: %s", dev, err) - ctx.Warn(msg) + 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) } -// netParseEthtoolFeature parses a line from the ethtool -k output and returns -// a NICCapability. -// -// The supplied line will look like the following: -// -// tx-checksum-ip-generic: off [fixed] -// -// [fixed] indicates that the feature may not be turned on/off. Note: it makes -// no difference whether a privileged user runs `ethtool -k` when determining -// whether [fixed] appears for a feature. -func netParseEthtoolFeature(line string) *NICCapability { - parts := strings.Fields(line) - cap := strings.TrimSuffix(parts[0], ":") - enabled := parts[1] == "on" - fixed := len(parts) == 3 && parts[2] == "[fixed]" - return &NICCapability{ - Name: cap, - IsEnabled: enabled, - CanEnable: !fixed, +// 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,12 +172,6 @@ func netDevicePCIAddress(netDevDir, netDevName string) *string { return &pciAddr } -func (nic *NIC) setNicAttrSysFs(paths *linuxpath.Paths, dev string) { - // Get speed and duplex from /sys/class/net/$DEVICE/ directory - nic.Speed = readFile(filepath.Join(paths.SysClassNet, dev, "speed")) - nic.Duplex = readFile(filepath.Join(paths.SysClassNet, dev, "duplex")) -} - func readFile(path string) string { contents, err := os.ReadFile(path) if err != nil { @@ -262,97 +179,3 @@ func readFile(path string) string { } return strings.TrimSpace(string(contents)) } - -func autoNegCap(m map[string][]string) *NICCapability { - autoNegotiation := NICCapability{Name: "auto-negotiation", IsEnabled: false, CanEnable: false} - - an, anErr := util.ParseBool(strings.Join(m["Auto-negotiation"], "")) - aan, aanErr := util.ParseBool(strings.Join(m["Advertised auto-negotiation"], "")) - if an && aan && aanErr == nil && anErr == nil { - autoNegotiation.IsEnabled = true - } - - san, err := util.ParseBool(strings.Join(m["Supports auto-negotiation"], "")) - if san && err == nil { - autoNegotiation.CanEnable = true - } - - return &autoNegotiation -} - -func pauseFrameUseCap(m map[string][]string) *NICCapability { - pauseFrameUse := NICCapability{Name: "pause-frame-use", IsEnabled: false, CanEnable: false} - - apfu, err := util.ParseBool(strings.Join(m["Advertised pause frame use"], "")) - if apfu && err == nil { - pauseFrameUse.IsEnabled = true - } - - spfu, err := util.ParseBool(strings.Join(m["Supports pause frame use"], "")) - if spfu && err == nil { - pauseFrameUse.CanEnable = true - } - - return &pauseFrameUse -} - -func parseNicAttrEthtool(out *bytes.Buffer) map[string][]string { - // The out variable will now contain something that looks like the - // following. - // - //Settings for eth0: - // Supported ports: [ TP ] - // Supported link modes: 10baseT/Half 10baseT/Full - // 100baseT/Half 100baseT/Full - // 1000baseT/Full - // Supported pause frame use: No - // Supports auto-negotiation: Yes - // Supported FEC modes: Not reported - // Advertised link modes: 10baseT/Half 10baseT/Full - // 100baseT/Half 100baseT/Full - // 1000baseT/Full - // Advertised pause frame use: No - // Advertised auto-negotiation: Yes - // Advertised FEC modes: Not reported - // Speed: 1000Mb/s - // Duplex: Full - // Auto-negotiation: on - // Port: Twisted Pair - // PHYAD: 1 - // Transceiver: internal - // MDI-X: off (auto) - // Supports Wake-on: pumbg - // Wake-on: d - // Current message level: 0x00000007 (7) - // drv probe link - // Link detected: yes - - scanner := bufio.NewScanner(out) - // Skip the first line - scanner.Scan() - m := make(map[string][]string) - var name string - for scanner.Scan() { - var fields []string - if strings.Contains(scanner.Text(), ":") { - line := strings.Split(scanner.Text(), ":") - name = strings.TrimSpace(line[0]) - str := strings.Trim(strings.TrimSpace(line[1]), "[]") - switch str { - case - "Not reported", - "Unknown": - continue - } - fields = strings.Fields(str) - } else { - fields = strings.Fields(strings.Trim(strings.TrimSpace(scanner.Text()), "[]")) - } - - for _, f := range fields { - m[name] = append(m[name], strings.TrimSpace(f)) - } - } - - return m -} diff --git a/pkg/net/net_linux_test.go b/pkg/net/net_linux_test.go index 3de5bdb4..71066e80 100644 --- a/pkg/net/net_linux_test.go +++ b/pkg/net/net_linux_test.go @@ -10,141 +10,136 @@ package net import ( - "bytes" - "os" + "fmt" "reflect" - "strings" + "sort" "testing" + + "github.com/safchain/ethtool" ) -func TestParseEthtoolFeature(t *testing.T) { - if _, ok := os.LookupEnv("GHW_TESTING_SKIP_NET"); ok { - t.Skip("Skipping network tests.") +func TestAlwaysFailUntilWeFigureOutTheOutputDifferences(t *testing.T) { + t.Fail() +} + +func TestNetDeviceCapabilitiesFromEthHandle(t *testing.T) { + type testCase struct { + name string + dev string + // dev(string) -> map[string]FeatureState + feats map[string]map[string]ethtool.FeatureState + // dev(string) -> error + errs map[string]error + expected []*NICCapability + expectedError error } - tests := []struct { - line string - expected *NICCapability - }{ + testCases := []testCase{ { - line: "scatter-gather: off", - expected: &NICCapability{ - Name: "scatter-gather", - IsEnabled: false, - CanEnable: true, - }, + name: "nil data", + dev: "foodev", + expectedError: fmt.Errorf("unsupported device: foodev"), }, { - line: "scatter-gather: on", - expected: &NICCapability{ - Name: "scatter-gather", - IsEnabled: true, - CanEnable: true, + name: "empty data", + dev: "foodev", + feats: map[string]map[string]ethtool.FeatureState{ + "foodev": map[string]ethtool.FeatureState{}, }, + expected: []*NICCapability{}, }, { - line: "scatter-gather: off [fixed]", - expected: &NICCapability{ - Name: "scatter-gather", - IsEnabled: false, - CanEnable: false, + name: "minimal data", + dev: "foodev", + feats: map[string]map[string]ethtool.FeatureState{ + "foodev": map[string]ethtool.FeatureState{ + "foo": ethtool.FeatureState{ + Available: true, + NeverChanged: true, + }, + "bar": ethtool.FeatureState{ + Available: true, + Requested: true, + }, + }, }, - }, - } - - for x, test := range tests { - actual := netParseEthtoolFeature(test.line) - if !reflect.DeepEqual(test.expected, actual) { - t.Fatalf("In test %d, expected %v == %v", x, test.expected, actual) - } - } -} - -func TestParseNicAttrEthtool(t *testing.T) { - if _, ok := os.LookupEnv("GHW_TESTING_SKIP_NET"); ok { - t.Skip("Skipping network tests.") - } - - tests := []struct { - input string - expected *NIC - }{ - { - input: `Settings for eth0: - Supported ports: [ TP ] - Supported link modes: 10baseT/Half 10baseT/Full - 100baseT/Half 100baseT/Full - 1000baseT/Full - Supported pause frame use: No - Supports auto-negotiation: Yes - Supported FEC modes: Not reported - Advertised link modes: 10baseT/Half 10baseT/Full - 100baseT/Half 100baseT/Full - 1000baseT/Full - Advertised pause frame use: No - Advertised auto-negotiation: Yes - Advertised FEC modes: Not reported - Speed: 1000Mb/s - Duplex: Full - Auto-negotiation: on - Port: Twisted Pair - PHYAD: 1 - Transceiver: internal - MDI-X: off (auto) - Supports Wake-on: pumbg - Wake-on: d - Current message level: 0x00000007 (7) - drv probe link - Link detected: yes -`, - expected: &NIC{ - Speed: "1000Mb/s", - Duplex: "Full", - SupportedPorts: []string{"TP"}, - AdvertisedLinkModes: []string{ - "10baseT/Half", - "10baseT/Full", - "100baseT/Half", - "100baseT/Full", - "1000baseT/Full", + expected: []*NICCapability{ + { + Name: "bar", + CanEnable: true, }, - SupportedLinkModes: []string{ - "10baseT/Half", - "10baseT/Full", - "100baseT/Half", - "100baseT/Full", - "1000baseT/Full", + { + Name: "foo", + CanEnable: true, }, - Capabilities: []*NICCapability{ - { - Name: "auto-negotiation", - IsEnabled: true, - CanEnable: true, + }, + }, + { + name: "minimal data and errors", + dev: "foodev", + feats: map[string]map[string]ethtool.FeatureState{ + "foodev": map[string]ethtool.FeatureState{ + "foo": ethtool.FeatureState{ + Available: true, + NeverChanged: true, }, - { - Name: "pause-frame-use", - IsEnabled: false, - CanEnable: false, + "bar": ethtool.FeatureState{ + Available: true, + Requested: true, }, }, }, + errs: map[string]error{ + "foodev": fmt.Errorf("fake error"), + }, + expected: nil, + expectedError: fmt.Errorf("fake error"), + }, + { + name: "only errors", + dev: "foodev", + feats: map[string]map[string]ethtool.FeatureState{}, + errs: map[string]error{ + "foodev": fmt.Errorf("fake error"), + }, + expected: nil, + expectedError: fmt.Errorf("fake error"), }, } - for x, test := range tests { - m := parseNicAttrEthtool(bytes.NewBufferString(test.input)) - actual := &NIC{} - actual.Speed = strings.Join(m["Speed"], "") - actual.Duplex = strings.Join(m["Duplex"], "") - actual.SupportedLinkModes = m["Supported link modes"] - actual.SupportedPorts = m["Supported ports"] - actual.SupportedFECModes = m["Supported FEC modes"] - actual.AdvertisedLinkModes = m["Advertised link modes"] - actual.AdvertisedFECModes = m["Advertised FEC modes"] - actual.Capabilities = append(actual.Capabilities, autoNegCap(m)) - actual.Capabilities = append(actual.Capabilities, pauseFrameUseCap(m)) - if !reflect.DeepEqual(test.expected, actual) { - t.Fatalf("In test %d\nExpected:\n%+v\nActual:\n%+v\n", x, *test.expected, *actual) - } + for _, tCase := range testCases { + t.Run(tCase.name, func(t *testing.T) { + fc := fakeCollector{ + feats: tCase.feats, + errs: tCase.errs, + } + got, err := netDeviceCapabilitiesFromEthHandle(fc, tCase.dev) + if (err != nil) != (tCase.expectedError != nil) { + t.Fatalf("got error %v expected error %v", err, tCase.expectedError) + } + sort.Slice(got, func(i, j int) bool { + return got[i].Name < got[j].Name + }) + if !reflect.DeepEqual(got, tCase.expected) { + t.Errorf("got %v expected %v", got, tCase.expected) + } + }) + } + +} + +type fakeCollector struct { + // dev(string) -> map[string]FeatureState + feats map[string]map[string]ethtool.FeatureState + errs map[string]error +} + +func (fc fakeCollector) FeaturesWithState(intf string) (map[string]ethtool.FeatureState, error) { + if err, ok := fc.errs[intf]; ok { + return nil, err + } + if feats, ok := fc.feats[intf]; ok { + return feats, nil } + return nil, fmt.Errorf("unsupported device: %s", intf) }