From 0f16edfe614a571c22d3c9c17d964a180047d8c3 Mon Sep 17 00:00:00 2001 From: flamingcow Date: Sat, 25 Jul 2026 22:54:04 -0700 Subject: [PATCH] Count link and syscall failures instead of printing them --- counters.go | 72 +++++++++++++++++++++++------------------------------ frame.go | 19 -------------- main.go | 60 +++++++++++++++++++------------------------- rx.go | 22 ++++------------ system.go | 37 +++++---------------------- 5 files changed, 68 insertions(+), 142 deletions(-) diff --git a/counters.go b/counters.go index d287996..16555a1 100644 --- a/counters.go +++ b/counters.go @@ -1,27 +1,31 @@ package main import ( - "fmt" "os" "strconv" "strings" ) -var nicFields = []string{ - "rx_packets", "tx_packets", - "rx_errors", "tx_errors", - "rx_dropped", "tx_dropped", - "rx_crc_errors", "rx_missed_errors", - "rx_length_errors", "rx_over_errors", - "rx_frame_errors", "rx_fifo_errors", - "collisions", -} +// Everything the driver counts against a frame the link failed to carry, split +// by direction of travel so each counter belongs to exactly one direction: the +// transmitting interface owns the tx fields and the receiving interface the rx +// ones. Reading both sets off both interfaces would report every drop twice. +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", + } +) type nicCounters struct { - stats map[string]uint64 - carrierChanges uint64 - carrierUp uint64 - carrierDown uint64 + tx uint64 + rx 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 { - c := nicCounters{stats: make(map[string]uint64, len(nicFields))} - base := "/sys/class/net/" + ifname - for _, f := range nicFields { +func sumFields(base string, fields []string) uint64 { + var total uint64 + for _, f := range fields { if v, ok := readUint(base + "/statistics/" + f); ok { - c.stats[f] = v + total += v } } - c.carrierChanges, _ = readUint(base + "/carrier_changes") - c.carrierUp, _ = readUint(base + "/carrier_up_count") + 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 } - -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, " ") -} diff --git a/frame.go b/frame.go index 007aaaa..4696431 100644 --- a/frame.go +++ b/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 -} diff --git a/main.go b/main.go index 96a236b..a811f2d 100644 --- a/main.go +++ b/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 - nicRX nicCounters + nicNow uint64 + 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 - 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 + d.nicNow = tx.tx + rx.rx + tx.carrierDown } 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 - } } } } diff --git a/rx.go b/rx.go index c4f52fe..fa5b4c0 100644 --- a/rx.go +++ b/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 { - continue + if crc32.Checksum(pay, crcTable) != p.crc { + 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: - } -} diff --git a/system.go b/system.go index ab7c2c6..7ba79ca 100644 --- a/system.go +++ b/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 - 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, checkRings(fd, ifname, wantRxRing, wantTxRing)) out = append(out, checkCoalesce(fd, ifname, wantCoalesceUsecs, wantCoalesceUsecs)) out = append(out, checkTimestamping(fd, ifname)) out = append(out, checkFlowRules(fd, ifname, ethertypes))