From 38c2cc4da2acf31fa9ffcc8f61544a3e0a5b89d0 Mon Sep 17 00:00:00 2001 From: flamingcow Date: Mon, 17 Aug 2026 12:45:41 -0700 Subject: [PATCH] Hardware lock-in cleanup: ice RX counter set and by-driver map collapsed to the single ixgbe nicRxFields list (newNICPoller drops its redundant driver gate), checkCoalesce loses the try-tx-then-EINVAL fallback and sets rx-usecs shared with tx as the one path, ice-attributed comments neutralized, dead errs.total removed with its redundant test assertion; ice counter reference set recorded in docs/nics/e810; verified on hardware (20.00G, zero errors, ECD 41m) --- counters.go | 50 ++++++++-------------------------- docs/nics/e810/README.md | 4 +++ docs/nics/x520/README.md | 4 +-- main.go | 4 --- main_test.go | 3 -- system.go | 59 ++++++++++++++-------------------------- 6 files changed, 38 insertions(+), 86 deletions(-) diff --git a/counters.go b/counters.go index ebb2866..11afbed 100644 --- a/counters.go +++ b/counters.go @@ -1,7 +1,6 @@ package main import ( - "fmt" "os" "path/filepath" "strconv" @@ -27,34 +26,17 @@ var nicTxFields = []string{ "collisions", } -// 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 per driver, so nothing contains -// anything else in its list. -var nicRxStatsByDriver = map[string][]string{ - // The reference set: illegal_bytes and the faults move while every frame - // still arrives intact — as close to a bit error rate as the link reports. - "ice": { - "rx_crc_errors.nic", - "rx_jabber.nic", - "rx_undersize.nic", - "rx_oversize.nic", - "rx_fragments.nic", - "rx_dropped.nic", - "illegal_bytes.nic", - "mac_local_faults.nic", - "mac_remote_faults.nic", - }, - // The 82599 exposes no jabber, fragment, illegal-byte or fault counters - // (docs/nics/x520/). - "ixgbe": { - "rx_crc_errors", - "rx_missed_errors", - "rx_length_errors", - "rx_long_length_errors", - "rx_short_length_errors", - }, +// Receiving is the hardware's, and is taken from the driver's own array: +// sysfs flattens and overlaps it (crc errors fold into rx_errors, so summing +// rx_errors alongside rx_crc_errors charges every bad frame twice). Named +// individually, so nothing contains anything else in the list; the 82599 +// exposes no jabber, fragment, illegal-byte or fault counters (docs/nics/x520/). +var nicRxFields = []string{ + "rx_crc_errors", + "rx_missed_errors", + "rx_length_errors", + "rx_long_length_errors", + "rx_short_length_errors", } func ifDriver(name string) (string, error) { @@ -113,15 +95,7 @@ type nicPoller struct { } func newNICPoller(fd int, txName, rxName string, total *atomic.Uint64, measuring *atomic.Bool) (*nicPoller, error) { - drv, err := ifDriver(rxName) - if err != nil { - return nil, err - } - want, ok := nicRxStatsByDriver[drv] - if !ok { - return nil, fmt.Errorf("%s: no rx error statistic set for driver %s", rxName, drv) - } - rx, err := newStatReader(fd, rxName, want) + rx, err := newStatReader(fd, rxName, nicRxFields) if err != nil { return nil, err } diff --git a/docs/nics/e810/README.md b/docs/nics/e810/README.md index b5659e7..3274d14 100644 --- a/docs/nics/e810/README.md +++ b/docs/nics/e810/README.md @@ -7,6 +7,10 @@ Out of the box. The measurement path was originally built against it — its dat - `irdma` autoloads and binds ice ports, making `ETHTOOL_SCHANNELS` fail EBUSY ("Cannot change channels when RDMA is active") and failing the channels host check. `sudo rmmod irdma` (usage count 0; returns on reboot). The appliance kernel has no irdma. - ice refuses channel changes while ntuple rules exist — cabletest clears its own stale rules first. +## RX error counter reference set + +The richest per-frame RX error breakdown of the NICs used — `illegal_bytes` and the MAC faults move while every frame still arrives intact, as close to a bit error rate as a link reports. `ethtool -S` names: `rx_crc_errors.nic`, `rx_jabber.nic`, `rx_undersize.nic`, `rx_oversize.nic`, `rx_fragments.nic`, `rx_dropped.nic`, `illegal_bytes.nic`, `mac_local_faults.nic`, `mac_remote_faults.nic`. Read the driver's own array, never sysfs beside it: ice folds crc errors and jabbers into `rx_errors`, so summing `rx_errors` alongside `rx_crc_errors` charges every bad frame twice. + ## Module I2C: multi-byte framing works, writes are policy-blocked - topo-I2C (0x06E2/E3) has offset-size control: params bit[7] repeated-start, [6:5] address length, [3:0] data size. **Reads work perfectly.** diff --git a/docs/nics/x520/README.md b/docs/nics/x520/README.md index 8ceabdc..3aedad3 100644 --- a/docs/nics/x520/README.md +++ b/docs/nics/x520/README.md @@ -4,8 +4,8 @@ - PCIe Gen2 ×8 (5 GT/s, 32 Gb/s raw, ~25–26 Gb/s/dir effective vs 20 needed) — enough for 2×10G full duplex at the default mix; the 64 B case was host-bound already on the E810. Verify 5 GT/s ×8 trained (`lspci -vv`). - Loss attribution survives here: missed-packet (RXMPC → `rx_missed_errors`) and per-queue drop (QPRDC) counters — "prove host-side zero" works. -- The RX error counter set is the 82599's slimmer one (`counters.go`): no jabber, fragment, illegal-byte or MAC-fault counters exist — crc/missed/length errors are what this link reports; the ice set remains the richer reference. -- ixgbe's mixed rx/tx interrupt vectors reject a tx-specific coalesce value; `checkCoalesce` falls back to rx-shared-with-tx on EINVAL, which is the normal path here. The full rx=8160/tx=4096 ring ask is taken as-is. +- The RX error counter set is the 82599's slimmer one (`counters.go`): no jabber, fragment, illegal-byte or MAC-fault counters exist — crc/missed/length errors are what this link reports; the richer ice reference set is recorded in [../e810/README.md](../e810/README.md). +- ixgbe's mixed rx/tx interrupt vectors take one moderation value per vector, so `checkCoalesce` sets rx-usecs shared with tx. The full rx=8160/tx=4096 ring ask is taken as-is. - **`allow_unsupported_sfp=1` is mandatory** (`ixgbe_main.c:165`): the FS module's honest 10GBASE-T EEPROM fails Intel qualification and kills the whole port probe (error -95, no netdev). `load-ixgbe` passes it. ## The smoothed-bucket rate: read-time buckets, backward smear, no hardware stamps diff --git a/main.go b/main.go index 32ba5ea..aeef438 100644 --- a/main.go +++ b/main.go @@ -97,10 +97,6 @@ type errs struct { internal uint64 } -func (e errs) total() uint64 { - return e.lost + e.corrupt + e.link + e.internal -} - func (e errs) add(o errs) errs { return errs{ lost: e.lost + o.lost, corrupt: e.corrupt + o.corrupt, diff --git a/main_test.go b/main_test.go index 98f4c58..e8d139f 100644 --- a/main_test.go +++ b/main_test.go @@ -257,9 +257,6 @@ func TestErrsBetweenBuckets(t *testing.T) { if got != want { t.Errorf("errsBetween = %+v, want %+v", got, want) } - if got.total() != 55 { - t.Errorf("total = %d, want 55", got.total()) - } } // A reset re-bases from a fresh capture while the ring still holds buckets from diff --git a/system.go b/system.go index f5a4d51..aaf80c7 100644 --- a/system.go +++ b/system.go @@ -59,9 +59,7 @@ func rxRings(fd int, ifname string) (uint64, error) { return nfc.data, nil } -// Returns the installed rule locations and the total filter capacity. ice -// only honours filters near the top of that range, which is why ethtool's own -// rule manager allocates downwards from the end. +// Returns the installed rule locations and the total filter capacity. func allRuleLocations(fd int, ifname string) ([]uint32, uint32, error) { cnt := ethtoolRxnfc{cmd: ethtoolGRXCLSRLCNT} if err := ethtoolCall(fd, ifname, unsafe.Pointer(&cnt)); err != nil { @@ -111,7 +109,8 @@ func deleteRule(fd int, ifname string, loc uint32) error { return ethtoolCall(fd, ifname, unsafe.Pointer(&nfc)) } -// ice rejects RX_CLS_LOC_ANY, so the caller must supply a free location. +// RX_CLS_LOC_ANY is not honoured here, so the caller supplies a free location, +// allocated downward from the top to match ethtool's own rule manager. func insertEtherRule(fd int, ifname string, ethType uint16, queue uint64, loc uint32) error { nfc := ethtoolRxnfc{cmd: ethtoolSRXCLSRLINS} nfc.fs.flowType = etherFlow @@ -589,7 +588,10 @@ func checkLinkUp(fd int, ifname string) checkResult { return res } -func checkCoalesce(fd int, ifname string, rxUsecs, txUsecs uint32) checkResult { +// ixgbe runs mixed rx/tx interrupt vectors — one moderation value per vector, +// so rx-usecs covers both, a tx-specific value is rejected, and get reports +// the tx side as zero. +func checkCoalesce(fd int, ifname string, usecs uint32) checkResult { res := checkResult{item: ifname + " coalesce"} ec, err := getCoalesce(fd, ifname) if err != nil { @@ -597,49 +599,28 @@ func checkCoalesce(fd int, ifname string, rxUsecs, txUsecs uint32) checkResult { return res } desc := func(e ethtoolCoalesce) string { - return fmt.Sprintf("adaptive rx=%d tx=%d rx-usecs=%d tx-usecs=%d", - e.useAdaptiveRxCoalesce, e.useAdaptiveTxCoalesce, e.rxCoalesceUsecs, e.txCoalesceUsecs) + return fmt.Sprintf("adaptive rx=%d tx=%d rx-usecs=%d shared with tx", + e.useAdaptiveRxCoalesce, e.useAdaptiveTxCoalesce, e.rxCoalesceUsecs) } - settled := func(e ethtoolCoalesce, tx uint32) bool { - return e.useAdaptiveRxCoalesce == 0 && e.useAdaptiveTxCoalesce == 0 && - e.rxCoalesceUsecs == rxUsecs && e.txCoalesceUsecs == tx - } - if settled(ec, txUsecs) { + if ec.useAdaptiveRxCoalesce == 0 && ec.useAdaptiveTxCoalesce == 0 && + ec.rxCoalesceUsecs == usecs && ec.txCoalesceUsecs == 0 { res.state = desc(ec) return res } was := desc(ec) - set := func(tx uint32) error { - s := ec - s.cmd = unix.ETHTOOL_SCOALESCE - s.useAdaptiveRxCoalesce = 0 - s.useAdaptiveTxCoalesce = 0 - s.rxCoalesceUsecs = rxUsecs - s.txCoalesceUsecs = tx - return ethtoolCall(fd, ifname, unsafe.Pointer(&s)) - } - err = set(txUsecs) - // A driver running mixed rx/tx vectors has one moderation value per - // vector and rejects a tx-specific one: rx-usecs covers both, and get - // reports the tx side as zero. - if err == unix.EINVAL { - if settled(ec, 0) { - res.state = desc(ec) + " (tx shares rx)" - return res - } - if err = set(0); err == nil { - res.fixed = true - res.state = fmt.Sprintf("was %s, now adaptive off rx-usecs=%d shared with tx", was, rxUsecs) - return res - } - } - if err != nil { + s := ec + s.cmd = unix.ETHTOOL_SCOALESCE + s.useAdaptiveRxCoalesce = 0 + s.useAdaptiveTxCoalesce = 0 + s.rxCoalesceUsecs = usecs + s.txCoalesceUsecs = 0 + if err := ethtoolCall(fd, ifname, unsafe.Pointer(&s)); err != nil { res.err = err res.state = "could not set" return res } res.fixed = true - res.state = fmt.Sprintf("was %s, now adaptive off rx-usecs=%d tx-usecs=%d", was, rxUsecs, txUsecs) + res.state = fmt.Sprintf("was %s, now adaptive off rx-usecs=%d shared with tx", was, usecs) return res } @@ -753,7 +734,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. out = append(out, checkRings(fd, ifname, wantRxRing, wantTxRing)) - out = append(out, checkCoalesce(fd, ifname, wantCoalesceUsecs, wantCoalesceUsecs)) + out = append(out, checkCoalesce(fd, ifname, wantCoalesceUsecs)) out = append(out, checkFlowRules(fd, ifname, ethertypes)) } return out