On the Ubuntu builders, the Linux integration tests fail:

=== RUN   TestIntegrationClientLinkInfos
    client_linux_integration_test.go:21: failed to open client: netlink receive: no such file or directory
--- FAIL: TestIntegrationClientLinkInfos (0.00s)
=== RUN   TestIntegrationClientNetlinkStrict
    client_linux_integration_test.go:42: failed to open client: netlink receive: no such file or directory
--- FAIL: TestIntegrationClientNetlinkStrict (0.00s)

Full log: https://launchpadlibrarian.net/638901184/buildlog_ubuntu-lunar-amd64.golang-github-mdlayher-ethtool_0.0~git20221205.a2a5239-1_BUILDING.txt.gz

Test fails as follows on Ubuntu 22.10 (kernel 5.19) and Debian bookworm (kernel 6.0, as of writing):

=== RUN   TestIntegrationClientLinkInfos
    client_linux_integration_test.go:23: failed to fetch link infos: netlink receive: operation not supported
--- FAIL: TestIntegrationClientLinkInfos (0.00s)

A git bisect reveals that the test started failing from commit 07f2ecdc165a478e1c8d93e0d17201498ab3acf5 onwards.

This failing test is preventing the packaging of this project in Debian, and subsequently the updating of the prometheus-node-exporter package, whose version 1.5.0 depends on this module.

Another test that fails on Debian CI infrastructure (s390x):

=== RUN   TestFEC
    client_linux_test.go:893: unexpected request header bytes (-want +got):
          []uint8{
        - 	0x10, 0x00, 0x01, 0x80, 0x0b, 0x00, 0x02, 0x00,                                                 // -|........|
        + 	0x00, 0x10, 0x80, 0x01, 0x00, 0x0b, 0x00, 0x02,                                                 // +|........|
          	0x65, 0x6e, 0x70, 0x37, 0x73, 0x30, 0x00, 0x00,                                                 //  |enp7s0..|
        - 	0x18, 0x00, 0x02, 0x80, 0x04, 0x00, 0x01, 0x00, 0x10, 0x00, 0x03, 0x80, 0x0c, 0x00, 0x01, 0x80, // -|................|
        - 	0x07, 0x00, 0x02, 0x00, 0x52, 0x53,                                                             // -|....RS|
        + 	0x00, 0x18, 0x80, 0x02, 0x00, 0x04, 0x00, 0x01, 0x00, 0x10, 0x80, 0x03, 0x00, 0x0c, 0x80, 0x01, // +|................|
        + 	0x00, 0x07, 0x00, 0x02, 0x52, 0x53, 0x00,                                                       // +|....RS.|
          	0x00, 0x00, 0x05, 0x00, 0x03,                                                                   //  |.....|
        - 	0x00,                                                                                           // -|.|
          	0x00, 0x00, 0x00, 0x00,                                                                         //  |....|
          }
--- FAIL: TestFEC (0.00s)

s390x is big-endian, and the byte ordering in the "got" response looks almost certainly like this is a wrong endianness decode issue.

Some network card and SFP module combinations (e.g. Mellanox ConnectX-4 with a Flexoptix P.B1625G.10.AD) need to explicitly be configured to use RS forward error correction, otherwise they won’t link.

I changed the get() function to only set the COMPACT_BITSETS flag for query commands so that the resulting request byte sequence matches the ethtool(8) byte sequence 1:1 (see TestFEC). Functionally, there is no change in behavior, as can be verified by looking at the corresponding Linux kernel code :)

Another test that fails on Debian CI infrastructure (s390x):

