From 49a1a2f829bb0421378aed2ec0a77ac17202cb42 Mon Sep 17 00:00:00 2001 From: flamingcow Date: Tue, 4 Aug 2026 11:27:30 -0700 Subject: [PATCH] Read rx errors from the driver stats array and add the pcs counters --- counters.go | 118 ++++++++++++++++++++++++++++++----------------- main.go | 15 +++++- system.go | 130 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 219 insertions(+), 44 deletions(-) diff --git a/counters.go b/counters.go index daf3659..4ce8f7d 100644 --- a/counters.go +++ b/counters.go @@ -11,22 +11,39 @@ import ( // Split by direction of travel so each counter belongs to exactly one // direction: the transmitting interface owns the tx fields and the receiving one // the rx fields. Reading both sets off both interfaces double counts every drop. -var ( - nicTxFields = []string{ - "tx_errors", "tx_dropped", "tx_fifo_errors", - "tx_carrier_errors", "tx_aborted_errors", "tx_window_errors", - "collisions", - } - nicRxFields = []string{ - "rx_errors", "rx_dropped", "rx_crc_errors", "rx_missed_errors", - "rx_length_errors", "rx_over_errors", "rx_frame_errors", "rx_fifo_errors", - } -) +// +// The two sides come from different places, and deliberately not from both at +// once. dev_get_stats adds the core's own drop counts on top of whatever the +// driver reports, so a sysfs field is the driver's counter plus the stack's and +// overlaps any driver counter read beside it. +// +// Sending is the stack's story: what it refused, and what the qdisc discarded +// because we outran it, neither of which the driver ever sees. +var nicTxFields = []string{ + "tx_errors", "tx_dropped", "tx_fifo_errors", + "tx_carrier_errors", "tx_aborted_errors", "tx_window_errors", + "collisions", +} -type nicCounters struct { - tx uint64 - rx uint64 - carrierDown uint64 +// Receiving is the hardware's, and is taken from the driver's own array, which +// sysfs both flattens and overlaps: ice folds crc errors and jabbers into +// rx_errors, so the old sum of rx_errors alongside rx_crc_errors charged every +// bad frame check twice. Named individually here, so nothing contains anything +// else in the list. +var nicRxStats = []string{ + "rx_crc_errors.nic", + "rx_jabber.nic", + "rx_undersize.nic", + "rx_oversize.nic", + "rx_fragments.nic", + "rx_dropped.nic", + // Below the frame: a 64b/66b block that decoded to no legal symbol, and the + // fault ordered sets the pcs sends when it loses sync. A cable going + // marginal moves these while every frame still arrives intact, which is as + // close to a bit error rate as this link will report. + "illegal_bytes.nic", + "mac_local_faults.nic", + "mac_remote_faults.nic", } func readUint(path string) (uint64, bool) { @@ -41,49 +58,64 @@ func readUint(path string) (uint64, bool) { return v, true } +// Every counter here was read once before the test began, so one that stops +// reading now is the interface going away underneath us. Stopping is the only +// honest answer: a tester that quietly gives up counting errors goes on +// reporting a clean link, which is the one lie it must never tell. +func mustReadUint(path string) uint64 { + v, ok := readUint(path) + if !ok { + panic("reading " + path) + } + return v +} + func sumFields(base string, fields []string) uint64 { var total uint64 for _, f := range fields { - if v, ok := readUint(base + "/statistics/" + f); ok { - total += v - } + total += mustReadUint(base + "/statistics/" + f) } return total } -func readNIC(ifname string) nicCounters { - base := "/sys/class/net/" + ifname - c := nicCounters{ - tx: sumFields(base, nicTxFields), - rx: sumFields(base, nicRxFields), - } - c.carrierDown, _ = readUint(base + "/carrier_down_count") - return c -} - -// A poll is thirty-odd sysfs attributes and each one costs microseconds of open -// and read, so it runs here rather than on the goroutine that has a frame to -// draw every sixteen milliseconds. Off that path the interval can be short -// enough that an unplugged cable shows up as a link error while the hand is -// still on it. +// A poll is an ioctl and a handful of sysfs attributes, each costing +// microseconds of open and read, so it runs here rather than on the goroutine +// that has a frame to draw every sixteen milliseconds. Off that path the +// interval can be short enough that an unplugged cable shows up as a link error +// while the hand is still on it. const nicInterval = 200 * time.Millisecond type nicPoller struct { - txName, rxName string - total *atomic.Uint64 - raw uint64 + txBase string + rx *statReader + total *atomic.Uint64 + raw uint64 } +func newNICPoller(fd int, txName, rxName string, total *atomic.Uint64) (*nicPoller, error) { + rx, err := newStatReader(fd, rxName, nicRxStats) + if err != nil { + return nil, err + } + return &nicPoller{ + txBase: "/sys/class/net/" + txName, + rx: rx, + total: total, + }, nil +} + +// Carrier is a property of the netdev rather than a counter of either side, and +// is taken from the transmitting interface only: across both directions each +// interface transmits exactly once, so every flap is counted once. func (p *nicPoller) read() uint64 { - tx := readNIC(p.txName) - rx := readNIC(p.rxName) - return tx.tx + rx.rx + tx.carrierDown + return sumFields(p.txBase, nicTxFields) + p.rx.sum() + + mustReadUint(p.txBase+"/carrier_down_count") } -// Sysfs nic counters run from boot and restart from zero whenever the driver -// resets its statistics, so only their forward motion is accumulated. Taking -// raw differences instead charges a boot's worth of errors to the first sample -// and turns a reset into a near-2^64 underflow. +// Nic counters run from boot and restart from zero whenever the driver resets +// its statistics, so only their forward motion is accumulated. Taking raw +// differences instead charges a boot's worth of errors to the first sample and +// turns a reset into a near-2^64 underflow. func (p *nicPoller) poll() { raw := p.read() if raw > p.raw { diff --git a/main.go b/main.go index 1a9735a..5347e26 100644 --- a/main.go +++ b/main.go @@ -48,6 +48,7 @@ type direction struct { probeSpec *frameSpec probeTxFD int probeRxFD int + statFD int cable *cableStats // Guards everything the sampler touches. The counters are read on their own @@ -456,7 +457,18 @@ func buildDirection(label string, tx, rx endpoint, sizes []int, cfg config) (*di streams: newLossWindows(cfg.streams), cable: newCableStats(), } - d.poller = &nicPoller{txName: tx.name, rxName: rx.name, total: &d.nic} + // Held open for the life of the run: the stats ioctl is issued five times a + // second and reopening a socket for each one is pure overhead. + statFD, err := unix.Socket(unix.AF_INET, unix.SOCK_DGRAM, 0) + if err != nil { + return nil, fmt.Errorf("%s stats socket: %w", label, err) + } + d.statFD = statFD + + d.poller, err = newNICPoller(statFD, tx.name, rx.name, &d.nic) + if err != nil { + return nil, fmt.Errorf("%s: %w", label, err) + } d.win = newRateWindow(int(rateWindowSpan/sampleInterval) + 1) for i := 0; i < cfg.streams; i++ { @@ -563,6 +575,7 @@ func (d *direction) close() { } unix.Close(d.probeTxFD) unix.Close(d.probeRxFD) + unix.Close(d.statFD) } type config struct { diff --git a/system.go b/system.go index d71a2a9..84b4efc 100644 --- a/system.go +++ b/system.go @@ -1,6 +1,7 @@ package main import ( + "bytes" "encoding/binary" "fmt" "os" @@ -327,6 +328,135 @@ func getCoalesce(fd int, ifname string) (ethtoolCoalesce, error) { return ec, err } +const ( + ethSSStats = 1 + ethGstringLen = 32 +) + +// Each of these has a __u32 or __u8 tail the kernel fills past the end of the +// struct, so they are only ever the header on a larger buffer. +type ethtoolSsetInfo struct { + cmd uint32 + reserved uint32 + ssetMask uint64 + data [1]uint32 + _ [4]byte +} + +type ethtoolGstrings struct { + cmd uint32 + stringSet uint32 + len uint32 +} + +type ethtoolStatsHdr struct { + cmd uint32 + nStats uint32 +} + +// The stats payload is an array of __u64 immediately after the header, so the +// buffer is allocated as uint64 to guarantee it lands on an eight byte +// boundary. A []byte carries no such guarantee. +func statsBuf(n int) []uint64 { return make([]uint64, n) } + +func statCount(fd int, ifname string) (uint32, error) { + req := ethtoolSsetInfo{cmd: unix.ETHTOOL_GSSET_INFO, ssetMask: 1 << ethSSStats} + if err := ethtoolCall(fd, ifname, unsafe.Pointer(&req)); err != nil { + return 0, err + } + // A cleared mask bit means the driver does not have that string set at all, + // in which case no count was written into the tail. + if req.ssetMask == 0 { + return 0, fmt.Errorf("driver has no ETH_SS_STATS string set") + } + return req.data[0], nil +} + +func statNames(fd int, ifname string) ([]string, error) { + n, err := statCount(fd, ifname) + if err != nil { + return nil, err + } + if n == 0 { + return nil, fmt.Errorf("driver reports zero statistics") + } + + hdrLen := int(unsafe.Sizeof(ethtoolGstrings{})) + buf := statsBuf((hdrLen + int(n)*ethGstringLen + 7) / 8) + hdr := (*ethtoolGstrings)(unsafe.Pointer(&buf[0])) + hdr.cmd = unix.ETHTOOL_GSTRINGS + hdr.stringSet = ethSSStats + hdr.len = n + if err := ethtoolCall(fd, ifname, unsafe.Pointer(&buf[0])); err != nil { + return nil, err + } + + raw := unsafe.Slice((*byte)(unsafe.Pointer(&buf[0])), len(buf)*8)[hdrLen:] + names := make([]string, hdr.len) + for i := range names { + s := raw[i*ethGstringLen : (i+1)*ethGstringLen] + if k := bytes.IndexByte(s, 0); k >= 0 { + s = s[:k] + } + names[i] = string(s) + } + return names, nil +} + +func statValues(fd int, ifname string, n uint32, into []uint64) error { + hdr := (*ethtoolStatsHdr)(unsafe.Pointer(&into[0])) + hdr.cmd = unix.ETHTOOL_GSTATS + hdr.nStats = n + return ethtoolCall(fd, ifname, unsafe.Pointer(&into[0])) +} + +// The ioctl indexes counters by position, and the ordering is a property of the +// driver build, so names are resolved to indices once rather than per poll. +type statReader struct { + fd int + ifname string + n uint32 + want []int + buf []uint64 +} + +func newStatReader(fd int, ifname string, want []string) (*statReader, error) { + names, err := statNames(fd, ifname) + if err != nil { + return nil, fmt.Errorf("%s statistics: %w", ifname, err) + } + idx := make(map[string]int, len(names)) + for i, s := range names { + idx[s] = i + } + r := &statReader{ + fd: fd, + ifname: ifname, + n: uint32(len(names)), + buf: statsBuf(1 + len(names)), + } + for _, w := range want { + i, ok := idx[w] + if !ok { + return nil, fmt.Errorf("%s has no statistic %q", ifname, w) + } + r.want = append(r.want, i) + } + return r, nil +} + +func (r *statReader) sum() uint64 { + if err := statValues(r.fd, r.ifname, r.n, r.buf); err != nil { + panic(fmt.Sprintf("reading %s statistics: %v", r.ifname, err)) + } + vals := r.buf[1:] + var total uint64 + for _, i := range r.want { + total += vals[i] + } + return total +} + func checkGovernor(want string) checkResult { res := checkResult{item: "cpu governor"} paths, err := filepath.Glob("/sys/devices/system/cpu/cpu*/cpufreq/scaling_governor")