From dd47fcf11c337ca295b481a673f04a9c7d0f2dcf Mon Sep 17 00:00:00 2001 From: snokvist Date: Thu, 10 Sep 2026 08:36:17 +0200 Subject: [PATCH 1/3] mt7612u: block D - the validation the backend was missing Issue #419's Validation bullet: a regress.py matrix against the MediaTek kernel driver, more ctest cells, and on-air figures for the README with the counterpart stated in the same breath. The matrix runs, on two MT7612U units, both bands, twelve cells over three runs, every one green first time - and the kernel mt76x2u decodes our frames as 802.11a/g at 6 Mbps with a per-antenna breakdown, which is an independent decoder rather than our own reading of our own transmit. Three new ctest cells (five MediaTek total): mt7612u_rx_queue the RX hand-off queue, extracted from Mt7612uRadio into a header so it can be tested - the first cell to cover backend BEHAVIOUR rather than a lookup mt7612u_usb_ids_vs_mt76 the id table really is the complete vendor table from the pinned reference. An id we FORGOT is invisible to mt7612u_usb_ids mt7612u_initvals_generated wires up a --check that existed and ran nowhere Two bugs the running found, both latent for a same-model pair: _wlan_iface_for_dut matched by VID:PID and took the first hit, so a kernel cell could tcpdump the adapter devourer had claimed; and --keep-logs called os._exit after print() on a block-buffered pipe, discarding the table it exists to preserve. A third, older one: _count_tcpdump_hits counted a trailing blank line, so every kernel-RX figure ever published was one high. Four reviewers over two rounds. Ten mutations of the queue are now caught that were not, including two truncating pushes and a pop_begin that discarded the drain at teardown; and the README footnote's stated reason was simply false - the part airs HT MCS7 fine, it is the column's USRP method this bench cannot reproduce. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj --- CMakeLists.txt | 42 +++ README.md | 28 +- docs/mt7612u.md | 181 ++++++++++-- src/mt7612u/Mt7612uRadio.cpp | 67 ++--- src/mt7612u/Mt7612uRadio.h | 31 +- src/mt7612u/Mt7612uRxQueue.h | 160 ++++++++++ tests/README.md | 28 +- tests/mt7612u_rx_queue_selftest.cpp | 439 ++++++++++++++++++++++++++++ tests/mt7612u_usb_ids_vs_mt76.py | 161 ++++++++++ tests/regress.py | 286 ++++++++++++++++-- tools/extract_mt7612u_tables.py | 7 +- 11 files changed, 1308 insertions(+), 122 deletions(-) create mode 100644 src/mt7612u/Mt7612uRxQueue.h create mode 100644 tests/mt7612u_rx_queue_selftest.cpp create mode 100644 tests/mt7612u_usb_ids_vs_mt76.py diff --git a/CMakeLists.txt b/CMakeLists.txt index 9f9e8ecd..ceb4882b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -386,6 +386,7 @@ if(DEVOURER_MT7612U) src/mt7612u/usb.cpp src/mt7612u/Mt7612uRadio.cpp src/mt7612u/Mt7612uRadio.h src/mt7612u/Mt7612uMapping.h + src/mt7612u/Mt7612uRxQueue.h src/mt7612u/internal.h src/mt7612u/regs.h src/mt7612u/initvals.h @@ -893,6 +894,25 @@ target_include_directories(Mt7612uMappingSelftest PRIVATE add_test(NAME mt7612u_mapping COMMAND Mt7612uMappingSelftest) +# Headless guard for the RX hand-off queue, the one piece of backend BEHAVIOUR +# (as opposed to descriptor lookups) that can be exercised without a radio. Its +# two load-bearing properties - a full ring drops the newest frame and counts +# it, and a popped slot outlives the lock so user code can run unlocked - are +# both invisible on a healthy bench and both turn into a wedge, a reorder or a +# use-after-free when broken. Like the mapping cell it compiles regardless of +# DEVOURER_MT7612U: it is header-only and needs no libusb. +add_executable(Mt7612uRxQueueSelftest + tests/mt7612u_rx_queue_selftest.cpp +) +# The first selftest in the tree to use directly. It links today +# without this because libdevourer already pulls the thread runtime in, and on +# glibc >= 2.34 pthread is part of libc anyway — but that is two accidents, not +# a dependency, and this costs nothing. +find_package(Threads REQUIRED) +target_link_libraries(Mt7612uRxQueueSelftest PRIVATE devourer Threads::Threads) + +add_test(NAME mt7612u_rx_queue COMMAND Mt7612uRxQueueSelftest) + # Headless guard for the TX quiesce seam (ITransport::quiesce_tx via # RtlAdapter): the explicit "stop TX and wait it out" call every device makes # before anything is released. UsbTransport's cancel/drain is validated on @@ -1382,6 +1402,28 @@ if(Python3_Interpreter_FOUND) COMMAND ${Python3_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/tests/nitroqam_waterfall.py --self-test ) + + # The two MediaTek artifacts that are transcriptions of reference/mt76 — + # the USB-id gate table and the generated MAC initvals. Both were + # hand-checked once and then trusted; the id table was wrong the first time + # (11 of 16 entries, from the host kernel tree rather than the pinned one) + # and nothing caught it. Both SKIP (77) when the submodule is absent, which + # is the case in CI: `actions/checkout` here does not fetch submodules, so + # these are bench cells, not CI cells. Anyone who can regenerate initvals.h + # can run them. + add_test( + NAME mt7612u_usb_ids_vs_mt76 + COMMAND ${Python3_EXECUTABLE} + ${CMAKE_CURRENT_SOURCE_DIR}/tests/mt7612u_usb_ids_vs_mt76.py + ) + set_tests_properties(mt7612u_usb_ids_vs_mt76 PROPERTIES SKIP_RETURN_CODE 77) + + add_test( + NAME mt7612u_initvals_generated + COMMAND ${Python3_EXECUTABLE} + ${CMAKE_CURRENT_SOURCE_DIR}/tools/extract_mt7612u_tables.py --check + ) + set_tests_properties(mt7612u_initvals_generated PROPERTIES SKIP_RETURN_CODE 77) endif() # --- selftest aggregate ---------------------------------------------------- diff --git a/README.md b/README.md index 8d38347c..d68c1da3 100644 --- a/README.md +++ b/README.md @@ -109,10 +109,19 @@ Bandwidth cells are devourer's measured on-air TX throughput (Mbps, HT MCS7, | **RTL8832BU** (11ax) | 2T2R | — | — | — | — | Wi-Fi-only SKU of the 8852B die; rides the 8852BU code path. Not benchmarked. 5/10 MHz capable; HE ER SU + DCM extended range | | **RTL8832CU** (11ax) | 2T2R + BT | 40 | 33 | 32 | 32 | TP-Link Archer TX50UH (`35bc:0101`); Wi-Fi 6E tri-band (2.4/5/6 GHz). 5/10 and 160 MHz capable; HE ER SU + DCM extended range. Host-push injection over USB 2.0 (~50% duty ceiling); [6G TX+RX validated](tests/kestrel_8832cu_6g_txrx.sh) | | **RTL8852CU** (11ax) | 2T2R + BT | — | — | — | — | "8852" branding of the same 8852C die; rides the 8832CU code path. Not benchmarked. 5/10 and 160 MHz capable; HE ER SU + DCM extended range | +| **MT7612U** (MediaTek) | 2T2R | ‖ | ‖ | — | — | Alfa AWUS036ACM / Aukey USBAC1200 (`0e8d:7612`) and 15 OEM ids; the one non-Realtek backend, opt-in with `-DDEVOURER_MT7612U=ON`. 20/40/80 MHz; no 5/10 MHz, no fast retune. [Measured record](docs/mt7612u.md) | `†` = works on-air but the reading varies run-to-run (bracketed = best clean reading). +`‖` = **measured, but not by this column's method**, so not comparable with the +rows above. These cells are USRP duty cycle × PHY rate; the MT7612U bench has +no USRP, and its HT MCS7 / 20 MHz figures — 34.03 Mbit/s single-frame, 44.55 +with A-MPDU — are receiver frame counts instead, which is the instrument this +project deliberately does not judge TX by. Both directions are validated +against the kernel `mt76x2u` driver on 2.4 GHz and UNII-1: +[the regression matrix](docs/mt7612u.md#on-air-against-the-kernel-driver). + These cells are single-frame injection (the default TX path), measured as channel occupancy × PHY rate. A-MPDU (`SetAmpduMode`) does **not** move them on a chip already near the PHY ceiling — it raises *goodput* (delivered payload) @@ -137,6 +146,21 @@ long preamble only). It intentionally does not advertise VHT, LDPC, SGI, STBC, or experimental 5/10 MHz operation; see [the validation record](docs/rtl8733b.md) for the tested and deferred matrix. +The **MT7612U** backend (`src/mt7612u/`) is the one non-Realtek family, behind +the same `IRadio` contract and sharing none of the Realtek HAL — this is 32-bit +registers over EP0 plus an in-band MCU on EP8/EP5, with firmware uploaded at +bring-up. It is OFF by default; build it with `-DDEVOURER_MT7612U=ON`, and note +that the firmware is *not* embedded the way every Realtek blob is: `mt7662.bin` +and `mt7662_rom_patch.bin` ship zstd-compressed in linux-firmware under their +own licence, so they are searched for at runtime +(`DeviceConfig.mt7612u.firmware_dir`). Monitor RX with per-chain RSSI, raw +injection at any rate the TXWI can express, hardware ACK, A-MPDU and 20/40/80 +MHz tuning all work; there is no fast retune (≈50 ms fast / ≈530 ms full), no +5/10 MHz, no HE, no 160 MHz, no beacons, and unicast injection is a 40× cliff. +`SetTxMode` is refused — the C library has no session-default rate, so the rate +goes in each frame's radiotap header instead, where it always wins. [Every +number and its counterpart](docs/mt7612u.md). + > Heads up — some Realtek sticks ship in "ZeroCD" mode and first enumerate as > a USB flash drive holding a Windows installer (`0bda:1a2b` is the canonical > offender). If the device won't open, check `lsusb`; `usb_modeswitch` flips @@ -371,7 +395,9 @@ per-chip quirks notes at the bottom. Headless selftests run with `ctest`. Hardware regression is `tests/regress.py`: a TX/RX matrix between devourer and the kernel driver across plugged-in adapters, with optional full-pair, encoding-sweep, and -third-adapter-sniffer modes — see [`tests/README.md`](tests/README.md). +third-adapter-sniffer modes — see [`tests/README.md`](tests/README.md). It +covers the MediaTek backend too (`--mt7612u-fw-dir`, and `--tx-pid`/`--rx-pid` +accept a sysfs id so two adapters of one model can be told apart). ## License diff --git a/docs/mt7612u.md b/docs/mt7612u.md index 156a4ab6..8d82a827 100644 --- a/docs/mt7612u.md +++ b/docs/mt7612u.md @@ -2,8 +2,10 @@ Everything below was measured on one MT7612U (`0e8d:7612`, `MT_ASIC_VERSION` `0x76120044`, MT7662 MAC core, 2T2R, SuperSpeed) against an RTL8812AU witness -running this project's own `rxdemo`/`txdemo`. Read `## Offline tests` and -`## Counterparts` before quoting any number here. +running this project's own `rxdemo`/`txdemo` — except `## On-air, against the +kernel driver`, which puts a second, USB 2.0 unit of the same part on the other +end and the MediaTek kernel driver on one side of it. Read `## Offline tests` +and `## Counterparts` before quoting any number here. **It is wired in.** `DEVOURER_MT7612U` (default OFF) compiles the subtree into `libdevourer` and `WiFiDriver::CreateRadio` constructs an `Mt7612uRadio`, so a @@ -306,7 +308,34 @@ and nothing was ever queued on the healthy path. ## Offline tests -`make -C src/mt7612u check` runs four binaries. No hardware, no privileges. +`ctest` runs five MediaTek cells. The first three are C++ and need neither +hardware nor the `DEVOURER_MT7612U` option — the code they cover is +header-only. The last two are Python and need the `reference/mt76` submodule: + +| cell | what it holds | +|---|---| +| `mt7612u_usb_ids` | that the 16 MediaTek pairs never claim a device one of the 91 Realtek ids owns. The gate runs *before* the Realtek `SYS_CFG2` read, so an id we wrongly own is refused with no second chance | +| `mt7612u_mapping` | the descriptor translations — RSSI bias, per-chain signal, rate codes, TID offset, widths. Every one of these has been wrong once | +| `mt7612u_rx_queue` | the RX hand-off queue's two load-bearing properties: a full ring drops the **newest** frame and counts it, and a popped slot outlives the queue lock. Broken, the first reorders frames or wedges the part and the second is a use-after-free — and both look like a healthy link until a packet processor falls behind | +| `mt7612u_usb_ids_vs_mt76` | that the id table above really is the complete `mt76x2u_device_table` from the pinned reference, byte-checked. An id we *forgot* is invisible to `mt7612u_usb_ids` — the adapter just falls through to the Realtek path — and the first draft of that header had 11 of the 16, taken from the host's kernel tree | +| `mt7612u_initvals_generated` | that the checked-in `initvals.h` still matches what `tools/extract_mt7612u_tables.py` produces from the pinned reference | + +The last two SKIP when `reference/mt76` is absent, which is the case in CI — +`actions/checkout` here does not fetch submodules. They are bench cells. Both +are also registered only `if(Python3_Interpreter_FOUND)`, so on a host without +Python they are *absent* rather than skipped, and `ctest` says nothing at all. + +What still has no cell is the *wiring*: `Mt7612uRadio`'s calls into the queue — +the reset before the ring is armed, the commit after the processor returns — +are covered by nothing headless. Deleting the `pop_commit()` compiles clean and +passes the whole suite. + +`mt7612u_rx_queue` was mutation-tested five ways: dropping the oldest instead of +the newest, not counting the drop, using the whole allocation instead of leaving +the sentinel slot, a `reset()` that forgets the counter, and a `wake()` that does +nothing. Each fails the cell, and each names the property it broke. + +`make -C src/mt7612u check` runs four more binaries. No hardware, no privileges. | test | what it holds | |---|---| @@ -320,14 +349,114 @@ frame-shape fix, and reverting the header-length fix each make the suite fail, with the RX one reporting `QoS Control zeroed by the pad fold: aa aa`. `tools/extract_mt7612u_tables.py --check` byte-compares the generated -`initvals.h` against `reference/mt76` at the pinned commit. +`initvals.h` against `reference/mt76` at the pinned commit. It is the +`mt7612u_initvals_generated` cell above; until now nothing ran it. + +## On-air, against the kernel driver + +`tests/regress.py` runs its four-cell matrix on this part, so "devourer's +MediaTek backend interoperates with the MediaTek kernel driver" is a command +you can re-run rather than a story. Both ends are MT7612U — the SuperSpeed unit +transmitting, a second **USB 2.0** unit receiving: + +``` +sudo python3 tests/regress.py --channel 36 --tx-pid 2-1 --rx-pid 5-1 \ + --mt7612u-fw-dir --duration 15 +``` + +Cells are `hits / frames offered`, 15 s each. Twelve cells over three runs — +ch36 twice and ch6 once — and every one passed on its first attempt: + +| **ch36** (UNII-1, 5180 MHz) | RX = devourer | RX = kernel `mt76x2u` | +|---|---|---| +| **TX = devourer** | 8500 / 9383 ✓ | 8921 / 9288 ✓ | +| **TX = kernel** (scapy) | 400 / 496 ✓ | 458 / 459 ✓ | + +| **ch6** (2.4 GHz, 2437 MHz) | RX = devourer | RX = kernel `mt76x2u` | +|---|---|---| +| **TX = devourer** | 8600 / 9433 ✓ | 8531 / 9354 ✓ | +| **TX = kernel** (scapy) | 400 / 497 ✓ | 458 / 488 ✓ | + +The kernel-RX figures are `tcpdump` record counts. The harness published each +of them one higher until this change: it counted `splitlines()`, and the log's +trailing newline made a phantom record — which is how the baseline cell came to +report a suspiciously perfect `459 / 459` when 458 of 459 arrived. + +The ch36 table above is the second run; the first, ~4 minutes earlier, agrees +to within ordinary run-to-run variance — baseline 466 vs 458 and devourer-TX to +kernel-RX 9077 vs 8921, both exact `tcpdump` counts, so ~1.7 % and not a +sampling artefact. + +Some detail from the per-cell logs that the hit counts do not show: + +- **The kernel driver decodes our frames at 6 Mbps on both bands.** `tcpdump` + on the `mt76x2u` side reads `6.0 Mb/s 5180 MHz 11a ... SA:57:42:75:05:d6:00` + at a −31 dBm mode on ch36 and `6.0 Mb/s 2437 MHz 11g` at −35 dBm on ch6, each + with a per-antenna breakdown. That is an independent decoder confirming the + rate `SetTxMode`'s refusal predicts, not our own reading of our own transmit. + (6 Mbps because `txdemo` builds rate-less frames; the part itself airs + whatever the TXWI says — see `## TX rate is authoritative`.) +- **Nothing was dropped at the hand-off queue, in any of the four cells that + ran a devourer receiver.** The queue only exists on that side; the other four + are `tcpdump`. The busiest carried 20391 frames — ambient included — over the + ~28 s the receiver was up, and `StopRxLoop` logged the plain "RX stopped + after N frames" line rather than the DROPPED one. +- **TX reported 0 failed in all four devourer-TX cells** (9288–9433 submitted + each) — with the standing caveat that on this backend `failed` counts + refusals rather than wire deaths (Counterparts). +- The hit counters are **sampled on the devourer side only**: `rx.txhit` fires + at `hits <= 10` or every 100th, so a reported 8500 means 8500–8599. The + kernel-RX counts are `tcpdump` lines, granularity 1. `frames offered` is + `tx.stats` for the devourer rows and the injector's own tally for the kernel + rows, both exact. The matrix's own table prints `tx.frame` instead, sampled + every 500th and therefore low — which is why a cell there can show more hits + than TX. + +### What this does not show + +- **The kernel side is the host's in-tree `mt76x2u`** (6.14.0-37, mainline + mt76), not a build of `reference/mt76 @ be5ce79`. They are the same codebase + at different revisions. Loading the reference tree instead would mean + replacing the `mt76` core module — refcount 9 on this host, `mt7921e` among + its users, i.e. the machine's own Wi-Fi. The exact pinned revision is held + instead by the two `reference/`-comparing cells above, which is a stronger + check of the *transcription* and no check at all of the *interop*. Note that + `regress.py` has a designed way round this — its VM mode exists precisely to + run kernel cells against an out-of-tree driver in a pinned-kernel guest, + where the host's module refcount is irrelevant. That VM was never set up for + MediaTek, so this is "not done", not "not possible". +- **A hit count is not a throughput number.** The bench discipline in this repo + is to judge TX by SDR duty cycle × PHY rate and never by receiver frame + counts (`tests/ampdu_onair_ab.sh` says so at the top); there is no USRP on + this bench, which is why the README's bandwidth cells for this part carry a + `‖` — measured, but by the receiver-count method that column is not — rather + than a number that would read as comparable with the Realtek rows. +- **A ✓ is one frame.** The pass threshold is `regress.py`'s default of 1, so + the marks say "the path is not dead" and only the counts say how well it + works. Nor can the artifacts show a cell was not re-run: what they show is + three consecutive runs whose every cell passed, with no re-run in between. +- **The matrix reports; it does not gate.** `regress.py` exits 0 whatever the + cells say — a run in which all four failed exits 0 too. It is output to read, + not a check to wire into anything. +- **Both adapters are ~20 cm apart** on one bench, so every RSSI here is + near-field and says nothing about range. +- **The kernel-TX cells are scapy injection** through `mt76x2u`, which airs + ~33 frames/s. The ~500-vs-~9000 asymmetry between the rows is the injector's + rate, not a receiver difference. +- **UNII-2/3 is untested.** This bench runs `iw reg set SE`, and ETSI has no + ch149, so the third column of the README's table has no MediaTek reading and + is not going to get one here without changing the regulatory domain. ## Counterparts Stated because the numbers above are uniformly favourable. -- **One physical unit, one sample.** No second MT7612U, no second board - revision, no vendor-driver A/B beyond the register diff. +- **Two units, one design.** Everything above the regression matrix was + measured on a single SuperSpeed MT7612U. The matrix adds a second unit — same + `0e8d:7612`, same `bcdDevice`, but a USB 2.0 one — which is enough to say the + backend is not fitted to one adapter's quirks and not enough to call it a + second sample of the *silicon*. No second board revision, no second OEM id, + and no vendor-driver A/B beyond the register diff and the interop matrix. - **Two adapters share one diagnostic route.** The C library's log sink is a single process-global pair, so `Mt7612uRadio` keeps a registry and routes to the first live radio's `Logger`. With two MediaTek adapters open, the second @@ -343,9 +472,12 @@ Stated because the numbers above are uniformly favourable. URBs where this counts frames, and restarts at zero when a ring is torn down, so `failed` would go backwards across an RX restart. Needs a monotonic wire-failure counter in the library. -- **One witness generation.** Every on-air number is an RTL8812AU running this - project's `rxdemo`. `paggr`, `bw` and `rate` are that implementation's - reading, not an independent instrument. +- **Effectively one witness implementation.** Every on-air number outside the + regression matrix is an RTL8812AU running this project's `rxdemo`: `paggr`, + `bw` and `rate` are that implementation's reading, not an independent + instrument. The matrix's kernel-RX cells add a second one — `tcpdump` over + `mt76x2u` — which is genuinely independent code, but on the same silicon + family, and it reports only legacy rate and dBm. - **TX power was verified against the kernel's registers, not against radiated power.** No spectrum analyser, no power meter. Register equality is not dBm — the correctness claim is "identical to what mt76 programs", nothing @@ -363,18 +495,20 @@ Stated because the numbers above are uniformly favourable. firmware-running bit. - **The 48 ms fast retune is our implementation, not the floor.** The floor is unmeasured. -- **The library compiles in CI, and two ctest cells cover the integration; the - subtree's own four tests still do not run there.** With `DEVOURER_MT7612U=ON` - the whole platform matrix (gcc, clang, MSVC, mingw, macOS) builds the subtree - and the sanitizer job links it, so a portability or lifetime regression is - caught. `mt7612u_usb_ids` and `mt7612u_mapping` run on every configuration — - but both cover pure LOOKUPS, not the backend's behaviour, which has no - automated coverage beyond compiling. Every hardware claim in this document is - hand-run. - The four offline tests under `src/mt7612u/tests/` and the table generator's - `--check` are still driven only by `src/mt7612u/Makefile`, which no workflow - invokes — nor is there a lifecycle soak of the kind the Realtek backends - carry. +- **Three ctest cells run in CI, two more only on a bench, and the subtree's own + four still run nowhere automatically.** With `DEVOURER_MT7612U=ON` the whole + platform matrix (gcc, clang, MSVC, mingw, macOS) builds the subtree and the + sanitizer job links it, so a portability or lifetime regression is caught. + `mt7612u_usb_ids`, `mt7612u_mapping` and `mt7612u_rx_queue` run on every + configuration. That last one is the first cell to cover backend *behaviour* + rather than a lookup — but it covers the queue and nothing else. Bring-up, + the teardown ordering, the 1 Hz tick and TX still have no automated coverage, + and the two `reference/`-comparing cells SKIP in CI because the submodule is + not fetched there. Every hardware claim in this document is hand-run except + the regression matrix, which is `tests/regress.py`. + The four offline tests under `src/mt7612u/tests/` are still driven only by + `src/mt7612u/Makefile`, which no workflow invokes — nor is there a lifecycle + soak of the kind the Realtek backends carry. - **80 MHz, VHT on air, and NSS=2 are unexercised.** The rate word encodes them and the RX path decodes them; neither has been transmitted. - **The RX path delivers no FCS** (see above). That is a measured hardware @@ -415,7 +549,10 @@ Ordered, and honest about which are unknowns rather than typing: 2. `mt76x2_phy_tssi_compensate()` — periodic temperature correction. Without it output power drifts with die temperature. 3. Cold-boot verification on a host with switchable USB power. -4. A second sample, and a second witness generation. +4. A witness on different silicon. The regression matrix added a second witness + *implementation* (`tcpdump` over `mt76x2u`), and a second *unit* of the part + — but not a second board revision, and not a decoder outside the MediaTek + family. 5. 80 MHz; VHT and NSS=2 on air. 6. Retune tuning — batch registers via `CMD_RANDOM_WRITE`, drop the inter-command sleep, skip the `RXDCOC` on a fast path. Worth doing only if 10–20 ms is diff --git a/src/mt7612u/Mt7612uRadio.cpp b/src/mt7612u/Mt7612uRadio.cpp index 5d9df62a..84ac8592 100644 --- a/src/mt7612u/Mt7612uRadio.cpp +++ b/src/mt7612u/Mt7612uRadio.cpp @@ -378,12 +378,13 @@ void Mt7612uRadio::StartRxLoop(Action_ParsedRadioPacket packetProcessor) { throw std::runtime_error("MT7612U RX loop is already active"); _rx_processor = std::move(packetProcessor); _rx_stop = false; - /* 64 slots at the part's 3836-byte max MPDU is ~245 KB, about 45 ms of - * headroom at the measured 1400 fps - enough to ride out a slow processor - * without letting the producer block. Allocated here, not per frame. */ - _rx_q.assign(64, RxSlot{}); - _rx_q_head = _rx_q_tail = 0; - _rx_queue_dropped = 0; + /* 64 slots is about 45 ms of headroom at the measured 1400 fps - enough to + * ride out a slow processor without letting the producer block. The ring + * itself is allocated here; each slot's payload buffer then grows to the + * largest frame that slot has held (~245 KB in total at the part's + * 3836-byte max MPDU), so the event thread does still allocate during the + * first pass round the ring and on any frame-size step-up. */ + _rx_q.reset(64); /* Ring first, receiver second - see rule 1 in the header. */ if (mt7612u_rx_start(_dev, &Mt7612uRadio::rx_trampoline, this) != 0) @@ -422,15 +423,8 @@ void Mt7612uRadio::StartRxLoop(Action_ParsedRadioPacket packetProcessor) { * contract every other backend keeps. See the queue's comment in the header * for why delivering on the event thread wedges the hardware. */ for (;;) { - RxSlot *slot = nullptr; - { - std::unique_lock lock(_rx_q_mu); - _rx_q_cv.wait_for(lock, std::chrono::milliseconds(20), [this] { - return _rx_q_head != _rx_q_tail || _rx_stop.load(); - }); - if (_rx_q_head != _rx_q_tail) - slot = &_rx_q[_rx_q_tail]; - } + mt7612u::RxQueue::Slot *slot = + _rx_q.pop_begin(std::chrono::milliseconds(20), _rx_stop); if (!slot) { if (_rx_stop.load() || g_devourer_should_stop) break; @@ -438,18 +432,15 @@ void Mt7612uRadio::StartRxLoop(Action_ParsedRadioPacket packetProcessor) { } /* Outside the queue lock: user code runs here, and it may call back into - * this object. The producer never writes the slot at _rx_q_tail, so this - * reference stays valid until the tail is advanced below. */ + * this object. The producer never writes the slot at the tail, so this + * reference stays valid until pop_commit() below. */ Packet packet{}; packet.RxAtrib = slot->attrib; packet.Data = std::span(slot->data.data(), slot->data.size()); if (_rx_processor) _rx_processor(packet); - { - std::lock_guard lock(_rx_q_mu); - _rx_q_tail = (_rx_q_tail + 1) % _rx_q.size(); - } + _rx_q.pop_commit(slot); } StopRxLoop(); @@ -458,7 +449,7 @@ void Mt7612uRadio::StartRxLoop(Action_ParsedRadioPacket packetProcessor) { void Mt7612uRadio::StopRxLoop() { _rx_stop = true; /* Wake the consumer immediately rather than leaving it to time out. */ - _rx_q_cv.notify_all(); + _rx_q.wake(); /* Held across the WHOLE teardown, so a second caller blocks here instead of * returning while the first is still inside mt7612u_rx_stop(). That early @@ -491,10 +482,13 @@ void Mt7612uRadio::StopRxLoop() { * existed, and mt7612u_rx_start() then returned success WITHOUT arming. */ _rx_active.store(false, std::memory_order_release); - const uint64_t dropped = _rx_queue_dropped.load(std::memory_order_relaxed); + const uint64_t dropped = _rx_q.dropped(); if (dropped) - _logger->warn("MT7612U RX stopped after {} frames, {} DROPPED at the " - "hand-off queue - the packet processor could not keep up", + /* "received" is the count off the air, so it INCLUDES the dropped ones - + * spelling that out because "N frames, D DROPPED" reads as N delivered. */ + _logger->warn("MT7612U RX stopped after {} frames received, of which {} " + "DROPPED at the hand-off queue - the packet processor could " + "not keep up", _rx_frames.load(std::memory_order_relaxed), dropped); else _logger->info("MT7612U RX stopped after {} frames", @@ -543,25 +537,10 @@ void Mt7612uRadio::on_rx(const void *frame, size_t len, /* Copy and hand off. The library's buffer is reused the moment this returns, * and the consumer now runs on another thread, so the frame cannot be passed - * by reference the way it could when the processor ran here. */ - { - std::lock_guard lock(_rx_q_mu); - const size_t next = (_rx_q_head + 1) % _rx_q.size(); - if (next == _rx_q_tail) { - /* Full: the consumer is slower than the air. Drop the NEWEST rather than - * block - blocking here is precisely the wedge this queue exists to - * prevent, and dropping the oldest would reorder frames. Counted so the - * loss is visible instead of silent. */ - _rx_queue_dropped.fetch_add(1, std::memory_order_relaxed); - return; - } - RxSlot &slot = _rx_q[_rx_q_head]; - slot.attrib = packet.RxAtrib; - slot.data.assign(static_cast(frame), - static_cast(frame) + len); - _rx_q_head = next; - } - _rx_q_cv.notify_one(); + * by reference the way it could when the processor ran here. A full queue + * drops this frame and counts it rather than blocking - blocking here is + * precisely the wedge the queue exists to prevent. */ + _rx_q.push(packet.RxAtrib, static_cast(frame), len); } void Mt7612uRadio::SetMonitorChannel(SelectedChannel channel) { diff --git a/src/mt7612u/Mt7612uRadio.h b/src/mt7612u/Mt7612uRadio.h index 6c4ee6b9..1e25b917 100644 --- a/src/mt7612u/Mt7612uRadio.h +++ b/src/mt7612u/Mt7612uRadio.h @@ -15,6 +15,7 @@ #include "RxPacket.h" #include "UsbDeviceLock.h" #include "logger.h" +#include "mt7612u/Mt7612uRxQueue.h" #include "mt7612u/mt7612u.h" /* @@ -141,34 +142,14 @@ class Mt7612uRadio : public IRadio { std::atomic _rx_stop{false}; std::atomic _rx_active{false}; std::atomic _rx_frames{0}; - std::atomic _rx_queue_dropped{0}; /* Frames cross from the C library's event thread to the StartRxLoop thread * here, rather than the processor being invoked where the frame arrives. - * - * That is not a style choice. The library's event thread is the SOLE - * servicer of both RX and TX completions, so a processor that transmits - - * examples/chanmig does, from its RX callback - would park that thread in - * mt_async_tx_submit waiting for a TX slot only that same thread can free. - * MAC RX stays enabled, EP4 stops being drained, and this part wedges below - * the USB level where only a physical replug recovers it. Anything taking - * _mu from the processor has a milder version of the same problem: it stalls - * the drain for a 526 ms channel change. - * - * Delivering on the StartRxLoop thread also restores the contract every - * Realtek backend keeps - the processor runs on the thread that called - * StartRxLoop - and keeps the library's "must not block, must not call back" - * rule an internal invariant instead of one silently exported to consumers. - * The copy costs ~2 MB/s at the measured 1400 fps. */ - struct RxSlot { - std::vector data; - rx_pkt_attrib attrib{}; - }; - std::vector _rx_q; - size_t _rx_q_head = 0; /* next slot to write */ - size_t _rx_q_tail = 0; /* next slot to read */ - std::mutex _rx_q_mu; - std::condition_variable _rx_q_cv; + * That is not a style choice - it is what keeps a transmitting processor + * from wedging the part below the USB level. The queue's own header carries + * the argument and the two properties (drop the newest and count it; the + * popped slot outlives the lock) that its selftest holds. */ + mt7612u::RxQueue _rx_q; std::atomic _tx_submitted{0}; std::atomic _tx_failed{0}; diff --git a/src/mt7612u/Mt7612uRxQueue.h b/src/mt7612u/Mt7612uRxQueue.h new file mode 100644 index 00000000..e9cf16c2 --- /dev/null +++ b/src/mt7612u/Mt7612uRxQueue.h @@ -0,0 +1,160 @@ +#ifndef MT7612U_RX_QUEUE_H +#define MT7612U_RX_QUEUE_H + +#include +#include +#include +#include +#include +#include +#include + +#include "RxPacket.h" + +namespace mt7612u { + +/* + * The hand-off between the C library's USB event thread and the thread that + * called StartRxLoop. + * + * WHY IT EXISTS. The library's event thread is the SOLE servicer of both RX + * and TX completions, so a packet processor that transmits - examples/chanmig + * does, from its RX callback - would park that thread in mt_async_tx_submit + * waiting for a TX slot only that same thread can free. MAC RX stays enabled, + * EP4 stops being drained, and this part wedges below the USB level, where + * libusb_reset_device, the sysfs authorized toggle and rebinding the kernel + * driver all fail and only a physical replug recovers it. Anything that merely + * takes the radio's control lock from the processor has a milder version of the + * same problem: it stalls the drain for a 526 ms channel change. + * + * Running the processor on the StartRxLoop thread instead also restores the + * contract every Realtek backend keeps, and keeps the library's "must not + * block, must not call back" rule an internal invariant rather than one + * silently exported to consumers. The copy costs ~2 MB/s at the measured + * 1400 fps. + * + * TWO PROPERTIES THAT ARE NOT FREE TO CHANGE. + * + * 1. A full queue drops the NEWEST frame and counts it. Blocking the producer + * is precisely the wedge above; dropping the oldest would reorder frames + * under a consumer that is only briefly behind. The count is what keeps the + * loss visible instead of silent - StopRxLoop logs it. + * + * 2. pop_begin() hands back a pointer that stays valid, without the lock held, + * until pop_commit(). That is what lets user code run outside the queue + * lock, and it holds because the producer never writes the slot at the tail: + * one slot of the ring is always left empty, so `full` is one short of the + * allocation. Capacity is therefore slots - 1. + */ +class RxQueue { +public: + struct Slot { + std::vector data; + rx_pkt_attrib attrib{}; + }; + + /* Allocate `slots` ring entries, rewind, and zero the drop count. + * + * Only safe while no producer is running AND no slot is outstanding: this + * reallocates the ring, so it invalidates any pointer pop_begin() handed + * back, and even a same-size reset empties every slot under a consumer still + * reading one. The radio calls it before arming the ring, with the previous + * session's event thread already joined - see StartRxLoop. + * + * `slots` counts the always-empty sentinel, so usable capacity is slots - 1; + * values below 2 are raised to 2, since a ring of one could never hold a + * frame and would drop the entire stream silently. */ + void reset(size_t slots) { + if (slots < 2) + slots = 2; + std::lock_guard lock(_mu); + _q.assign(slots, Slot{}); + _head = _tail = 0; + _dropped.store(0, std::memory_order_relaxed); + } + + /* Producer side, called from the library's event thread. Returns false when + * the frame was dropped, having counted it. */ + bool push(const rx_pkt_attrib &attrib, const uint8_t *frame, size_t len) { + { + std::lock_guard lock(_mu); + /* Before the modulo: an un-reset queue has no slots to write and a + * `% 0` would be undefined rather than a drop. */ + if (_q.empty()) { + _dropped.fetch_add(1, std::memory_order_relaxed); + return false; + } + const size_t next = (_head + 1) % _q.size(); + if (next == _tail) { + _dropped.fetch_add(1, std::memory_order_relaxed); + return false; + } + Slot &slot = _q[_head]; + slot.attrib = attrib; + slot.data.assign(frame, frame + len); + _head = next; + } + _cv.notify_one(); + return true; + } + + /* Consumer side. Waits up to `wait` for a frame; `stop` ends the WAIT early + * but does not end the DRAIN - a queued frame is still handed back after + * stop is set, and nullptr means "the ring is empty", never "we are + * stopping". That is load-bearing: the radio's consumer loop leaves only on + * a nullptr, so a pop_begin that short-circuited on `stop` would silently + * discard whatever was still queued at teardown. + * + * The returned slot is valid, without the lock held, until pop_commit() - + * the producer refuses at a full ring rather than writing the tail slot. */ + Slot *pop_begin(std::chrono::milliseconds wait, const std::atomic &stop) { + std::unique_lock lock(_mu); + _cv.wait_for(lock, wait, [this, &stop] { + return _head != _tail || stop.load(); + }); + if (_head == _tail) + return nullptr; + return &_q[_tail]; + } + + /* Releases the slot pop_begin() handed back - pass back the same pointer. + * + * Checking it is not ceremony: the pairing is otherwise enforced by nothing, + * and a second commit for one pop would advance the tail past an + * undelivered frame, silently and uncounted. Handing back a slot that is no + * longer the tail is therefore a no-op rather than a skipped frame. */ + void pop_commit(const Slot *slot) { + std::lock_guard lock(_mu); + if (_q.empty() || _head == _tail || slot != &_q[_tail]) + return; + _tail = (_tail + 1) % _q.size(); + } + + /* Wakes every waiter without enqueuing anything - the stop path, so a + * consumer usually returns at once instead of sitting out its timeout after + * StopRxLoop. Usually, not always: the stop flag lives outside this lock, so + * a consumer that evaluates the predicate just before the flag moves still + * sleeps out the wait. The timeout stays the bound - 20 ms in the radio. */ + void wake() { _cv.notify_all(); } + + uint64_t dropped() const { return _dropped.load(std::memory_order_relaxed); } + + /* Frames the ring can hold at once. One short of the allocation; see + * property 2 above. */ + size_t capacity() const { + std::lock_guard lock(_mu); + return _q.empty() ? 0 : _q.size() - 1; + } + +private: + std::vector _q; + size_t _head = 0; /* next slot to write */ + size_t _tail = 0; /* next slot to read */ + std::atomic _dropped{0}; + mutable std::mutex _mu; + std::condition_variable _cv; +}; + +} // namespace mt7612u + +#endif /* MT7612U_RX_QUEUE_H */ diff --git a/tests/README.md b/tests/README.md index 81a807c0..33ced777 100644 --- a/tests/README.md +++ b/tests/README.md @@ -99,6 +99,21 @@ probe on kernels 6.15+ (`failed to download firmware`, `error -22`), but - `iw`, `tcpdump`, `ip` on PATH - Passwordless `sudo`, or run directly as root +### For MediaTek (MT7612U) DUTs + +- devourer configured with `-DDEVOURER_MT7612U=ON`. Without it `rxdemo` + still builds and still refuses the adapter at `CreateRadio`, by design — + falling through to the Realtek path would misdetect it as a Jaguar1. +- `mt7662.bin` + `mt7662_rom_patch.bin`, decompressed, passed with + `--mt7612u-fw-dir`. They ship as `.bin.zst` in linux-firmware. +- `mt76x2u` for the kernel-side cells. + +`regress.py` checks the first two before the first cell — each otherwise +produces a cell that reads exactly like a dead radio — and only if `build/` has +a CMake cache to read. A missing `mt76x2u` is a warning rather than an error, +since a devourer-only run does not need it; it shows up later as +`no wlan iface appeared for 0e8d:7612 after 20.0s`. + ### Adaptive-hopset validation (`hopset_adaptive_jammer.sh`) Three radios — a transmitting authority, a lockstep receiver running the @@ -204,7 +219,16 @@ per-cell stdout/stderr logs end up at `/tmp/devourer-regress-last/`. - `--duration SECONDS` — per-cell injection/measurement window (default 15) - `--pass-threshold N` — min hits to pass (default 1) - `--tx-pid 0xNNNN` / `--rx-pid 0xNNNN` — pick specific DUTs (defaults to - the first two auto-detected) + the first two auto-detected). Each also accepts a **sysfs id** (`2-1`, + `3-2.2`), which is the only way to name one of two adapters that share a + model — the normal case for a MediaTek matrix, where there is one PID + worth having. +- `--mt7612u-fw-dir DIR` — where `mt7662.bin` + `mt7662_rom_patch.bin` live, + for MediaTek DUTs (env: `DEVOURER_MT7612U_FW_DIR`). Unlike every Realtek + backend the MediaTek firmware is not embedded: it ships zstd-compressed in + linux-firmware under its own licence, so the pair has to be decompressed + somewhere first. Unset lets the backend search its own defaults + (`/lib/firmware/mediatek`, then `./firmware`). - `--no-baseline-abort` — run all 4 cells even if kernel-kernel fails (useful when one chipset has no working kernel driver on the host) - `--no-rf-reset` — skip the per-cell USB port-level authorize-cycle. @@ -219,7 +243,7 @@ per-cell stdout/stderr logs end up at `/tmp/devourer-regress-last/`. - `--keep-logs` — symlink the temp log dir at `/tmp/devourer-regress-last` Environment variable equivalents: `DEVOURER_VM_NAME`, `DEVOURER_VM_SSH`, -`DEVOURER_SNIFFER_IFACE`. +`DEVOURER_SNIFFER_IFACE`, `DEVOURER_MT7612U_FW_DIR`. ### `--sniffer-iface` — on-air encoding verification + attribution diff --git a/tests/mt7612u_rx_queue_selftest.cpp b/tests/mt7612u_rx_queue_selftest.cpp new file mode 100644 index 00000000..27fd27e3 --- /dev/null +++ b/tests/mt7612u_rx_queue_selftest.cpp @@ -0,0 +1,439 @@ +/* Headless guard for the MT7612U RX hand-off queue + * (src/mt7612u/Mt7612uRxQueue.h). + * + * The queue exists because delivering RX on the C library's event thread + * wedges the part below the USB level, and the two properties that make it + * safe are both invisible on a healthy bench: a full queue must drop the + * NEWEST frame and count it (blocking is the wedge; dropping the oldest + * reorders), and the slot pop_begin() returns must stay valid while the + * consumer runs user code with the lock released. On hardware a broken + * version of either looks like "the link is fine" right up until a slow + * packet processor turns it into a silent reorder, a use-after-free, or a + * replug. Hence a cell rather than a comment. + * + * What this does NOT cover: the ordering rules around the queue - ring before + * receiver, quiesce before the drain is removed - which need a device. Those + * are hand-run and recorded in docs/mt7612u.md. */ +#include "mt7612u/Mt7612uRxQueue.h" + +#include +#include +#include +#include + +using mt7612u::RxQueue; + +namespace { + +int fails; + +void expect(const char *what, bool ok) { + if (!ok) { + std::fprintf(stderr, "mt7612u_rx_queue: FAIL %s\n", what); + fails++; + } +} + +/* A frame whose every byte is `tag`, so a slot mix-up is visible in the + * payload and not only in the attrib. `pkt_len` carries the true length, which + * is what pop_tag checks the delivered payload against. */ +bool push_tagged(RxQueue &q, uint8_t tag, size_t len = 8) { + std::vector frame(len, tag); + rx_pkt_attrib a{}; + a.pkt_len = static_cast(len); + a.priority = tag; /* travels with the frame; checked on the way out */ + return q.push(a, frame.data(), frame.size()); +} + +/* Pops one frame and returns its tag, or a negative code: + * -1 nothing queued + * -2 the payload is empty + * -3 attrib and payload came from different frames + * -4 the payload is not the length the frame was pushed with + * -5 the payload was not tagged all the way to its LAST byte + * -4 and -5 exist because the consumer's frame length comes only from + * data.size() (Mt7612uRadio builds its span from it) while a processor that + * trusts attrib.pkt_len would then read past the end. A push that truncated + * every frame passed this cell until both were added. */ +int pop_tag(RxQueue &q, const std::atomic &stop) { + RxQueue::Slot *s = q.pop_begin(std::chrono::milliseconds(1), stop); + if (!s) + return -1; + int tag = s->data.empty() ? -2 : s->data[0]; + if (tag >= 0 && s->attrib.priority != tag) + tag = -3; + if (tag >= 0 && s->data.size() != s->attrib.pkt_len) + tag = -4; + if (tag >= 0 && s->data.back() != s->data.front()) + tag = -5; + q.pop_commit(s); + return tag; +} + +} // namespace + +int main() { + std::atomic stop{false}; + + /* --- capacity is one short of the allocation --- + * These four pin the SENTINEL-SLOT MECHANISM, which the header documents + * (capacity() == slots - 1) but which is not the property the design rests + * on. The property is "the producer never writes the slot the consumer is + * holding" — asserted mechanism-independently further down, under "the + * popped slot survives the producer filling the rest of the ring". A rewrite + * that kept a count instead of a sentinel would fail these four and still be + * correct; it would be changing the documented capacity, so they are here + * deliberately, as the allocation's documentation and not as the guarantee. */ + { + RxQueue q; + q.reset(4); + expect("reset(4) holds 3", q.capacity() == 3); + expect("push 1 accepted", push_tagged(q, 1)); + expect("push 2 accepted", push_tagged(q, 2)); + expect("push 3 accepted", push_tagged(q, 3)); + /* The 4th must be refused: the tail slot is never written, which is what + * lets the consumer hold a popped slot without the lock. */ + expect("push 4 refused (ring keeps one slot empty)", !push_tagged(q, 4)); + expect("refusal counted", q.dropped() == 1); + } + + /* --- a full queue drops the NEWEST, never the oldest --- */ + { + RxQueue q; + q.reset(4); + push_tagged(q, 10); + push_tagged(q, 11); + push_tagged(q, 12); + expect("overflow frame refused", !push_tagged(q, 99)); + /* 99 is the frame that was dropped; 10, 11, 12 survive IN ORDER. A + * drop-oldest queue would answer 11, 12, 99 here. */ + expect("oldest survives", pop_tag(q, stop) == 10); + expect("order preserved (2nd)", pop_tag(q, stop) == 11); + expect("order preserved (3rd)", pop_tag(q, stop) == 12); + expect("the newest is the one that went missing", + pop_tag(q, stop) == -1); + } + + /* --- the ring wraps, and stays FIFO across the wrap --- */ + { + RxQueue q; + q.reset(4); + for (int round = 0; round < 5; ++round) { + const uint8_t tag = static_cast(20 + round); + expect("wrap: push accepted", push_tagged(q, tag)); + expect("wrap: same frame comes back", pop_tag(q, stop) == 20 + round); + } + expect("wrap: nothing dropped", q.dropped() == 0); + } + + /* --- reset() rewinds AND zeroes the count --- */ + { + RxQueue q; + q.reset(2); + expect("reset(2) holds 1", q.capacity() == 1); + push_tagged(q, 1); + expect("2nd refused at capacity 1", !push_tagged(q, 2)); + expect("count is 1 before reset", q.dropped() == 1); + q.reset(8); + expect("reset zeroes the drop count", q.dropped() == 0); + expect("reset empties the ring", pop_tag(q, stop) == -1); + expect("reset resizes", q.capacity() == 7); + } + + /* --- a ring of one could never hold a frame: raised to exactly 2 --- */ + { + RxQueue q; + q.reset(1); + expect("reset(1) is raised to the smallest usable ring", + q.capacity() == 1); + expect("reset(1) can still take a frame", push_tagged(q, 7)); + expect("reset(1) hands it back", pop_tag(q, stop) == 7); + q.reset(0); + expect("reset(0) likewise", q.capacity() == 1); + } + + /* --- frames of different lengths arrive whole --- + * The consumer's frame length comes only from data.size(); attrib.pkt_len is + * what a processor would trust. A push that dropped a byte, or truncated to + * one, passed every other cell here. */ + { + RxQueue q; + q.reset(8); + for (size_t len : {size_t{1}, size_t{2}, size_t{60}, size_t{1500}, + size_t{3836}}) { + expect("varying length accepted", push_tagged(q, 70, len)); + RxQueue::Slot *s = q.pop_begin(std::chrono::milliseconds(1), stop); + expect("varying length handed back", s != nullptr); + if (s) { + expect("payload is the length it was pushed with", s->data.size() == len); + expect("payload is intact to its last byte", + s->data.front() == 70 && s->data.back() == 70); + q.pop_commit(s); + } + } + /* A zero-length frame is a real shape on this wire (an ACK with the FCS + * and the FCE trailer already stripped can reach here empty). It must be + * queued, not silently dropped. */ + rx_pkt_attrib a{}; + const uint8_t empty_frame[1] = {0}; + expect("a zero-length frame is accepted", q.push(a, empty_frame, 0)); + RxQueue::Slot *s = q.pop_begin(std::chrono::milliseconds(1), stop); + expect("a zero-length frame is handed back", s != nullptr); + if (s) { + expect("...as zero length", s->data.empty()); + q.pop_commit(s); + } + } + + /* --- an un-reset queue drops rather than dividing by zero --- */ + { + RxQueue q; + expect("un-reset capacity is 0", q.capacity() == 0); + expect("un-reset push refused", !push_tagged(q, 1)); + expect("un-reset push counted", q.dropped() == 1); + expect("un-reset pop is empty", pop_tag(q, stop) == -1); + q.pop_commit(nullptr); /* must not fall off the end of an empty vector */ + } + + /* --- the popped slot survives the producer filling the rest of the ring --- + * This is the property that lets user code run outside the queue lock. */ + { + RxQueue q; + q.reset(4); + push_tagged(q, 30); + RxQueue::Slot *held = q.pop_begin(std::chrono::milliseconds(1), stop); + expect("held slot handed back", held != nullptr); + if (held) { + /* Fill every slot the producer is allowed to touch, plus one refused. */ + push_tagged(q, 31); + push_tagged(q, 32); + expect("producer stops at the held slot", !push_tagged(q, 33)); + expect("held payload untouched", held->data[0] == 30); + expect("held attrib untouched", held->attrib.priority == 30); + /* A commit for a slot that is no longer the tail must be a no-op, not a + * skipped frame. Without the identity check a second commit here would + * advance past tag 31 and lose it, silently and uncounted. */ + q.pop_commit(held); + q.pop_commit(held); + expect("a repeat commit does not skip the next frame", + pop_tag(q, stop) == 31); + } + /* And the slot is only reusable after the commit. */ + expect("commit frees a slot", push_tagged(q, 34)); + } + + /* --- a commit for a slot that was never popped changes nothing --- */ + { + RxQueue q; + q.reset(4); + push_tagged(q, 60); + push_tagged(q, 61); + RxQueue::Slot bogus{}; + q.pop_commit(&bogus); + expect("a stray commit does not consume", pop_tag(q, stop) == 60); + expect("...nor reorder what follows", pop_tag(q, stop) == 61); + } + + /* --- a stale commit that has wrapped back ONTO the tail is still a no-op --- + * The identity check alone does not cover this: hold a slot pointer, drain + * the ring until the tail comes round to that same slot, and the pointer + * matches again while the ring is empty. Advancing there desynchronises head + * from tail, and the queue then hands out capacity-1 slots of stale bytes as + * if they were frames. */ + { + RxQueue q; + q.reset(2); /* capacity 1, so the tail wraps every other frame */ + push_tagged(q, 80); + RxQueue::Slot *first = q.pop_begin(std::chrono::milliseconds(1), stop); + expect("wrap-stale: first slot handed back", first != nullptr); + q.pop_commit(first); + push_tagged(q, 81); + RxQueue::Slot *second = q.pop_begin(std::chrono::milliseconds(1), stop); + expect("wrap-stale: second slot handed back", second != nullptr); + expect("wrap-stale: it is the other slot", second != first); + q.pop_commit(second); /* tail is now back on `first`, ring empty */ + q.pop_commit(first); /* the stale pointer matches again */ + expect("wrap-stale: ring is still empty", pop_tag(q, stop) == -1); + expect("wrap-stale: and still usable", push_tagged(q, 82)); + expect("wrap-stale: delivering the right frame", pop_tag(q, stop) == 82); + } + + /* --- `stop` ends the WAIT, never the DRAIN --- + * The radio's consumer loop leaves only when pop_begin returns nullptr + * (Mt7612uRadio::StartRxLoop), so a pop_begin that short-circuited on `stop` + * would silently discard everything still queued at teardown - up to 63 + * frames. Deterministic on purpose: the concurrent block below happens to + * catch that mutation most of the time, which is not the same as catching + * it. */ + { + RxQueue q; + q.reset(8); + std::atomic stopped{true}; /* set BEFORE a single pop */ + push_tagged(q, 50); + push_tagged(q, 51); + push_tagged(q, 52); + expect("stop drains, 1st", pop_tag(q, stopped) == 50); + expect("stop drains, 2nd", pop_tag(q, stopped) == 51); + expect("stop drains, 3rd", pop_tag(q, stopped) == 52); + expect("...and only then reports empty", pop_tag(q, stopped) == -1); + } + + /* --- pop_begin returns on stop instead of waiting out its timeout --- */ + { + RxQueue q; + q.reset(4); + std::atomic local_stop{false}; + const auto t0 = std::chrono::steady_clock::now(); + expect("empty queue times out", + q.pop_begin(std::chrono::milliseconds(30), local_stop) == nullptr); + const auto waited = std::chrono::steady_clock::now() - t0; + expect("...having actually waited", + waited >= std::chrono::milliseconds(25)); + + local_stop.store(true); + const auto t1 = std::chrono::steady_clock::now(); + expect("stop returns nullptr", + q.pop_begin(std::chrono::seconds(30), local_stop) == nullptr); + expect("...immediately, not after the timeout", + std::chrono::steady_clock::now() - t1 < + std::chrono::seconds(5)); + } + + /* --- wake() releases a waiter that is ALREADY blocked when stop flips --- + * StopRxLoop sets the flag and then calls wake(). A waiter that entered + * wait_for before the flag moved has no notification of its own coming, so + * without the wake it sits out the rest of its timeout - 20 ms in the radio, + * but the teardown then joins nothing and the contract "the ring is down + * when StopRxLoop returns" is what pays for it. Timeout here is 30 s so a + * missing wake fails the cell rather than passing slowly. */ + { + RxQueue q; + q.reset(4); + std::atomic late_stop{false}; + std::atomic woke{false}; + std::thread waiter([&] { + q.pop_begin(std::chrono::seconds(30), late_stop); + woke.store(true); + }); + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + const auto t0 = std::chrono::steady_clock::now(); + late_stop.store(true); + q.wake(); + waiter.join(); + expect("waiter released by wake()", woke.load()); + expect("...promptly, not at the timeout", + std::chrono::steady_clock::now() - t0 < std::chrono::seconds(5)); + } + + /* --- wake() releases EVERY waiter, which is what notify_all buys --- + * One consumer today, so a notify_one would be invisible in the radio; the + * header says "every waiter", and an untested "every" is a comment. */ + { + RxQueue q; + q.reset(4); + std::atomic late_stop{false}; + std::atomic released{0}; + std::thread a([&] { + q.pop_begin(std::chrono::seconds(30), late_stop); + released.fetch_add(1); + }); + std::thread b([&] { + q.pop_begin(std::chrono::seconds(30), late_stop); + released.fetch_add(1); + }); + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + const auto t0 = std::chrono::steady_clock::now(); + late_stop.store(true); + q.wake(); + a.join(); + b.join(); + expect("both waiters released by one wake()", released.load() == 2); + /* Without the time bound a notify_one still passes this cell - the second + * waiter just sits out its 30 s timeout and then reports itself released. */ + expect("...both promptly, not one of them at the timeout", + std::chrono::steady_clock::now() - t0 < std::chrono::seconds(5)); + } + + /* --- a push wakes a BLOCKED consumer, rather than leaving it to time out --- + * Deleting the producer's notify passes every other cell in this file, + * because they all poll. In the radio it would turn hand-off latency into + * the 20 ms poll period, per frame, on a video link. */ + { + RxQueue q; + q.reset(4); + std::atomic never{false}; + std::atomic got{false}; + const auto t0 = std::chrono::steady_clock::now(); + std::thread consumer([&] { + RxQueue::Slot *s = q.pop_begin(std::chrono::seconds(30), never); + if (s) { + got.store(s->data[0] == 41); + q.pop_commit(s); + } + }); + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + push_tagged(q, 41); + consumer.join(); + expect("push delivered to a blocked consumer", got.load()); + expect("...on the notify, not on a timeout", + std::chrono::steady_clock::now() - t0 < std::chrono::seconds(5)); + } + + /* --- concurrent producer and consumer: nothing is invented, nothing is + * lost except what the queue says it dropped --- */ + { + RxQueue q; + q.reset(8); + constexpr int kFrames = 20000; + std::atomic run_stop{false}; + std::atomic received{0}; + std::atomic out_of_order{0}; + + std::thread consumer([&] { + int last = -1; + for (;;) { + RxQueue::Slot *s = q.pop_begin(std::chrono::milliseconds(5), run_stop); + if (!s) { + if (run_stop.load()) + break; + continue; + } + const int seq = s->attrib.pkt_len; + if (seq <= last) + out_of_order.fetch_add(1); + last = seq; + received.fetch_add(1); + q.pop_commit(s); + } + }); + + int pushed = 0; + for (int i = 1; i <= kFrames; ++i) { + std::vector frame(16, static_cast(i)); + rx_pkt_attrib a{}; + a.pkt_len = static_cast(i); /* monotonic sequence */ + if (q.push(a, frame.data(), frame.size())) + pushed++; + } + run_stop.store(true); + q.wake(); + consumer.join(); + + /* Every frame is either delivered or counted. Both counts are read after + * the join, so neither is a snapshot of a moving value. */ + expect("delivered + dropped == offered", + received.load() + static_cast(q.dropped()) == kFrames); + expect("accepted == delivered", pushed == received.load()); + expect("delivery is in order", out_of_order.load() == 0); + /* Not an assertion about the drop count: at this speed the consumer may + * or may not fall behind, and a cell that required drops would be a cell + * that fails on a fast machine. */ + } + + if (fails) { + std::fprintf(stderr, "mt7612u_rx_queue: %d failure(s)\n", fails); + return 1; + } + std::printf("mt7612u_rx_queue: all checks passed\n"); + return 0; +} diff --git a/tests/mt7612u_usb_ids_vs_mt76.py b/tests/mt7612u_usb_ids_vs_mt76.py new file mode 100644 index 00000000..bf5381f0 --- /dev/null +++ b/tests/mt7612u_usb_ids_vs_mt76.py @@ -0,0 +1,161 @@ +#!/usr/bin/env python3 +"""Hold src/mt7612u/Mt7612uUsbIds.h to the vendor table it was transcribed from. + +The header claims to be "the complete mt76x2u_device_table, transcribed from +reference/mt76 @ be5ce79". That claim is load-bearing in a way the C++ cell +cannot see: Mt7612uUsbIdsSelftest proves the ids we DO list never collide with +a Realtek one, but an id we forgot is invisible to it — the device simply falls +through to the Realtek path and is misdetected as a Jaguar1, which is the exact +failure the gate exists to prevent. The first draft of that header had 11 of +the 16 entries, taken from the host's kernel tree rather than from the pinned +reference, and nothing caught it. + +So this compares the two tables directly, and pins the SHA-256 of the vendor +file the same way tools/extract_mt7612u_tables.py pins its sources — otherwise +"matches the vendor table" would only mean "matches whatever happens to be +checked out". + +Bench cell, not a CI cell: reference/ is a git submodule and CI checks the repo +out without submodules, so this SKIPS (exit 77) there. It runs for anyone who +has fetched the reference tree, which is everyone who can regenerate initvals.h. +""" + +from __future__ import annotations + +import hashlib +import re +import sys +from pathlib import Path + +UPSTREAM = "openwrt/mt76 commit be5ce79" +VENDOR_REL = "reference/mt76/mt76x2/usb.c" +VENDOR_SHA256 = "6e6292552a4b3a8f9a9e773d226c0255042d84203ff19e8621fb7650e60c02fd" +HEADER_REL = "src/mt7612u/Mt7612uUsbIds.h" + +SUBMODULE_HINT = ( + f"reference/mt76 is a pinned git submodule ({UPSTREAM}); fetch it with\n" + f" git submodule update --init reference/mt76" +) + +SKIP = 77 + +# { USB_DEVICE(0x0b05, 0x1833) }, /* Asus USB-AC54 */ +VENDOR_ENTRY = re.compile( + r"USB_DEVICE\(\s*0x([0-9a-fA-F]{4})\s*,\s*0x([0-9a-fA-F]{4})\s*\)" +) +# {0x0b05, 0x1833}, /* Asus USB-AC54 */ +HEADER_ENTRY = re.compile( + r"\{\s*0x([0-9a-fA-F]{4})\s*,\s*0x([0-9a-fA-F]{4})\s*\}" +) + + +def vendor_ids(text: str) -> list[tuple[int, int]]: + """The ids inside mt76x2u_device_table[] only. Taking every USB_DEVICE() in + the file would silently absorb a second table if one is ever added.""" + table = re.search( + r"mt76x2u_device_table\[\]\s*=\s*\{(.*?)\n\};", text, re.S + ) + if not table: + raise SystemExit(f"mt76x2u_device_table[] not found in {VENDOR_REL}") + return [ + (int(v, 16), int(p, 16)) for v, p in VENDOR_ENTRY.findall(table.group(1)) + ] + + +def header_ids(text: str) -> list[tuple[int, int]]: + table = re.search(r"kUsbIds\[\]\s*=\s*\{(.*?)\n\};", text, re.S) + if not table: + raise SystemExit(f"kUsbIds[] not found in {HEADER_REL}") + return [ + (int(v, 16), int(p, 16)) for v, p in HEADER_ENTRY.findall(table.group(1)) + ] + + +def fmt(ids) -> str: + return ", ".join(f"{v:04x}:{p:04x}" for v, p in ids) + + +def main() -> int: + root = Path(__file__).resolve().parent.parent + vendor_path = root / VENDOR_REL + header_path = root / HEADER_REL + + if not vendor_path.exists(): + print(f"SKIP: missing {VENDOR_REL}\n{SUBMODULE_HINT}") + return SKIP + + raw = vendor_path.read_bytes() + actual = hashlib.sha256(raw).hexdigest() + if actual != VENDOR_SHA256: + print( + f"{VENDOR_REL} is not the pinned revision:\n" + f" sha256 {actual}\n" + f" expected {VENDOR_SHA256} ({UPSTREAM})\n" + f"The id comparison below would be against a different tree than " + f"{HEADER_REL} names. Re-verify the table against the new " + f"revision, then update VENDOR_SHA256 here and the provenance " + f"comment in the header.", + file=sys.stderr, + ) + return 1 + + vendor = vendor_ids(raw.decode("utf-8")) + ours = header_ids(header_path.read_text(encoding="utf-8")) + + # Before the set comparison, which is blind to multiplicity: a duplicated + # entry would otherwise pass, and be reported as "17 ids match" a 16-entry + # table. Harmless to is_usb_id(), but this cell is the thing that says the + # table is a faithful transcription, so it has to notice. + seen: set[tuple[int, int]] = set() + dupes: list[tuple[int, int]] = [] + for i in ours: + if i in seen: + dupes.append(i) + seen.add(i) + if dupes: + print( + f"{HEADER_REL} lists {len(dupes)} id(s) twice: {fmt(dupes)}", + file=sys.stderr, + ) + return 1 + + missing = [i for i in vendor if i not in ours] + extra = [i for i in ours if i not in vendor] + + if missing: + print( + f"{HEADER_REL} is MISSING {len(missing)} id(s) the vendor driver " + f"claims: {fmt(missing)}\n" + f"Each is an MT7662-MAC adapter that would fall through to the " + f"Realtek path and be misdetected.", + file=sys.stderr, + ) + if extra: + print( + f"{HEADER_REL} claims {len(extra)} id(s) the vendor driver does " + f"not: {fmt(extra)}\n" + f"The gate runs BEFORE the Realtek SYS_CFG2 read, so an id we " + f"wrongly own is refused outright with no second chance.", + file=sys.stderr, + ) + if missing or extra: + return 1 + + note = "" + if vendor != ours: + # Same set, different order. Not a defect — the header is read by + # humans next to the vendor file, so say it and carry on. + note = " (same ids, different order to the vendor file)\n" + + print( + f"mt7612u_usb_ids_vs_mt76: {len(vendor)} ids match {VENDOR_REL} " + f"at {UPSTREAM}\n{note}" + f" covers only which backend gets to LOOK at a device; the library's " + f"MT_ASIC_VERSION identify stays authoritative after the handle opens", + end="\n", + ) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/regress.py b/tests/regress.py index 84c2aa21..64ab0aa8 100755 --- a/tests/regress.py +++ b/tests/regress.py @@ -53,6 +53,7 @@ import dataclasses import glob import os +import re import shlex import shutil import signal @@ -161,6 +162,49 @@ def _handler(signum, _frame): # injector both use this. CANONICAL_SA = "57:42:75:05:d6:00" + +def _mediatek_duts() -> dict[str, str]: + """The MediaTek ids, read from the C++ gate table rather than copied. + + src/mt7612u/Mt7612uUsbIds.h is what WiFiDriver::CreateRadio actually + consults, so a second hand-maintained list here would drift — and the + failure mode of drift is silent: a plugged adapter this table has never + heard of is simply not discovered, and the matrix reports "need 2 DUTs". + An unreadable header is loud rather than empty for the same reason.""" + root = Path(__file__).resolve().parent.parent + header = root / "src" / "mt7612u" / "Mt7612uUsbIds.h" + unavailable = "MediaTek DUTs unavailable —" + try: + text = header.read_text(encoding="utf-8") + except OSError as e: + sys.stderr.write(f"{unavailable} cannot read {header}: {e}\n") + return {} + body = re.search(r"kUsbIds\[\]\s*=\s*\{(.*?)\n\};", text, re.S) + if not body: + sys.stderr.write(f"{unavailable} no kUsbIds[] in {header}\n") + return {} + out: dict[str, str] = {} + # The trailing comma is OPTIONAL. Requiring it dropped the last entry when + # anything reformatted the table, leaving `out` non-empty so no warning + # fired and that one adapter merely undiscoverable — the exact silent drift + # this function exists to rule out. tests/mt7612u_usb_ids_vs_mt76.py parses + # the same table and must agree with this. + for vid, pid, comment in re.findall( + r"\{\s*0x([0-9a-fA-F]{4})\s*,\s*0x([0-9a-fA-F]{4})\s*\}\s*,?" + r"(?:[^\S\n]*/\*\s*(.*?)\s*\*/)?", + body.group(1), + ): + label = comment.split("—")[0].strip() or "MediaTek" + out[f"{vid.lower()}:{pid.lower()}"] = f"MT7612U ({label})" + if not out: + sys.stderr.write(f"{unavailable} kUsbIds[] parsed empty in {header}\n") + return out + + +# The MediaTek half, kept separately as well as merged below: a cell has to +# know which side of the matrix needs firmware and a MediaTek-enabled build. +MEDIATEK_DUTS = _mediatek_duts() + # Map every supported PID to the chipset family used for log readability. # Detection of the kernel driver claiming the device is dynamic (via sysfs); # this table is informational only. @@ -178,6 +222,7 @@ def _handler(signum, _frame): "0bda:c82c": "RTL8822CU (Jaguar3)", "0bda:a81a": "RTL8812EU (Jaguar3 EU)", "0bda:e822": "RTL8822EU (Jaguar3 EU)", + **MEDIATEK_DUTS, } # Required external tools on the host. Each entry: (binary, install hint). @@ -395,25 +440,22 @@ def wait_for_wlan_iface(self, dut: "Dut", timeout: float = 20.0) -> str: def _wlan_iface_for_dut(self, dut: "Dut") -> Optional[str]: if not self.is_remote: - # Local mode: walk /sys/bus/usb/devices for the DUT, then - # look at /net/ for a wlan name. - for d in glob.glob("/sys/bus/usb/devices/*"): - try: - with open(f"{d}/idVendor") as f: - if f.read().strip() != dut.vid: - continue - with open(f"{d}/idProduct") as f: - if f.read().strip() != dut.pid: - continue - except (FileNotFoundError, PermissionError): - continue - net_dir = f"{d}:1.0/net" - if not os.path.isdir(net_dir): - return None - ifaces = os.listdir(net_dir) - return ifaces[0] if ifaces else None - return None - # Remote: ssh and iterate /sys/bus/usb/devices/ over there. + # Local mode: the DUT's sysfs id IS its address on this machine, so + # go straight to it. Searching by VID:PID instead returns the first + # match, which on a rig with two adapters of the same model is a + # coin flip - and the wrong side of that flip puts a cell's kernel + # tcpdump on the very adapter devourer has claimed. + net_dir = f"/sys/bus/usb/devices/{dut.iface_id}/net" + if not os.path.isdir(net_dir): + return None + ifaces = os.listdir(net_dir) + return ifaces[0] if ifaces else None + # Remote: ssh and iterate /sys/bus/usb/devices/ over there. The host's + # sysfs id is meaningless inside the guest, so this matches on VID:PID + # and takes what it finds - which cannot tell two adapters of the same + # model apart. Rather than pick one at random (the local branch's old + # bug, which silently pointed a cell at the wrong radio), list them all + # and refuse when there is more than one. r = self.run([ "sh", "-c", "for d in /sys/bus/usb/devices/*; do " @@ -421,11 +463,17 @@ def _wlan_iface_for_dut(self, dut: "Dut") -> Optional[str]: f" [ \"$(cat $d/idVendor)\" = \"{dut.vid}\" ] || continue; " f" [ \"$(cat $d/idProduct)\" = \"{dut.pid}\" ] || continue; " " ls \"$d:1.0/net\" 2>/dev/null | head -1; " - " break; " "done", ]) - out = (r.stdout or "").strip() - return out or None + ifaces = [line for line in (r.stdout or "").split() if line] + if len(ifaces) > 1: + raise RuntimeError( + f"VM mode found {len(ifaces)} interfaces for {dut.vidpid} " + f"({', '.join(ifaces)}) and cannot tell them apart — the " + f"guest's sysfs ids are not the host's. Use two adapters of " + f"different models, or run local mode." + ) + return ifaces[0] if ifaces else None def iface_to_monitor(self, iface: str, channel: int) -> None: """Put a wlan iface into monitor mode on a channel.""" @@ -497,6 +545,81 @@ def preflight(devourer_root: Path, kh: KernelHost) -> None: sys.exit(2) +def preflight_mediatek(devourer_root: Path, duts: list[Dut]) -> None: + """Prerequisites that only bite when a MediaTek DUT is in the run. + + Both of these produce a cell that reads on the matrix exactly like a dead + radio - 0 hits, no explanation - so they are worth catching before the + first authorize-cycle rather than after four failed cells. + + `duts` is the SELECTED pair (or, for --full-matrix, everything), never + simply everything plugged in: a MediaTek adapter sharing the bench with two + Realteks must not stop an all-Realtek matrix from running.""" + if not any(d.is_mediatek for d in duts): + return + + missing = [] + cache = devourer_root / "build" / "CMakeCache.txt" + if cache.is_file(): + # A build without the backend still BUILDS rxdemo; it just refuses the + # adapter at CreateRadio, which is by design (falling through to the + # Realtek path would misdetect it as a Jaguar1). + # + # Read the VALUE, don't string-match "=ON": the cache records whatever + # the operator typed, and `-DDEVOURER_MT7612U=1` — idiomatic CMake — + # lands as `:BOOL=1`. Matching the literal would have failed a + # correctly configured build, which is this function's own failure mode + # inverted. + m = re.search(r"^DEVOURER_MT7612U:BOOL=(.*)$", + cache.read_text(errors="replace"), re.M) + value = (m.group(1).strip() if m else "").upper() + # CMake's false constants; anything else (1, ON, TRUE, YES...) is true. + if value in ("", "OFF", "0", "FALSE", "NO", "N", "IGNORE", + "NOTFOUND") or value.endswith("-NOTFOUND"): + missing.append( + f" - a MediaTek DUT is plugged but {cache.parent} was " + f"configured without the backend " + f"(DEVOURER_MT7612U={value or 'unset'}) — reconfigure with " + f"`cmake -S {devourer_root} -B {devourer_root}/build " + f"-DDEVOURER_MT7612U=ON` and rebuild, or unplug it" + ) + + # Same search order the backend uses (DeviceConfig.mt7612u.firmware_dir, + # then /lib/firmware/mediatek, then ./firmware) so the message names the + # paths that will actually be tried. + tried = ([_MT7612U_FW_DIR] if _MT7612U_FW_DIR else []) + [ + "/lib/firmware/mediatek", "firmware", + ] + if not any((Path(d) / "mt7662.bin").is_file() + and (Path(d) / "mt7662_rom_patch.bin").is_file() + for d in tried): + missing.append( + " - MediaTek firmware (mt7662.bin + mt7662_rom_patch.bin) not " + "found in: " + ", ".join(tried) + ". It ships zstd-compressed in " + "linux-firmware; decompress the pair somewhere and pass " + "`--mt7612u-fw-dir `." + ) + + if missing: + sys.stderr.write("Prerequisites not met:\n" + "\n".join(missing) + "\n") + sys.exit(2) + + # A warning, not an error: a run can legitimately skip the kernel side. + # But name the flags that actually do it — `--modes` is parsed only under + # `--encoding-matrix`, so on the default 4-cell matrix it is silently + # ignored and the operator still waits out wait_for_wlan_iface and still + # loses the run to the baseline abort. + if shutil.which("modinfo") and run(["modinfo", "mt76x2u"]).returncode != 0: + sys.stderr.write( + "warning: kernel module mt76x2u not available — every kernel-side " + "cell of a MediaTek matrix will fail to find a wlan iface, and on " + "the default matrix the kernel/kernel baseline then aborts the " + "run. Either `--encoding-matrix --modes devourer:devourer` for the " + "devourer-only rows, or `--no-baseline-abort` to see the rest fail " + "anyway.\n" + ) + + # --------------------------------------------------------------------------- # DUT discovery — find plugged-in adapters via sysfs (host side only; # the VM sees DUTs only when we explicitly hand them over via virsh). @@ -519,6 +642,18 @@ def iface_id(self) -> str: """Interface address sysfs expects for bind/unbind.""" return f"{self.sysfs_id}:1.0" + @property + def is_mediatek(self) -> bool: + return self.vidpid in MEDIATEK_DUTS + + @property + def topology(self) -> tuple[str, str]: + """(bus, dotted port path) from the sysfs id — "2-1" -> ("2", "1"), + "3-2.2" -> ("3", "2.2"). This is what tells two adapters of the SAME + model apart, which VID:PID cannot.""" + bus, _, port = self.sysfs_id.partition("-") + return bus, port + def discover_duts() -> list[Dut]: duts: list[Dut] = [] @@ -578,6 +713,11 @@ def attach_to_host_kernel(dut: Dut) -> None: # --no-rf-reset is passed. _RF_RESET_ENABLED: bool = True +# Directory holding mt7662.bin + mt7662_rom_patch.bin, set by main() from +# --mt7612u-fw-dir. Empty means "let the backend search its own defaults" +# (/lib/firmware/mediatek, then ./firmware). +_MT7612U_FW_DIR: str = "" + def usb_port_power_cycle(dut: Dut, settle_s: float = 2.0) -> None: """Toggle the USB port-level `authorized` flag to force a chip-power @@ -669,12 +809,62 @@ def fmt(self, threshold: int) -> str: # --------------------------------------------------------------------------- +def _sysfs_id_still_holds(dut: Dut) -> bool: + """True when dut.sysfs_id still names a device with this VID:PID.""" + base = f"/sys/bus/usb/devices/{dut.sysfs_id}" + try: + with open(f"{base}/idVendor") as f: + if f.read().strip() != dut.vid: + return False + with open(f"{base}/idProduct") as f: + return f.read().strip() == dut.pid + except OSError: + return False + + +def _count_plugged(dut: Dut) -> int: + """How many devices with this VID:PID are on the host bus right now.""" + return sum(1 for d in discover_duts() if d.vidpid == dut.vidpid) + + def _devourer_env(dut: Dut, channel: int, tx_encoding: Optional[dict] = None) -> dict[str, str]: env = os.environ.copy() env["DEVOURER_VID"] = f"0x{dut.vid}" env["DEVOURER_PID"] = f"0x{dut.pid}" env["DEVOURER_CHANNEL"] = str(channel) + # Pin the PHYSICAL device, not just its model. discover_duts() found this + # adapter at a specific sysfs id and every other part of a cell (the + # authorize-cycle, the kernel unbind, the wlan iface lookup) addresses it + # that way; without this the demo opens the first adapter with a matching + # VID:PID, which on a rig with two of the same model can be the OTHER one - + # including, for a devourer-to-devourer cell, the one already claimed. + # + # The sysfs id is resolved ONCE, at discovery. An authorize-cycle keeps it, + # but a device that re-enumerates onto another bus (a SuperSpeed part + # falling back to its companion HS controller, say) moves - and usb_select + # is deliberately strict, so a stale pin would fail to open rather than + # silently take the wrong adapter. That is the right trade on a same-model + # pair and the wrong one when there is nothing to confuse it with, so: keep + # the pin, and drop back to VID:PID only when exactly one such device is + # plugged and therefore nothing is ambiguous. + bus, port = dut.topology + if bus and port: + if _sysfs_id_still_holds(dut) or _count_plugged(dut) != 1: + env["DEVOURER_USB_BUS"] = bus + env["DEVOURER_USB_PORT"] = port + else: + sys.stderr.write( + f"warning: {dut.vidpid} is no longer at {dut.sysfs_id} — it " + f"re-enumerated. Falling back to VID:PID, which is " + f"unambiguous here because it is the only one plugged.\n" + ) + if dut.is_mediatek and _MT7612U_FW_DIR: + # The MediaTek firmware is not embedded: it ships in linux-firmware + # under its own licence and zstd-compressed, so the backend searches a + # directory at runtime. Without this the cell fails at bring-up, which + # reads on the matrix exactly like a dead radio. + env["DEVOURER_MT7612U_FW_DIR"] = _MT7612U_FW_DIR if tx_encoding: # The TX rate/mode is a single DEVOURER_TX_RATE string read by # txdemo (-> RtlJaguarDevice::SetTxMode): @@ -909,8 +1099,13 @@ def _count_devourer_tx_attempts(log_path: Path) -> tuple[int, int]: def _count_tcpdump_hits(log_path: Path) -> int: + # Blank lines do not count. tcpdump's log ends with a trailing newline, so + # splitlines() yielded one phantom record and EVERY kernel-RX cell was + # published one frame high - including a baseline that read a suspiciously + # perfect "459 hits / 459 TX" when 458 of 459 arrived. try: - return sum(1 for _ in log_path.read_text(errors="replace").splitlines()) + return sum(1 for line in log_path.read_text(errors="replace").splitlines() + if line.strip()) except FileNotFoundError: return 0 @@ -1465,11 +1660,13 @@ def main(): ) ap.add_argument( "--tx-pid", - help="USB PID hex of TX adapter (default: first auto-detected DUT)", + help="USB PID hex of TX adapter, or a sysfs id like `2-1` when two " + "plugged adapters share a PID (default: first auto-detected DUT)", ) ap.add_argument( "--rx-pid", - help="USB PID hex of RX adapter (default: second auto-detected DUT)", + help="USB PID hex of RX adapter, or a sysfs id like `5-1` when two " + "plugged adapters share a PID (default: second auto-detected DUT)", ) ap.add_argument( "--keep-logs", action="store_true", @@ -1524,6 +1721,16 @@ def main(): "--encoding-matrix runs. Always host-local; never moved to the " "VM. Env: DEVOURER_SNIFFER_IFACE.", ) + ap.add_argument( + "--mt7612u-fw-dir", + default=os.environ.get("DEVOURER_MT7612U_FW_DIR", ""), + help="directory holding mt7662.bin + mt7662_rom_patch.bin, for " + "MediaTek DUTs (env: DEVOURER_MT7612U_FW_DIR). Unlike the Realtek " + "backends the MediaTek firmware is not embedded — it ships " + "zstd-compressed in linux-firmware — so the pair has to be " + "decompressed somewhere first. Unset means let the backend search " + "its own defaults: /lib/firmware/mediatek, then ./firmware.", + ) ap.add_argument( "--no-rf-reset", action="store_true", @@ -1537,8 +1744,9 @@ def main(): args = ap.parse_args() # Apply RF-reset toggle before any cell runs. - global _RF_RESET_ENABLED + global _RF_RESET_ENABLED, _MT7612U_FW_DIR _RF_RESET_ENABLED = not args.no_rf_reset + _MT7612U_FW_DIR = args.mt7612u_fw_dir if args.vm_name and not args.vm_ssh: sys.stderr.write("--vm-name requires --vm-ssh\n") @@ -1567,10 +1775,20 @@ def main(): def pick(pid_arg, default_idx): if pid_arg is None: return duts[default_idx] + # A sysfs id ("2-1") rather than a PID: the only way to name ONE of two + # adapters that share a model, which is the normal case for a MediaTek + # matrix (there is one MediaTek PID worth having). + for d in duts: + if d.sysfs_id == pid_arg: + return d for d in duts: if d.pid == pid_arg.lower().removeprefix("0x"): return d - sys.stderr.write(f"No plugged DUT has PID {pid_arg}\n") + sys.stderr.write( + f"No plugged DUT has PID or sysfs id {pid_arg}. Plugged:\n" + + "".join(f" - {d.vidpid} ({d.chipset}) at {d.sysfs_id}\n" + for d in duts) + ) sys.exit(2) if args.encoding_matrix: @@ -1579,6 +1797,7 @@ def pick(pid_arg, default_idx): if tx_dut.sysfs_id == rx_dut.sysfs_id: sys.stderr.write("TX and RX must be different physical devices.\n") sys.exit(2) + preflight_mediatek(args.devourer_root, [tx_dut, rx_dut]) print(f"Encoding matrix mode:") print(f" TX adapter: {tx_dut.vidpid} ({tx_dut.chipset}) at {tx_dut.sysfs_id}") print(f" RX adapter: {rx_dut.vidpid} ({rx_dut.chipset}) at {rx_dut.sysfs_id}") @@ -1631,10 +1850,12 @@ def pick(pid_arg, default_idx): print(f"(logs kept at {kept} — symlink, valid until next run)", flush=True) sys.stdout.flush() + sys.stderr.flush() os._exit(0) return if args.full_matrix: + preflight_mediatek(args.devourer_root, duts) print(f"Full matrix mode over {len(duts)} adapters:") for d in duts: print(f" - {d.vidpid} ({d.chipset}) at {d.sysfs_id}") @@ -1671,6 +1892,8 @@ def pick(pid_arg, default_idx): kept.unlink() kept.symlink_to(tmpdir) print(f"(logs kept at {kept} — symlink, valid until next run)") + sys.stdout.flush() # see the note in the 4-cell branch below + sys.stderr.flush() os._exit(0) return @@ -1679,6 +1902,7 @@ def pick(pid_arg, default_idx): if tx_dut.sysfs_id == rx_dut.sysfs_id: sys.stderr.write("TX and RX must be different physical devices.\n") sys.exit(2) + preflight_mediatek(args.devourer_root, [tx_dut, rx_dut]) print(f"TX: {tx_dut.vidpid} ({tx_dut.chipset}) at {tx_dut.sysfs_id}") print(f"RX: {rx_dut.vidpid} ({rx_dut.chipset}) at {rx_dut.sysfs_id}") @@ -1714,6 +1938,14 @@ def pick(pid_arg, default_idx): kept.unlink() kept.symlink_to(tmpdir) print(f"(logs kept at {kept} — symlink, valid until next run)") + # os._exit skips the TemporaryDirectory cleanup on purpose - it is + # what keeps the logs - but it also skips the stdout flush, and + # stdout is block-buffered whenever this is piped or redirected. + # Without these the whole markdown table is discarded, which is + # exactly the output the flag exists to preserve. The + # encoding-matrix branch already learned this. + sys.stdout.flush() + sys.stderr.flush() os._exit(0) diff --git a/tools/extract_mt7612u_tables.py b/tools/extract_mt7612u_tables.py index bac92b4b..61f10b8f 100755 --- a/tools/extract_mt7612u_tables.py +++ b/tools/extract_mt7612u_tables.py @@ -18,6 +18,7 @@ import argparse import hashlib import re +import sys from pathlib import Path UPSTREAM = "openwrt/mt76 commit be5ce79" @@ -202,7 +203,11 @@ def load(source_root: Path) -> list[tuple[int, int, str]]: for key, (relative, expected_hash) in SOURCES.items(): path = source_root / relative if not path.exists(): - raise SystemExit(f"missing {path}\n{SUBMODULE_HINT}") + # 77, not 1: this is registered as a ctest cell, and ctest reads 77 + # as SKIP. A shell still sees a non-zero exit, so nothing that + # drives this script by hand starts passing on a missing submodule. + print(f"missing {path}\n{SUBMODULE_HINT}", file=sys.stderr) + raise SystemExit(77) source = path.read_bytes() actual_hash = hashlib.sha256(source).hexdigest() if actual_hash != expected_hash: From 29ab93e5dbf521a27cee0b8dcd3be27a25ee753c Mon Sep 17 00:00:00 2001 From: snokvist Date: Thu, 10 Sep 2026 11:56:22 +0200 Subject: [PATCH 2/3] docs(mt7612u): republish the matrix from the fixed parser The kernel-RX figures came from _count_tcpdump_hits, which counted a trailing blank line - so the table had to be re-measured, not corrected. Twenty cells over five runs now, all green; the tcpdump counts agree with the harness exactly. What re-running actually showed is worth more than the +1: the same devourer-TX -> kernel-RX cell read 9077, 8921 and 6758 across three ch36 runs against a near-constant ~9400 offered. A 26% spread, same pair, same command, minutes apart. The verdict repeats and the counts do not, which is the honest reason they do not belong in a bandwidth column. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj --- docs/mt7612u.md | 44 ++++++++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/docs/mt7612u.md b/docs/mt7612u.md index 8d82a827..3aaa933b 100644 --- a/docs/mt7612u.md +++ b/docs/mt7612u.md @@ -364,48 +364,52 @@ sudo python3 tests/regress.py --channel 36 --tx-pid 2-1 --rx-pid 5-1 \ --mt7612u-fw-dir --duration 15 ``` -Cells are `hits / frames offered`, 15 s each. Twelve cells over three runs — -ch36 twice and ch6 once — and every one passed on its first attempt: +Cells are `hits / frames offered`, 15 s each. Twenty cells over five runs — +ch36 three times, ch6 twice — and every one passed on its first attempt. The +last run of each band: | **ch36** (UNII-1, 5180 MHz) | RX = devourer | RX = kernel `mt76x2u` | |---|---|---| -| **TX = devourer** | 8500 / 9383 ✓ | 8921 / 9288 ✓ | -| **TX = kernel** (scapy) | 400 / 496 ✓ | 458 / 459 ✓ | +| **TX = devourer** | 7000 / 9375 ✓ | 6758 / 9451 ✓ | +| **TX = kernel** (scapy) | 400 / 477 ✓ | 474 / 475 ✓ | | **ch6** (2.4 GHz, 2437 MHz) | RX = devourer | RX = kernel `mt76x2u` | |---|---|---| -| **TX = devourer** | 8600 / 9433 ✓ | 8531 / 9354 ✓ | -| **TX = kernel** (scapy) | 400 / 497 ✓ | 458 / 488 ✓ | +| **TX = devourer** | 8600 / 9436 ✓ | 8631 / 9425 ✓ | +| **TX = kernel** (scapy) | 400 / 474 ✓ | 450 / 474 ✓ | The kernel-RX figures are `tcpdump` record counts. The harness published each of them one higher until this change: it counted `splitlines()`, and the log's -trailing newline made a phantom record — which is how the baseline cell came to -report a suspiciously perfect `459 / 459` when 458 of 459 arrived. +trailing newline made a phantom record — which is how an earlier baseline cell +came to report a suspiciously perfect `459 / 459` when 458 of 459 arrived. -The ch36 table above is the second run; the first, ~4 minutes earlier, agrees -to within ordinary run-to-run variance — baseline 466 vs 458 and devourer-TX to -kernel-RX 9077 vs 8921, both exact `tcpdump` counts, so ~1.7 % and not a -sampling artefact. +**The verdict repeats; the counts do not.** Across the three ch36 runs the +devourer-TX → kernel-RX cell read 9077, 8921 and 6758 against a near-constant +~9400 offered — a 26 % spread on the same pair, same channel, same command, +minutes apart. 5 GHz here carries heavy ambient traffic and an injection with +no rate control contends with it. Every run says "the path works"; none of them +says how well, which is the other reason these counts do not belong in the +README's bandwidth column. Some detail from the per-cell logs that the hit counts do not show: - **The kernel driver decodes our frames at 6 Mbps on both bands.** `tcpdump` on the `mt76x2u` side reads `6.0 Mb/s 5180 MHz 11a ... SA:57:42:75:05:d6:00` - at a −31 dBm mode on ch36 and `6.0 Mb/s 2437 MHz 11g` at −35 dBm on ch6, each + at a −29 dBm mode on ch36 and `6.0 Mb/s 2437 MHz 11g` at −35 dBm on ch6, each with a per-antenna breakdown. That is an independent decoder confirming the rate `SetTxMode`'s refusal predicts, not our own reading of our own transmit. (6 Mbps because `txdemo` builds rate-less frames; the part itself airs whatever the TXWI says — see `## TX rate is authoritative`.) -- **Nothing was dropped at the hand-off queue, in any of the four cells that - ran a devourer receiver.** The queue only exists on that side; the other four - are `tcpdump`. The busiest carried 20391 frames — ambient included — over the +- **Nothing was dropped at the hand-off queue, in any cell that ran a devourer + receiver.** The queue only exists on that side; the other half of each matrix + is `tcpdump`. The busiest carried 21842 frames — ambient included — over the ~28 s the receiver was up, and `StopRxLoop` logged the plain "RX stopped after N frames" line rather than the DROPPED one. -- **TX reported 0 failed in all four devourer-TX cells** (9288–9433 submitted - each) — with the standing caveat that on this backend `failed` counts - refusals rather than wire deaths (Counterparts). +- **TX reported 0 failed in every devourer-TX cell** (9375–9451 submitted each + in the two runs above) — with the standing caveat that on this backend + `failed` counts refusals rather than wire deaths (Counterparts). - The hit counters are **sampled on the devourer side only**: `rx.txhit` fires - at `hits <= 10` or every 100th, so a reported 8500 means 8500–8599. The + at `hits <= 10` or every 100th, so a reported 7000 means 7000–7099. The kernel-RX counts are `tcpdump` lines, granularity 1. `frames offered` is `tx.stats` for the devourer rows and the injector's own tally for the kernel rows, both exact. The matrix's own table prints `tx.frame` instead, sampled From ad8b3a3523cb9218d8209ae09204f729c166c69f Mon Sep 17 00:00:00 2001 From: snokvist Date: Thu, 10 Sep 2026 12:08:27 +0200 Subject: [PATCH 3/3] qodo: five real defects, and one probe that could never fail 4 The RX consumer loop tested the stop flags only when the queue ran dry, so on a busy channel SIGINT could not reach StopRxLoop for as long as frames kept arriving - exactly when a consumer wants out. Tested at the top of every iteration now. 9 _rx_frames was lifetime-monotonic while the queue's drop count was reset per session, so a restart paired a cumulative received count with one session's drops. Zeroed together. 8 bringup's drain_cb casts its cookie to std::atomic* but the mtu, caps and linkstat gates passed plain unsigned long - fetch_add on storage holding no atomic object. All three are atomic now. 10 _wlan_iface_for_dut addressed the DUT only at its discovery-time sysfs id, so a re-enumeration failed the kernel cells while _devourer_env recovered. Same fallback, same condition: only when one device with that VID:PID is plugged. 11 bf_report_fcs_selftest passed the whole angle_bytes length as vbytes, which puts mu_start at len(raw) - 4 - so BOTH parse_mu_snr calls returned None and their equality held whatever the parser did. A dead probe, and the reverted-bound mutation walks straight through it. It now parses a real series and asserts so. While fixing it: the fcs_present bound is NOT observable on this fixture at any vbytes - the series collapses first, measured at 2, 4, 10, 20 and 30 - so there is no check comparing the two bounds, and a comment says why rather than leaving one that cannot fail. Declined, with evidence: #7, StartRxLoop after InitWrite. InitWrite never creates an async ring - tx.cpp:229 falls back to a synchronous bulk transfer when d->a is null, and mt_async_start is reached only through mt7612u_rx_start - so the `if (d->a) return 0` early-out it describes is unreachable from that sequence. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj --- src/mt7612u/Mt7612uRadio.cpp | 15 +++++++++++---- src/mt7612u/tools/bringup.cpp | 15 ++++++++++----- tests/bf_report_fcs_selftest.py | 22 ++++++++++++++++++---- tests/regress.py | 20 +++++++++++++++++++- 4 files changed, 58 insertions(+), 14 deletions(-) diff --git a/src/mt7612u/Mt7612uRadio.cpp b/src/mt7612u/Mt7612uRadio.cpp index 84ac8592..4e3cd222 100644 --- a/src/mt7612u/Mt7612uRadio.cpp +++ b/src/mt7612u/Mt7612uRadio.cpp @@ -385,6 +385,10 @@ void Mt7612uRadio::StartRxLoop(Action_ParsedRadioPacket packetProcessor) { * 3836-byte max MPDU), so the event thread does still allocate during the * first pass round the ring and on any frame-size step-up. */ _rx_q.reset(64); + /* Zeroed with the queue's drop count, so the teardown line reports both + * over the same interval. Left lifetime-monotonic, it paired a cumulative + * received count with one session's drops. */ + _rx_frames.store(0, std::memory_order_relaxed); /* Ring first, receiver second - see rule 1 in the header. */ if (mt7612u_rx_start(_dev, &Mt7612uRadio::rx_trampoline, this) != 0) @@ -423,13 +427,16 @@ void Mt7612uRadio::StartRxLoop(Action_ParsedRadioPacket packetProcessor) { * contract every other backend keeps. See the queue's comment in the header * for why delivering on the event thread wedges the hardware. */ for (;;) { + /* Tested at the TOP, not only when the queue runs dry. A busy channel can + * keep the ring non-empty indefinitely, and checking the flags only on the + * empty path meant SIGINT could not reach StopRxLoop for as long as frames + * kept arriving - i.e. exactly when a consumer most wants to stop. */ + if (_rx_stop.load() || g_devourer_should_stop) + break; mt7612u::RxQueue::Slot *slot = _rx_q.pop_begin(std::chrono::milliseconds(20), _rx_stop); - if (!slot) { - if (_rx_stop.load() || g_devourer_should_stop) - break; + if (!slot) continue; - } /* Outside the queue lock: user code runs here, and it may call back into * this object. The producer never writes the slot at the tail, so this diff --git a/src/mt7612u/tools/bringup.cpp b/src/mt7612u/tools/bringup.cpp index 04e41dc7..17cfd941 100644 --- a/src/mt7612u/tools/bringup.cpp +++ b/src/mt7612u/tools/bringup.cpp @@ -886,7 +886,8 @@ static int gate_mtu(uint8_t chan, int count) for (k = 0; k < sizeof sizes / sizeof sizes[0]; k++) { int len = sizes[k]; long ok_sync = 0, ok_async = 0; - unsigned long drained = 0; + /* atomic because drain_cb increments it from the RX event thread. */ + std::atomic drained{0}; int i, pass; if ((size_t)len > sizeof frame) continue; @@ -1396,7 +1397,8 @@ static void drain_cb(void *user, const void *frame, size_t len, static int gate_caps(uint8_t chan) { struct mt7612u_caps c; - unsigned long drained = 0; + /* atomic: drain_cb runs on the RX event thread. */ + std::atomic drained{0}; uint64_t t1, t2; int64_t delta; int bad = 0; @@ -1512,7 +1514,8 @@ static int gate_caps(uint8_t chan) rx_teardown(); mt_mac_stop(&dev); - printf("\n%lu frames drained from EP 4 while the receiver was on\n", drained); + printf("\n%lu frames drained from EP 4 while the receiver was on\n", + drained.load()); printf("\nGATE caps: %s\n", bad ? "FAIL" : "PASS"); return bad; } @@ -1748,7 +1751,8 @@ static int gate_rxbytes(uint8_t chan, int secs) * * Read-and-clear, so each line is the second that just passed. */ -static unsigned long linkstat_drained; +/* atomic: drain_cb runs on the RX event thread. */ +static std::atomic linkstat_drained{0}; static int gate_linkstat(uint8_t chan, int secs, int with_rx) { @@ -1803,7 +1807,8 @@ static int gate_linkstat(uint8_t chan, int secs, int with_rx) } if (with_rx) { rx_teardown(); - printf(" %lu frames reached the ring over the run\n", linkstat_drained); + printf(" %lu frames reached the ring over the run\n", + linkstat_drained.load()); } mt_mac_stop(&dev); return 0; diff --git a/tests/bf_report_fcs_selftest.py b/tests/bf_report_fcs_selftest.py index 73c16236..94be1b25 100644 --- a/tests/bf_report_fcs_selftest.py +++ b/tests/bf_report_fcs_selftest.py @@ -76,13 +76,27 @@ def ev(body): # --- parse_mu_snr: the FCS-present bound must not have moved -------------- if with_fcs: - a = bf.parse_mu_snr(with_fcs, with_fcs["ns"] if "ns" in with_fcs else 52, - len(with_fcs["angle_bytes"])) + # vbytes has to leave room for the MU report: parse_mu_snr computes + # mu_start = 29 + nc + vbytes and gives up when mu_start + 4 >= len(raw). + # Passing the whole angle_bytes length put mu_start at len(raw) - 4, so + # BOTH calls below returned None and their equality held whatever the + # parser did. 20 lands inside the report and yields a real series. + MU_VBYTES = 20 + ns_ = with_fcs["ns"] if "ns" in with_fcs else 52 + a = bf.parse_mu_snr(with_fcs, ns_, MU_VBYTES) legacy = dict(with_fcs) legacy.pop("fcs_present", None) # a dict from before the field existed - b = bf.parse_mu_snr(legacy, legacy["ns"] if "ns" in legacy else 52, - len(legacy["angle_bytes"])) + b = bf.parse_mu_snr(legacy, ns_, MU_VBYTES) + check(a is not None, "MU-SNR actually parses (a None here is a dead probe)") check(a == b, "FCS-present MU-SNR identical with and without the new key") + # What this fixture CANNOT hold: that the fcs_present bound itself moved. + # parse_mu_snr stops when the smooth SNR series collapses, and on this + # capture that always fires before either end bound - measured at vbytes + # 2, 4, 10, 20 and 30, len - 2 and len give the same series every time. + # A check comparing them would pass with the bound reverted, so there + # isn't one. The bound is covered where it IS observable: parse_frame's + # four-byte angle trim, asserted above. + print("bf_report_fcs: " + ("FAIL" if fails else "PASS")) sys.exit(1 if fails else 0) diff --git a/tests/regress.py b/tests/regress.py index 64ab0aa8..4b10f8d1 100755 --- a/tests/regress.py +++ b/tests/regress.py @@ -447,7 +447,25 @@ def _wlan_iface_for_dut(self, dut: "Dut") -> Optional[str]: # tcpdump on the very adapter devourer has claimed. net_dir = f"/sys/bus/usb/devices/{dut.iface_id}/net" if not os.path.isdir(net_dir): - return None + # Either the driver has not bound yet - the caller polls for + # that - or the device re-enumerated somewhere else. Fall back + # the same way _devourer_env does, and on the same condition: + # only when exactly one device with this VID:PID is plugged, so + # there is nothing to confuse it with. + if _sysfs_id_still_holds(dut) or _count_plugged(dut) != 1: + return None + for d in discover_duts(): + if d.vidpid != dut.vidpid: + continue + moved = f"/sys/bus/usb/devices/{d.iface_id}/net" + if os.path.isdir(moved): + sys.stderr.write( + f"warning: {dut.vidpid} moved from {dut.sysfs_id} " + f"to {d.sysfs_id}; using its interface there.\n") + net_dir = moved + break + else: + return None ifaces = os.listdir(net_dir) return ifaces[0] if ifaces else None # Remote: ssh and iterate /sys/bus/usb/devices/ over there. The host's