Read rx errors from the driver stats array and add the pcs counters

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 {