From d0b571510fe5e8f7494857ddad2762f89f009ee7 Mon Sep 17 00:00:00 2001 From: Jacob Young Date: Thu, 27 Apr 2023 05:13:46 -0400 Subject: [PATCH 1/8] Add Speed, Duplex, SupportedLinkModes, SupportedPorts, SupportedFECModes, AdvertisedLinkModes, AdvertisedFECModes to NIC struct. Update test case to validate new fields. Update README. Signed-off-by: Jacob Young --- README.md | 14 +++++++++++ pkg/net/net.go | 19 ++++++++++---- pkg/net/net_linux.go | 23 +++++++++++------ pkg/net/net_linux_test.go | 52 +++++++++++++++++++++++++++++---------- 4 files changed, 83 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index 410e36e9..c14d08af 100644 --- a/README.md +++ b/README.md @@ -715,6 +715,20 @@ Each `ghw.NIC` struct contains the following fields: * `ghw.NIC.PCIAddress` is the PCI device address of the device backing the NIC. this is not-nil only if the backing device is indeed a PCI device; more backing devices (e.g. USB) will be added in future versions. +* `ghw.NIC.Speed` is a string showing the current link speed. This + field will be present even if ethtool is not available. +* `ghw.NIC.Duplex` is a string showing the current link duplex. This + field will be present even if ethtool is not available. +* `ghw.NIC.SupportedLinkModes` is a string slice containing a list of + supported link modes +* `ghw.NIC.SupportedPorts` is a string slice containing the list of + supported port types (MII, TP, FIBRE) +* `ghw.NIC.SupportedFECModes` is a string slice containing a list of + supported FEC Modes. +* `ghw.NIC.AdvertisedLinkModes` is a string slice containing the + link modes being advertised during auto negotiation. +* `ghw.NIC.AdvertisedFECModes` is a string slice containing the FEC + modes advertised during auto negotiation. The `ghw.NICCapability` struct contains the following fields: diff --git a/pkg/net/net.go b/pkg/net/net.go index dfc1dc52..2d7855a0 100644 --- a/pkg/net/net.go +++ b/pkg/net/net.go @@ -21,11 +21,20 @@ type NICCapability struct { } type NIC struct { - Name string `json:"name"` - MacAddress string `json:"mac_address"` - IsVirtual bool `json:"is_virtual"` - Capabilities []*NICCapability `json:"capabilities"` - PCIAddress *string `json:"pci_address,omitempty"` + Name string `json:"name"` + MacAddress string `json:"mac_address"` + IsVirtual bool `json:"is_virtual"` + Capabilities []*NICCapability `json:"capabilities"` + PCIAddress *string `json:"pci_address,omitempty"` + Speed string `json:"speed"` + Duplex string `json:"duplex"` + SupportedLinkModes []string `json:"supported_link_modes,omitempty"` + SupportedPorts []string `json:"supported_ports,omitempty"` + SupportedFECModes []string `json:"supported_fec_modes,omitempty"` + AdvertisedLinkModes []string `json:"advertised_link_modes,omitempty"` + AdvertisedFECModes []string `json:"advertised_fec_modes,omitempty"` + SupportedWakeOnModes []string `json:"supported_wake_on_modes,omitempty"` + AdvertisedWakeOnModes []string `json:"advertised_wake_on_modes,omitempty"` // TODO(fromani): add other hw addresses (USB) when we support them } diff --git a/pkg/net/net_linux.go b/pkg/net/net_linux.go index 6d4300f8..ee4ba09c 100644 --- a/pkg/net/net_linux.go +++ b/pkg/net/net_linux.go @@ -68,7 +68,7 @@ func nics(ctx *context.Context) []*NIC { mac := netDeviceMacAddress(paths, filename) nic.MacAddress = mac if etAvailable { - nic.Capabilities = netDeviceCapabilities(ctx, filename) + nic.netDeviceParseEthtool(ctx, filename) } else { nic.Capabilities = []*NICCapability{} } @@ -106,19 +106,29 @@ func ethtoolInstalled() bool { return err == nil } -func netDeviceCapabilities(ctx *context.Context, dev string) []*NICCapability { - caps := make([]*NICCapability, 0) +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) - caps = append(caps, autoNegCap(m)) - caps = append(caps, pauseFrameUseCap(m)) + 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) @@ -154,7 +164,7 @@ func netDeviceCapabilities(ctx *context.Context, dev string) []*NICCapability { scanner.Scan() for scanner.Scan() { line := strings.TrimPrefix(scanner.Text(), "\t") - caps = append(caps, netParseEthtoolFeature(line)) + n.Capabilities = append(n.Capabilities, netParseEthtoolFeature(line)) } } else { @@ -162,7 +172,6 @@ func netDeviceCapabilities(ctx *context.Context, dev string) []*NICCapability { ctx.Warn(msg) } - return caps } diff --git a/pkg/net/net_linux_test.go b/pkg/net/net_linux_test.go index 6da777e4..6539b091 100644 --- a/pkg/net/net_linux_test.go +++ b/pkg/net/net_linux_test.go @@ -66,7 +66,7 @@ func TestParseNicAttrEthtool(t *testing.T) { tests := []struct { input string - expected []*NICCapability + expected *NIC }{ { input: `Settings for eth0: @@ -96,16 +96,35 @@ func TestParseNicAttrEthtool(t *testing.T) { drv probe link Link detected: yes `, - expected: []*NICCapability{ - { - Name: "auto-negotiation", - IsEnabled: true, - CanEnable: true, + expected: &NIC{ + Speed: "1000Mb/s", + Duplex: "Full", + SupportedPorts: []string{"TP"}, + AdvertisedLinkModes: []string{ + "10baseT/Half", + "10baseT/Full", + "100baseT/Half", + "100baseT/Full", + "1000baseT/Full", }, - { - Name: "pause-frame-use", - IsEnabled: false, - CanEnable: false, + SupportedLinkModes: []string{ + "10baseT/Half", + "10baseT/Full", + "100baseT/Half", + "100baseT/Full", + "1000baseT/Full", + }, + Capabilities: []*NICCapability{ + { + Name: "auto-negotiation", + IsEnabled: true, + CanEnable: true, + }, + { + Name: "pause-frame-use", + IsEnabled: false, + CanEnable: false, + }, }, }, }, @@ -113,9 +132,16 @@ func TestParseNicAttrEthtool(t *testing.T) { for x, test := range tests { m := parseNicAttrEthtool(bytes.NewBufferString(test.input)) - actual := make([]*NICCapability, 0) - actual = append(actual, autoNegCap(m)) - actual = append(actual, pauseFrameUseCap(m)) + 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) } From 53f970423ed7d7449b2a467236dabfbb2c025b38 Mon Sep 17 00:00:00 2001 From: Jacob Young Date: Thu, 27 Apr 2023 05:20:15 -0400 Subject: [PATCH 2/8] Get NIC Speed and Duplex from SysFs if ethtool is not available. Update ParseBool to return `false` for empty strings. Signed-off-by: Jacob Young --- pkg/net/net_linux.go | 34 ++++++++++++++++++++++++++++++++++ pkg/util/util.go | 3 +++ pkg/util/util_test.go | 4 ++++ 3 files changed, 41 insertions(+) diff --git a/pkg/net/net_linux.go b/pkg/net/net_linux.go index ee4ba09c..b9950106 100644 --- a/pkg/net/net_linux.go +++ b/pkg/net/net_linux.go @@ -71,6 +71,8 @@ func nics(ctx *context.Context) []*NIC { nic.netDeviceParseEthtool(ctx, filename) } else { nic.Capabilities = []*NICCapability{} + // Sets NIC struct fields from data in SysFs + nic.setNicAttrSysFs(paths, filename) } nic.PCIAddress = netDevicePCIAddress(paths.SysClassNet, filename) @@ -248,6 +250,38 @@ 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 := ioutil.ReadFile(path) + if err != nil { + return "" + } + return strings.TrimSpace(string(contents)) +} + +func (nic *NIC) setNicAttrEthtool(ctx *context.Context, dev string) error { + path, _ := exec.LookPath("ethtool") + cmd := exec.Command(path, dev) + var out bytes.Buffer + cmd.Stdout = &out + err := cmd.Run() + if err != nil { + msg := fmt.Sprintf("could not grab NIC link info for %s: %s", dev, err) + ctx.Warn(msg) + return err + } + + m := parseNicAttrEthtool(&out) + nic.updateNicAttrEthtool(m) + + return nil +} + func autoNegCap(m map[string][]string) *NICCapability { autoNegotiation := NICCapability{Name: "auto-negotiation", IsEnabled: false, CanEnable: false} diff --git a/pkg/util/util.go b/pkg/util/util.go index 5824f2f1..5d57bda2 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -71,6 +71,9 @@ func ParseBool(str string) (bool, error) { "off": false, "yes": true, "no": false, + // Return false instead of an error on empty strings + // For example from empty files in SysClassNet/Device + "": false, } if b, ok := ExtraBools[strings.ToLower(str)]; ok { return b, nil diff --git a/pkg/util/util_test.go b/pkg/util/util_test.go index e249a393..77f5f325 100644 --- a/pkg/util/util_test.go +++ b/pkg/util/util_test.go @@ -75,6 +75,10 @@ func TestParseBool(t *testing.T) { item: "1", expected: true, }, + { + item: "", + expected: false, + }, { item: "on", expected: true, From 4ee98654e91e379c13114f961884ad165e14838a Mon Sep 17 00:00:00 2001 From: Jacob Young Date: Mon, 1 May 2023 11:51:56 -0400 Subject: [PATCH 3/8] Update README Signed-off-by: Jacob Young --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index c14d08af..dbcb4a3a 100644 --- a/README.md +++ b/README.md @@ -711,7 +711,8 @@ Each `ghw.NIC` struct contains the following fields: device * `ghw.NIC.Capabilities` is an array of pointers to `ghw.NICCapability` structs that can describe the things the NIC supports. These capabilities match the - returned values from the `ethtool -k ` call on Linux + returned values from the `ethtool -k ` call on Linux as well as the + AutoNegotiation and PauseFrameUse capabilities from `ethtool`. * `ghw.NIC.PCIAddress` is the PCI device address of the device backing the NIC. this is not-nil only if the backing device is indeed a PCI device; more backing devices (e.g. USB) will be added in future versions. From 203daa7ffbe4a439de771196a76a29a2d278e749 Mon Sep 17 00:00:00 2001 From: Jacob Young Date: Wed, 10 May 2023 21:43:45 -0400 Subject: [PATCH 4/8] Clarify NIC.Speed in README.md Co-authored-by: Jay Pipes Signed-off-by: Jacob Young --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index dbcb4a3a..40c6ad63 100644 --- a/README.md +++ b/README.md @@ -716,8 +716,8 @@ Each `ghw.NIC` struct contains the following fields: * `ghw.NIC.PCIAddress` is the PCI device address of the device backing the NIC. this is not-nil only if the backing device is indeed a PCI device; more backing devices (e.g. USB) will be added in future versions. -* `ghw.NIC.Speed` is a string showing the current link speed. This - field will be present even if ethtool is not available. +* `ghw.NIC.Speed` is a string showing the current link speed. On Linux, this + field will be present even if `ethtool` is not available. * `ghw.NIC.Duplex` is a string showing the current link duplex. This field will be present even if ethtool is not available. * `ghw.NIC.SupportedLinkModes` is a string slice containing a list of From 5891895a6e3b2c68b5421524b070c02e93fc632f Mon Sep 17 00:00:00 2001 From: Jacob Young Date: Wed, 10 May 2023 21:44:07 -0400 Subject: [PATCH 5/8] Clarify NIC.Duplex in README.md Co-authored-by: Jay Pipes Signed-off-by: Jacob Young --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 40c6ad63..e8e184e3 100644 --- a/README.md +++ b/README.md @@ -718,8 +718,8 @@ Each `ghw.NIC` struct contains the following fields: devices (e.g. USB) will be added in future versions. * `ghw.NIC.Speed` is a string showing the current link speed. On Linux, this field will be present even if `ethtool` is not available. -* `ghw.NIC.Duplex` is a string showing the current link duplex. This - field will be present even if ethtool is not available. +* `ghw.NIC.Duplex` is a string showing the current link duplex. On Linux, this + field will be present even if `ethtool` is not available. * `ghw.NIC.SupportedLinkModes` is a string slice containing a list of supported link modes * `ghw.NIC.SupportedPorts` is a string slice containing the list of From e344f1f4e1c2dcedad6b318f6898251bb9efae20 Mon Sep 17 00:00:00 2001 From: Jacob Young Date: Wed, 10 May 2023 21:49:19 -0400 Subject: [PATCH 6/8] Remove refactored function Signed-off-by: Jacob Young --- pkg/net/net_linux.go | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/pkg/net/net_linux.go b/pkg/net/net_linux.go index b9950106..cbdea304 100644 --- a/pkg/net/net_linux.go +++ b/pkg/net/net_linux.go @@ -174,7 +174,6 @@ func (n *NIC) netDeviceParseEthtool(ctx *context.Context, dev string) { ctx.Warn(msg) } - } // netParseEthtoolFeature parses a line from the ethtool -k output and returns @@ -264,24 +263,6 @@ func readFile(path string) string { return strings.TrimSpace(string(contents)) } -func (nic *NIC) setNicAttrEthtool(ctx *context.Context, dev string) error { - path, _ := exec.LookPath("ethtool") - cmd := exec.Command(path, dev) - var out bytes.Buffer - cmd.Stdout = &out - err := cmd.Run() - if err != nil { - msg := fmt.Sprintf("could not grab NIC link info for %s: %s", dev, err) - ctx.Warn(msg) - return err - } - - m := parseNicAttrEthtool(&out) - nic.updateNicAttrEthtool(m) - - return nil -} - func autoNegCap(m map[string][]string) *NICCapability { autoNegotiation := NICCapability{Name: "auto-negotiation", IsEnabled: false, CanEnable: false} From fc6625aed162b8191cbf2641ece1bf3e7adf443e Mon Sep 17 00:00:00 2001 From: Jacob Young Date: Wed, 10 May 2023 22:10:36 -0400 Subject: [PATCH 7/8] missing import and need to dereference variables for error output Signed-off-by: Jacob Young --- pkg/net/net_linux_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/net/net_linux_test.go b/pkg/net/net_linux_test.go index 6539b091..3de5bdb4 100644 --- a/pkg/net/net_linux_test.go +++ b/pkg/net/net_linux_test.go @@ -13,6 +13,7 @@ import ( "bytes" "os" "reflect" + "strings" "testing" ) @@ -143,7 +144,7 @@ func TestParseNicAttrEthtool(t *testing.T) { 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) + t.Fatalf("In test %d\nExpected:\n%+v\nActual:\n%+v\n", x, *test.expected, *actual) } } } From 413cc04a5f48ce1570a2c6287d72b160c467835e Mon Sep 17 00:00:00 2001 From: Jacob Young Date: Wed, 10 May 2023 23:02:01 -0400 Subject: [PATCH 8/8] Getting ahead of myself. Removing WakeOnLAN fields until implemented. Signed-off-by: Jacob Young --- pkg/net/net.go | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/pkg/net/net.go b/pkg/net/net.go index 2d7855a0..82d3226a 100644 --- a/pkg/net/net.go +++ b/pkg/net/net.go @@ -21,20 +21,18 @@ type NICCapability struct { } type NIC struct { - Name string `json:"name"` - MacAddress string `json:"mac_address"` - IsVirtual bool `json:"is_virtual"` - Capabilities []*NICCapability `json:"capabilities"` - PCIAddress *string `json:"pci_address,omitempty"` - Speed string `json:"speed"` - Duplex string `json:"duplex"` - SupportedLinkModes []string `json:"supported_link_modes,omitempty"` - SupportedPorts []string `json:"supported_ports,omitempty"` - SupportedFECModes []string `json:"supported_fec_modes,omitempty"` - AdvertisedLinkModes []string `json:"advertised_link_modes,omitempty"` - AdvertisedFECModes []string `json:"advertised_fec_modes,omitempty"` - SupportedWakeOnModes []string `json:"supported_wake_on_modes,omitempty"` - AdvertisedWakeOnModes []string `json:"advertised_wake_on_modes,omitempty"` + Name string `json:"name"` + MacAddress string `json:"mac_address"` + IsVirtual bool `json:"is_virtual"` + Capabilities []*NICCapability `json:"capabilities"` + PCIAddress *string `json:"pci_address,omitempty"` + Speed string `json:"speed"` + Duplex string `json:"duplex"` + SupportedLinkModes []string `json:"supported_link_modes,omitempty"` + SupportedPorts []string `json:"supported_ports,omitempty"` + SupportedFECModes []string `json:"supported_fec_modes,omitempty"` + AdvertisedLinkModes []string `json:"advertised_link_modes,omitempty"` + AdvertisedFECModes []string `json:"advertised_fec_modes,omitempty"` // TODO(fromani): add other hw addresses (USB) when we support them }