Count link and syscall failures instead of printing them

counters.go

@@ -1,27 +1,31 @@
 package main
 
 import (
-	"fmt"
 	"os"
 	"strconv"
 	"strings"
 )
 
-var nicFields = []string{
+// Everything the driver counts against a frame the link failed to carry, split
-	"rx_packets", "tx_packets",
+// by direction of travel so each counter belongs to exactly one direction: the
-	"rx_errors", "tx_errors",
+// transmitting interface owns the tx fields and the receiving interface the rx
-	"rx_dropped", "tx_dropped",
+// ones. Reading both sets off both interfaces would report every drop twice.
-	"rx_crc_errors", "rx_missed_errors",
+var (
-	"rx_length_errors", "rx_over_errors",
+	nicTxFields = []string{
-	"rx_frame_errors", "rx_fifo_errors",
+		"tx_errors", "tx_dropped", "tx_fifo_errors",
-	"collisions",
+		"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",
+	}
+)
 
 type nicCounters struct {
-	stats          map[string]uint64
+	tx          uint64
-	carrierChanges uint64
+	rx          uint64
-	carrierUp      uint64
+	carrierDown uint64
-	carrierDown    uint64
 }
 
 func readUint(path string) (uint64, bool) {
@@ -36,36 +40,22 @@ func readUint(path string) (uint64, bool) {
 	return v, true
 }
 
-func readNIC(ifname string) nicCounters {
+func sumFields(base string, fields []string) uint64 {
-	c := nicCounters{stats: make(map[string]uint64, len(nicFields))}
+	var total uint64
-	base := "/sys/class/net/" + ifname
+	for _, f := range fields {
-	for _, f := range nicFields {
 		if v, ok := readUint(base + "/statistics/" + f); ok {
-			c.stats[f] = v
+			total += v
 		}
 	}
-	c.carrierChanges, _ = readUint(base + "/carrier_changes")
+	return total
-	c.carrierUp, _ = readUint(base + "/carrier_up_count")
+}
+
+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
 }
-
-func (c nicCounters) diff(prev nicCounters) string {
-	var parts []string
-	for _, f := range nicFields {
-		now, ok := c.stats[f]
-		if !ok {
-			continue
-		}
-		if d := now - prev.stats[f]; d != 0 && !strings.HasSuffix(f, "_packets") {
-			parts = append(parts, fmt.Sprintf("%s=%d", f, d))
-		}
-	}
-	if d := c.carrierChanges - prev.carrierChanges; d != 0 {
-		parts = append(parts, fmt.Sprintf("carrier_changes=%d", d))
-	}
-	if d := c.carrierDown - prev.carrierDown; d != 0 {
-		parts = append(parts, fmt.Sprintf("carrier_down=%d", d))
-	}
-	return strings.Join(parts, " ")
-}

frame.go

@@ -156,22 +156,3 @@ func parseHeader(buf []byte) (parsed, bool) {
 	}
 	return p, true
 }
-
-func firstDiff(got, want []byte) (int, int) {
-	n := len(got)
-	if len(want) < n {
-		n = len(want)
-	}
-	for i := 0; i < n; i++ {
-		if got[i] != want[i] {
-			bits := 0
-			x := got[i] ^ want[i]
-			for x != 0 {
-				bits += int(x & 1)
-				x >>= 1
-			}
-			return i, bits
-		}
-	}
-	return -1, 0
-}

main.go

@@ -44,7 +44,6 @@ type direction struct {
 	streams []lossWindow
 	txFDs   []int
 	rxFDs   []int
-	reports chan string
 
 	probeSpec *frameSpec
 	probeTxFD int
@@ -59,8 +58,8 @@ type direction struct {
 	drops       uint64
 	errBase     sample
 	dropBase    uint64
-	nicTX       nicCounters
+	nicNow      uint64
-	nicRX       nicCounters
+	nicBase     uint64
 }
 
 // Smoothing has to be steady against high-frequency noise yet still chase a
@@ -192,6 +191,7 @@ type sample struct {
 	crcErr, badMagic  uint64
 	badLen            uint64
 	txErrs, txShort   uint64
+	rxErrs            uint64
 }
 
 func lookupEndpoint(name string) (endpoint, error) {
@@ -247,6 +247,7 @@ func (d *direction) snapshot() sample {
 		s.crcErr += r.crcErr.Load()
 		s.badMagic += r.badMagic.Load()
 		s.badLen += r.badLen.Load()
+		s.rxErrs += r.rxErrs.Load()
 	}
 	for i := range d.streams {
 		s.lost += d.streams[i].lost.Load()
@@ -264,6 +265,7 @@ func (d *direction) reset() {
 	d.dropBase = d.drops
 	d.heldFrames = heldValue{}
 	d.heldSent = heldValue{}
+	d.nicBase = d.nicNow
 	d.cable.reset()
 }
 
@@ -300,7 +302,8 @@ var intervalCols = []colSpec{
 	{title: "CRC", width: 7, right: true},
 	{title: "BADMAG", width: 7, right: true},
 	{title: "KDROP", width: 11, right: true},
-	{title: "ERRORS", width: 11, right: true},
+	{title: "LINK", width: 13, right: true},
+	{title: "ERRORS", width: 13, right: true},
 	{title: "MIN ns", width: 9, right: true},
 	{title: "LEN m", width: 6, right: true},
 }
@@ -314,7 +317,8 @@ type view struct {
 	rxFrames, rxGot  uint64
 	lost, late       uint64
 	crc, badMagic    uint64
-	kdrop, errors    uint64
+	kdrop, link      uint64
+	errors           uint64
 	cable            cableView
 }
 
@@ -334,7 +338,12 @@ func (d *direction) counters(now sample) view {
 		kdrop:    d.drops - d.dropBase,
 		cable:    d.cable.view(),
 	}
-	v.errors = v.lost + v.crc + v.badMagic + (now.badLen - b.badLen) + v.kdrop
+	// A frame the stack refused and a frame the driver dropped are the same
+	// failure seen from either side of the ring, and never the same frame twice:
+	// a send that fails never reaches the driver to be dropped.
+	v.link = d.nicNow - d.nicBase +
+		(now.txErrs - b.txErrs) + (now.rxErrs - b.rxErrs)
+	v.errors = v.lost + v.crc + v.badMagic + (now.badLen - b.badLen) + v.kdrop + v.link
 	return v
 }
 
@@ -398,25 +407,19 @@ func (d *direction) row(elapsed time.Duration, v view, target float64, length st
 		statusCell(v.crc),
 		statusCell(v.badMagic),
 		statusCell(v.kdrop),
+		statusCell(v.link),
 		statusCell(v.errors),
 		paint(v.cable.minText(), cCyan),
 		paint(length, cCyan),
 	}
 }
 
-func (d *direction) reportNIC() []string {
+// Read once a second rather than per frame, since these are sysfs files; the
+// display uses whatever the last sample left behind.
+func (d *direction) sampleNIC() {
 	tx := readNIC(d.tx.name)
 	rx := readNIC(d.rx.name)
-	var parts []string
+	d.nicNow = tx.tx + rx.rx + tx.carrierDown
-	if s := tx.diff(d.nicTX); s != "" {
-		parts = append(parts, paint("  ▲ "+d.tx.name+" tx-side: "+s, cYellow))
-	}
-	if s := rx.diff(d.nicRX); s != "" {
-		parts = append(parts, paint("  ▲ "+d.rx.name+" rx-side: "+s, cYellow))
-	}
-	d.nicTX = tx
-	d.nicRX = rx
-	return parts
 }
 
 func buildDirection(label string, tx, rx endpoint, patIdx int, sizes []int, cfg config) (*direction, error) {
@@ -426,10 +429,8 @@ func buildDirection(label string, tx, rx endpoint, patIdx int, sizes []int, cfg
 		tx:      tx,
 		rx:      rx,
 		streams: newLossWindows(cfg.streams),
-		reports: make(chan string, 64),
+		cable:   newCableStats(),
 	}
-	d.nicTX = readNIC(tx.name)
-	d.nicRX = readNIC(rx.name)
 
 	for i := 0; i < cfg.streams; i++ {
 		et := uint16(etherBase + i)
@@ -453,7 +454,6 @@ func buildDirection(label string, tx, rx endpoint, patIdx int, sizes []int, cfg
 	// The probe carries its own ethertype so it lands on a socket of its own, but
 	// it is left unsteered: it is a few frames a second and does not need a queue
 	// to itself, and the stamps are taken at the wire either way.
-	d.cable = newCableStats()
 	d.probeSpec = newFrameSpec(patIdx, rx.mac, tx.mac, cfg.probeEther, []int{probeSize})
 	fd, err := openTxSocket(tx.idx)
 	if err != nil {
@@ -471,6 +471,10 @@ func buildDirection(label string, tx, rx endpoint, patIdx int, sizes []int, cfg
 		return nil, fmt.Errorf("%s probe rx timestamps: %w", label, err)
 	}
 	d.probeRxFD = fd
+
+	// Whatever the interfaces have counted before now is not ours.
+	d.sampleNIC()
+	d.nicBase = d.nicNow
 	return d, nil
 }
 
@@ -497,7 +501,6 @@ func (d *direction) start(wg *sync.WaitGroup, doneTx, doneRx *atomic.Bool, cfg c
 			spec:    d.specs[i],
 			stats:   d.rxStats[i],
 			streams: d.streams,
-			reports: d.reports,
 			ready:   rxReady,
 		}
 		wg.Add(1)
@@ -762,6 +765,7 @@ func run(aName, bName, sizesArg, patArg string,
 			// Length needs both directions, so every row is sampled before any of
 			// them is printed.
 			for i, d := range dirs {
+				d.sampleNIC()
 				rows[i] = d.view(&d.prevConsole, secs)
 			}
 			length := "-"
@@ -772,18 +776,6 @@ func run(aName, bName, sizesArg, patArg string,
 				for _, line := range stats.emit(d.row(elapsed, rows[i], target, length)) {
 					fmt.Println(line)
 				}
-				for _, line := range d.reportNIC() {
-					fmt.Println(line)
-				}
-				for {
-					select {
-					case msg := <-d.reports:
-						fmt.Println(paint("  ✗ "+d.short+": "+msg, cRed))
-						continue
-					default:
-					}
-					break
-				}
 			}
 		}
 	}

rx.go

@@ -1,7 +1,6 @@
 package main
 
 import (
-	"fmt"
 	"hash/crc32"
 	"sync"
 	"sync/atomic"
@@ -15,7 +14,8 @@ type rxStats struct {
 	badMagic atomic.Uint64
 	badLen   atomic.Uint64
 	crcErr   atomic.Uint64
-	_        [24]byte
+	rxErrs   atomic.Uint64
+	_        [16]byte
 }
 
 type rxWorker struct {
@@ -24,7 +24,6 @@ type rxWorker struct {
 	spec    *frameSpec
 	stats   *rxStats
 	streams []lossWindow
-	reports chan string
 	ready   *sync.WaitGroup
 }
 
@@ -44,7 +43,7 @@ func (w *rxWorker) run(done *atomic.Bool) {
 		n, err := recvmmsg(w.fd, hdrs, unix.MSG_WAITFORONE)
 		if n <= 0 {
 			if err != nil && err != unix.EAGAIN && err != unix.EINTR {
-				w.report(fmt.Sprintf("recvmmsg: %v", err))
+				w.stats.rxErrs.Add(1)
 			}
 			continue
 		}
@@ -67,20 +66,9 @@ func (w *rxWorker) run(done *atomic.Bool) {
 				continue
 			}
 			pay := buf[minFrame : minFrame+p.payLen]
-			if crc32.Checksum(pay, crcTable) == p.crc {
+			if crc32.Checksum(pay, crcTable) != p.crc {
-				continue
+				w.stats.crcErr.Add(1)
 			}
-			w.stats.crcErr.Add(1)
-			off, bits := firstDiff(pay, w.spec.ref[:p.payLen])
-			w.report(fmt.Sprintf("payload mismatch stream=%d seq=%d len=%d first-diff-offset=%d bits=%d",
-				p.stream, p.seq, p.payLen, off, bits))
 		}
 	}
 }
-
-func (w *rxWorker) report(msg string) {
-	select {
-	case w.reports <- msg:
-	default:
-	}
-}

system.go

@@ -6,7 +6,6 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
-	"time"
 	"unsafe"
 
 	"golang.org/x/sys/unix"
@@ -380,24 +379,6 @@ func checkLinkUp(fd int, ifname string) checkResult {
 	return res
 }
 
-func checkCarrier(ifname string, wait time.Duration) checkResult {
-	res := checkResult{item: ifname + " carrier"}
-	deadline := time.Now().Add(wait)
-	for {
-		v, ok := readUint("/sys/class/net/" + ifname + "/carrier")
-		if ok && v == 1 {
-			res.state = "present"
-			return res
-		}
-		if time.Now().After(deadline) {
-			res.err = fmt.Errorf("no carrier after %s", wait)
-			res.fatal = true
-			return res
-		}
-		time.Sleep(100 * time.Millisecond)
-	}
-}
-
 func checkCoalesce(fd int, ifname string, rxUsecs, txUsecs uint32) checkResult {
 	res := checkResult{item: ifname + " coalesce"}
 	ec, err := getCoalesce(fd, ifname)
@@ -430,18 +411,18 @@ func checkCoalesce(fd int, ifname string, rxUsecs, txUsecs uint32) checkResult {
 	return res
 }
 
-func checkRings(fd int, ifname string, rxWant, txWant uint32) (checkResult, bool) {
+func checkRings(fd int, ifname string, rxWant, txWant uint32) checkResult {
 	res := checkResult{item: ifname + " rings"}
 	rp, err := getRings(fd, ifname)
 	if err != nil {
 		res.err = err
-		return res, false
+		return res
 	}
 	rx := min(rxWant, rp.rxMaxPending)
 	tx := min(txWant, rp.txMaxPending)
 	if rp.rxPending == rx && rp.txPending == tx {
 		res.state = fmt.Sprintf("rx=%d tx=%d", rp.rxPending, rp.txPending)
-		return res, false
+		return res
 	}
 	was := fmt.Sprintf("rx=%d tx=%d", rp.rxPending, rp.txPending)
 	rp.cmd = unix.ETHTOOL_SRINGPARAM
@@ -450,11 +431,11 @@ func checkRings(fd int, ifname string, rxWant, txWant uint32) (checkResult, bool
 	if err := ethtoolCall(fd, ifname, unsafe.Pointer(&rp)); err != nil {
 		res.err = err
 		res.state = "could not set"
-		return res, false
+		return res
 	}
 	res.fixed = true
 	res.state = fmt.Sprintf("was %s, now rx=%d tx=%d (link reset)", was, rx, tx)
-	return res, true
+	return res
 }
 
 func withIoctlSocket(fn func(fd int) []checkResult) []checkResult {
@@ -475,13 +456,7 @@ func configureSystem(ifnames []string, ethertypes []uint16) []checkResult {
 
 			// Ring changes reprogram the queues, so flow rules pointing at those
 			// queues have to be installed afterwards.
-			carrierWait := 3 * time.Second
+			out = append(out, checkRings(fd, ifname, wantRxRing, wantTxRing))
-			ringRes, ringReset := checkRings(fd, ifname, wantRxRing, wantTxRing)
-			out = append(out, ringRes)
-			if ringReset {
-				carrierWait = 10 * time.Second
-			}
-			out = append(out, checkCarrier(ifname, carrierWait))
 			out = append(out, checkCoalesce(fd, ifname, wantCoalesceUsecs, wantCoalesceUsecs))
 			out = append(out, checkTimestamping(fd, ifname))
 			out = append(out, checkFlowRules(fd, ifname, ethertypes))