=== RUN   TestLinuxClientLinkModes
=== RUN   TestLinuxClientLinkModes/OK
    client_linux_test.go:292: unexpected link mode (-want +got):
          []*ethtool.LinkMode{
          	&{
          		Interface:     {Index: 1, Name: "eth0"},
          		SpeedMegabits: 1000,
          		Ours: []ethtool.AdvertisedLinkMode{
        - 			{Index: 4, Name: "1000baseT/Half"},
        + 			{Index: 28, Name: "56000baseCR4/Full"},
        - 			{Index: 5, Name: "1000baseT/Full"},
        + 			{Index: 29, Name: "56000baseSR4/Full"},
          		},
          		Peer:   nil,
          		Duplex: s"Half",
          	},
          	&{
          		Interface:     {Index: 2, Name: "eth1"},
          		SpeedMegabits: 10000,
          		Ours: []ethtool.AdvertisedLinkMode{
        - 			{Index: 10, Name: "FIBRE"},
        + 			{Index: 18, Name: "10000baseKX4/Full"},
        - 			{Index: 12, Name: "10000baseT/Full"},
        + 			{Index: 20, Name: "10000baseR/FEC"},
          		},
          		Peer:   nil,
          		Duplex: s"Full",
          	},
          }
--- FAIL: TestLinuxClientLinkModes (0.00s)
    --- FAIL: TestLinuxClientLinkModes/OK (0.00s)

s390x is big-endian, so possibly this is an endian decode issue.

It looks like ethtool can support querying a link to find out what kind of time-stamping capabilities it has with the -T option.

> ethtool -T enp1s0
Time stamping parameters for enp1s0:
Capabilities:
        hardware-transmit     (SOF_TIMESTAMPING_TX_HARDWARE)
        software-transmit     (SOF_TIMESTAMPING_TX_SOFTWARE)
        hardware-receive      (SOF_TIMESTAMPING_RX_HARDWARE)
        software-receive      (SOF_TIMESTAMPING_RX_SOFTWARE)
        software-system-clock (SOF_TIMESTAMPING_SOFTWARE)
        hardware-raw-clock    (SOF_TIMESTAMPING_RAW_HARDWARE)
PTP Hardware Clock: 0
Hardware Transmit Timestamp Modes:
        off                   (HWTSTAMP_TX_OFF)
        on                    (HWTSTAMP_TX_ON)
Hardware Receive Filter Modes:
        none                  (HWTSTAMP_FILTER_NONE)
        all                   (HWTSTAMP_FILTER_ALL)

The results of this query could then be used in setsockopt to enable these capabilities when using sendmsg and recvmsg and then parsing the control message that gets returned to get timestamps of userspace, kernel, and hardware. Enabling these options would work with https://pkg.go.dev/github.com/mdlayher/socket as long as Recvmsg and Sendmsg are used and there is a utility function for assisting in parsing the control message, but this is outside the scope of this issue.

Would you be open to adding support for this query? I propose something along the lines of:

// TimestampInfo holds the decoded response from `ethtool -T`
// NOTE: I am not suggesting that this is the struct we should use, but
// it is similar to how I have a working version already.
type TimestampInfo struct {
  HardwareRx bool
  HardwareTx bool

  SoftwareRx bool
  SoftwareTx bool

  SoftwareSystem bool
  HardwareRaw bool

  PTPClockIndex int
}

// TimestampingInfo essentiall does `ethtool -T DEV` and decodes the response into *TimestampInfo
func (c *Client) TimestampingInfo(ifi Interface) (*TimestampInfo, error) {
  // TBD
}

Additional References:

• https://kernel.googlesource.com/pub/scm/network/ethtool/ethtool/+/refs/tags/v5.14/ethtool.c#1653
• https://www.kernel.org/doc/html/latest/networking/timestamping.html

This proposes an interface for partial updates of link mode information. Tests coming up: wanted to make sure the overall design was acceptable first. The motivation behind disjoining LinkMode and LinkModeUpdate is that:

1. The sets of readable and writable fields are not the same, e.g. ETHTOOL_A_LINKMODES_MASTER_SLAVE_STATE is only readable and ETHTOOL_A_LINKMODES_LANES is only writable per docs.
2. This allows for a natural (for golang) partial update interface via nullable fields.

One point on which I waver is whether slices should have encoding/json-type semantics (nil <> empty slice) as proposed here, or perhaps be pointers like everything else. Let me know what you think.

The folks at Talos Systems (CC @smira) are setting up a manual ethtool genetlink watcher, but it'd be very reasonable for us to provide a nicer interface in this package for handling that functionality.

See https://github.com/talos-systems/talos/blob/5811f4dda1b62848eefae9be56e8b91d443f4d34/internal/app/machined/pkg/controllers/network/watch/ethtool.go.