From 15e4074b9a4b68b794ab3063ccc0c5c7432f7a93 Mon Sep 17 00:00:00 2001 From: snokvist Date: Tue, 8 Sep 2026 18:04:19 +0200 Subject: [PATCH 01/13] docs(mt7612u): what a fully-userspace AP needs, and its limits Gap analysis, no code. The AP brain (probe/auth/assoc, DHCP/ARP/ICMP, WPA2 4-way) already exists in tests/ and is backend-agnostic, so the work is a few hundred lines of C backend primitives: StartBeacon (load the reserved page + arm the MAC beacon function), StopBeacon, per-station/GTK key install (hardware CCMP - a gain over Realtek's software CCMP), an AP RX filter, and confirming the auto-ACK covers SetAckResponder. Addressing, the station table, crypto slots, ACKed TX and the beacon timer are already present. Limitations named with workarounds: USB has no pre-TBTT interrupt so dynamic TIM/power-save is the one hard case (static beacon sidesteps it entirely for always-on FPV clients); BlockAck RX reordering and per-station rate control are software (decline BA / fixed-or-RSSI rate); multi-client is harness work. Verification reuses the existing beacon_*/ap_* harnesses. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj --- docs/mt7612u-ap-mode.md | 179 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 179 insertions(+) create mode 100644 docs/mt7612u-ap-mode.md diff --git a/docs/mt7612u-ap-mode.md b/docs/mt7612u-ap-mode.md new file mode 100644 index 00000000..abb6182b --- /dev/null +++ b/docs/mt7612u-ap-mode.md @@ -0,0 +1,179 @@ +# MT7612U as a fully-userspace access point — what is missing + +**Status: research and gap analysis, no code.** File:line references are to the +merged subtree at `daabab7` (`src/mt7612u/`), the mt76 reference at `be5ce79`, +and devourer's existing AP work. A companion implementation/verification plan +is at the end. + +## The claim, and why it holds + +The gap between "MT7612U injector" (what the subtree is) and "MT7612U userspace +AP" is small — a few hundred lines of C in the backend, and no new AP logic at +all. Two reasons: + +1. **devourer already has the AP brain, and it is backend-agnostic.** The + probe/auth/assoc responder, the DHCP/ARP/ICMP data plane, and the WPA2 4-way + handshake with software CCMP all live in `tests/` (`ap_responder.cpp`, + `ap_wpa2.cpp`, `probe_responder.cpp`, `beacon_*.cpp`), driven entirely + through the `IRtlDevice` interface — `StartBeacon` + the RX callback + + `send_packet`. `docs/ap-mode.md` documents a complete open and WPA2-PSK AP + validated against real Linux stations on this stack. None of it is + Realtek-specific; it works against any backend that implements the beacon + and ACK primitives. + +2. **MediaTek's MAC offloads in hardware exactly what an AP needs most.** The + beacon is auto-transmitted from a reserved page at each TBTT, TSF-stamped by + the MAC; ACK is SIFS-timed by the MAC against the programmed address; + 802.11 sequence numbering is a MAC function; and CCMP has real per-station + key hardware (`MT_WCID_KEY`, `MT_SKEY`). On Realtek, devourer does CCMP in + **software** because the security TX-desc field is absent on most + generations (`docs/ap-mode.md`: "only Jaguar1 has + `SET_TX_DESC_SEC_TYPE_8812`"). So on MT the encrypted data plane, and GTK + rekey which is explicitly out of scope on Realtek, become **hardware** — + this part is a capability *gain*, not a gap. + +## What the MT7612U backend already has + +Verified in the merged subtree: + +| AP need | present today | where | +|---|---|---| +| Port MAC + BSSID programmed | yes — `MT_MAC_ADDR_DW0/1`, `MT_MAC_BSSID_DW0/1`, MBSS_MODE=3, MBEACON_N | `init.c:206‑216` (`mac_setaddr`) | +| Station table (WCID) | yes — `mt_wcid_setup(idx, mac)` writes `MT_WCID_ATTR` + address; all zeroed at init | `tx.c:95`, `init.c:236` | +| Crypto key slots | present and zeroed — `MT_WCID_KEY`, `MT_SKEY`, `MT_SKEY_MODE` | `regs.h:239‑245`, `init.c:242‑246` | +| ACKed unicast TX | yes — `no_ack=0` sets `MT_TXWI_ACK_CTL_REQ`; BA-window field present | `tx.c:164‑167` | +| Beacon-interval timer regs | defined — `MT_BEACON_TIME_CFG` INTVAL/TIMER_EN/TBTT_EN/BEACON_TX, `MBEACON_N` | `regs.h:176‑180,169` | +| RX filter control | yes — managed default `0x00015f97`, monitor clears to error-only | `init.c:278,494‑509` | +| Register block copy | yes — `mt_wr_copy()` for reserved-page writes | used in `init.c` | + +So the addressing, the station table, the crypto slots, the ACK path and the +beacon *timer* are already in place. The receiver runs (per the #414 tick), and +`mt_tx_build()` already produces `[TXWI][802.11]` which is exactly the reserved- +page beacon shape. + +## The gap — the driver primitives to add + +Each has a direct mt76 recipe. Estimates are the C-library side only. + +1. **`StartBeacon` — load the beacon + arm the MAC beacon function (~70 LOC).** + - Add two register defines our `regs.h` lacks: `MT_BEACON_BASE` (0xc000) and + `MT_BCN_OFFSET(n)` (0x041c + n·4). Everything else is present. + - Program the 5 USB beacon slots: `slot_size = (8192/5) & ~63 = 1600`, + offsets via `MT_BCN_OFFSET` — mt76 `mt76x02_set_beacon_offsets` + (`mt76x02_beacon.c:10`, `N_BCN_SLOTS=5` in `mt76x02_usb_core.c:126`). + - Write `[TXWI][beacon MPDU]` into `MT_BEACON_BASE` with `mt_wr_copy()` — + mt76 `mt76x02_write_beacon`/`mt76x02_mac_set_beacon` + (`mt76x02_beacon.c:24,54`). `mt_tx_build()` already emits that shape. + - Enable: set `MT_BEACON_TIME_CFG` `BEACON_TX | TBTT_EN | TIMER_EN` with + `INTVAL = interval_tu` — mt76 `mt76x02_mac_set_beacon_enable` + (`mt76x02_beacon.c:69`). Point `MT_MAC_BSSID_*` at the AP BSSID, **unicast** + (`0x02…`) — `docs/ap-mode.md`'s hardest-won finding: an I/G-set BSSID makes + the station drop auth before it reaches the air. + - 802.11 sequence numbering: let the MAC number it (the HW-seq path mt76 + uses for beacons) or number it per update in software; `beacon_wire_check` + expects +1 per beacon. + +2. **`StopBeacon` (~10 LOC).** Clear `BEACON_TX | TBTT_EN | TIMER_EN`. Note the + `IRtlDevice` contract: the chip beacons autonomously, so a session that ends + without a power-cycle **must** call this or the beacon contaminates the next + run (`src/IRtlDevice.h:424`). + +3. **Per-station / group key install (~50 LOC).** `MT_WCID_KEY(idx)` + + `MT_WCID_ATTR` PKEY_MODE/PAIRWISE for pairwise, `MT_SKEY` + `MT_SKEY_MODE` + for the GTK — mt76 `mt76x02_mac_wcid_set_key` / `mac_shared_key_setup` + (`mt76x02_mac.c`). The cipher enum (`MT76X02_CIPHER_*`) is small. This buys + **hardware CCMP**; the software CCMP in `ap_wpa2.cpp` still works as the + fallback/portable path. + +4. **AP RX filter (~15 LOC).** A third mode beside monitor: accept probe-req + (broadcast), auth/assoc to the BSSID, and data addressed to us; keep + dropping foreign-BSS and error frames. Derive the mask from the managed + default `0x00015f97` by clearing the "not-to-me" drops the AP needs to hear. + +5. **`SetAckResponder` — likely already covered.** On MT the MAC auto-ACKs + frames matching the programmed `MT_MAC_ADDR`, so pointing it at the BSSID + (done in `mac_setaddr`) is the ACK responder. **Open item to confirm on + hardware:** whether MT needs an explicit op-mode/net-type register beyond + beacon-enable + address match (Realtek needs `net_type = AP`); mt76 sets no + distinct AP opmode reg for this part, but verify auth arrives at retry=0. + +6. **`UpdateBeaconPayload` (~10 LOC, optional).** Re-write the reserved page in + place. Only needed for dynamic beacon content (below). + +### The two integration layers + +- **The mt7612u C library** gains items 1–4 above (~150 LOC, all with recipes). +- **`RtlMt7612uDevice`** (the wrapper from the integration PR) exposes them as + `StartBeacon`/`StopBeacon`/`UpdateBeaconPayload`/`SetAckResponder` over the C + ABI, so the **existing** C++ AP harnesses in `tests/` run unchanged. No AP + logic is written — it already exists. + +## Limitations and shortfalls of a userspace AP on MediaTek — and workarounds + +1. **Power-save / TIM is the real fight — USB has no pre-TBTT interrupt.** + A dynamic beacon (TIM bitmap, buffered multicast/broadcast) must be rewritten + just before each TBTT. The kernel fakes the interrupt with an hrtimer firing + 8 ms pre-TBTT plus a high-priority workqueue that refills up to `N_BCN_SLOTS` + buffered frames (`mt76x02u_pre_tbtt_work`, `mt76x02_usb_core.c:128‑217`). In + userspace that is a timer thread racing a 1–2 ms USB write, and a dense + 25 TU beacon (needed so a scanning supplicant catches the AP) fires it ~40×/s. + - **Workaround (recommended for FPV/video-return):** a **static** beacon is + free — the MAC auto-beacons from the reserved page with no host + involvement. Advertise DTIM=1, no buffering, and do not support + power-saving clients (the return-video use case has always-on clients). + Then `StartBeacon` alone suffices and the pre-TBTT machinery is never + needed. + - **If PS clients are required:** port the hrtimer+worker as a userspace + timer thread that calls `UpdateBeaconPayload` pre-TBTT. This is the one + place the USB-userspace shape genuinely fights the protocol; budget for + jitter and missed updates. + +2. **BlockAck RX reordering is software.** TX aggregation is hardware, but if a + client sends A-MPDU the AP must own a reorder buffer. + - **Workaround:** negotiate **no** BlockAck (decline ADDBA) — clients fall + back to non-aggregated data, lower uplink throughput but correct. Implement + a reorder window only if client→AP throughput demands it. + +3. **No firmware rate control.** `txwi.rate` airs verbatim (proven — there is no + rate LUT), so the AP picks every client's TX rate in host software. + - **Workaround:** a fixed rate, or a simple picker off the per-chain RSSI the + RX path already reports (`rssi[0..1]`). Adequate for a handful of clients. + +4. **Multiple concurrent clients.** The WCID table is 256 entries and SKEY is + per-BSS in hardware, so the silicon supports many clients; the current AP + harness handles one (`docs/ap-mode.md` scope). Extending is **harness** work + (a client table, per-client PTK), not driver work. + +5. **Out of scope, standard AP-stack breadth.** WMM/QoS EDCA parameter sets, + DFS/radar on 5 GHz DFS channels, 802.11w management-frame protection, band + steering, and a real DHCP pool. None are MT-specific gaps; they are AP-stack + features the harness does not implement. + +6. **Management-frame timing is fine.** Probe/auth/assoc responses are tens-of-ms + tolerant and the userspace RX→TX round-trip is a few ms, proven on the + Realtek stack (`docs/ap-mode.md`); only SIFS-timed ACK must be hardware, and + it is. + +**Net:** an open or WPA2-PSK AP serving a few always-on clients is very +achievable and *easier* on MT7612U than on Realtek (hardware CCMP + GTK). The +static-beacon path avoids the one hard USB limitation entirely. Power-save +clients and BlockAck reordering are where a userspace MT AP stops being worth +it, and both have clean "don't support it" workarounds for the return-video +use case. + +## End-to-end verification (reuse, don't rebuild) + +devourer's existing AP checks are backend-agnostic and become the acceptance +suite once the MT backend implements the primitives: + +- `tests/beacon_wire_check.cpp` — beacon frame control, +1 seq per beacon, live TSF. +- `tests/beacon_kernel_scan.sh` — a real `rtw88` station's `iw scan` lists the AP. +- `tests/probe_responder.cpp` — active-scan probe response, no beacon. +- `tests/ap_responder.cpp` + `tests/ap_ping_demo.sh` — open assoc → DHCP lease → + ping 0% loss. +- `tests/ap_wpa2.cpp` + `tests/ap_wpa2_demo.sh` — WPA2 4-way → encrypted DHCP → + encrypted ping (here, exercise the **hardware** CCMP path). + +Success = a real Linux station associates and passes IP traffic against the +MT7612U backend, open and WPA2-PSK, on both 2.4 and 5 GHz, with the static +beacon. From a609ef6b402e734de7dff3c63f1eecd4107c05bf Mon Sep 17 00:00:00 2001 From: snokvist Date: Thu, 10 Sep 2026 21:56:57 +0200 Subject: [PATCH 02/13] mt7612u: static reserved-page AP beacon and address match, on the merged subtree The MediaTek AP driver layer: a beacon the MAC auto-transmits from its reserved page at every TBTT, and the address match that makes it an AP rather than a beacon generator. mt_beacon_init / _write / _set_enable the beacon engine and slot 0 mt_ap_set_bssid the APC slot the MAC matches a BSS in regs.h MBSS mask correction, beacon regs tx.cpp MT_TXOPT_BEACON: hardware TSF + seq bringup: `beacon`, `ap` the two gates that measured it Device-verified 2026-09-08 (docs/mt7612u-ap-mode.md): beacon on air on both bands by a kernel station's `iw scan` and an independent RTL8812AU witness; timestamp advancing exactly 102400 us per beacon, so the MAC is inserting the live TSF; sequence +1 per beacon; and a real station's three auth frames arriving with 0 retried, which is the hardware auto-ACK - an un-ACKed frame comes back with FC Retry set. BCN_BYPASS_MASK is INVERTED: a set bit SUPPRESSES that slot. mt_beacon_init sets all sixteen and mt_beacon_write clears the one it loaded. Getting that backwards gives a running beacon timer, an advancing TSF, and nothing on air. Squashed with its own post-merge repair, because the four original commits predate the subtree's C++ migration (#421) and do not build on today's master: beacon.c was compiled by NOTHING (the Makefile globs *.cpp and the CMake list is explicit), ap_ctx used C11 _Atomic and the C11 free functions, ap_cb took its cookie through implicit void* conversions, and a memset ran over a no-longer-trivially-copyable type. Kept as one commit so the series bisects. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj --- CMakeLists.txt | 1 + docs/mt7612u-ap-mode.md | 89 +++++-- src/mt7612u/beacon.cpp | 148 ++++++++++++ src/mt7612u/internal.h | 14 ++ src/mt7612u/regs.h | 32 ++- src/mt7612u/tests/frame_shape.cpp | 52 ++++ src/mt7612u/tools/bringup.cpp | 378 ++++++++++++++++++++++++++++++ src/mt7612u/tx.cpp | 8 + 8 files changed, 703 insertions(+), 19 deletions(-) create mode 100644 src/mt7612u/beacon.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index ceb4882b..77b83465 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -374,6 +374,7 @@ endif() if(DEVOURER_MT7612U) target_sources(devourer PRIVATE src/mt7612u/async.cpp + src/mt7612u/beacon.cpp src/mt7612u/caps.cpp src/mt7612u/eeprom.cpp src/mt7612u/fw.cpp diff --git a/docs/mt7612u-ap-mode.md b/docs/mt7612u-ap-mode.md index abb6182b..65ce24a4 100644 --- a/docs/mt7612u-ap-mode.md +++ b/docs/mt7612u-ap-mode.md @@ -1,9 +1,31 @@ # MT7612U as a fully-userspace access point — what is missing -**Status: research and gap analysis, no code.** File:line references are to the -merged subtree at `daabab7` (`src/mt7612u/`), the mt76 reference at `be5ce79`, -and devourer's existing AP work. A companion implementation/verification plan -is at the end. +**Status: Stages A and B implemented and device-verified** (see "Verified on +hardware" below); Stages C–E (association responder, data plane, WPA2) still +need the `IRtlDevice` wrapper and the existing C++ harnesses. File:line +references are to the merged subtree at `daabab7` (`src/mt7612u/`), the mt76 +reference at `be5ce79`, and devourer's existing AP work. + +## Verified on hardware (2026-09-08, MT7612U at USB 2-1) + +Measured against a second MT7612U bound to the kernel `mt76x2u` driver as a +station, plus an RTL8812AU running `rxdemo` as an independent on-air witness. + +| Claim | Evidence | +|---|---| +| Beacon reaches the air | Kernel station's `iw scan` lists `SSID: MT7612U-AP`, `beacon interval: 100 TUs`, `capability: ESS (0x0001)`, correct `DS Parameter set` and basic-rate flags — on ch149, ch36 **and** ch6 (both bands) | +| Independent radio decodes it | 8812AU witness: 177 frames on a channel measured empty beforehand, each `len:71` (our 67-byte MPDU + FCS) at `rate:4` (OFDM 6M), ~9.8/s | +| HW TSF timestamp (`FLAGS_TS`) | `wlan.fixed.timestamp` advances **102400 µs** per beacon — exactly 100 TU | +| HW sequence (`ACK_CTL_NSEQ`) | `wlan.seq` increments **+1 per beacon** (2140, 2141, 2142 …) | +| Beacon interval math | On-air spacing 102.4 ms, confirming `INTVAL = interval_tu << 4` (1/16 TU) | +| Corrected MBSS masks | `MT_MAC_BSSID_DW1` reads `0x003fa127` — upper bits exactly mt76's `MBSS_MODE=3 / MBEACON_N=7 / LOCAL_BIT` | +| Hardware auto-ACK (Gate B) | A real station's **3 auth frames, 0 retried**. An un-ACKed frame is retransmitted with FC Retry set, so retry=0 is the ACK | +| APC BSSID slot programmed | `MT_MAC_APC_BSSID_L(0)=0x50efa540` (device MAC `40:a5:ef:50:…`) | +| StopBeacon contract | After the process exits the BSSID is absent from a fresh scan — nothing left airing | + +Not yet done: probe **responses**, auth/assoc **responses** and the data plane — +those are the existing backend-agnostic C++ harnesses' job (Stages C–E), not +driver work. ## The claim, and why it holds @@ -72,6 +94,26 @@ Each has a direct mt76 recipe. Estimates are the C-library side only. - 802.11 sequence numbering: let the MAC number it (the HW-seq path mt76 uses for beacons) or number it per update in software; `beacon_wire_check` expects +1 per beacon. + - **Trap — a silent no-transmit.** `MT_BCN_BYPASS_MASK` (0x108c) is + **inverted**. `0xffff`, the value `mt76x02_init_beacon_config()` writes, is + "suppress every slot" — the guard mt76 raises *during* an update ("prevent + corrupt transmissions during update"). A slot only airs once its bit is + **cleared**, which mt76 does after loading the page: + `0xff00 | ~(0xff00 >> beacon_data_count)` (`mt76x02_usb_core.c:223`, + identically `mt76x02_mmio.c:43`); for one beacon in slot 0 that is + `0xffffff7f`. Leave it at `0xffff` and the beacon timer runs, the TSF + advances and every register reads correct — while nothing reaches the air. + - **Trap — the beacon TXWI needs two extra bits.** `MT_TXWI_FLAGS_TS` (BIT 3; + MAC inserts the TSF timestamp) and `MT_TXWI_ACK_CTL_NSEQ` (MAC assigns the + sequence number), exactly as `mt76x02_mac_write_txwi()` sets them for + beacon/probe-resp subtypes. Without them the beacon airs with a frozen + timestamp and a constant sequence number. + - **Trap — `regs.h` had the MBSS masks two bits high.** `MBSS_MODE`, + `MBEACON_N` and `MBSS_LOCAL_BIT` were transcribed as 19:18 / 22:20 / 23 + instead of mt76's 17:16 / 20:18 / 21. Harmless while the port only + injected (only `mac_setaddr` uses them, and beacons were never generated), + but it programmed MBSS_MODE=4 (invalid) and MBEACON_N=15. Corrected; + `MT_MAC_BSSID_DW1` now reads `0x003f____`, matching mt76 bit for bit. 2. **`StopBeacon` (~10 LOC).** Clear `BEACON_TX | TBTT_EN | TIMER_EN`. Note the `IRtlDevice` contract: the chip beacons autonomously, so a session that ends @@ -85,17 +127,34 @@ Each has a direct mt76 recipe. Estimates are the C-library side only. **hardware CCMP**; the software CCMP in `ap_wpa2.cpp` still works as the fallback/portable path. -4. **AP RX filter (~15 LOC).** A third mode beside monitor: accept probe-req - (broadcast), auth/assoc to the BSSID, and data addressed to us; keep - dropping foreign-BSS and error frames. Derive the mask from the managed - default `0x00015f97` by clearing the "not-to-me" drops the AP needs to hear. - -5. **`SetAckResponder` — likely already covered.** On MT the MAC auto-ACKs - frames matching the programmed `MT_MAC_ADDR`, so pointing it at the BSSID - (done in `mac_setaddr`) is the ACK responder. **Open item to confirm on - hardware:** whether MT needs an explicit op-mode/net-type register beyond - beacon-enable + address match (Realtek needs `net_type = AP`); mt76 sets no - distinct AP opmode reg for this part, but verify auth arrives at retry=0. +4. **AP RX filter — one line, not ~15 LOC.** Measured: the managed default + `0x00015f97` already leaves `OTHER_BSS`, `BCAST` and `MCAST` **undropped**, + so a probe request with a wildcard BSSID and auth addressed to us both + arrive unchanged; mt76 clears `OTHER_BSS` for every mode too + (`mt76x02_configure_filter`), and defines no AP-specific filter. The single + change an AP wants is clearing `DUP` (`0x00015f17`) so a station's + retransmission stays visible — dropping duplicates hides exactly the + retry evidence that tells you whether your ACKs are landing. + +5. **`SetAckResponder` — covered by address match. OPEN ITEM NOW RESOLVED.** + On MT the MAC auto-ACKs frames matching the programmed `MT_MAC_ADDR`, so + pointing it at the BSSID (done in `mac_setaddr`) is the ACK responder. + Confirmed on hardware: **there is no AP op-mode/net-type register on this + part.** mt76 sets none — its only AP-specific work in `mt76x02_sta_add` is a + *software* PS flag (`MT_WCID_FLAG_CHECK_PS`) plus a GTK restriction — and a + real station's auth arrived at **retry=0** with nothing but address match + + beacon enable. Address match + beacon *is* the AP. + - **But one register the gap analysis missed:** `mac_setaddr()` zeroes all + eight APC BSSID slots, which is right for an injector and wrong for an AP. + The MAC matches the BSS against `MT_MAC_APC_BSSID_L/H(idx)` for the slot + the MBSS index selects, so an AP must publish its BSSID there — + `mt76x02_mac_set_bssid()` (`mt76x02_mac.c`), 4 lines. Without it the MAC + matches nothing for the BSS. + - The simplest consistent choice is to make the AP BSSID the device's own + MAC: `mac_setaddr()` already programs it into `MT_MAC_ADDR` (what the MAC + ACKs against) and `MT_MAC_BSSID`, so only the APC slot is left to write. + Advertising any *other* BSSID in the beacon leaves a station addressing + auth to an address the MAC does not answer for. 6. **`UpdateBeaconPayload` (~10 LOC, optional).** Re-write the reserved page in place. Only needed for dynamic beacon content (below). diff --git a/src/mt7612u/beacon.cpp b/src/mt7612u/beacon.cpp new file mode 100644 index 00000000..12bd5f16 --- /dev/null +++ b/src/mt7612u/beacon.cpp @@ -0,0 +1,148 @@ +/* SPDX-License-Identifier: BSD-3-Clause-Clear */ +/* + * Static reserved-page beacon. + * + * The MT76 MAC auto-transmits a beacon written to MT_BEACON_BASE at every + * TBTT, filling the TSF timestamp and the 802.11 sequence number in hardware + * (MT_TXWI_FLAGS_TS + MT_TXWI_ACK_CTL_NSEQ, requested via MT_TXOPT_BEACON). + * So an always-on AP needs no host involvement per beacon: no pre-TBTT timer, + * no worker thread. That is the one USB-userspace limitation the AP design + * engineers out - see docs/mt7612u-ap-mode.md. + * + * Dynamic beacon content (a live TIM bitmap for power-saving clients) is NOT + * covered here; it would need the pre-TBTT machinery mt76 runs on PCIe/USB. + * + * Ported from mt76/mt76x02_beacon.c and mt76x02_usb_core.c @ be5ce79. + * Copyright (C) 2016 Felix Fietkau, (C) 2018 Lorenzo Bianconi / Stanislaw Gruszka. + */ +#include "internal.h" + +/* mt76x02u: 5 USB beacon slots, each (8192 / 5) & ~63 = 1600 bytes. The 8 kB + * reserved page is shared with PS-buffered frames upstream; we use slot 0. */ +#define MT_BCN_NSLOTS 5 +#define MT_BCN_SLOT_SIZE ((8192 / MT_BCN_NSLOTS) & ~63) + +/* mt76x02_set_beacon_offsets(): each slot's (byte offset / 64) is packed into + * MT_BCN_OFFSET, four slots to a 32-bit register. */ +static void mt_beacon_set_offsets(struct mt7612u_dev *d) +{ + uint32_t regs[4] = { 0 }; + int i; + + for (i = 0; i < MT_BCN_NSLOTS; i++) { + uint32_t val = (uint32_t)i * MT_BCN_SLOT_SIZE; + + regs[i / 4] |= (val / 64) << (8 * (i % 4)); + } + for (i = 0; i < 4; i++) + mt_wr(d, MT_BCN_OFFSET(i), regs[i]); +} + +/* + * mt76x02_init_beacon_config(): quiet the beacon engine, select sync mode, + * suppress every beacon slot while the page is being set up, and lay out the + * slot offsets. Run once before the first beacon is written. The address + * programming (BSSID, MBSS mode, per-slot beacon count) is done by + * mac_setaddr() at init. BCN_BYPASS_MASK=0xffff suppresses all slots here; + * mt_beacon_write() clears the bit for the slot it loads so that one airs. + */ +void mt_beacon_init(struct mt7612u_dev *d) +{ + mt_clear(d, MT_BEACON_TIME_CFG, + MT_BEACON_TIME_CFG_TIMER_EN | MT_BEACON_TIME_CFG_TBTT_EN | + MT_BEACON_TIME_CFG_BEACON_TX); + mt_set(d, MT_BEACON_TIME_CFG, MT_BEACON_TIME_CFG_SYNC_MODE); + mt_wr(d, MT_BCN_BYPASS_MASK, 0xffff); + mt_beacon_set_offsets(d); +} + +/* + * mt76x02_mac_set_beacon(): write [TXWI][beacon MPDU] into slot 0. + * + * mt_tx_build() emits [TXINFO 4][TXWI 20][802.11][pad], which is the shape the + * TX queue wants; the reserved page wants no TXINFO and no trailing zero word, + * so skip the first 4 bytes and copy from the TXWI onward. A beacon header is + * 24 bytes (4-aligned), so mt_tx_build() inserts no interior L2 pad and the + * copied region is exactly [TXWI][MPDU] rounded up to a word. + */ +int mt_beacon_write(struct mt7612u_dev *d, const void *frame, size_t len, + const struct mt7612u_tx_rate *rate) +{ + uint8_t buf[MT_BCN_SLOT_SIZE]; + int total; + + if (len + MT_TXWI_LEN > MT_BCN_SLOT_SIZE) { + ERR("beacon %zu B + TXWI exceeds the %d B slot", len, + (int)MT_BCN_SLOT_SIZE); + return -1; + } + total = mt_tx_build(d, buf, sizeof buf, frame, len, rate, 0xff, + MT_TXOPT_BEACON, 0, 0); + if (total < 0) + return -1; + + mt_wr_copy(d, MT_BEACON_BASE, buf + MT_DMA_HDR_LEN, + total - MT_DMA_HDR_LEN); + + /* + * Unsuppress the slot just written. BCN_BYPASS_MASK is inverted: a set + * bit suppresses that slot (mt_beacon_init() set all 16). mt76 clears the + * low N bits down from bit 7 for N written beacons - 0xff00 | ~(0xff00 >> + * beacon_data_count) in mt76x02u_pre_tbtt_work() - and the static path + * writes exactly one (slot 0), so N = 1. Without this the beacon never + * airs even though the TSF and beacon timer run. + */ + mt_wr(d, MT_BCN_BYPASS_MASK, 0xff00u | ~(0xff00u >> 1)); + return 0; +} + +/* + * mt76x02_mac_set_bssid(): the per-BSS address the MAC matches receptions + * against. mac_setaddr() zeroes all eight APC slots at init, which is right for + * an injector; an AP must publish its own BSSID in the slot its MBSS index + * selects (0 for a single BSS) or the MAC matches nothing for the BSS - a + * station's auth is then neither accepted nor auto-ACKed, and it retries + * forever. Called by the AP path; the injector never needs it. + */ +int mt_ap_set_bssid(struct mt7612u_dev *d, uint8_t idx, const uint8_t *addr) +{ + uint32_t lo = (uint32_t)addr[0] | ((uint32_t)addr[1] << 8) | + ((uint32_t)addr[2] << 16) | ((uint32_t)addr[3] << 24); + uint32_t hi = (uint32_t)addr[4] | ((uint32_t)addr[5] << 8); + + idx &= 7; + if (mt_wr_chk(d, MT_MAC_APC_BSSID_L(idx), lo)) + return -1; + /* mt_rmw() skips the write entirely when its read half fails, so an + * unchecked call can leave bytes 4-5 zero - a half-programmed BSSID that + * matches nothing while the L half still reads back correct. mt76 returns + * void here because MMIO cannot fail; USB can. */ + return mt_rmw(d, MT_MAC_APC_BSSID_H(idx), MT_MAC_APC_BSSID_H_ADDR, hi); +} + +/* + * mt76x02_mac_set_beacon_enable(), static path. No pre-TBTT timer: the MAC + * transmits the reserved-page beacon on its own once BEACON_TX|TBTT_EN|TIMER_EN + * are set. interval_tu is the beacon interval in TU (1024 us); the register + * counts in 1/16 TU, so it is shifted left by 4 (mt76x02_bss_info_changed). + */ +int mt_beacon_set_enable(struct mt7612u_dev *d, int on, unsigned interval_tu) +{ + const uint32_t bits = MT_BEACON_TIME_CFG_BEACON_TX | + MT_BEACON_TIME_CFG_TBTT_EN | + MT_BEACON_TIME_CFG_TIMER_EN; + + if (on) { + /* INTVAL is 16 bits of 1/16-TU, so the interval caps at 4095 TU. */ + if (interval_tu == 0 || interval_tu > 0xffffu / 16) { + ERR("beacon interval %u TU out of range (1..4095)", interval_tu); + return -1; + } + mt_rmw(d, MT_BEACON_TIME_CFG, MT_BEACON_TIME_CFG_INTVAL, + FIELD_PREP(MT_BEACON_TIME_CFG_INTVAL, interval_tu << 4)); + mt_set(d, MT_BEACON_TIME_CFG, bits); + } else { + mt_clear(d, MT_BEACON_TIME_CFG, bits); + } + return 0; +} diff --git a/src/mt7612u/internal.h b/src/mt7612u/internal.h index 7fab68e5..e95056f8 100644 --- a/src/mt7612u/internal.h +++ b/src/mt7612u/internal.h @@ -324,6 +324,7 @@ int mt_hdrlen_from_fc(const uint8_t *frame); #define MT_TXOPT_RATE_LUT 0x01 /* set MT_TXWI_FLAGS_TX_RATE_LUT */ #define MT_TXOPT_AMPDU 0x02 /* AMPDU flag + density + BA window */ #define MT_TXOPT_QSEL_MGMT 0x04 /* mt76 uses MT_QSEL_MGMT for aggregated TX */ +#define MT_TXOPT_BEACON 0x08 /* HW timestamp (FLAGS_TS) + HW sequence (ACK_CTL_NSEQ) */ int mt_tx_build(struct mt7612u_dev *d, uint8_t *buf, size_t bufsz, const void *frame, size_t len, const struct mt7612u_tx_rate *rate, uint8_t wcid, unsigned opts, @@ -332,6 +333,19 @@ int mt_tx_raw(struct mt7612u_dev *d, const void *frame, size_t len, const struct mt7612u_tx_rate *rate, uint8_t wcid, unsigned opts); void mt_wcid_setup(struct mt7612u_dev *d, uint8_t idx, const uint8_t *mac); +/* --- beacon.c --- */ +/* Static reserved-page beacon. mt_beacon_init() prepares the beacon engine + * (offsets, bypass, sync) once; mt_beacon_write() loads slot 0; mt_beacon_set_enable() + * arms or disarms auto-TX. No pre-TBTT host work - the MAC beacons on its own. */ +void mt_beacon_init(struct mt7612u_dev *d); +int mt_beacon_write(struct mt7612u_dev *d, const void *frame, size_t len, + const struct mt7612u_tx_rate *rate); +int mt_beacon_set_enable(struct mt7612u_dev *d, int on, unsigned interval_tu); +/* Publish the AP's BSSID in APC slot `idx` so the MAC matches and auto-ACKs + * frames addressed to the BSS. mac_setaddr() zeroes every slot at init. + * Returns 0 on success, -1 if either half of the address failed to program. */ +int mt_ap_set_bssid(struct mt7612u_dev *d, uint8_t idx, const uint8_t *addr); + /* --- radiotap.c --- */ int mt_radiotap_parse(const uint8_t *buf, size_t len, struct mt7612u_tx_rate *r); diff --git a/src/mt7612u/regs.h b/src/mt7612u/regs.h index 80cc7639..9fe9bfd3 100644 --- a/src/mt7612u/regs.h +++ b/src/mt7612u/regs.h @@ -166,19 +166,40 @@ enum mt_mcu_cr_mode { MT_RF_CR, MT_BBP_CR, MT_RF_BBP_CR, MT_HL_TEMP_CR_UPDATE }; #define MT_MAC_ADDR_DW1_U2ME_MASK GENMASK(23, 16) #define MT_MAC_BSSID_DW0 0x1010 #define MT_MAC_BSSID_DW1 0x1014 -#define MT_MAC_BSSID_DW1_MBSS_MODE GENMASK(19, 18) -#define MT_MAC_BSSID_DW1_MBEACON_N GENMASK(22, 20) -#define MT_MAC_BSSID_DW1_MBSS_LOCAL_BIT BIT(23) +/* + * These three were transcribed two bits high (MBSS_MODE at 19:18, MBEACON_N at + * 22:20, LOCAL_BIT at 23) - the whole group shifted left by 2 versus mt76's + * mt76x02_regs.h. It was harmless while this HAL only injected: mac_setaddr() + * is the only user, and MBSS_MODE / MBEACON_N steer beacon generation, which + * an injector never exercises. The AP path beacons, so the true positions + * matter - the shifted masks programmed MBSS_MODE=4 (invalid) and MBEACON_N=15. + */ +#define MT_MAC_BSSID_DW1_ADDR GENMASK(15, 0) +#define MT_MAC_BSSID_DW1_MBSS_MODE GENMASK(17, 16) +#define MT_MAC_BSSID_DW1_MBEACON_N GENMASK(20, 18) +#define MT_MAC_BSSID_DW1_MBSS_LOCAL_BIT BIT(21) +#define MT_MAC_BSSID_DW1_MBSS_MODE_B2 BIT(22) +#define MT_MAC_BSSID_DW1_MBEACON_N_B3 BIT(23) #define MT_MAX_LEN_CFG 0x1018 #define MT_XIFS_TIME_CFG 0x1100 #define MT_XIFS_TIME_CFG_OFDM_SIFS GENMASK(15, 8) #define MT_BKOFF_SLOT_CFG 0x1104 #define MT_BKOFF_SLOT_CFG_CC_DELAY GENMASK(11, 8) #define MT_BEACON_TIME_CFG 0x1114 -#define MT_BEACON_TIME_CFG_INTVAL GENMASK(15, 0) +#define MT_BEACON_TIME_CFG_INTVAL GENMASK(15, 0) /* in 1/16 TU units */ #define MT_BEACON_TIME_CFG_TIMER_EN BIT(16) +#define MT_BEACON_TIME_CFG_SYNC_MODE GENMASK(18, 17) #define MT_BEACON_TIME_CFG_TBTT_EN BIT(19) #define MT_BEACON_TIME_CFG_BEACON_TX BIT(20) +/* Reserved-page beacon (mt76x02_beacon.c). BCN_OFFSET packs four slot offsets + * (each offset/64) per register. BCN_BYPASS_MASK is per-slot and INVERTED: a + * set bit SUPPRESSES that slot (0xffff = all suppressed, used during an + * update); a slot only airs once its bit is CLEARED. mt76 enables N written + * beacons with 0xff00 | ~(0xff00 >> N). */ +#define MT_BCN_OFFSET_BASE 0x041c +#define MT_BCN_OFFSET(_n) (MT_BCN_OFFSET_BASE + ((_n) << 2)) +#define MT_BCN_BYPASS_MASK 0x108c +#define MT_BEACON_BASE 0xc000 #define MT_TSF_TIMER_DW0 0x111c #define MT_TSF_TIMER_DW1 0x1120 #define MT_MAC_STATUS 0x1200 @@ -222,6 +243,8 @@ enum mt_mcu_cr_mode { MT_RF_CR, MT_BBP_CR, MT_RF_BBP_CR, MT_HL_TEMP_CR_UPDATE }; #define MT_RX_FILTR_CFG_PROMISC BIT(2) #define MT_RX_FILTR_CFG_OTHER_BSS BIT(3) #define MT_RX_FILTR_CFG_VER_ERR BIT(4) +#define MT_RX_FILTR_CFG_MCAST BIT(5) +#define MT_RX_FILTR_CFG_BCAST BIT(6) #define MT_RX_FILTR_CFG_DUP BIT(7) #define MT_RX_FILTR_CFG_CTRL_RSV BIT(16) #define MT_AUTO_RSP_CFG 0x1404 @@ -411,6 +434,7 @@ enum mt_ee_field { #define MT_RXWI_LEN 32 #define MT_DMA_HDR_LEN 4 +#define MT_TXWI_FLAGS_TS BIT(3) /* MAC inserts the TSF timestamp (beacon/probe-resp) */ #define MT_TXWI_FLAGS_AMPDU BIT(4) #define MT_TXWI_FLAGS_MPDU_DENSITY GENMASK(7, 5) #define MT_TXWI_ACK_CTL_BA_WINDOW GENMASK(7, 2) diff --git a/src/mt7612u/tests/frame_shape.cpp b/src/mt7612u/tests/frame_shape.cpp index 8fcb72d2..4a869807 100644 --- a/src/mt7612u/tests/frame_shape.cpp +++ b/src/mt7612u/tests/frame_shape.cpp @@ -492,6 +492,57 @@ static void test_tx_ring_ceiling(void) } } +/* + * A reserved-page beacon must carry two TXWI bits an injected frame must not: + * MT_TXWI_FLAGS_TS (the MAC fills the TSF timestamp) and MT_TXWI_ACK_CTL_NSEQ + * (the MAC assigns the sequence number). Without them the witness sees a frozen + * TSF and a constant sequence number - exactly what Gate A checks on air. This + * pins the bits at build time so the on-air gate is not the first to notice. + */ +static void test_beacon_txwi(void) +{ + struct mt7612u_dev d; + uint8_t buf[128]; + uint8_t beacon[36], data[36]; + struct mt7612u_tx_rate bcn_rate = { + .phy = MT7612U_PHY_OFDM, .mcs = 0, .nss = 1, + .bw = MT7612U_BW_20, .no_ack = 1, + }; + struct mt7612u_tx_rate data_rate = { + .phy = MT7612U_PHY_OFDM, .mcs = 0, .nss = 1, .bw = MT7612U_BW_20, + }; + int total; + uint8_t flags_lo, ack_ctl; + + printf("beacon TXWI (MT_TXOPT_BEACON sets TS + NSEQ):\n"); + + memset(&d, 0, sizeof d); + d.chainmask = 0x0202; + + memset(beacon, 0, sizeof beacon); + beacon[0] = 0x80; /* FC: mgmt beacon, 24-byte header */ + memset(data, 0, sizeof data); + data[0] = 0x08; /* FC: data, 24-byte header */ + + total = mt_tx_build(&d, buf, sizeof buf, beacon, sizeof beacon, + &bcn_rate, 0xff, MT_TXOPT_BEACON, 0, 0); + if (total < 0) { printf(" FAIL beacon build returned %d\n", total); fails++; return; } + flags_lo = buf[4]; /* TXWI flags, low byte (TXWI is at buf+4) */ + ack_ctl = buf[8]; /* TXWI ack_ctl */ + if (!(flags_lo & MT_TXWI_FLAGS_TS)) { printf(" FAIL beacon missing FLAGS_TS\n"); fails++; } + if (!(ack_ctl & MT_TXWI_ACK_CTL_NSEQ)) { printf(" FAIL beacon missing ACK_CTL_NSEQ\n"); fails++; } + if (ack_ctl & MT_TXWI_ACK_CTL_REQ) { printf(" FAIL beacon requested an ACK\n"); fails++; } + + total = mt_tx_build(&d, buf, sizeof buf, data, sizeof data, + &data_rate, 0xff, 0, 0, 0); + if (total < 0) { printf(" FAIL data build returned %d\n", total); fails++; return; } + flags_lo = buf[4]; + ack_ctl = buf[8]; + if (flags_lo & MT_TXWI_FLAGS_TS) { printf(" FAIL data frame set FLAGS_TS\n"); fails++; } + if (ack_ctl & MT_TXWI_ACK_CTL_NSEQ) { printf(" FAIL data frame set NSEQ\n"); fails++; } + if (!(ack_ctl & MT_TXWI_ACK_CTL_REQ)) { printf(" FAIL data frame did not request an ACK\n"); fails++; } +} + int main(void) { test_hdrlen(); @@ -501,6 +552,7 @@ int main(void) test_chan_group(); test_vht_bandwidth(); test_ht_bandwidth(); + test_beacon_txwi(); printf("frame_shape: %s\n", fails ? "FAIL" : "PASS"); return fails ? 1 : 0; } diff --git a/src/mt7612u/tools/bringup.cpp b/src/mt7612u/tools/bringup.cpp index 17cfd941..44ad22a9 100644 --- a/src/mt7612u/tools/bringup.cpp +++ b/src/mt7612u/tools/bringup.cpp @@ -541,6 +541,376 @@ static int gate_adopt(const char *sel) return rc; } +/* + * Stage A: a static beacon on air. The MAC auto-transmits it from the reserved + * page, so there is nothing to loop over here except watching the TSF advance; + * the RTL8812AU witness (rxdemo) and a kernel station's `iw scan` decide + * PASS/FAIL. The beacon is ALWAYS disabled before returning - a beacon left + * armed keeps airing after the process exits and contaminates the next run. + */ +static int build_beacon(uint8_t chan, const uint8_t *bssid, uint8_t *out, + size_t outsz) +{ + /* 5 GHz: OFDM basic set. 2.4 GHz: CCK + OFDM basic set. */ + static const uint8_t rates_5g[] = { 0x8c, 0x12, 0x98, 0x24, + 0xb0, 0x48, 0x60, 0x6c }; + static const uint8_t rates_2g[] = { 0x82, 0x84, 0x8b, 0x96, + 0x0c, 0x12, 0x18, 0x24 }; + static const char ssid[] = "MT7612U-AP"; + const uint8_t *rates = chan <= 14 ? rates_2g : rates_5g; + const int ssidlen = (int)sizeof ssid - 1; + uint8_t *p = out; + + if (outsz < 128) + return -1; + + *p++ = 0x80; *p++ = 0x00; /* FC: mgmt, beacon */ + *p++ = 0x00; *p++ = 0x00; /* duration */ + memset(p, 0xff, 6); p += 6; /* addr1 = broadcast */ + memcpy(p, bssid, 6); p += 6; /* addr2 = SA (BSSID) */ + memcpy(p, bssid, 6); p += 6; /* addr3 = BSSID */ + *p++ = 0x00; *p++ = 0x00; /* seq ctl (HW assigns) */ + + memset(p, 0, 8); p += 8; /* timestamp (HW fills) */ + *p++ = 0x64; *p++ = 0x00; /* beacon interval = 100 TU */ + *p++ = 0x01; *p++ = 0x00; /* capability: ESS */ + + *p++ = 0; *p++ = (uint8_t)ssidlen; /* SSID IE */ + memcpy(p, ssid, (size_t)ssidlen); p += ssidlen; + *p++ = 1; *p++ = 8; memcpy(p, rates, 8); p += 8; /* Supported Rates */ + *p++ = 3; *p++ = 1; *p++ = chan; /* DS Parameter Set */ + *p++ = 5; *p++ = 4; /* TIM (DTIM=1, empty) */ + *p++ = 0; *p++ = 1; *p++ = 0; *p++ = 0; + + return (int)(p - out); +} + +static int gate_beacon(uint8_t chan, int secs) +{ + /* The AP's BSSID is the device's own MAC, which mac_setaddr() has already + * programmed into MT_MAC_ADDR (what the MAC auto-ACKs against) and + * MT_MAC_BSSID. Advertising anything else in the beacon would leave a + * station addressing auth to an address the MAC does not answer for. It + * is a real, unicast address, which is what a STA requires (an I/G-set + * BSSID makes it drop auth before the air - docs/ap-mode.md). */ + const uint8_t *bssid; + struct mt7612u_tx_rate rate = { + .phy = MT7612U_PHY_OFDM, .mcs = 0, .nss = 1, + .bw = MT7612U_BW_20, .no_ack = 1, + }; + uint8_t bcn[128]; + int n, rc = 1; + + if (mt_eeprom_init(&dev)) return 1; + bssid = dev.macaddr; + n = build_beacon(chan, bssid, bcn, sizeof bcn); + if (n < 0) { printf("GATE A: FAIL - beacon build\n"); return 1; } + if (mt_init_hardware(&dev, NULL)) { + printf("GATE A: FAIL - init_hardware\n"); return 1; + } + if (mt_set_channel(&dev, chan, MT7612U_BW_20)) { + printf("GATE A: FAIL - set_channel\n"); return 1; + } + /* TX-only: beaconing never reads EP 4. */ + if (mt_mac_start(&dev, MT_RX_DRAIN_NONE)) { + printf("GATE A: FAIL - mac_start\n"); return 1; + } + + mt_beacon_init(&dev); + if (mt_beacon_write(&dev, bcn, (size_t)n, &rate)) { + printf("GATE A: FAIL - beacon_write\n"); goto out; + } + if (mt_beacon_set_enable(&dev, 1, 100)) { + printf("GATE A: FAIL - beacon_set_enable\n"); goto out; + } + + printf("beacon armed: ch%u, BSSID %02x:%02x:%02x:%02x:%02x:%02x, " + "SSID \"MT7612U-AP\", 100 TU, OFDM 6M, %d B MPDU\n", + chan, bssid[0], bssid[1], bssid[2], bssid[3], bssid[4], bssid[5], n); + printf("MT_BEACON_TIME_CFG=0x%08x (bit16 TIMER bit19 TBTT bit20 TX)\n", + mt_rr(&dev, MT_BEACON_TIME_CFG)); + printf("MT_MAC_BSSID_DW1 =0x%08x (MBSS_MODE 17:16 should read 3)\n", + mt_rr(&dev, MT_MAC_BSSID_DW1)); + printf("witness: run rxdemo on the 8812AU and grep the BSSID; " + "or `iw dev scan | grep MT7612U-AP`\n"); + + /* Watch the TSF advance - proof the beacon timer is running. DW0 is the + * low word on this silicon (mt76's debug read has it backwards). */ + { + uint64_t prev = 0; + int good = 0; + + for (int s = 0; s < secs && !g_stop; s++) { + uint32_t lo = mt_rr(&dev, MT_TSF_TIMER_DW0); + uint32_t hi = mt_rr(&dev, MT_TSF_TIMER_DW1); + uint64_t tsf = ((uint64_t)hi << 32) | lo; + + if (s) + printf(" t=%ds TSF=%llu (+%llu us)\n", s, + (unsigned long long)tsf, + (unsigned long long)(tsf - prev)); + if (s && tsf > prev) + good++; + prev = tsf; + if (!wait_ms(1000)) + break; + } + /* A running TSF is necessary, not sufficient - the witness is the + * real gate - but a frozen TSF means no beacons are being sent. */ + if (good == 0) { + printf("GATE A: FAIL - TSF did not advance; beacon timer is dead\n"); + goto out; + } + printf("TSF advanced on %d sample(s) - beacon timer is live\n", good); + } + rc = 0; + /* This is a LOCAL precondition only: an advancing TSF proves the beacon + * timer runs, not that a frame reaches the air. The witness (rxdemo / + * `iw scan`) is the actual Gate A. */ + printf("\nGATE A (local): beacon armed, timer live. On-air PASS/FAIL is " + "the witness's call - grep the 8812AU for our SSID/BSSID.\n"); + +out: + mt_beacon_set_enable(&dev, 0, 0); /* never leave a beacon airing */ + mt_mac_stop(&dev); + return rc; +} + +/* + * Stage B: the beacon plus a receiver, so a real station can probe, authenticate + * and associate against us. + * + * The measurement that matters is the RETRY BIT. An ACK is SIFS-timed and can + * only come from the MAC, so it cannot be observed directly from userspace - + * but a station that does not get one retransmits with FC Retry set. Auth + * arriving at retry=0 is therefore the proof that the hardware auto-ACKed it; + * a pile of retry=1 auths is the proof it did not. + */ +struct ap_ctx { + /* std::atomic, not C11 _Atomic: this file is C++ since the subtree + * migration, and ap_cb runs on the RX event thread while the gate's own + * thread reads the counters. */ + std::atomic probe_req{0}, auth{0}, auth_retry{0}; + std::atomic assoc{0}, assoc_retry{0}; + std::atomic data_to_us{0}, mgmt_other{0}; + uint8_t bssid[6]; +}; + +static void ap_cb(void *user, const void *frame, size_t len, + const struct mt7612u_rx_info *info) +{ + struct ap_ctx *c = static_cast(user); + const uint8_t *f = static_cast(frame); + unsigned fc, type, subtype; + int retry, to_us; + + (void)info; + if (len < 16) return; + fc = (unsigned)f[0] | ((unsigned)f[1] << 8); + type = (fc >> 2) & 3; + subtype = (fc >> 4) & 0xf; + retry = (f[1] & 0x08) != 0; /* FC Retry */ + to_us = memcmp(f + 4, c->bssid, 6) == 0; /* addr1 == our BSSID */ + + if (type == 2) { /* data */ + if (to_us) + c->data_to_us.fetch_add(1, std::memory_order_relaxed); + return; + } + if (type != 0) return; /* control */ + + switch (subtype) { + case 4: /* probe request (usually broadcast) */ + c->probe_req.fetch_add(1, std::memory_order_relaxed); + break; + case 11: /* authentication */ + if (!to_us) break; + c->auth.fetch_add(1, std::memory_order_relaxed); + if (retry) + c->auth_retry.fetch_add(1, std::memory_order_relaxed); + break; + case 0: case 2: /* (re)association request */ + if (!to_us) break; + c->assoc.fetch_add(1, std::memory_order_relaxed); + if (retry) + c->assoc_retry.fetch_add(1, std::memory_order_relaxed); + break; + default: + if (to_us) + c->mgmt_other.fetch_add(1, std::memory_order_relaxed); + break; + } +} + +static int gate_ap(uint8_t chan, int secs) +{ + struct ap_ctx ctx{}; + struct mt7612u_tx_rate rate = { + .phy = MT7612U_PHY_OFDM, .mcs = 0, .nss = 1, + .bw = MT7612U_BW_20, .no_ack = 1, + }; + uint8_t bcn[128]; + int n, rc = 1, rx_up = 0; + unsigned pr, au, aur, as, asr, dt; + + if (secs <= 0 || secs > 3600) { + printf("GATE B: FAIL - duration %d out of range (1..3600 s)\n", secs); + return 1; + } + if (mt_eeprom_init(&dev)) return 1; + memcpy(ctx.bssid, dev.macaddr, 6); + n = build_beacon(chan, dev.macaddr, bcn, sizeof bcn); + if (n < 0) { printf("GATE B: FAIL - beacon build\n"); return 1; } + + if (mt_init_hardware(&dev, NULL)) { + printf("GATE B: FAIL - init_hardware\n"); return 1; + } + if (mt_set_channel(&dev, chan, MT7612U_BW_20)) { + printf("GATE B: FAIL - set_channel\n"); return 1; + } + /* Ring first, receiver second - RX must never run with EP 4 undrained. */ + if (mt7612u_rx_start(&dev, ap_cb, &ctx)) { + printf("GATE B: FAIL - rx_start\n"); return 1; + } + rx_up = 1; + if (mt_mac_start(&dev, MT_RX_DRAIN_RING)) { + printf("GATE B: FAIL - mac_start\n"); mt7612u_rx_stop(&dev); return 1; + } + /* + * AP receive filter. The managed default mt_mac_start() just wrote already + * leaves OTHER_BSS, BCAST and MCAST undropped, so a probe request with a + * wildcard BSSID reaches us - mt76 clears OTHER_BSS for every mode too. + * The one change an AP needs is DUP: dropping duplicates would hide exactly + * the retransmissions this gate measures. Clear the bit in place rather + * than re-write a copied literal, so this cannot drift from the default. + */ + mt_clear(&dev, MT_RX_FILTR_CFG, MT_RX_FILTR_CFG_DUP); + + /* + * The address-match half of "being an AP": the MAC auto-ACKs against + * MT_MAC_ADDR (already our MAC) and matches the BSS against this slot, + * which mac_setaddr() zeroed. There is no separate AP op-mode register on + * this part - mt76 sets none either; address match + beacon IS the AP. + * + * Slot 0 is only right for a globally-administered MAC. Under MBSS_MODE=3 + * the hardware takes the BSS index from the address bits, and mt76 uses + * 1 + (((macaddr[0] ^ addr[0]) >> 2) & 7) whenever the locally-administered + * bit is set (mt76x02_util.c). Refuse loudly rather than guess: a cloned + * 02:/06:/0a: MAC would match nothing and void every result below. + */ + if (dev.macaddr[0] & 0x02) { + printf("GATE B: FAIL - MAC %02x:.. is locally administered; APC slot 0 " + "is not the slot this MAC selects (mt76 derives 1+n)\n", + dev.macaddr[0]); + goto out; + } + if (mt_ap_set_bssid(&dev, 0, dev.macaddr)) { + printf("GATE B: FAIL - could not program the APC BSSID slot\n"); + goto out; + } + + mt_beacon_init(&dev); + if (mt_beacon_write(&dev, bcn, (size_t)n, &rate)) { + printf("GATE B: FAIL - beacon_write\n"); goto out; + } + if (mt_beacon_set_enable(&dev, 1, 100)) { + printf("GATE B: FAIL - beacon_set_enable\n"); goto out; + } + + printf("AP up: ch%u BSSID/MAC %02x:%02x:%02x:%02x:%02x:%02x SSID \"MT7612U-AP\"\n", + chan, dev.macaddr[0], dev.macaddr[1], dev.macaddr[2], + dev.macaddr[3], dev.macaddr[4], dev.macaddr[5]); + /* Read back BOTH halves of the BSSID: mt_rmw() skips its write when the + * read fails, so printing only the L half would show a correct-looking + * address for a BSSID whose top two bytes never landed. */ + printf("MT_RX_FILTR_CFG=0x%08x APC_BSSID(0)=%04x%08x AUTO_RSP_CFG=0x%08x\n", + mt_rr(&dev, MT_RX_FILTR_CFG), + (unsigned)(mt_rr(&dev, MT_MAC_APC_BSSID_H(0)) & MT_MAC_APC_BSSID_H_ADDR), + mt_rr(&dev, MT_MAC_APC_BSSID_L(0)), + mt_rr(&dev, MT_AUTO_RSP_CFG)); + printf("stimulus: on a station radio run\n" + " sudo iw dev scan (probe requests)\n" + " sudo wpa_supplicant ... / iw dev connect MT7612U-AP\n"); + printf("listening %d s ...\n", secs); + + if (!wait_ticking(secs * 1000.0)) + printf("(interrupted)\n"); + + /* + * Receiver loss belongs next to the verdict: a retried auth we simply + * missed biases the result toward PASS, which is the direction that + * produces a false hardware conclusion. Sample it while the ring still + * EXISTS - mt7612u_rx_stop() tears the ring down and takes its counters + * with it, which reads back as a flat zero and looks like a clean capture. + */ + { + struct mt7612u_stats st; + + mt7612u_get_stats(&dev, &st); + printf("rx frames %llu err %llu invalid %llu dropped %llu\n", + (unsigned long long)st.rx_frames, + (unsigned long long)st.rx_err, + (unsigned long long)st.rx_invalid, + (unsigned long long)st.rx_dropped); + } + + /* + * Now stop the producer, BEFORE reading the verdict counters. ap_cb() runs + * on the RX event thread, and auth/auth_retry are two independent relaxed + * atomics - sampling them live can catch one increment half-applied and + * invert the verdict outright (auth=0 with auth_retry=1 reads as "no auth + * reached us"; auth_retry>auth reads as "every auth was a retry"). + * mt_async_stop() joins the event thread, so after this no callback can + * run. gate_ack orders it the same way. + */ + mt7612u_rx_stop(&dev); + rx_up = 0; + + pr = ctx.probe_req.load(std::memory_order_relaxed); + au = ctx.auth.load(std::memory_order_relaxed); + aur = ctx.auth_retry.load(std::memory_order_relaxed); + as = ctx.assoc.load(std::memory_order_relaxed); + asr = ctx.assoc_retry.load(std::memory_order_relaxed); + dt = ctx.data_to_us.load(std::memory_order_relaxed); + + printf("\nprobe-req %u | auth %u (retry %u) | assoc %u (retry %u) | data-to-us %u | other-mgmt %u\n", + pr, au, aur, as, asr, dt, + ctx.mgmt_other.load(std::memory_order_relaxed)); + + if (!au) { + printf("GATE B: INCONCLUSIVE - no auth reached us " + "(probe-req %u). Did a station try to connect?\n", pr); + } else if (aur == 0) { + printf("GATE B: PASS - %u auth frame(s), none retried: " + "the MAC auto-ACKed them\n", au); + rc = 0; + } else if (aur < au) { + printf("GATE B: PARTIAL - %u auth, %u retried: ACKs land but not always\n", + au, aur); + rc = 0; + } else { + printf("GATE B: FAIL - every auth (%u) was a retry: nothing is ACKing\n", au); + } + +out: + mt_beacon_set_enable(&dev, 0, 0); /* never leave a beacon airing */ + /* Retract the BSS address too, so the teardown matches the contract the + * beacon half states. Inert in practice (the MAC is stopped and + * mac_setaddr() re-zeroes every slot on the next bring-up), but leaving + * half the AP identity programmed contradicts what this gate promises. */ + { + static const uint8_t zero[6] = { 0 }; + + mt_ap_set_bssid(&dev, 0, zero); + } + /* rx_up: the verdict path already stopped the ring so the counters could + * be read with the producer joined; stopping twice must not happen. */ + if (rx_up) + mt7612u_rx_stop(&dev); + mt_mac_stop(&dev); + return rc; +} + /* Gate E: inject frames. The witness is a separate radio - our own RX seeing * these would prove nothing. */ static int gate_tx(uint8_t chan, int count, int phy, int mcs) @@ -2609,6 +2979,12 @@ int main(int argc, char **argv) argc > 3 ? atoi(argv[3]) : 200, argc > 4 ? atoi(argv[4]) : MT7612U_PHY_OFDM, argc > 5 ? atoi(argv[5]) : 0); + } else if (!strcmp(cmd, "beacon")) { + rc = gate_beacon(argc > 2 ? (uint8_t)atoi(argv[2]) : 149, + argc > 3 ? atoi(argv[3]) : 10); + } else if (!strcmp(cmd, "ap")) { + rc = gate_ap(argc > 2 ? (uint8_t)atoi(argv[2]) : 149, + argc > 3 ? atoi(argv[3]) : 30); } else if (!strcmp(cmd, "chan")) { rc = gate_chan(argc > 2 ? (uint8_t)atoi(argv[2]) : 149, argc > 3 ? argv[3] : NULL); @@ -2622,6 +2998,8 @@ int main(int argc, char **argv) fprintf(stderr, "unknown subcommand '%s'\n", cmd); fprintf(stderr, "usage: bringup [regs|fw|init|chan|tx|rx|hop|gateg] [chan] [count] [phy 0=CCK 1=OFDM 2=HT 4=VHT] [mcs]\n"); fprintf(stderr, " bringup adopt (the mt_adopt path a libusb-owning consumer uses)\n"); + fprintf(stderr, " bringup beacon [chan] [secs] (Stage A: static AP beacon on air)\n"); + fprintf(stderr, " bringup ap [chan] [secs] (Stage B: beacon + RX, probe/auth/assoc)\n"); fprintf(stderr, " bringup [sweep|coding|vht] [chan] [count] [bw 0=20 1=40 2=80]\n"); fprintf(stderr, " the witness must listen at the same width (DEVOURER_BW=40|80)\n"); rc = 2; diff --git a/src/mt7612u/tx.cpp b/src/mt7612u/tx.cpp index d3b3a598..3f30c7b8 100644 --- a/src/mt7612u/tx.cpp +++ b/src/mt7612u/tx.cpp @@ -135,6 +135,10 @@ int mt_tx_build(struct mt7612u_dev *d, uint8_t *buf, size_t bufsz, if (opts & MT_TXOPT_AMPDU) fl |= MT_TXWI_FLAGS_AMPDU | FIELD_PREP(MT_TXWI_FLAGS_MPDU_DENSITY, 4); + /* Beacon/probe-resp: the MAC fills the TSF timestamp field, as + * mt76x02_mac_write_txwi() does for these subtypes. */ + if (opts & MT_TXOPT_BEACON) + fl |= MT_TXWI_FLAGS_TS; put_le16(txwi + 0, fl); } /* "A frame may narrow below the channel but never widen it" was only @@ -162,6 +166,10 @@ int mt_tx_build(struct mt7612u_dev *d, uint8_t *buf, size_t bufsz, /* ack_ctl bit0 REQ: set it only when an ACK is wanted. Leaving it * clear is how a frame becomes no-ACK, per packet. */ txwi[4] = rate->no_ack ? 0 : MT_TXWI_ACK_CTL_REQ; + /* Beacon: let the MAC assign the 802.11 sequence number (mt76 sets this + * for IEEE80211_TX_CTL_ASSIGN_SEQ frames), so each beacon airs seq+1. */ + if (opts & MT_TXOPT_BEACON) + txwi[4] |= MT_TXWI_ACK_CTL_NSEQ; if (opts & MT_TXOPT_AMPDU) txwi[4] |= FIELD_PREP(MT_TXWI_ACK_CTL_BA_WINDOW, 63); txwi[5] = wcid; /* 0xff = none */ From e2efaedd714baac7a82151270c69272b71186736 Mon Sep 17 00:00:00 2001 From: snokvist Date: Thu, 10 Sep 2026 20:50:15 +0200 Subject: [PATCH 03/13] mt7612u: put the beacon plane behind IRadio, so devourer can be the AP The AP work was device-verified as bring-up gates driving the C library directly. Nothing reached it through IRadio, so pointing devourer's own AP harnesses at a MediaTek adapter beaconed nothing: StartBeacon fell through to the base class's `return false`. That was the whole gap. tests/ap_responder.cpp and tests/ap_wpa2.cpp already take an IRadio* - #415 made them genuinely backend-agnostic - and SetAckResponder, ReadTsf and WriteTsf were already implemented here. What was missing was three methods over primitives that were already proven on air. Three public entry points (31 now, api_link updated), because the backend is meant to reach the subtree through the public header and not through internal.h: mt7612u_beacon_start radiotap-framed or bare MPDU; publishes addr3 in APC slot 0, loads the reserved page, arms TBTT mt7612u_beacon_update reload in place, engine untouched mt7612u_beacon_stop clear the timer bits Two configurations are REFUSED rather than half-served, because each airs a beacon that scans perfectly and then ACKs nothing - the operator ends up debugging the RF link instead of the configuration: - a BSSID that is not the adapter's own MAC. The MAC ACKs against MT_MAC_ADDR and this call does not retarget it. - a locally-administered adapter MAC. Under MBSS_MODE=3 the hardware derives the BSS index from the address bits (mt76: 1 + n), so slot 0 is the wrong slot and the match would silently never fire. The Stage B gate already refused this; now the library does. Stop() silences the beacon before it releases the device. The MAC beacons autonomously once armed, so a beacon outliving this object airs until the adapter is power-cycled and contaminates the next run on that channel. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj --- src/mt7612u/Mt7612uRadio.cpp | 58 ++++++++++++++++ src/mt7612u/Mt7612uRadio.h | 9 +++ src/mt7612u/beacon.cpp | 97 +++++++++++++++++++++++++++ src/mt7612u/include/mt7612u/mt7612u.h | 39 +++++++++++ src/mt7612u/tests/api_link.c | 3 + 5 files changed, 206 insertions(+) diff --git a/src/mt7612u/Mt7612uRadio.cpp b/src/mt7612u/Mt7612uRadio.cpp index 4e3cd222..8fa266ae 100644 --- a/src/mt7612u/Mt7612uRadio.cpp +++ b/src/mt7612u/Mt7612uRadio.cpp @@ -634,6 +634,15 @@ void Mt7612uRadio::Stop() { StopRxLoop(); } catch (...) { } + /* Before anything else lets go of the device: the MAC beacons AUTONOMOUSLY + * once armed, so a beacon that outlives this object keeps airing until the + * adapter is power-cycled and contaminates whatever runs next on that + * channel. Bench-bitten on the Realtek side, and the bring-up gate silences + * its beacon on every exit path for the same reason. */ + try { + StopBeacon(); + } catch (...) { + } stop_tick(); /* joins; must not run with _mu held */ /* Take the device out under _mu, then close it OUTSIDE - mt7612u_close() @@ -841,6 +850,55 @@ void Mt7612uRadio::ClearAckResponder() { mt7612u_clear_ack_responder(_dev); } +/* The beacon plane. Thin on purpose: the sequence these wrap is the one the + * bring-up harness's Stage A and Stage B gates run, device-verified on + * 2026-09-08 - beacon on air on both bands, hardware TSF and sequence, and a + * real station's auth arriving at retry=0, which is the auto-ACK. Putting it + * behind IRadio is what lets devourer's existing backend-agnostic AP + * harnesses (tests/ap_responder.cpp, tests/ap_wpa2.cpp - both already take an + * IRadio*) drive this part with no MediaTek-specific code in them. + * + * Under _mu with the rest of the control plane: every one of these is a + * register write, and the 1 Hz tick is issuing MCU traffic on its own thread. */ +bool Mt7612uRadio::StartBeacon(const uint8_t *beacon, size_t len, + int interval_tu) { + std::lock_guard lock(_mu); + if (!_dev || !beacon || len == 0 || interval_tu <= 0) + return false; + if (mt7612u_beacon_start(_dev, beacon, len, + static_cast(interval_tu)) != 0) + return false; + _beacon_active = true; + _logger->info("MT7612U beaconing every {} TU", interval_tu); + return true; +} + +bool Mt7612uRadio::UpdateBeaconPayload(const uint8_t *beacon, size_t len) { + std::lock_guard lock(_mu); + /* "Requires an active StartBeacon; returns false otherwise" - and without + * the guard this would load a beacon into a disarmed engine and report + * success for something that never airs. */ + if (!_dev || !_beacon_active || !beacon || len == 0) + return false; + return mt7612u_beacon_update(_dev, beacon, len) == 0; +} + +bool Mt7612uRadio::StopBeacon() { + std::lock_guard lock(_mu); + if (!_dev || !_beacon_active) + return false; + const bool ok = mt7612u_beacon_stop(_dev) == 0; + /* Cleared either way. A failed stop leaves the MAC beaconing and there is + * nothing further this object can do about it, but reporting the beacon as + * still ours would make the destructor try again on a device that is about + * to be closed. */ + _beacon_active = false; + if (!ok) + _logger->error("MT7612U beacon stop failed - the MAC may still be airing " + "it; a power-cycle is the only certain silence"); + return ok; +} + /* The absolute dBm the actuator should carry: the base plus whatever offset is * live, clamped to the part's 0-30 range. One place, so the base setter, the * offset setter and the bring-up replay cannot drift apart. */ diff --git a/src/mt7612u/Mt7612uRadio.h b/src/mt7612u/Mt7612uRadio.h index 1e25b917..e5d5c92f 100644 --- a/src/mt7612u/Mt7612uRadio.h +++ b/src/mt7612u/Mt7612uRadio.h @@ -100,6 +100,9 @@ class Mt7612uRadio : public IRadio { void WriteTsf(uint64_t tsf) override; devourer::TxStats GetTxStats() override; bool SetAckResponder(const devourer::MacAddr &mac) override; + bool StartBeacon(const uint8_t *beacon, size_t len, int interval_tu) override; + bool UpdateBeaconPayload(const uint8_t *beacon, size_t len) override; + bool StopBeacon() override; void ClearAckResponder() override; private: @@ -160,6 +163,12 @@ class Mt7612uRadio : public IRadio { std::condition_variable _tick_cv; bool _tick_stop = false; + /* StopBeacon/UpdateBeaconPayload are documented to return false when no + * beacon is active, and the MAC keeps beaconing after the host process dies, + * so the destructor needs to know too. Guarded by _mu like the rest of the + * control plane. */ + bool _beacon_active = false; + int _txpwr_dbm = 20; /* the absolute dBm limit mt7612u_set_txpower takes */ int _txpwr_offset_qdb = 0; /* sticky, folded onto _txpwr_dbm */ }; diff --git a/src/mt7612u/beacon.cpp b/src/mt7612u/beacon.cpp index 12bd5f16..93c3f0b8 100644 --- a/src/mt7612u/beacon.cpp +++ b/src/mt7612u/beacon.cpp @@ -15,6 +15,7 @@ * Ported from mt76/mt76x02_beacon.c and mt76x02_usb_core.c @ be5ce79. * Copyright (C) 2016 Felix Fietkau, (C) 2018 Lorenzo Bianconi / Stanislaw Gruszka. */ +#include #include "internal.h" /* mt76x02u: 5 USB beacon slots, each (8192 / 5) & ~63 = 1600 bytes. The 8 kB @@ -146,3 +147,99 @@ int mt_beacon_set_enable(struct mt7612u_dev *d, int on, unsigned interval_tu) } return 0; } + +/* --- public ABI --------------------------------------------------------- + * + * The three calls devourer's IRadio beacon surface maps onto. Everything they + * do is the sequence bringup's Stage A and Stage B gates run and that was + * device-verified on 2026-09-08 (docs/mt7612u-ap-mode.md); this is that + * sequence behind the public header, so a consumer does not have to reach into + * internal.h to be an AP. + */ + +/* Split a radiotap-framed buffer into rate + MPDU, exactly as + * mt7612u_send_packet() does. A bare MPDU (no radiotap) is not an error here - + * IRadio's contract strips the header "if present" - and takes the rate a + * beacon wants: OFDM 6 Mbps, the basic rate every station must decode. */ +static int beacon_split(const void *buf, size_t len, const uint8_t **mpdu, + size_t *mpdu_len, struct mt7612u_tx_rate *r) +{ + const uint8_t *p = (const uint8_t *)buf; + int rlen; + + if (!p || len == 0) return -1; + + rlen = mt_radiotap_parse(p, len, r); + if (rlen > 0 && (size_t)rlen < len) { + *mpdu = p + rlen; + *mpdu_len = len - (size_t)rlen; + } else { + r->phy = MT7612U_PHY_OFDM; + r->mcs = 0; + r->nss = 1; + r->bw = MT7612U_BW_20; + r->no_ack = 1; /* a broadcast beacon is never ACKed */ + *mpdu = p; + *mpdu_len = len; + } + /* addr3 lives at offset 16, so anything shorter has no BSSID to publish + * and is not a beacon whatever else it is. */ + if (*mpdu_len < 24) { + ERR("beacon: %zu B is too short for an 802.11 header", *mpdu_len); + return -1; + } + return 0; +} + +int mt7612u_beacon_start(struct mt7612u_dev *dev, const void *buf, size_t len, + unsigned interval_tu) +{ + struct mt7612u_tx_rate rate; + const uint8_t *mpdu = NULL; + size_t mpdu_len = 0; + + if (!dev) return -1; + if (beacon_split(buf, len, &mpdu, &mpdu_len, &rate)) return -1; + + /* Both refusals are in the header's contract. They are refusals rather + * than warnings because each one airs a beacon that looks perfect on a + * scan and then ACKs nothing - the operator debugs the RF link instead of + * the configuration. */ + if (dev->macaddr[0] & 0x02) { + ERR("beacon: adapter MAC %02x:.. is locally administered; APC slot 0 " + "is not the slot that MAC selects (mt76 derives 1+n)", + dev->macaddr[0]); + return -1; + } + if (memcmp(mpdu + 16, dev->macaddr, 6) != 0) { + ERR("beacon: BSSID %02x:%02x:%02x:%02x:%02x:%02x is not the adapter's " + "own MAC; the MAC ACKs against MT_MAC_ADDR and this call does not " + "retarget it, so that BSS would answer nothing", + mpdu[16], mpdu[17], mpdu[18], mpdu[19], mpdu[20], mpdu[21]); + return -1; + } + + if (mt_ap_set_bssid(dev, 0, dev->macaddr)) return -1; + mt_beacon_init(dev); + if (mt_beacon_write(dev, mpdu, mpdu_len, &rate)) return -1; + return mt_beacon_set_enable(dev, 1, interval_tu); +} + +int mt7612u_beacon_update(struct mt7612u_dev *dev, const void *buf, size_t len) +{ + struct mt7612u_tx_rate rate; + const uint8_t *mpdu = NULL; + size_t mpdu_len = 0; + + if (!dev) return -1; + if (beacon_split(buf, len, &mpdu, &mpdu_len, &rate)) return -1; + /* No mt_beacon_init() and no set_enable(): the engine is already armed and + * re-initialising it would re-suppress every slot mid-flight. */ + return mt_beacon_write(dev, mpdu, mpdu_len, &rate); +} + +int mt7612u_beacon_stop(struct mt7612u_dev *dev) +{ + if (!dev) return -1; + return mt_beacon_set_enable(dev, 0, 0); +} diff --git a/src/mt7612u/include/mt7612u/mt7612u.h b/src/mt7612u/include/mt7612u/mt7612u.h index a99165e0..2366525f 100644 --- a/src/mt7612u/include/mt7612u/mt7612u.h +++ b/src/mt7612u/include/mt7612u/mt7612u.h @@ -279,6 +279,45 @@ size_t mt7612u_send_packets(struct mt7612u_dev *dev, int mt7612u_set_ack_responder(struct mt7612u_dev *dev, const uint8_t mac[6]); void mt7612u_clear_ack_responder(struct mt7612u_dev *dev); +/* + * Hardware beacon, from the MAC's reserved page. + * + * mt7612u_beacon_start() loads the beacon and arms the TBTT timer; the MAC + * then transmits it on its own at every TBTT, stamping the live 64-bit TSF + * into the timestamp field and assigning the 802.11 sequence number. There is + * no host involvement per beacon and no host jitter. `buf` is one + * radiotap-framed MPDU, the same contract as mt7612u_send_packet(); a bare + * MPDU with no radiotap header is accepted too and airs at OFDM 6 Mbps, the + * rate a beacon wants. + * + * The BSSID is taken from addr3 and published in APC slot 0, which is what + * makes the MAC match - and therefore auto-ACK - frames addressed to the BSS. + * Two configurations are REFUSED rather than half-served, because both air a + * beacon that no station can associate to: + * + * - addr3 different from the adapter's own MAC. The port identity the MAC + * ACKs against is MT_MAC_ADDR, and this call does not retarget it; a BSSID + * that disagrees with it beacons fine and ACKs nothing. + * - a locally-administered adapter MAC (bit 1 of byte 0). Under MBSS_MODE=3 + * the hardware derives the BSS index from the address bits and mt76 uses + * 1 + (((macaddr[0] ^ addr[0]) >> 2) & 7), so slot 0 is the wrong slot and + * the match would silently never fire. + * + * mt7612u_beacon_update() replaces the loaded beacon in place; the interval, + * TBTT phase and BSSID are untouched. The swap is not atomic against TBTT - a + * beacon airing during the write may still carry the previous content. + * + * mt7612u_beacon_stop() clears the timer bits. It matters: the MAC beacons + * AUTONOMOUSLY once armed, so killing the host process does NOT silence it, + * and a beacon left airing contaminates whatever runs next on that channel. + * + * All three return 0 on success, negative on failure. + */ +int mt7612u_beacon_start(struct mt7612u_dev *dev, const void *buf, size_t len, + unsigned interval_tu); +int mt7612u_beacon_update(struct mt7612u_dev *dev, const void *buf, size_t len); +int mt7612u_beacon_stop(struct mt7612u_dev *dev); + /* * TX/RX counters from the async rings. Zeroed when no ring is running, and * taken under the ring's own lock - reading the fields directly would race diff --git a/src/mt7612u/tests/api_link.c b/src/mt7612u/tests/api_link.c index f6bf5d8e..42cf8572 100644 --- a/src/mt7612u/tests/api_link.c +++ b/src/mt7612u/tests/api_link.c @@ -34,6 +34,9 @@ static void *const api[] = { (void *)mt7612u_send_packets, (void *)mt7612u_set_ack_responder, (void *)mt7612u_clear_ack_responder, + (void *)mt7612u_beacon_start, + (void *)mt7612u_beacon_update, + (void *)mt7612u_beacon_stop, (void *)mt7612u_get_stats, (void *)mt7612u_link_stats_start, (void *)mt7612u_link_stats, From 2e78a5877952950a49e8dcee8464e1fa1050ecd9 Mon Sep 17 00:00:00 2001 From: snokvist Date: Thu, 10 Sep 2026 21:06:25 +0200 Subject: [PATCH 04/13] mt7612u: the beacon plane's review round - eight defects, one reproduced Review of the previous commit found a genuine correctness bug in the new public ABI and seven smaller ones. The reproduced one first: BARE-MPDU BEACONS AIRED WITH AN UNINITIALIZED RATE WORD. beacon_split() assigned five of mt7612u_tx_rate's nine fields; sgi, ldpc, stbc and power_adj were left indeterminate. All four are read downstream - mt_tx_rate_word() folds the first three into the 16-bit word the MAC transmits VERBATIM, and a non-zero power_adj short-circuits the derived per-rate TX power in mt_tx_build(). tests/ap_responder.cpp hands us a bare MPDU, so this was the live path, and my own hardware run went out with whatever was on the stack. The radiotap branch escaped only because mt_radiotap_parse() memsets its output. Zero-initialised now. - mt_radiotap_parse() has THREE return classes and beacon_split saw two: a NEGATIVE return means "this IS a radiotap header and it is malformed". Swallowing it parsed the radiotap bytes as an 802.11 header and read the BSSID out of the middle of it. Both sibling entry points refuse on <= 0; so does this now. - no_ack is forced, not taken from the caller's radiotap. Cleared, it becomes MT_TXWI_ACK_CTL_REQ - an ACK request on a broadcast beacon. Both bring-up gates hardcode it; the ABI was the weaker path. - the header length is checked. mt_beacon_write() documents that it relies on an unpadded 24-byte header; a QoS-data or 4-address frame passed every check and would have landed in the reserved page as [TXWI][hdr][2 pad][body]. - _beacon_active was not failure-atomic. beacon_start() disarms the engine before it re-arms, so a failed RE-arm left the beacon dead while the flag still reported the previous one live - which is how UpdateBeaconPayload came to return true for every write into a disarmed engine, the exact fault its guard exists to prevent. - StopBeacon() cleared _beacon_active on FAILURE, so a caller retrying after a transient stall was told "no beacon was active" and walked away from one the MAC was still airing. It stays active now. - the bypass-mask unsuppress is mt_wr_chk, not mt_wr. That single write decides whether the beacon airs at all, and mt_ap_set_bssid twenty lines below already argues why USB writes get checked and MMIO ones do not. - hw_beacon_txtsf said false next to the function that implements it. The branch's own evidence is 102400 us per beacon. - a memset over mt7612u_dev survived in frame_shape.cpp - added by this branch, missed by the migration commit that claimed to have fixed it. Invisible to CI, which never compiles src/mt7612u/tests/. StopBeacon now retracts the WHOLE identity rather than half of it: the APC BSSID slots are zeroed and, when beacon_start was what retargeted the port MAC, that is restored too. It tracks who took it (beacon_took_identity) so it cannot disarm an ACK responder the caller owns. The bring-up gate already did this; the public path was weaker than the gate it claims to reproduce. Re-verified on hardware after the changes: beacon on air, a real station associated, AUTH and ASSOC both at retry=0, 6/6 pings at 1.1 ms. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj --- src/mt7612u/Mt7612uRadio.cpp | 33 ++++-- src/mt7612u/beacon.cpp | 143 ++++++++++++++++++++++---- src/mt7612u/include/mt7612u/mt7612u.h | 26 +++-- src/mt7612u/internal.h | 5 + src/mt7612u/tests/frame_shape.cpp | 3 +- 5 files changed, 165 insertions(+), 45 deletions(-) diff --git a/src/mt7612u/Mt7612uRadio.cpp b/src/mt7612u/Mt7612uRadio.cpp index 8fa266ae..da59a48e 100644 --- a/src/mt7612u/Mt7612uRadio.cpp +++ b/src/mt7612u/Mt7612uRadio.cpp @@ -865,6 +865,13 @@ bool Mt7612uRadio::StartBeacon(const uint8_t *beacon, size_t len, std::lock_guard lock(_mu); if (!_dev || !beacon || len == 0 || interval_tu <= 0) return false; + /* Cleared BEFORE the call, not just set after it. mt7612u_beacon_start() + * runs mt_beacon_init(), which disarms the timer and suppresses every slot, + * so a failure after that point leaves the beacon dead - and a re-arm that + * fails (an over-long beacon body, say) would otherwise keep reporting the + * PREVIOUS arm as live, which is how UpdateBeaconPayload comes to return + * true for every write into a disarmed engine. */ + _beacon_active = false; if (mt7612u_beacon_start(_dev, beacon, len, static_cast(interval_tu)) != 0) return false; @@ -887,16 +894,18 @@ bool Mt7612uRadio::StopBeacon() { std::lock_guard lock(_mu); if (!_dev || !_beacon_active) return false; - const bool ok = mt7612u_beacon_stop(_dev) == 0; - /* Cleared either way. A failed stop leaves the MAC beaconing and there is - * nothing further this object can do about it, but reporting the beacon as - * still ours would make the destructor try again on a device that is about - * to be closed. */ + if (mt7612u_beacon_stop(_dev) != 0) { + /* Deliberately still active. A caller retrying after a transient USB + * stall must not be told "already stopped" - false means "no beacon was + * active" in this interface, and reading a failed stop as that walks away + * from a beacon the MAC is still airing. Stop() calls this inside a + * try/catch, so a retry there costs nothing. */ + _logger->error("MT7612U beacon stop FAILED - the MAC is still airing it; " + "retry, or power-cycle the adapter"); + return false; + } _beacon_active = false; - if (!ok) - _logger->error("MT7612U beacon stop failed - the MAC may still be airing " - "it; a power-cycle is the only certain silence"); - return ok; + return true; } /* The absolute dBm the actuator should carry: the base plus whatever offset is @@ -1005,7 +1014,11 @@ devourer::AdapterCaps Mt7612uRadio::GetAdapterCaps() { c.ldpc_rx_flag = true; /* the RXWI carries the per-frame LDPC bit */ c.per_chain_rssi = true; c.hw_rx_timestamp = false; /* the RXWI TSF field is not parsed */ - c.hw_beacon_txtsf = false; /* no hardware beacon function ported */ + /* The MAC inserts the live 64-bit TSF into the beacon it auto-transmits; + * measured at 102400 us per beacon, exactly 100 TU (docs/mt7612u-ap-mode.md). + * True since the beacon plane landed - it read false while the function it + * describes sat three hundred lines above. */ + c.hw_beacon_txtsf = true; /* Measured on air: 0 frames at the stimulus radio unarmed, 3500+ armed. */ c.ack_responder_ok = true; /* Unmeasured, so false rather than optimistic - nothing here drives the diff --git a/src/mt7612u/beacon.cpp b/src/mt7612u/beacon.cpp index 93c3f0b8..78bfdf79 100644 --- a/src/mt7612u/beacon.cpp +++ b/src/mt7612u/beacon.cpp @@ -53,7 +53,7 @@ void mt_beacon_init(struct mt7612u_dev *d) MT_BEACON_TIME_CFG_TIMER_EN | MT_BEACON_TIME_CFG_TBTT_EN | MT_BEACON_TIME_CFG_BEACON_TX); mt_set(d, MT_BEACON_TIME_CFG, MT_BEACON_TIME_CFG_SYNC_MODE); - mt_wr(d, MT_BCN_BYPASS_MASK, 0xffff); + mt_wr(d, MT_BCN_BYPASS_MASK, 0xffff); /* suppress all while we set up */ mt_beacon_set_offsets(d); } @@ -92,9 +92,14 @@ int mt_beacon_write(struct mt7612u_dev *d, const void *frame, size_t len, * beacon_data_count) in mt76x02u_pre_tbtt_work() - and the static path * writes exactly one (slot 0), so N = 1. Without this the beacon never * airs even though the TSF and beacon timer run. + * + * Checked, unlike mt_wr(): this single write decides whether the beacon + * airs at all, and mt76 can leave it unchecked because MMIO cannot fail + * while USB can - the same argument mt_ap_set_bssid() makes below. A + * silent failure here is an AP that beacons nothing while every other + * step reports success. */ - mt_wr(d, MT_BCN_BYPASS_MASK, 0xff00u | ~(0xff00u >> 1)); - return 0; + return mt_wr_chk(d, MT_BCN_BYPASS_MASK, 0xff00u | ~(0xff00u >> 1)); } /* @@ -169,25 +174,60 @@ static int beacon_split(const void *buf, size_t len, const uint8_t **mpdu, if (!p || len == 0) return -1; + /* ZEROED FIRST. The bare-MPDU branch below sets five of this struct's + * nine fields, and sgi/ldpc/stbc go straight into the 16-bit rate word + * the MAC transmits verbatim, while power_adj short-circuits the derived + * per-rate TX power in mt_tx_build(). Left indeterminate, a bare-MPDU + * beacon - which is what tests/ap_responder.cpp hands us - airs with + * whatever was on the stack. The radiotap branch only escaped this + * because mt_radiotap_parse() memsets its output. */ + *r = mt7612u_tx_rate{}; + rlen = mt_radiotap_parse(p, len, r); + /* Three return classes, not two: <0 means "this IS a radiotap header and + * it is malformed". Treating that as a bare MPDU would parse the radiotap + * bytes as an 802.11 header and read the BSSID out of the middle of it. + * mt7612u_send_packet() refuses on <= 0; so does this. */ + if (rlen < 0) { + ERR("beacon: malformed radiotap header"); + return -1; + } if (rlen > 0 && (size_t)rlen < len) { *mpdu = p + rlen; *mpdu_len = len - (size_t)rlen; } else { + /* rlen == 0 (no radiotap) or rlen == len (a header with no frame + * after it): treat the buffer as a bare MPDU. OFDM 6 Mbps is the + * basic rate every station must decode, which is what a beacon wants. */ r->phy = MT7612U_PHY_OFDM; r->mcs = 0; r->nss = 1; r->bw = MT7612U_BW_20; - r->no_ack = 1; /* a broadcast beacon is never ACKed */ *mpdu = p; *mpdu_len = len; } - /* addr3 lives at offset 16, so anything shorter has no BSSID to publish - * and is not a beacon whatever else it is. */ + /* Unconditionally, whatever the caller's radiotap said: a beacon is + * broadcast, and mt_tx_build() turns a cleared no_ack into + * MT_TXWI_ACK_CTL_REQ - an ACK request on a frame no one may ACK. Both + * bring-up gates hardcode this; the ABI must not be weaker. */ + r->no_ack = 1; + + /* addr3 lives at offset 16, so anything shorter has no BSSID to publish. */ if (*mpdu_len < 24) { ERR("beacon: %zu B is too short for an 802.11 header", *mpdu_len); return -1; } + /* mt_beacon_write() documents that it relies on a 24-byte, 4-aligned + * header so mt_tx_build() inserts no interior L2 pad. A QoS-data or + * 4-address frame (26 or 30) passes every check above and would land in + * the reserved page as [TXWI][hdr][2 pad][body] - a layout that function + * is written not to expect. Enforce what it assumes. */ + if (mt_hdrlen_from_fc(*mpdu) != 24) { + ERR("beacon: header is %d B, not the 24 a beacon has - the reserved " + "page needs an unpadded [TXWI][MPDU]", + mt_hdrlen_from_fc(*mpdu)); + return -1; + } return 0; } @@ -197,29 +237,61 @@ int mt7612u_beacon_start(struct mt7612u_dev *dev, const void *buf, size_t len, struct mt7612u_tx_rate rate; const uint8_t *mpdu = NULL; size_t mpdu_len = 0; + const uint8_t *ta, *bssid; + uint8_t idx; if (!dev) return -1; if (beacon_split(buf, len, &mpdu, &mpdu_len, &rate)) return -1; - /* Both refusals are in the header's contract. They are refusals rather - * than warnings because each one airs a beacon that looks perfect on a - * scan and then ACKs nothing - the operator debugs the RF link instead of - * the configuration. */ - if (dev->macaddr[0] & 0x02) { - ERR("beacon: adapter MAC %02x:.. is locally administered; APC slot 0 " - "is not the slot that MAC selects (mt76 derives 1+n)", - dev->macaddr[0]); + ta = mpdu + 10; /* addr2 - the transmitter, i.e. the port identity */ + bssid = mpdu + 16; /* addr3 */ + + if (ta[0] & 0x01) { + ERR("beacon addr2 must be unicast; a station cannot unicast-auth to " + "a multicast BSSID"); return -1; } - if (memcmp(mpdu + 16, dev->macaddr, 6) != 0) { - ERR("beacon: BSSID %02x:%02x:%02x:%02x:%02x:%02x is not the adapter's " - "own MAC; the MAC ACKs against MT_MAC_ADDR and this call does not " - "retarget it, so that BSS would answer nothing", - mpdu[16], mpdu[17], mpdu[18], mpdu[19], mpdu[20], mpdu[21]); - return -1; + + /* + * mt76x02_add_interface(): the port identity FOLLOWS the interface + * address. IRadio says the same thing - "addr2/addr3 set the port + * MAC/BSSID" - and devourer's AP harnesses rely on it ("MACID = BSSID, + * set by StartBeacon", tests/ap_responder.cpp). Without this the MAC + * would keep ACKing for the adapter's factory MAC while beaconing a + * different BSSID, so a station's auth is never acknowledged and it + * retries until it gives up. + * + * mt7612u_set_ack_responder() is that register write, and it saves the + * factory identity so ClearAckResponder() can put it back. The two share + * one identity and one save slot by construction - there is only one + * MT_MAC_ADDR on this part - so a caller that arms a responder AND + * beacons is setting the same thing twice, and the restore is whichever + * of the two runs last. + */ + if (memcmp(ta, dev->macaddr, 6) != 0) { + /* Only claim the identity if nobody else already holds it. When a + * caller armed an ACK responder first, the saved factory MAC is + * theirs and restoring it on beacon stop would silently disarm them. */ + const int was_taken = dev->ack_saved; + if (mt7612u_set_ack_responder(dev, ta)) + return -1; + if (!was_taken) + dev->beacon_took_identity = 1; } - if (mt_ap_set_bssid(dev, 0, dev->macaddr)) return -1; + /* + * The APC slot the hardware will match this BSS in. Under MBSS_MODE=3 the + * index comes from the address bits, and mt76 computes + * idx = 1 + (((macaddr[0] ^ addr[0]) >> 2) & 7) + * for a locally-administered address, 0 otherwise (mt76x02_util.c:310). + * It runs that AFTER retargeting the identity, so macaddr == addr and the + * XOR is zero: the expression collapses to 1. Getting this wrong is + * silent - slot 0 for an 02:/06:/0a: BSSID matches nothing, and the AP + * beacons perfectly while acknowledging nobody. + */ + idx = (ta[0] & 0x02) ? 1 : 0; + + if (mt_ap_set_bssid(dev, idx, bssid)) return -1; mt_beacon_init(dev); if (mt_beacon_write(dev, mpdu, mpdu_len, &rate)) return -1; return mt_beacon_set_enable(dev, 1, interval_tu); @@ -240,6 +312,33 @@ int mt7612u_beacon_update(struct mt7612u_dev *dev, const void *buf, size_t len) int mt7612u_beacon_stop(struct mt7612u_dev *dev) { + static const uint8_t zero[6] = { 0 }; + int rc; + if (!dev) return -1; - return mt_beacon_set_enable(dev, 0, 0); + + rc = mt_beacon_set_enable(dev, 0, 0); + + /* + * Retract the WHOLE identity, not just the timer. Leaving the APC slot + * programmed means the MAC keeps matching and auto-ACKing for a BSS that + * no longer exists, so a session that stops beaconing and carries on as an + * injector or a monitor drags that residue with it. The bring-up gate + * already zeroes the slot on every exit path for exactly this reason; the + * public path was the weaker of the two. + * + * Slot 1 and slot 0 are both cleared because beacon_start picks between + * them by the address's locally-administered bit, and stop does not have + * the beacon any more to re-derive which one it used. + */ + mt_ap_set_bssid(dev, 0, zero); + mt_ap_set_bssid(dev, 1, zero); + + /* And the port MAC, if this call's opposite number was what retargeted + * it. mt7612u_clear_ack_responder() is the restore. */ + if (dev->beacon_took_identity) { + mt7612u_clear_ack_responder(dev); + dev->beacon_took_identity = 0; + } + return rc; } diff --git a/src/mt7612u/include/mt7612u/mt7612u.h b/src/mt7612u/include/mt7612u/mt7612u.h index 2366525f..ba17374f 100644 --- a/src/mt7612u/include/mt7612u/mt7612u.h +++ b/src/mt7612u/include/mt7612u/mt7612u.h @@ -290,18 +290,22 @@ void mt7612u_clear_ack_responder(struct mt7612u_dev *dev); * MPDU with no radiotap header is accepted too and airs at OFDM 6 Mbps, the * rate a beacon wants. * - * The BSSID is taken from addr3 and published in APC slot 0, which is what - * makes the MAC match - and therefore auto-ACK - frames addressed to the BSS. - * Two configurations are REFUSED rather than half-served, because both air a - * beacon that no station can associate to: + * addr2 becomes the MAC's port identity and addr3 is published in the APC slot + * the hardware will match that BSS in - together, those two ARE "being an AP" + * on this part; there is no AP op-mode register and mt76 sets none either. + * Retargeting the identity is required, not a convenience: without it the MAC + * keeps ACKing for the adapter's factory MAC while beaconing a different + * BSSID, so a station's auth is never acknowledged and it retries until it + * gives up. The slot index follows mt76 - 1 for a locally-administered + * address, 0 otherwise - because under MBSS_MODE=3 the hardware derives it + * from the address bits, and slot 0 for an 02:/06:/0a: BSSID matches nothing. * - * - addr3 different from the adapter's own MAC. The port identity the MAC - * ACKs against is MT_MAC_ADDR, and this call does not retarget it; a BSSID - * that disagrees with it beacons fine and ACKs nothing. - * - a locally-administered adapter MAC (bit 1 of byte 0). Under MBSS_MODE=3 - * the hardware derives the BSS index from the address bits and mt76 uses - * 1 + (((macaddr[0] ^ addr[0]) >> 2) & 7), so slot 0 is the wrong slot and - * the match would silently never fire. + * The identity is one register plane with one saved copy, shared with + * mt7612u_set_ack_responder(): a caller doing both is setting the same thing + * twice, and mt7612u_clear_ack_responder() is what restores the factory MAC. + * mt7612u_beacon_stop() deliberately does NOT restore it - it silences the + * beacon and leaves the identity alone rather than clobbering a responder the + * caller may own. * * mt7612u_beacon_update() replaces the loaded beacon in place; the interval, * TBTT phase and BSSID are untouched. The swap is not atomic against TBTT - a diff --git a/src/mt7612u/internal.h b/src/mt7612u/internal.h index e95056f8..d227ab34 100644 --- a/src/mt7612u/internal.h +++ b/src/mt7612u/internal.h @@ -214,6 +214,11 @@ struct mt7612u_dev { /* Oracle-diff log: every EP0 write we emit, in order. */ uint8_t ack_saved_mac[6]; int ack_saved; + /* Set when mt7612u_beacon_start() was the one that retargeted the port + * identity, so mt7612u_beacon_stop() restores it - and does NOT when a + * caller had already armed an ACK responder, because then the identity is + * theirs and restoring would silently disarm it. */ + int beacon_took_identity; struct mt_async *a; FILE *wrlog; FILE *mculog; diff --git a/src/mt7612u/tests/frame_shape.cpp b/src/mt7612u/tests/frame_shape.cpp index 4a869807..911f0047 100644 --- a/src/mt7612u/tests/frame_shape.cpp +++ b/src/mt7612u/tests/frame_shape.cpp @@ -501,7 +501,7 @@ static void test_tx_ring_ceiling(void) */ static void test_beacon_txwi(void) { - struct mt7612u_dev d; + struct mt7612u_dev d{}; uint8_t buf[128]; uint8_t beacon[36], data[36]; struct mt7612u_tx_rate bcn_rate = { @@ -516,7 +516,6 @@ static void test_beacon_txwi(void) printf("beacon TXWI (MT_TXOPT_BEACON sets TS + NSEQ):\n"); - memset(&d, 0, sizeof d); d.chainmask = 0x0202; memset(beacon, 0, sizeof beacon); From 543d89ee4dd38210f271189a149f61c0e3896c78 Mon Sep 17 00:00:00 2001 From: snokvist Date: Thu, 10 Sep 2026 21:15:01 +0200 Subject: [PATCH 05/13] mt7612u: a harness that actually tests StopBeacon, because nothing did I reported "the beacon was gone after the process exited" as evidence that StopBeacon works. It was not evidence of anything. Both AP harnesses end in `_exit(0)` (tests/ap_responder.cpp:end), which bypasses every destructor - so the radio's Stop(), and with it StopBeacon(), never ran in any of those runs. The beacon that was "gone" in one run was still airing in another, confirmed live at `last seen: 308 ms ago`, -32 dBm. The MAC beacons autonomously from the reserved page; nothing was silencing it. So: a harness that drives the transitions explicitly and lets an external station be the witness. PHASE 1 armed - scan MUST see the SSID -> seen PHASE 2 stopped - scan MUST NOT -> gone PHASE 3 re-armed - scan MUST see it again -> seen then an explicit stop -> gone `iw scan` alone is not the witness - its BSS cache holds an entry for ~30 s after the beacon dies, which reported a stopped beacon as present. `iw scan flush` is what distinguishes the two, and a phase-3 scan at +8 s still misses it: StartBeacon copies a 1600-byte page over EP0 and reads back the identity, so the re-arm is not instant. Both of those cost a false reading before they were understood, and the file says so. It also holds the two contract points that need no radio: a second StopBeacon returns false (no beacon is active), and UpdateBeaconPayload with nothing armed refuses instead of reporting success for a write into a disarmed engine. Not a ctest cell - it needs an adapter and a second radio to look. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj --- tests/mt7612u_beacon_stop_check.cpp | 154 ++++++++++++++++++++++++++++ 1 file changed, 154 insertions(+) create mode 100644 tests/mt7612u_beacon_stop_check.cpp diff --git a/tests/mt7612u_beacon_stop_check.cpp b/tests/mt7612u_beacon_stop_check.cpp new file mode 100644 index 00000000..4be8cf6c --- /dev/null +++ b/tests/mt7612u_beacon_stop_check.cpp @@ -0,0 +1,154 @@ +/* + * Does StopBeacon actually silence the MAC? + * + * This exists because the obvious way to check it does not check it. Both AP + * harnesses (tests/ap_responder.cpp, tests/ap_wpa2.cpp) end in `_exit(0)`, + * which bypasses every destructor - so the radio's Stop(), and with it + * StopBeacon(), never run, and "the beacon was gone after the process exited" + * measures nothing. On this part that is not a cosmetic difference: the MAC + * beacons AUTONOMOUSLY from the reserved page once armed, so a session that + * skips the teardown leaves it airing until the adapter is power-cycled. + * + * Three phases, each long enough for a station to complete a scan: + * 1. armed - a scan MUST see the SSID + * 2. stopped - StopBeacon(), then a scan MUST NOT + * 3. re-armed - StartBeacon() again, to prove the stop left the engine + * usable rather than wedged + * + * The witness is external (`iw scan` from a station); this program only drives + * the transitions and prints when each phase begins, so the operator or a + * script can scan in the right window. It is not a ctest cell - it needs an + * adapter and a second radio to look. + * + * sudo DEVOURER_VID=0x0e8d DEVOURER_PID=0x7612 DEVOURER_CHANNEL=36 \ + * DEVOURER_MT7612U_FW_DIR= ./beacon_stop_check [phase_secs] + */ +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#include "DeviceConfig.h" +#include "UsbDeviceLock.h" +#include "UsbOpen.h" +#include "WiFiDriver.h" +#include "env_config.h" +#include "logger.h" + +namespace { + +/* BSSID and SSID of our own, so this cannot be confused with a neighbour or + * with ap_responder's devourerAP. Locally administered on purpose: that is the + * case that lands in APC slot 1 rather than 0, and the case the first draft of + * mt7612u_beacon_start() refused outright. */ +const uint8_t kBssid[6] = { 0x02, 0x4d, 0x54, 0x53, 0x54, 0x50 }; +const char kSsid[] = "mtStopCheck"; + +std::vector build_beacon(uint8_t chan) { + std::vector f = { + /* radiotap: 8-byte header + TX flags, the shape ap_responder uses */ + 0x00, 0x00, 0x0a, 0x00, 0x00, 0x80, 0x00, 0x00, 0x08, 0x00, + 0x80, 0x00, 0x00, 0x00, /* FC + duration */ + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, /* addr1 broadcast */ + }; + f.insert(f.end(), kBssid, kBssid + 6); /* addr2 */ + f.insert(f.end(), kBssid, kBssid + 6); /* addr3 */ + const uint8_t tail[] = { + 0x00, 0x00, /* seq (HW assigns) */ + 0, 0, 0, 0, 0, 0, 0, 0, /* timestamp (HW) */ + 0x64, 0x00, /* 100 TU */ + 0x01, 0x00, /* ESS */ + 0x00, (uint8_t)(sizeof kSsid - 1), + }; + f.insert(f.end(), tail, tail + sizeof tail); + f.insert(f.end(), kSsid, kSsid + sizeof kSsid - 1); + const uint8_t ies[] = { + 0x01, 0x08, 0x8c, 0x12, 0x98, 0x24, 0xb0, 0x48, 0x60, 0x6c, + 0x03, 0x01, chan, + }; + f.insert(f.end(), ies, ies + sizeof ies); + return f; +} + +void banner(const char *phase) { + std::printf("\n=== %s === (scan now)\n", phase); + std::fflush(stdout); +} + +} // namespace + +int main(int argc, char **argv) { + const int secs = argc > 1 ? atoi(argv[1]) : 20; + uint8_t chan = 36; + if (const char *c = std::getenv("DEVOURER_CHANNEL")) chan = (uint8_t)atoi(c); + + auto logger = std::make_shared(); + libusb_context *ctx = nullptr; + libusb_init(&ctx); + libusb_set_option(ctx, LIBUSB_OPTION_LOG_LEVEL, LIBUSB_LOG_LEVEL_WARNING); + + uint16_t vid = 0x0e8d, pid = 0x7612; + if (const char *v = std::getenv("DEVOURER_VID")) vid = (uint16_t)strtoul(v, 0, 0); + if (const char *p = std::getenv("DEVOURER_PID")) pid = (uint16_t)strtoul(p, 0, 0); + auto *h = libusb_open_device_with_vid_pid(ctx, vid, pid); + if (!h) { std::fprintf(stderr, "open %04x:%04x fail\n", vid, pid); return 1; } + + std::shared_ptr lk; + if (devourer::claim_interface_then_reset( + h, devourer::find_wifi_interface(h), logger, true, lk) != 0) + return 1; + + WiFiDriver wifi(logger); + auto dev = wifi.CreateRadio(h, ctx, lk, devourer_config_from_env()); + if (!dev) return 1; + dev->InitWrite(SelectedChannel{chan, 0, CHANNEL_WIDTH_20}); + + const auto bcn = build_beacon(chan); + int fails = 0; + + banner("PHASE 1: armed - the SSID mtStopCheck MUST appear"); + if (!dev->StartBeacon(bcn.data(), bcn.size(), 100)) { + std::fprintf(stderr, "FAIL: StartBeacon returned false\n"); + return 1; + } + std::this_thread::sleep_for(std::chrono::seconds(secs)); + + banner("PHASE 2: stopped - the SSID MUST be gone"); + if (!dev->StopBeacon()) { + std::fprintf(stderr, "FAIL: StopBeacon returned false\n"); + fails++; + } + /* Documented to return false the second time: no beacon is active. */ + if (dev->StopBeacon()) { + std::fprintf(stderr, "FAIL: a second StopBeacon returned true\n"); + fails++; + } + /* And an update with nothing armed must refuse rather than report success + * for a write into a disarmed engine. */ + if (dev->UpdateBeaconPayload(bcn.data(), bcn.size())) { + std::fprintf(stderr, "FAIL: UpdateBeaconPayload succeeded with no beacon\n"); + fails++; + } + std::this_thread::sleep_for(std::chrono::seconds(secs)); + + banner("PHASE 3: re-armed - the SSID MUST come back"); + if (!dev->StartBeacon(bcn.data(), bcn.size(), 100)) { + std::fprintf(stderr, "FAIL: StartBeacon after a stop returned false\n"); + fails++; + } + std::this_thread::sleep_for(std::chrono::seconds(secs)); + + /* Explicit, not left to the destructor - the point of this program is that + * the teardown path is the thing under test. */ + dev->StopBeacon(); + std::printf("\nlocal checks: %d failure(s). The SSID phases are the " + "witness's call.\n", fails); + return fails ? 1 : 0; +} From d7835a372ca90d883d3096e9a7e1b3add6a5c22f Mon Sep 17 00:00:00 2001 From: snokvist Date: Thu, 10 Sep 2026 21:15:40 +0200 Subject: [PATCH 06/13] docs(mt7612u-ap): say what is true now - it works, and what still is not The status block said Stages C-E "still need the IRtlDevice wrapper". Stale twice over: the interface was renamed to IRadio in #415, and the wrapper now exists and is verified end to end. Every IRtlDevice mention is fixed, and RtlMt7612uDevice is Mt7612uRadio. Added the IRadio-path evidence as its own section, separate from the bring-up gates' - they are different code and only one of them is what a consumer reaches. With its counterparts: WPA2 unrun, the station is the same silicon, 70 s longest run, near-field. Two of those counterparts cost me a false reading each and are worth the space: `iw scan` caches a BSS for ~30 s so it reports a stopped beacon as present, and neither AP harness silences the beacon on exit because both `_exit(0)` past the destructor. The "gap - the driver primitives to add" section keeps its pre-migration .c filenames. The reasoning there is still correct and rewriting the citations would be churn; the header now says not to expect them to resolve. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj --- docs/mt7612u-ap-mode.md | 70 ++++++++++++++++++++++++++++++++++------- 1 file changed, 59 insertions(+), 11 deletions(-) diff --git a/docs/mt7612u-ap-mode.md b/docs/mt7612u-ap-mode.md index 65ce24a4..37b78c58 100644 --- a/docs/mt7612u-ap-mode.md +++ b/docs/mt7612u-ap-mode.md @@ -1,10 +1,22 @@ -# MT7612U as a fully-userspace access point — what is missing - -**Status: Stages A and B implemented and device-verified** (see "Verified on -hardware" below); Stages C–E (association responder, data plane, WPA2) still -need the `IRtlDevice` wrapper and the existing C++ harnesses. File:line -references are to the merged subtree at `daabab7` (`src/mt7612u/`), the mt76 -reference at `be5ce79`, and devourer's existing AP work. +# MT7612U as a fully-userspace access point + +**Status: it works.** devourer's own AP harnesses run against this part +unmodified — beacon, probe, auth, assoc, and the ARP/ICMP data plane — with a +real Linux station associated. `StartBeacon`, `UpdateBeaconPayload` and +`StopBeacon` are implemented on `Mt7612uRadio`, so nothing in +`tests/ap_responder.cpp` or `tests/ap_wpa2.cpp` knows this is MediaTek. + +What is NOT done: WPA2. `tests/ap_wpa2.cpp` has not been run against this +backend, and the hardware CCMP path (`MT_WCID_KEY`, `MT_SKEY`) is untouched — +the claim below that hardware crypto is a capability *gain* on this part +remains unmeasured. Key install is the one item of the original gap list that +is still open. + +File:line references below are to the merged subtree (`src/mt7612u/`, all +`.cpp` since the C++ migration) and to `reference/mt76 @ be5ce79`. Some of the +citations in "The gap" section still name the pre-migration `.c` filenames and +pre-merge line numbers; they are kept because the reasoning is still correct, +but do not expect them to resolve. ## Verified on hardware (2026-09-08, MT7612U at USB 2-1) @@ -27,6 +39,42 @@ Not yet done: probe **responses**, auth/assoc **responses** and the data plane those are the existing backend-agnostic C++ harnesses' job (Stages C–E), not driver work. +## Verified through IRadio (2026-09-10) — devourer as the AP + +The section above is the bring-up gates driving the C library directly. This +one is devourer itself: `tests/ap_responder.cpp`, unmodified, built against +`libdevourer.a` and pointed at an MT7612U. A second MT7612U on the kernel +`mt76x2u` driver is the station. ch36, `iw reg set SE`. + +| Claim | Evidence | +|---|---| +| `StartBeacon` arms the MAC | `MT7612U beaconing every 100 TU`, then `ap_responder up on ch36 SSID devourerAP (beacon OK)` | +| The beacon is on air and correct | station `iw scan`: `SSID: devourerAP`, `BSS 02:42:75:05:d6:00`, `beacon interval: 100 TUs`, `capability: ESS (0x0001)`, `DS Parameter set: channel 36`, −32 dBm | +| A locally-administered BSSID works | that BSSID is `02:…`, so it lands in APC slot 1 by mt76's rule. The first draft of `mt7612u_beacon_start()` refused it outright | +| A real station associates | `wlx…: connected to 02:42:75:05:d6:00`, `freq: 5180.0` | +| The MAC auto-ACKs | AP side, three runs: `AUTH req … alg=0 seq=1 retry=0` and `ASSOC req … retry=0`. An un-ACKed frame is retransmitted with FC Retry set, so retry=0 IS the ACK | +| The data plane works | `6 packets transmitted, 6 received, 0% packet loss, rtt avg 0.808 ms`; AP side `data(arp/icmp)=8 responses_sent=16` | +| `StopBeacon` silences it | `tests/mt7612u_beacon_stop_check.cpp`: armed → SSID seen; stopped → gone; re-armed → seen again | + +### What this does not show + +- **WPA2 was not run.** Open network only. +- **The station is the same silicon** (MT7612U on `mt76x2u`), so this is not an + independent-generation witness. The RTL8812AU witness in the section above is. +- **One AP, one station, ~20 cm apart.** Every RSSI here is near-field. +- **Longest run 70 s.** No soak, no second station, no rekey, no roaming, and + no channel change while beaconing. +- **`iw scan` alone is not a witness for a beacon *stopping*.** Its BSS cache + holds an entry ~30 s after the beacon dies, and it reported a stopped beacon + as present until `iw scan flush` was used. A re-arm also takes long enough + (a 1600-byte page copy over EP0) that a scan at +8 s still misses it. +- **Neither AP harness silences the beacon on exit.** Both end in `_exit(0)`, + which bypasses the destructor, so `Stop()` and `StopBeacon()` never run and + the MAC keeps beaconing until the adapter is power-cycled. That is why the + StopBeacon evidence above comes from a purpose-built harness and not from + "the SSID was gone after the process exited" — which is what it looked like + once, by luck, and was false. + ## The claim, and why it holds The gap between "MT7612U injector" (what the subtree is) and "MT7612U userspace @@ -37,7 +85,7 @@ all. Two reasons: probe/auth/assoc responder, the DHCP/ARP/ICMP data plane, and the WPA2 4-way handshake with software CCMP all live in `tests/` (`ap_responder.cpp`, `ap_wpa2.cpp`, `probe_responder.cpp`, `beacon_*.cpp`), driven entirely - through the `IRtlDevice` interface — `StartBeacon` + the RX callback + + through the `IRadio` interface — `StartBeacon` + the RX callback + `send_packet`. `docs/ap-mode.md` documents a complete open and WPA2-PSK AP validated against real Linux stations on this stack. None of it is Realtek-specific; it works against any backend that implements the beacon @@ -116,9 +164,9 @@ Each has a direct mt76 recipe. Estimates are the C-library side only. `MT_MAC_BSSID_DW1` now reads `0x003f____`, matching mt76 bit for bit. 2. **`StopBeacon` (~10 LOC).** Clear `BEACON_TX | TBTT_EN | TIMER_EN`. Note the - `IRtlDevice` contract: the chip beacons autonomously, so a session that ends + `IRadio` contract: the chip beacons autonomously, so a session that ends without a power-cycle **must** call this or the beacon contaminates the next - run (`src/IRtlDevice.h:424`). + run (`src/IRadio.h:407-413`). 3. **Per-station / group key install (~50 LOC).** `MT_WCID_KEY(idx)` + `MT_WCID_ATTR` PKEY_MODE/PAIRWISE for pairwise, `MT_SKEY` + `MT_SKEY_MODE` @@ -162,7 +210,7 @@ Each has a direct mt76 recipe. Estimates are the C-library side only. ### The two integration layers - **The mt7612u C library** gains items 1–4 above (~150 LOC, all with recipes). -- **`RtlMt7612uDevice`** (the wrapper from the integration PR) exposes them as +- **`Mt7612uRadio`** (the wrapper from the integration PR) exposes them as `StartBeacon`/`StopBeacon`/`UpdateBeaconPayload`/`SetAckResponder` over the C ABI, so the **existing** C++ AP harnesses in `tests/` run unchanged. No AP logic is written — it already exists. From b084170cec2a4de7453e93be7915eb989ce720a2 Mon Sep 17 00:00:00 2001 From: snokvist Date: Thu, 10 Sep 2026 21:21:20 +0200 Subject: [PATCH 07/13] docs(mt7612u-ap): WPA2-PSK verified on hardware through IRadio tests/ap_wpa2.cpp, unmodified, against an MT7612U: the 4-way handshake completes with a verified msg2 MIC, GTK delivered in msg3, station keyed at msg4 - and 6/6 pings at 1.156 ms over the encrypted link afterwards. It needs the same four IRadio methods as the open-network harness and no others, so nothing further was required of the backend. Two counterparts recorded rather than glossed. The encryption was not independently captured: no third radio sniffed the Protected bit, and wpa_cli had no control socket to report the negotiated cipher, so "encrypted" rests on the verified MIC plus traffic reaching a CCMP-only station. And this is devourer's SOFTWARE CCMP - the same code the Realtek backends use. MT_WCID_KEY / MT_SKEY are still untouched, so the doc's claim that crypto becomes hardware on this part is still a claim. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj --- docs/mt7612u-ap-mode.md | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/docs/mt7612u-ap-mode.md b/docs/mt7612u-ap-mode.md index 37b78c58..ce3ca73e 100644 --- a/docs/mt7612u-ap-mode.md +++ b/docs/mt7612u-ap-mode.md @@ -6,11 +6,17 @@ real Linux station associated. `StartBeacon`, `UpdateBeaconPayload` and `StopBeacon` are implemented on `Mt7612uRadio`, so nothing in `tests/ap_responder.cpp` or `tests/ap_wpa2.cpp` knows this is MediaTek. -What is NOT done: WPA2. `tests/ap_wpa2.cpp` has not been run against this -backend, and the hardware CCMP path (`MT_WCID_KEY`, `MT_SKEY`) is untouched — -the claim below that hardware crypto is a capability *gain* on this part -remains unmeasured. Key install is the one item of the original gap list that -is still open. +**WPA2-PSK works too.** `tests/ap_wpa2.cpp`, also unmodified, completes the +4-way handshake against a real `wpa_supplicant` station and carries encrypted +traffic. It needs the same four `IRadio` methods as the open-network harness — +`InitWrite`, `StartBeacon`, `StartRxLoop`, `send_packet` — and no others, +because CCMP is done in software there. + +What is NOT done: **hardware** CCMP. `MT_WCID_KEY` and `MT_SKEY` are untouched, +so the claim below that hardware crypto is a capability *gain* on this part +remains unmeasured — what is measured is that the software path devourer +already had works here. Key install is the one item of the original gap list +that is still open. File:line references below are to the merged subtree (`src/mt7612u/`, all `.cpp` since the C++ migration) and to `reference/mt76 @ be5ce79`. Some of the @@ -55,10 +61,20 @@ one is devourer itself: `tests/ap_responder.cpp`, unmodified, built against | The MAC auto-ACKs | AP side, three runs: `AUTH req … alg=0 seq=1 retry=0` and `ASSOC req … retry=0`. An un-ACKed frame is retransmitted with FC Retry set, so retry=0 IS the ACK | | The data plane works | `6 packets transmitted, 6 received, 0% packet loss, rtt avg 0.808 ms`; AP side `data(arp/icmp)=8 responses_sent=16` | | `StopBeacon` silences it | `tests/mt7612u_beacon_stop_check.cpp`: armed → SSID seen; stopped → gone; re-armed → seen again | +| WPA2-PSK 4-way completes | AP side: `msg2 OK (SNonce, MIC verified) — PTK derived`, `sent msg3 (GTK, MIC)`, `msg4 OK — 4-WAY HANDSHAKE COMPLETE (station keyed)` against `wpa_supplicant` with `proto=RSN pairwise=CCMP group=CCMP` | +| Encrypted traffic flows | `6 packets transmitted, 6 received, 0% packet loss, rtt avg 1.156 ms` after the handshake — which requires both ends to agree on CCMP | ### What this does not show -- **WPA2 was not run.** Open network only. +- **The encryption was not independently captured.** The 4-way completing with + a verified MIC, and traffic flowing to a CCMP-only station, is strong + evidence that frames are protected — but no third radio sniffed the air to + confirm the Protected bit directly, and `wpa_cli` could not be queried for + the negotiated cipher (the supplicant was started without a control socket). +- **Hardware CCMP is untested.** The 4-way above is devourer's software CCMP, + the same code the Realtek backends use. `MT_WCID_KEY` / `MT_SKEY` are not + wired up, so the "crypto becomes hardware on MediaTek" claim is still a + claim. - **The station is the same silicon** (MT7612U on `mt76x2u`), so this is not an independent-generation witness. The RTL8812AU witness in the section above is. - **One AP, one station, ~20 cm apart.** Every RSSI here is near-field. From cfb2cfbc2c50744295f85500dd7065133f0f5e35 Mon Sep 17 00:00:00 2001 From: snokvist Date: Thu, 10 Sep 2026 21:25:10 +0200 Subject: [PATCH 08/13] tests: silence the beacon before _exit in the AP harnesses ap_responder, ap_wpa2 and ul_trigger_ap all arm a hardware beacon and then end in `_exit(0)`, which skips every destructor - so Stop(), and with it StopBeacon(), never ran. The chip beacons AUTONOMOUSLY once armed, so the beacon outlived the process and kept airing until the adapter was power-cycled, contaminating whatever ran next on that channel. Measured on MT7612U: a scan after the process was gone still found the SSID live at `last seen: 308 ms ago`, -32 dBm. IRadio.h has warned about exactly this since it was written ("killing the host process does NOT silence it - bench-bitten"), and beacon_update_probe.cpp already called StopBeacon before its own `_exit`. These three did not. One line each, before the exit, keeping the fast exit these harnesses want. Realtek has the same exposure - the beacon is hardware-autonomous on those parts too - so this is not a MediaTek fix. Verified on hardware, MT7612U, ch36, station scanning with `iw scan flush`: ap_responder beacon up during the run, 0 after exit (was 1, live) ap_wpa2 beacon up during the run, 0 after exit `iw scan` without `flush` is not a witness here: its BSS cache holds an entry ~30 s after the beacon dies and reports a stopped beacon as present. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj --- tests/ap_responder.cpp | 8 ++++++++ tests/ap_wpa2.cpp | 8 ++++++++ tests/ul_trigger_ap.cpp | 8 ++++++++ 3 files changed, 24 insertions(+) diff --git a/tests/ap_responder.cpp b/tests/ap_responder.cpp index 2bf1cf31..4505d461 100644 --- a/tests/ap_responder.cpp +++ b/tests/ap_responder.cpp @@ -290,5 +290,13 @@ int main(int argc, char** argv) { (unsigned long long)g_probe.load(), (unsigned long long)g_auth.load(), (unsigned long long)g_assoc.load(), (unsigned long long)g_data.load(), (unsigned long long)g_sent.load()); + /* The chip beacons AUTONOMOUSLY once StartBeacon arms it, and _exit(0) + * below skips every destructor - so without this the beacon keeps airing + * after the process is gone, until the adapter is power-cycled, and it + * contaminates whatever runs next on that channel. Measured on MT7612U: + * a scan after exit still found the SSID live at 308 ms. Realtek has the + * same exposure - IRadio.h calls it bench-bitten. beacon_update_probe.cpp + * already did this; these did not. */ + if (g_dev) g_dev->StopBeacon(); _exit(0); } diff --git a/tests/ap_wpa2.cpp b/tests/ap_wpa2.cpp index 119969aa..522616ad 100644 --- a/tests/ap_wpa2.cpp +++ b/tests/ap_wpa2.cpp @@ -423,5 +423,13 @@ int main(int argc, char** argv) { std::this_thread::sleep_for(std::chrono::milliseconds(1)); } fprintf(stderr, "sent=%llu 4way_state=%d\n", (unsigned long long)g_sent.load(), g_state); + /* The chip beacons AUTONOMOUSLY once StartBeacon arms it, and _exit(0) + * below skips every destructor - so without this the beacon keeps airing + * after the process is gone, until the adapter is power-cycled, and it + * contaminates whatever runs next on that channel. Measured on MT7612U: + * a scan after exit still found the SSID live at 308 ms. Realtek has the + * same exposure - IRadio.h calls it bench-bitten. beacon_update_probe.cpp + * already did this; these did not. */ + if (g_dev) g_dev->StopBeacon(); _exit(0); } diff --git a/tests/ul_trigger_ap.cpp b/tests/ul_trigger_ap.cpp index c61d514f..3e6ab6ae 100644 --- a/tests/ul_trigger_ap.cpp +++ b/tests/ul_trigger_ap.cpp @@ -446,5 +446,13 @@ int main(int argc, char** argv) { (unsigned long long)g_ul.load(), (unsigned long long)g_ul_tb.load(), (unsigned long long)g_data.load(), (unsigned long long)g_sent.load()); + /* The chip beacons AUTONOMOUSLY once StartBeacon arms it, and _exit(0) + * below skips every destructor - so without this the beacon keeps airing + * after the process is gone, until the adapter is power-cycled, and it + * contaminates whatever runs next on that channel. Measured on MT7612U: + * a scan after exit still found the SSID live at 308 ms. Realtek has the + * same exposure - IRadio.h calls it bench-bitten. beacon_update_probe.cpp + * already did this; these did not. */ + if (g_dev) g_dev->StopBeacon(); _exit(0); } From cc6c5e7f7c481c43e4b4bc6405c3aaa384872d5d Mon Sep 17 00:00:00 2001 From: snokvist Date: Thu, 10 Sep 2026 21:39:43 +0200 Subject: [PATCH 09/13] tests: one script for the whole MT7612U AP claim, on hardware Every number in docs/mt7612u-ap-mode.md's "Verified through IRadio" table was hand-run, and three of the readings were wrong. This is those steps as a script, with the three traps that produced the wrong readings written into it: - `iw scan` reports a beacon that stopped up to ~30 s ago as still present, out of its BSS cache. Every check here uses `scan flush`. - a bring-up that FAILS beacons nothing, so "no beacon" read as a pass when the AP had never started. Each cell proves the AP came up before it will believe an absence. - the harnesses used to leave a beacon airing on exit, so a stale one poisons the next cell. Cleanup power-cycles the port between cells. Three cells, each with an external witness (a second MT7612U on the kernel mt76x2u driver, told apart by sysfs id since the two share a PID): open ap_responder beacon -> scan, associate, ping, auth retry=0 wpa2 ap_wpa2 beacon -> scan, 4-way, encrypted ping stop beacon_stop_check armed -> stopped -> re-armed Writing it down immediately caught a flaw in itself that the hand runs had not: the stop cell slept a guessed 8 s after the phase banner before scanning, and reported "armed but not scannable". The banner prints BEFORE StartBeacon, and the arm is not instant - it copies a 1600-byte page over EP0 and reads the identity back. Phase 3 passed only because it happened to sleep 14 s. Both now wait for the log line that proves the arm happened, counting occurrences so the re-arm waits for the second one. Then a second self-inflicted one, worth recording because it is a bash trap and not a hardware one: `grep -c` PRINTS 0 and EXITS 1 when it matches nothing, so `grep -c ... || echo 0` emits "0\n0" and every later integer test dies on it. Result on the bench: open 6/6, wpa2 4/4, stop 4/4. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj --- tests/mt7612u_ap_onair.sh | 260 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 260 insertions(+) create mode 100755 tests/mt7612u_ap_onair.sh diff --git a/tests/mt7612u_ap_onair.sh b/tests/mt7612u_ap_onair.sh new file mode 100755 index 00000000..bae5ee98 --- /dev/null +++ b/tests/mt7612u_ap_onair.sh @@ -0,0 +1,260 @@ +#!/usr/bin/env bash +# mt7612u_ap_onair.sh — the whole MT7612U AP claim, end to end, on hardware. +# +# Every number in docs/mt7612u-ap-mode.md's "Verified through IRadio" table +# comes from this script. It exists because those numbers were hand-run first, +# and three of the readings were wrong in ways that a script would not have +# repeated: +# +# - "the beacon was gone after the process exited" measured nothing. Both AP +# harnesses used to end in _exit(0), so StopBeacon never ran; the beacon +# was still airing and one scan happened to miss it. +# - `iw scan` without `flush` reports a stopped beacon as present for ~30 s +# out of its BSS cache. +# - a bring-up that fails leaves no beacon either, so "no beacon" read as a +# pass when the AP had in fact never started. Every phase here checks the +# AP came up BEFORE it believes an absence. +# +# Three cells, each with its own witness: +# +# open ap_responder beacon -> scan, associate, ARP/ICMP ping +# wpa2 ap_wpa2 beacon -> scan, 4-way handshake, encrypted ping +# stop beacon_stop_check armed -> stopped -> re-armed, by scan +# +# Bench: two MT7612U. One is the AP (devourer claims it); the other stays on +# the kernel mt76x2u driver and is the station. They are told apart by sysfs +# id, not by PID - they share one. +# +# sudo tests/mt7612u_ap_onair.sh +# sudo AP_SYSFS=5-1 STA_SYSFS=2-1 CH=36 tests/mt7612u_ap_onair.sh open +# +# Env: AP_SYSFS, STA_SYSFS, CH, PSK, FW_DIR, SECS. Cells: open|wpa2|stop|all. + +set -u +ROOT="$(cd "$(dirname "$0")/.." && pwd)" +BUILD="${BUILD:-$ROOT/build}" +CELLS="${1:-all}" + +AP_SYSFS="${AP_SYSFS:-5-1}" +STA_SYSFS="${STA_SYSFS:-2-1}" +CH="${CH:-36}" +FREQ=$(( CH < 15 ? 2407 + CH * 5 : 5000 + CH * 5 )) +PSK="${PSK:-devourer123}" +SECS="${SECS:-40}" +FW_DIR="${FW_DIR:-}" +APIP=192.168.99.1 +STAIP=192.168.99.2 +OUT="${OUT:-/tmp/mt7612u-ap-onair}" + +[ "$(id -u)" = 0 ] || { echo "must run as root"; exit 2; } +mkdir -p "$OUT" +pass=0; fail=0 +say() { printf '%s\n' "$*"; } +ok() { pass=$((pass+1)); printf ' PASS %s\n' "$*"; } +bad() { fail=$((fail+1)); printf ' FAIL %s\n' "$*"; } + +cleanup() { + pkill -x apr_onair apw_onair bstop_onair 2>/dev/null + pkill -x wpa_supplicant 2>/dev/null + [ -n "${STA_IF:-}" ] && { ip addr flush dev "$STA_IF" 2>/dev/null + iw dev "$STA_IF" disconnect 2>/dev/null; } + # The MAC beacons autonomously. If a cell died before its teardown, only a + # port power-cycle is certain to silence it - and leaving one airing poisons + # the next run of this very script. + echo 0 > "/sys/bus/usb/devices/$AP_SYSFS/authorized" 2>/dev/null + sleep 2 + echo 1 > "/sys/bus/usb/devices/$AP_SYSFS/authorized" 2>/dev/null + sleep 3 +} +trap cleanup EXIT INT TERM + +# --- the station ----------------------------------------------------------- +STA_IF=$(ls "/sys/bus/usb/devices/$STA_SYSFS:1.0/net/" 2>/dev/null | head -1) +if [ -z "$STA_IF" ]; then + echo "$STA_SYSFS:1.0" > /sys/bus/usb/drivers_probe 2>/dev/null + sleep 3 + STA_IF=$(ls "/sys/bus/usb/devices/$STA_SYSFS:1.0/net/" 2>/dev/null | head -1) +fi +[ -n "$STA_IF" ] || { echo "no station iface at $STA_SYSFS (is mt76x2u bound?)"; exit 2; } +ip link set "$STA_IF" up 2>/dev/null +say "AP $AP_SYSFS station $STA_SYSFS ($STA_IF) ch$CH ($FREQ MHz)" + +# `flush` is not optional: without it the BSS cache reports a beacon that +# stopped up to ~30 s ago as still present, which is how a broken StopBeacon +# reads as working. +seen() { + local n + n=$(iw dev "$STA_IF" scan flush freq "$FREQ" 2>/dev/null | grep -c "SSID: $1") + printf '%s' "${n:-0}" +} + +apenv() { + set -- DEVOURER_VID=0x0e8d DEVOURER_PID=0x7612 DEVOURER_CHANNEL="$CH" \ + DEVOURER_BCN_TU=100 DEVOURER_TX_WITH_RX=thread "$@" + [ -n "$FW_DIR" ] && set -- DEVOURER_MT7612U_FW_DIR="$FW_DIR" "$@" + printf '%s\n' "$@" +} + +build() { # $1 = source stem, $2 = output name, $3.. = extra libs + local src="$1" out="$2"; shift 2 + g++ -std=c++20 -O2 -I"$ROOT/src" -I"$ROOT/examples/common" \ + "$ROOT/tests/$src.cpp" "$ROOT/examples/common/env_config.cpp" \ + "$BUILD/libdevourer.a" $(pkg-config --cflags --libs libusb-1.0) \ + "$@" -lpthread -o "/tmp/$out" || return 1 +} + +# A cell must prove the AP CAME UP before it may believe any absence. A failed +# bring-up beacons nothing, which otherwise reads as a pass. +came_up() { grep -q "beaconing every" "$1"; } + +# --- cell: open network ---------------------------------------------------- +cell_open() { + say "== open network (ap_responder) ==" + build ap_responder apr_onair || { bad "open: build"; return; } + env $(apenv) timeout $((SECS + 20)) /tmp/apr_onair "$SECS" \ + >"$OUT/open.jsonl" 2>"$OUT/open.log" & + local ap=$! + sleep 12 + came_up "$OUT/open.log" || { bad "open: AP did not come up (see $OUT/open.log)"; kill $ap 2>/dev/null; return; } + ok "open: beacon armed" + + [ "$(seen devourerAP)" = 1 ] && ok "open: beacon on air" || bad "open: beacon not scannable" + + ip addr flush dev "$STA_IF" 2>/dev/null + if timeout 30 iw dev "$STA_IF" connect -w devourerAP >/dev/null 2>&1; then + ok "open: station associated" + else + bad "open: station did not associate"; kill $ap 2>/dev/null; return + fi + + ip addr add "$STAIP/24" dev "$STA_IF" 2>/dev/null + ping -c 1 -W 2 -I "$STA_IF" "$APIP" >/dev/null 2>&1 # warm ARP + if ping -c 6 -W 1 -I "$STA_IF" "$APIP" 2>&1 | tee "$OUT/open.ping" | grep -q " 0% packet loss"; then + ok "open: data plane ($(grep -oE 'rtt [^ ]+ = [0-9./]+' "$OUT/open.ping" | head -1))" + else + bad "open: ping lost packets ($(grep -oE '[0-9]+% packet loss' "$OUT/open.ping" | head -1))" + fi + # retry=0 on auth IS the hardware ACK: an un-ACKed frame comes back with FC + # Retry set. This is the only evidence that the APC slot and port identity + # are both right. + grep -q "AUTH req .* retry=0" "$OUT/open.log" \ + && ok "open: hardware auto-ACK (auth at retry=0)" \ + || bad "open: no auth at retry=0 - the MAC did not ACK" + + iw dev "$STA_IF" disconnect 2>/dev/null; ip addr flush dev "$STA_IF" 2>/dev/null + wait $ap 2>/dev/null + sleep 3 + [ "$(seen devourerAP)" = 0 ] \ + && ok "open: nothing left airing after exit" \ + || bad "open: beacon STILL AIRING after exit" +} + +# --- cell: WPA2-PSK -------------------------------------------------------- +cell_wpa2() { + say "== WPA2-PSK (ap_wpa2) ==" + build ap_wpa2 apw_onair -lcrypto || { bad "wpa2: build"; return; } + env $(apenv) DEVOURER_WPA2_PSK="$PSK" timeout $((SECS + 20)) /tmp/apw_onair "$SECS" \ + >"$OUT/wpa2.jsonl" 2>"$OUT/wpa2.log" & + local ap=$! + sleep 12 + came_up "$OUT/wpa2.log" || { bad "wpa2: AP did not come up (see $OUT/wpa2.log)"; kill $ap 2>/dev/null; return; } + ok "wpa2: beacon armed" + + local wpa="$OUT/wpa.conf" + printf 'network={\n\tssid="devourerAP"\n\tpsk="%s"\n\tkey_mgmt=WPA-PSK\n\tproto=RSN\n\tpairwise=CCMP\n\tgroup=CCMP\n\tscan_ssid=1\n}\n' "$PSK" > "$wpa" + ip addr flush dev "$STA_IF" 2>/dev/null + wpa_supplicant -i "$STA_IF" -c "$wpa" -B >/dev/null 2>&1 + local i + for i in $(seq 1 20); do + grep -q "4-WAY HANDSHAKE COMPLETE" "$OUT/wpa2.log" && break + sleep 1 + done + if grep -q "4-WAY HANDSHAKE COMPLETE" "$OUT/wpa2.log"; then + ok "wpa2: 4-way complete (MIC verified, station keyed)" + else + bad "wpa2: 4-way did not complete"; pkill -x wpa_supplicant 2>/dev/null + kill $ap 2>/dev/null; return + fi + + ip addr add "$STAIP/24" dev "$STA_IF" 2>/dev/null + ping -c 1 -W 2 -I "$STA_IF" "$APIP" >/dev/null 2>&1 + if ping -c 6 -W 1 -I "$STA_IF" "$APIP" 2>&1 | tee "$OUT/wpa2.ping" | grep -q " 0% packet loss"; then + ok "wpa2: encrypted data plane ($(grep -oE 'rtt [^ ]+ = [0-9./]+' "$OUT/wpa2.ping" | head -1))" + else + bad "wpa2: encrypted ping lost packets" + fi + + pkill -x wpa_supplicant 2>/dev/null + ip addr flush dev "$STA_IF" 2>/dev/null + wait $ap 2>/dev/null + sleep 3 + [ "$(seen devourerAP)" = 0 ] \ + && ok "wpa2: nothing left airing after exit" \ + || bad "wpa2: beacon STILL AIRING after exit" +} + +# --- cell: the beacon lifecycle ------------------------------------------- +cell_stop() { + say "== beacon lifecycle (StartBeacon / StopBeacon / re-arm) ==" + build mt7612u_beacon_stop_check bstop_onair || { bad "stop: build"; return; } + local phase=24 + env $(apenv) timeout $((phase * 3 + 40)) /tmp/bstop_onair "$phase" \ + >"$OUT/stop.log" 2>&1 & + local ap=$! + + # Wait for the ARM ITSELF, not for the phase banner. The banner prints + # before StartBeacon, and the arm is not instant - it copies a 1600-byte + # page over EP0 and reads the identity back. Sleeping a guessed interval + # after the banner is how phase 1 of this very cell reported "armed but not + # scannable" while phase 3, which happened to sleep longer, passed. + # grep -c PRINTS 0 and EXITS 1 when it matches nothing, so `|| echo 0` + # appends a second line and every later [ -gt ] dies on "0\n0". + armed() { + local n + n=$(grep -c "beaconing every" "$OUT/stop.log" 2>/dev/null) + printf '%s' "${n:-0}" + } + wait_arm() { # $1 = the count to exceed, $2 = seconds to wait + local i + for i in $(seq 1 "$2"); do [ "$(armed)" -gt "$1" ] && return 0; sleep 1; done + return 1 + } + wait_gone() { # the log line that says StopBeacon ran, then a settle + local i + for i in $(seq 1 "$1"); do grep -q "PHASE 3" "$OUT/stop.log" && return 0; sleep 1; done + return 1 + } + + wait_arm 0 30 || { bad "stop: never armed"; kill $ap 2>/dev/null; return; } + sleep 4 + [ "$(seen mtStopCheck)" = 1 ] && ok "stop: armed - beacon on air" || bad "stop: armed but not scannable" + + local n_arms; n_arms=$(armed) + local i + for i in $(seq 1 60); do grep -q "PHASE 2" "$OUT/stop.log" && break; sleep 1; done + sleep 6 + [ "$(seen mtStopCheck)" = 0 ] && ok "stop: stopped - beacon gone" || bad "stop: STILL AIRING after StopBeacon" + + # The re-arm is the same non-instant operation: wait for the second + # "beaconing every", not for the banner that precedes it. + wait_arm "$n_arms" 60 || { bad "stop: re-arm never reported"; kill $ap 2>/dev/null; return; } + sleep 4 + [ "$(seen mtStopCheck)" = 1 ] && ok "stop: re-armed - beacon back" || bad "stop: re-arm did not air" + + wait $ap 2>/dev/null + grep -q "0 failure(s)" "$OUT/stop.log" \ + && ok "stop: local contract checks (2nd stop false, update-with-no-beacon false)" \ + || bad "stop: local contract checks failed" +} + +case "$CELLS" in + open) cell_open ;; + wpa2) cell_wpa2 ;; + stop) cell_stop ;; + all) cell_open; cleanup; cell_wpa2; cleanup; cell_stop ;; + *) echo "usage: $0 [open|wpa2|stop|all]"; exit 2 ;; +esac + +say "" +say "=== $pass passed, $fail failed (logs: $OUT) ===" +exit $(( fail > 0 )) From f3f08a931d3764e311f278ca3b2b84f2c9abc6a8 Mon Sep 17 00:00:00 2001 From: snokvist Date: Thu, 10 Sep 2026 21:56:09 +0200 Subject: [PATCH 10/13] mt7612u: upstream review - the slot index rested on a false premise Four blockers from an adversarial pre-PR review. The first is mine and the bench could not have caught it. THE APC SLOT INDEX. beacon_start derived it as (ta[0] & 2) ? 1 : 0, arguing mt76's `1 + (((macaddr[0] ^ addr[0]) >> 2) & 7)` collapses because the identity had just been retargeted to `ta`. It does not collapse here. dev->macaddr is written ONCE, from the EEPROM (eeprom.cpp), and MT_MAC_BSSID_DW0/DW1 - the register that actually carries MBSS_MODE, and the base the hardware derives the index from - is written once at init from the factory MAC. mt7612u_set_ack_responder moves only MT_MAC_ADDR. So the hardware was deriving its index from a different address than the host assumed, and the constant 1 was right only for adapters where 1 + (((factory[0] ^ ta[0]) >> 2) & 7) happens to be 1. This bench is one of them: factory 40:a5:ef:.., 0x40 ^ 0x02 >> 2 & 7 == 0. So every on-air run agreed with the wrong reasoning. On an adapter whose first byte is 0xe8 the AP would beacon perfectly and acknowledge nobody - the exact failure the header warns about. The bring-up gate already refused that case; the public ABI had flattened it to a constant. Fixed by making the retarget a real setaddr: mt_mac_set_bss_base() moves MT_MAC_BSSID with MT_MAC_ADDR, as mt76x02_mac_setaddr() does, so the premise is true by construction rather than by luck. Restored on stop. STOP COULD NOT FAIL. mt_beacon_set_enable's off path is mt_clear -> mt_rmw, which reports only its READ half, and the mt_ap_set_bssid returns were dropped - so mt7612u_beacon_stop returned non-zero only for a NULL device. That made StopBeacon's whole failure branch unreachable, and the new harness's assertion for it vacuous, while the real hazard (an EP0 stall during teardown leaving the MAC beaconing) reported success. Bracketed with the io_err delta. STOP()'S PROMISED RETRY DID NOT EXIST. It called StopBeacon once and discarded the result while the comment justified keeping _beacon_active on the strength of a retry. Three attempts now, then an error naming what was left airing. THE HEADER DOCUMENTED THE OPPOSITE OF THE CODE. mt7612u.h still described the pre-review behaviour ("deliberately does NOT restore it") after the code started restoring, and omitted every refusal the code enforces. Rewritten against the implementation. Also: - the reserved-page copy was the one write in mt_beacon_write that could fail silently; mt_wr_copy is void and gives up mid-loop, leaving a HALF WRITTEN beacon that then airs. Checked. - UpdateBeaconPayload overwrote a live unsuppressed slot. mt76 brackets the same write with BCN_BYPASS_MASK ("Prevent corrupt transmissions during update"); without it a TBTT mid-copy airs a torn beacon. - AdjustBeaconTiming / ...Fine / PinBeaconTbtt fell through to IRadio's default 0, which reads as "applied a 0 us shift" rather than "not implemented" - the one set of knobs on this backend that did not refuse loudly. - the slot bound admitted 12 bytes mt_tx_build then rejected with a misleading error. Docs: dropped a StopBeacon evidence row that the same document retracts two sections later and that predates the function; corrected the _exit(0) paragraph this branch made false; the WPA2 harness needs five IRadio methods, not four, and StopBeacon has nothing to do with the cipher path; and a "verified" row cited MT_WCID_KEY, which does not exist in this tree. Harness: seen() took one scan as authoritative. A scan can miss a 100 TU beacon, and it did - "beacon not scannable" in a run where the station then associated, pinged and got an auth at retry=0. Three tries, highest count, which is the conservative reading for both "present" and "gone". Re-verified: open 6/6, wpa2 4/4, stop 4/4. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj --- docs/mt7612u-ap-mode.md | 26 ++++--- src/mt7612u/Mt7612uRadio.cpp | 42 ++++++++++- src/mt7612u/Mt7612uRadio.h | 4 + src/mt7612u/beacon.cpp | 104 ++++++++++++++++++++++---- src/mt7612u/include/mt7612u/mt7612u.h | 57 ++++++++------ tests/mt7612u_ap_onair.sh | 19 ++++- tests/mt7612u_beacon_stop_check.cpp | 17 +++-- 7 files changed, 211 insertions(+), 58 deletions(-) diff --git a/docs/mt7612u-ap-mode.md b/docs/mt7612u-ap-mode.md index ce3ca73e..06c173c2 100644 --- a/docs/mt7612u-ap-mode.md +++ b/docs/mt7612u-ap-mode.md @@ -4,13 +4,16 @@ unmodified — beacon, probe, auth, assoc, and the ARP/ICMP data plane — with a real Linux station associated. `StartBeacon`, `UpdateBeaconPayload` and `StopBeacon` are implemented on `Mt7612uRadio`, so nothing in -`tests/ap_responder.cpp` or `tests/ap_wpa2.cpp` knows this is MediaTek. +`tests/ap_responder.cpp` or `tests/ap_wpa2.cpp` branches on the backend. (Both +carry one MediaTek-specific *comment* now, explaining why they silence the +beacon before `_exit`; no code depends on it.) **WPA2-PSK works too.** `tests/ap_wpa2.cpp`, also unmodified, completes the 4-way handshake against a real `wpa_supplicant` station and carries encrypted -traffic. It needs the same four `IRadio` methods as the open-network harness — -`InitWrite`, `StartBeacon`, `StartRxLoop`, `send_packet` — and no others, -because CCMP is done in software there. +traffic. It needs the same five `IRadio` methods as the open-network harness - +`InitWrite`, `StartBeacon`, `StartRxLoop`, `send_packet`, `StopBeacon` - and no +others. CCMP is done in software in the harness, so no key API is involved; +that is a separate point from the method count. What is NOT done: **hardware** CCMP. `MT_WCID_KEY` and `MT_SKEY` are untouched, so the claim below that hardware crypto is a capability *gain* on this part @@ -39,7 +42,6 @@ station, plus an RTL8812AU running `rxdemo` as an independent on-air witness. | Corrected MBSS masks | `MT_MAC_BSSID_DW1` reads `0x003fa127` — upper bits exactly mt76's `MBSS_MODE=3 / MBEACON_N=7 / LOCAL_BIT` | | Hardware auto-ACK (Gate B) | A real station's **3 auth frames, 0 retried**. An un-ACKed frame is retransmitted with FC Retry set, so retry=0 is the ACK | | APC BSSID slot programmed | `MT_MAC_APC_BSSID_L(0)=0x50efa540` (device MAC `40:a5:ef:50:…`) | -| StopBeacon contract | After the process exits the BSSID is absent from a fresh scan — nothing left airing | Not yet done: probe **responses**, auth/assoc **responses** and the data plane — those are the existing backend-agnostic C++ harnesses' job (Stages C–E), not @@ -84,12 +86,12 @@ one is devourer itself: `tests/ap_responder.cpp`, unmodified, built against holds an entry ~30 s after the beacon dies, and it reported a stopped beacon as present until `iw scan flush` was used. A re-arm also takes long enough (a 1600-byte page copy over EP0) that a scan at +8 s still misses it. -- **Neither AP harness silences the beacon on exit.** Both end in `_exit(0)`, - which bypasses the destructor, so `Stop()` and `StopBeacon()` never run and - the MAC keeps beaconing until the adapter is power-cycled. That is why the - StopBeacon evidence above comes from a purpose-built harness and not from - "the SSID was gone after the process exited" — which is what it looked like - once, by luck, and was false. +- **The beacon-stop evidence is from a purpose-built harness, not from process + exit.** Both AP harnesses used to end in `_exit(0)`, skipping the destructor, + so `StopBeacon` never ran and "the SSID was gone after exit" measured nothing + - it looked true once, by luck, and was false. They call `StopBeacon` + explicitly now, and `tests/mt7612u_beacon_stop_check.cpp` is what actually + exercises the transition. ## The claim, and why it holds @@ -126,7 +128,7 @@ Verified in the merged subtree: |---|---|---| | Port MAC + BSSID programmed | yes — `MT_MAC_ADDR_DW0/1`, `MT_MAC_BSSID_DW0/1`, MBSS_MODE=3, MBEACON_N | `init.c:206‑216` (`mac_setaddr`) | | Station table (WCID) | yes — `mt_wcid_setup(idx, mac)` writes `MT_WCID_ATTR` + address; all zeroed at init | `tx.c:95`, `init.c:236` | -| Crypto key slots | present and zeroed — `MT_WCID_KEY`, `MT_SKEY`, `MT_SKEY_MODE` | `regs.h:239‑245`, `init.c:242‑246` | +| Crypto key slots | the shared-key store is present and zeroed at init (`MT_SKEY`, `MT_SKEY_MODE`, `src/mt7612u/init.cpp` `wcid_and_key_clear()`). The per-station key store is NOT defined in this tree - `MT_WCID_KEY` does not exist here, which is part of why hardware CCMP is unreached | | ACKed unicast TX | yes — `no_ack=0` sets `MT_TXWI_ACK_CTL_REQ`; BA-window field present | `tx.c:164‑167` | | Beacon-interval timer regs | defined — `MT_BEACON_TIME_CFG` INTVAL/TIMER_EN/TBTT_EN/BEACON_TX, `MBEACON_N` | `regs.h:176‑180,169` | | RX filter control | yes — managed default `0x00015f97`, monitor clears to error-only | `init.c:278,494‑509` | diff --git a/src/mt7612u/Mt7612uRadio.cpp b/src/mt7612u/Mt7612uRadio.cpp index da59a48e..db0b54e3 100644 --- a/src/mt7612u/Mt7612uRadio.cpp +++ b/src/mt7612u/Mt7612uRadio.cpp @@ -640,7 +640,17 @@ void Mt7612uRadio::Stop() { * channel. Bench-bitten on the Realtek side, and the bring-up gate silences * its beacon on every exit path for the same reason. */ try { - StopBeacon(); + /* Retried, because the comment in StopBeacon promises one and because a + * beacon that survives this call survives the process: the MAC keeps + * airing it until the adapter is power-cycled. Three attempts, then say so + * at error level rather than closing the device in silence. */ + bool silenced = false; + for (int attempt = 0; attempt < 3 && !silenced; ++attempt) + silenced = StopBeacon(); + if (!silenced && _beacon_active) + _logger->error("MT7612U: closing the device with a beacon still armed - " + "the MAC will keep airing it until the adapter is " + "power-cycled"); } catch (...) { } stop_tick(); /* joins; must not run with _mu held */ @@ -890,6 +900,34 @@ bool Mt7612uRadio::UpdateBeaconPayload(const uint8_t *beacon, size_t len) { return mt7612u_beacon_update(_dev, beacon, len) == 0; } +/* The beacon-steer trio. Not implemented, and REFUSING QUIETLY IS THE PROBLEM: + * IRadio's default returns 0, which means "applied a 0 us shift" and is + * indistinguishable from "this backend cannot steer". tests/beacon_steer_check + * and the PTP harnesses run against any backend and would read a clean zero. + * Every other unsupported knob here says so out loud; these were the exception. + * + * Steering needs a pre-TBTT interrupt to re-time against, which this static + * reserved-page path does not have - mt76's own steering lives in + * mt76x02u_pre_tbtt_work(), a path this port does not run. */ +int32_t Mt7612uRadio::AdjustBeaconTiming(int32_t microseconds) { + (void)microseconds; + _logger->error("MT7612U: AdjustBeaconTiming is not implemented - the static " + "reserved-page beacon has no pre-TBTT hook to steer against"); + return 0; +} + +int32_t Mt7612uRadio::AdjustBeaconTimingFine(int32_t microseconds) { + (void)microseconds; + _logger->error("MT7612U: AdjustBeaconTimingFine is not implemented"); + return 0; +} + +int32_t Mt7612uRadio::PinBeaconTbtt(int32_t offset_us) { + (void)offset_us; + _logger->error("MT7612U: PinBeaconTbtt is not implemented"); + return 0; +} + bool Mt7612uRadio::StopBeacon() { std::lock_guard lock(_mu); if (!_dev || !_beacon_active) @@ -902,7 +940,7 @@ bool Mt7612uRadio::StopBeacon() { * try/catch, so a retry there costs nothing. */ _logger->error("MT7612U beacon stop FAILED - the MAC is still airing it; " "retry, or power-cycle the adapter"); - return false; + return false; /* _beacon_active deliberately left set: see above */ } _beacon_active = false; return true; diff --git a/src/mt7612u/Mt7612uRadio.h b/src/mt7612u/Mt7612uRadio.h index e5d5c92f..c0ede18b 100644 --- a/src/mt7612u/Mt7612uRadio.h +++ b/src/mt7612u/Mt7612uRadio.h @@ -103,6 +103,10 @@ class Mt7612uRadio : public IRadio { bool StartBeacon(const uint8_t *beacon, size_t len, int interval_tu) override; bool UpdateBeaconPayload(const uint8_t *beacon, size_t len) override; bool StopBeacon() override; + /* Refuse loudly rather than report a 0 us shift that was never applied. */ + int32_t AdjustBeaconTiming(int32_t microseconds) override; + int32_t AdjustBeaconTimingFine(int32_t microseconds) override; + int32_t PinBeaconTbtt(int32_t offset_us) override; void ClearAckResponder() override; private: diff --git a/src/mt7612u/beacon.cpp b/src/mt7612u/beacon.cpp index 78bfdf79..72105ad7 100644 --- a/src/mt7612u/beacon.cpp +++ b/src/mt7612u/beacon.cpp @@ -72,7 +72,10 @@ int mt_beacon_write(struct mt7612u_dev *d, const void *frame, size_t len, uint8_t buf[MT_BCN_SLOT_SIZE]; int total; - if (len + MT_TXWI_LEN > MT_BCN_SLOT_SIZE) { + /* The slot must hold the TXWI and the DMA header as well as the body; + * mt_tx_build() enforces the tighter bound anyway, but from here the + * error names the beacon rather than reporting a bad frame length. */ + if (len + MT_TXWI_LEN + MT_DMA_HDR_LEN > MT_BCN_SLOT_SIZE) { ERR("beacon %zu B + TXWI exceeds the %d B slot", len, (int)MT_BCN_SLOT_SIZE); return -1; @@ -82,8 +85,20 @@ int mt_beacon_write(struct mt7612u_dev *d, const void *frame, size_t len, if (total < 0) return -1; - mt_wr_copy(d, MT_BEACON_BASE, buf + MT_DMA_HDR_LEN, - total - MT_DMA_HDR_LEN); + /* Checked by the io_err delta, because mt_wr_copy() returns void and + * gives up mid-loop on the first failed vendor request - leaving a HALF + * WRITTEN beacon in the page, which then airs. That is worse than no + * beacon, and the paragraph below already argues why USB writes here get + * checked when mt76's MMIO ones do not. */ + { + const unsigned before = mt_io_errors(d); + mt_wr_copy(d, MT_BEACON_BASE, buf + MT_DMA_HDR_LEN, + total - MT_DMA_HDR_LEN); + if (mt_io_errors(d) != before) { + ERR("beacon: the reserved-page copy failed part way"); + return -1; + } + } /* * Unsuppress the slot just written. BCN_BYPASS_MASK is inverted: a set @@ -231,6 +246,30 @@ static int beacon_split(const void *buf, size_t len, const uint8_t **mpdu, return 0; } +/* + * The MBSS base address: MT_MAC_BSSID_DW0/DW1's address halves, leaving + * MBSS_MODE / MBEACON_N / LOCAL_BIT alone. + * + * This is the half of "retarget the identity" that mt76 does and devourer's + * ACK responder does not. mt76x02_mac_setaddr() moves mphy.macaddr, + * MT_MAC_ADDR and MT_MAC_BSSID together, and the whole per-BSS index + * derivation is written against that invariant. Move only MT_MAC_ADDR - which + * is all mt7612u_set_ack_responder() does, and all this function used to do - + * and the MBSS base is still the factory address, so the hardware derives the + * BSS index from a different address than the host thinks it does. That is + * silent: the AP beacons perfectly and matches nobody. + */ +static int mt_mac_set_bss_base(struct mt7612u_dev *d, const uint8_t *a) +{ + const uint32_t dw0 = (uint32_t)a[0] | ((uint32_t)a[1] << 8) | + ((uint32_t)a[2] << 16) | ((uint32_t)a[3] << 24); + const uint32_t dw1 = (uint32_t)a[4] | ((uint32_t)a[5] << 8); + + if (mt_wr_chk(d, MT_MAC_BSSID_DW0, dw0)) + return -1; + return mt_rmw(d, MT_MAC_BSSID_DW1, MT_MAC_BSSID_DW1_ADDR, dw1); +} + int mt7612u_beacon_start(struct mt7612u_dev *dev, const void *buf, size_t len, unsigned interval_tu) { @@ -268,13 +307,25 @@ int mt7612u_beacon_start(struct mt7612u_dev *dev, const void *buf, size_t len, * beacons is setting the same thing twice, and the restore is whichever * of the two runs last. */ - if (memcmp(ta, dev->macaddr, 6) != 0) { - /* Only claim the identity if nobody else already holds it. When a - * caller armed an ACK responder first, the saved factory MAC is - * theirs and restoring it on beacon stop would silently disarm them. */ + /* Unconditional, and BOTH registers. Unconditional because the old + * `ta != dev->macaddr` guard compared against the FACTORY address - + * dev->macaddr is written once, from the EEPROM, and nothing moves it - + * so a caller who had armed an ACK responder and then beaconed as the + * factory MAC kept the responder's address in MT_MAC_ADDR and ACKed for + * the wrong station all session. The retarget is idempotent and costs two + * EP0 writes; there is nothing to save by skipping it. + * + * Both registers because the index below is derived from the MBSS base, + * not from MT_MAC_ADDR. */ + { const int was_taken = dev->ack_saved; if (mt7612u_set_ack_responder(dev, ta)) return -1; + if (mt_mac_set_bss_base(dev, ta)) + return -1; + /* Only claim ownership if nobody else already held the identity: a + * caller who armed a responder first owns the saved factory address, + * and restoring it on beacon stop would silently disarm them. */ if (!was_taken) dev->beacon_took_identity = 1; } @@ -284,10 +335,13 @@ int mt7612u_beacon_start(struct mt7612u_dev *dev, const void *buf, size_t len, * index comes from the address bits, and mt76 computes * idx = 1 + (((macaddr[0] ^ addr[0]) >> 2) & 7) * for a locally-administered address, 0 otherwise (mt76x02_util.c:310). - * It runs that AFTER retargeting the identity, so macaddr == addr and the - * XOR is zero: the expression collapses to 1. Getting this wrong is - * silent - slot 0 for an 02:/06:/0a: BSSID matches nothing, and the AP - * beacons perfectly while acknowledging nobody. + * mt76 runs that AFTER mt76x02_mac_setaddr(), so its macaddr IS addr and + * the XOR is zero, collapsing the expression to 1. mt_mac_set_bss_base() + * above is what makes the same thing true here - without it the base + * stays the factory address, the hardware derives 1 + ((factory[0] ^ + * ta[0]) >> 2 & 7), and this constant is right only for the adapters + * where that happens to be 1. Getting it wrong is silent: the AP beacons + * perfectly and acknowledges nobody. */ idx = (ta[0] & 0x02) ? 1 : 0; @@ -305,18 +359,36 @@ int mt7612u_beacon_update(struct mt7612u_dev *dev, const void *buf, size_t len) if (!dev) return -1; if (beacon_split(buf, len, &mpdu, &mpdu_len, &rate)) return -1; - /* No mt_beacon_init() and no set_enable(): the engine is already armed and - * re-initialising it would re-suppress every slot mid-flight. */ + + /* Suppress the slot for the duration of the copy. mt_wr_copy() spans many + * 64-byte EP0 transactions, so a TBTT landing mid-copy would air a TORN + * beacon - leading bytes new, trailing bytes old. mt76 brackets the same + * write for the same reason ("Prevent corrupt transmissions during + * update", mt76x02_usb_core.c). mt_beacon_write() lowers the guard again + * on its way out, which is why this is the only half needed here. + * + * Still no mt_beacon_init() and no set_enable(): the engine is armed, and + * re-initialising it would clear the timer bits mid-flight. */ + mt_wr(dev, MT_BCN_BYPASS_MASK, 0xffff); return mt_beacon_write(dev, mpdu, mpdu_len, &rate); } int mt7612u_beacon_stop(struct mt7612u_dev *dev) { static const uint8_t zero[6] = { 0 }; + unsigned before; int rc; if (!dev) return -1; + /* The io_err delta is what makes a failed stop VISIBLE. mt_beacon_set_enable's + * off path is mt_clear() -> mt_rmw(), which reports only its READ half, and + * mt_ap_set_bssid()'s writes are checked but their returns were dropped. So + * this function used to be incapable of returning non-zero, which made + * Mt7612uRadio::StopBeacon's whole failure branch unreachable and the + * harness assertion for it vacuous - while the real hazard (an EP0 stall + * during teardown leaving the MAC beaconing) reported success. */ + before = mt_io_errors(dev); rc = mt_beacon_set_enable(dev, 0, 0); /* @@ -331,14 +403,16 @@ int mt7612u_beacon_stop(struct mt7612u_dev *dev) * them by the address's locally-administered bit, and stop does not have * the beacon any more to re-derive which one it used. */ - mt_ap_set_bssid(dev, 0, zero); - mt_ap_set_bssid(dev, 1, zero); + if (mt_ap_set_bssid(dev, 0, zero)) rc = -1; + if (mt_ap_set_bssid(dev, 1, zero)) rc = -1; /* And the port MAC, if this call's opposite number was what retargeted * it. mt7612u_clear_ack_responder() is the restore. */ if (dev->beacon_took_identity) { mt7612u_clear_ack_responder(dev); + if (mt_mac_set_bss_base(dev, dev->macaddr)) rc = -1; dev->beacon_took_identity = 0; } + if (mt_io_errors(dev) != before) rc = -1; return rc; } diff --git a/src/mt7612u/include/mt7612u/mt7612u.h b/src/mt7612u/include/mt7612u/mt7612u.h index ba17374f..4fdbf7c3 100644 --- a/src/mt7612u/include/mt7612u/mt7612u.h +++ b/src/mt7612u/include/mt7612u/mt7612u.h @@ -290,32 +290,47 @@ void mt7612u_clear_ack_responder(struct mt7612u_dev *dev); * MPDU with no radiotap header is accepted too and airs at OFDM 6 Mbps, the * rate a beacon wants. * - * addr2 becomes the MAC's port identity and addr3 is published in the APC slot - * the hardware will match that BSS in - together, those two ARE "being an AP" - * on this part; there is no AP op-mode register and mt76 sets none either. - * Retargeting the identity is required, not a convenience: without it the MAC - * keeps ACKing for the adapter's factory MAC while beaconing a different - * BSSID, so a station's auth is never acknowledged and it retries until it - * gives up. The slot index follows mt76 - 1 for a locally-administered - * address, 0 otherwise - because under MBSS_MODE=3 the hardware derives it - * from the address bits, and slot 0 for an 02:/06:/0a: BSSID matches nothing. + * addr2 becomes the MAC's identity - MT_MAC_ADDR (what it ACKs against) and + * the MT_MAC_BSSID base (what the per-BSS index is derived from), moved + * together the way mt76x02_mac_setaddr() moves them. Moving only the first + * leaves the hardware deriving its BSS index from a different address than the + * caller thinks, which is silent: the AP beacons perfectly and matches nobody. + * addr3 is then published in the APC slot that index selects - 1 for a + * locally-administered address, 0 otherwise. * - * The identity is one register plane with one saved copy, shared with - * mt7612u_set_ack_responder(): a caller doing both is setting the same thing - * twice, and mt7612u_clear_ack_responder() is what restores the factory MAC. - * mt7612u_beacon_stop() deliberately does NOT restore it - it silences the - * beacon and leaves the identity alone rather than clobbering a responder the - * caller may own. + * The identity is one register plane, shared with mt7612u_set_ack_responder(): + * a caller doing both is setting the same thing twice and the last writer + * wins. mt7612u_beacon_stop() restores the factory identity ONLY if this call + * was what moved it - if a responder was already armed, that address is the + * caller's and stop leaves it alone. * * mt7612u_beacon_update() replaces the loaded beacon in place; the interval, - * TBTT phase and BSSID are untouched. The swap is not atomic against TBTT - a - * beacon airing during the write may still carry the previous content. + * TBTT phase and BSSID are untouched. It suppresses the slot for the duration + * of the copy, so a beacon airing across an update carries the PREVIOUS + * content rather than a torn mixture of the two - but it may be skipped + * entirely. * - * mt7612u_beacon_stop() clears the timer bits. It matters: the MAC beacons - * AUTONOMOUSLY once armed, so killing the host process does NOT silence it, - * and a beacon left airing contaminates whatever runs next on that channel. + * mt7612u_beacon_stop() clears the timer bits, zeroes APC slots 0 and 1, and + * restores the identity as above. It matters that it is called: the MAC + * beacons AUTONOMOUSLY once armed, so killing the host process does NOT + * silence it, and a beacon left airing contaminates whatever runs next on that + * channel. * - * All three return 0 on success, negative on failure. + * What mt7612u_beacon_start() REFUSES, all silently fatal if allowed through: + * - a multicast addr2 (a station cannot unicast-auth to it) + * - an 802.11 header that is not 24 bytes. A QoS or 4-address frame makes + * mt_tx_build() insert an interior L2 pad, and the reserved page needs an + * unpadded [TXWI][MPDU] + * - a beacon body that does not fit the 1600-byte slot alongside its TXWI + * - an interval outside 1..4095 TU (INTVAL is 16 bits of 1/16 TU) + * + * And what it FORCES, whatever the caller's radiotap said: no_ack (a broadcast + * beacon must not request an ACK). A bare MPDU with no radiotap header is + * accepted and pinned to OFDM 6 Mbps, NSS 1, 20 MHz - the basic rate every + * station must decode. + * + * All three return 0 on success, negative on failure. A failed stop is a + * beacon still on the air; it is worth retrying. */ int mt7612u_beacon_start(struct mt7612u_dev *dev, const void *buf, size_t len, unsigned interval_tu); diff --git a/tests/mt7612u_ap_onair.sh b/tests/mt7612u_ap_onair.sh index bae5ee98..2654b752 100755 --- a/tests/mt7612u_ap_onair.sh +++ b/tests/mt7612u_ap_onair.sh @@ -82,10 +82,23 @@ say "AP $AP_SYSFS station $STA_SYSFS ($STA_IF) ch$CH ($FREQ MHz)" # `flush` is not optional: without it the BSS cache reports a beacon that # stopped up to ~30 s ago as still present, which is how a broken StopBeacon # reads as working. +# Scan up to three times and take the HIGHEST count. A scan can come back empty +# for its own reasons - colliding with another scan, a busy card, a dwell that +# misses a 100 TU beacon - and one empty result is not evidence of absence. +# Taking the max is the conservative reading in BOTH directions: it cannot turn +# a live beacon into a pass for "gone", and it stops a missed scan reporting a +# live beacon as absent. Observed: a "beacon not scannable" FAIL in a run where +# the station then associated, pinged, and got an auth at retry=0. seen() { - local n - n=$(iw dev "$STA_IF" scan flush freq "$FREQ" 2>/dev/null | grep -c "SSID: $1") - printf '%s' "${n:-0}" + local i n best=0 + for i in 1 2 3; do + n=$(iw dev "$STA_IF" scan flush freq "$FREQ" 2>/dev/null | grep -c "SSID: $1") + n=${n:-0} + [ "$n" -gt "$best" ] && best=$n + [ "$best" -gt 0 ] && break + sleep 2 + done + printf '%s' "$best" } apenv() { diff --git a/tests/mt7612u_beacon_stop_check.cpp b/tests/mt7612u_beacon_stop_check.cpp index 4be8cf6c..17d4e3cf 100644 --- a/tests/mt7612u_beacon_stop_check.cpp +++ b/tests/mt7612u_beacon_stop_check.cpp @@ -1,11 +1,12 @@ /* * Does StopBeacon actually silence the MAC? * - * This exists because the obvious way to check it does not check it. Both AP - * harnesses (tests/ap_responder.cpp, tests/ap_wpa2.cpp) end in `_exit(0)`, - * which bypasses every destructor - so the radio's Stop(), and with it - * StopBeacon(), never run, and "the beacon was gone after the process exited" - * measures nothing. On this part that is not a cosmetic difference: the MAC + * This exists because the obvious way to check it did not check it. Both AP + * harnesses used to end in `_exit(0)` with no teardown, which bypasses every + * destructor - so the radio's Stop(), and with it StopBeacon(), never ran, and + * "the beacon was gone after the process exited" measured nothing. They call + * StopBeacon explicitly now, but that still tests the call from one place at + * one moment; this drives the whole transition and lets a witness look. On this part that is not a cosmetic difference: the MAC * beacons AUTONOMOUSLY from the reserved page once armed, so a session that * skips the teardown leaves it airing until the adapter is power-cycled. * @@ -20,6 +21,12 @@ * script can scan in the right window. It is not a ctest cell - it needs an * adapter and a second radio to look. * + * Build: + * g++ -std=c++20 -O2 -Isrc -Iexamples/common \ + * tests/mt7612u_beacon_stop_check.cpp examples/common/env_config.cpp \ + * build/libdevourer.a $(pkg-config --cflags --libs libusb-1.0) \ + * -lpthread -o beacon_stop_check + * * sudo DEVOURER_VID=0x0e8d DEVOURER_PID=0x7612 DEVOURER_CHANNEL=36 \ * DEVOURER_MT7612U_FW_DIR= ./beacon_stop_check [phase_secs] */ From 424170b6f2e5e611db59ad7397648fd4cf033cab Mon Sep 17 00:00:00 2001 From: snokvist Date: Thu, 10 Sep 2026 22:10:24 +0200 Subject: [PATCH 11/13] qodo: twelve findings - two of them my own review fixes biting back Two came straight out of the previous round's changes, which is the useful kind: _beacon_active was cleared BEFORE the start call, to make it failure-atomic. But a start that fails before touching the engine - bad input, a failed identity retarget - then leaves the PREVIOUS beacon on the air while update, stop and the destructor all read it as inactive. An orphaned live beacon, which is the one outcome this whole change exists to prevent. mt7612u_beacon_start now returns three outcomes: 0 armed, -1 refused without touching hardware (flag unchanged, whatever was airing still is), -2 failed after the engine was disarmed and the library unwound the rest. beacon_start never checked io_err. I added that to stop last round and not to start, so a failed EP0 transfer while arming returned success - an AP that reports ready and airs nothing. Three more on the identity register, which has two owners and no arbitration: a failed start left it retargeted with no beacon, so the adapter answered for a BSS that does not exist. Every failure path unwinds now. a SetAckResponder issued AFTER StartBeacon was silently disarmed by the matching StopBeacon putting the factory address back. Ownership transfers to whoever wrote last. Two input-classification holes, both of which could put attacker- or caller-controlled bytes into the autonomous beacon page: mt_radiotap_parse returns 0 for "not a radiotap header" AND for "is one and it is malformed", and rlen == len (a header with no frame after it) fell to the bare-MPDU branch too. Byte 0 decides the shape now - a beacon's frame control is 0x80, radiotap's version must be 0 - and each shape is held to its own rules with no fallback to the other. mt7612u_beacon_update raised its suppression guard with an unchecked mt_wr. If that write never lands the copy runs against a live slot and a TBTT mid-copy airs a torn beacon, which is the single thing the guard is for. And four in the on-air harness, which matter because it runs as root: it wrote `authorized` to a caller-supplied sysfs path without checking what was on it - a stale AP_SYSFS would yank an unrelated device. Confirmed against 0e8d:7612 first. `pkill -x wpa_supplicant` would drop every wireless client on the host, and a name kill reaches a concurrent run of this same test. It tracks the PIDs it started. seen() matched SSID alone, so a neighbour running "devourerAP" could pass an arm check or fail a stop check. Matched on BSSID too. the harnesses called StopBeacon once and ignored the result immediately before _exit - and IRadio.h now says such a failure "must be retried". Three attempts, then a warning naming what is still airing. Re-verified on hardware: 14 passed, 0 failed. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj --- src/mt7612u/Mt7612uRadio.cpp | 24 +++--- src/mt7612u/beacon.cpp | 110 ++++++++++++++++++++++------ src/mt7612u/caps.cpp | 7 ++ tests/ap_responder.cpp | 20 +++-- tests/ap_wpa2.cpp | 20 +++-- tests/mt7612u_ap_onair.sh | 67 ++++++++++++----- tests/mt7612u_beacon_stop_check.cpp | 13 +++- tests/ul_trigger_ap.cpp | 20 +++-- 8 files changed, 204 insertions(+), 77 deletions(-) diff --git a/src/mt7612u/Mt7612uRadio.cpp b/src/mt7612u/Mt7612uRadio.cpp index db0b54e3..37307d6b 100644 --- a/src/mt7612u/Mt7612uRadio.cpp +++ b/src/mt7612u/Mt7612uRadio.cpp @@ -875,15 +875,21 @@ bool Mt7612uRadio::StartBeacon(const uint8_t *beacon, size_t len, std::lock_guard lock(_mu); if (!_dev || !beacon || len == 0 || interval_tu <= 0) return false; - /* Cleared BEFORE the call, not just set after it. mt7612u_beacon_start() - * runs mt_beacon_init(), which disarms the timer and suppresses every slot, - * so a failure after that point leaves the beacon dead - and a re-arm that - * fails (an over-long beacon body, say) would otherwise keep reporting the - * PREVIOUS arm as live, which is how UpdateBeaconPayload comes to return - * true for every write into a disarmed engine. */ - _beacon_active = false; - if (mt7612u_beacon_start(_dev, beacon, len, - static_cast(interval_tu)) != 0) + /* Three outcomes, not two, because a failed re-arm has to say whether the + * PREVIOUS beacon is still on the air: + * + * 0 armed + * -1 refused before the hardware was touched - whatever was airing still + * is, so the flag must NOT be cleared. Clearing it here was a way to + * orphan a live beacon: update, stop and the destructor would all then + * treat it as inactive and nobody would ever silence it. + * -2 failed after the engine was disarmed, and the library unwound the + * rest - so nothing is airing and the flag is false. */ + const int rc = mt7612u_beacon_start(_dev, beacon, len, + static_cast(interval_tu)); + if (rc == -2) + _beacon_active = false; + if (rc != 0) return false; _beacon_active = true; _logger->info("MT7612U beaconing every {} TU", interval_tu); diff --git a/src/mt7612u/beacon.cpp b/src/mt7612u/beacon.cpp index 72105ad7..85143437 100644 --- a/src/mt7612u/beacon.cpp +++ b/src/mt7612u/beacon.cpp @@ -198,22 +198,33 @@ static int beacon_split(const void *buf, size_t len, const uint8_t **mpdu, * because mt_radiotap_parse() memsets its output. */ *r = mt7612u_tx_rate{}; - rlen = mt_radiotap_parse(p, len, r); - /* Three return classes, not two: <0 means "this IS a radiotap header and - * it is malformed". Treating that as a bare MPDU would parse the radiotap - * bytes as an 802.11 header and read the BSSID out of the middle of it. - * mt7612u_send_packet() refuses on <= 0; so does this. */ - if (rlen < 0) { - ERR("beacon: malformed radiotap header"); - return -1; - } - if (rlen > 0 && (size_t)rlen < len) { + /* + * Which shape is this? An 802.11 beacon's first byte is its frame control, + * 0x80 - never 0. A radiotap header's first byte is its version, which must + * be 0. So byte 0 decides, and each shape is then held to its own rules + * rather than being allowed to fall back to the other: + * + * mt_radiotap_parse() returns 0 for BOTH "not a radiotap header" and "is + * one and it is malformed" (bad version, bad length, truncated present + * map), and negative for "declared a field past its own length". Treating + * any of those as a bare MPDU parses radiotap bytes as an 802.11 header and + * loads them into the beacon page - which then airs. + */ + if (p[0] == 0) { + rlen = mt_radiotap_parse(p, len, r); + if (rlen <= 0) { + ERR("beacon: radiotap header is malformed"); + return -1; + } + if ((size_t)rlen >= len) { + ERR("beacon: %d B of radiotap and no frame after it", rlen); + return -1; + } *mpdu = p + rlen; *mpdu_len = len - (size_t)rlen; } else { - /* rlen == 0 (no radiotap) or rlen == len (a header with no frame - * after it): treat the buffer as a bare MPDU. OFDM 6 Mbps is the - * basic rate every station must decode, which is what a beacon wants. */ + /* A bare MPDU. OFDM 6 Mbps is the basic rate every station must + * decode, which is what a beacon wants. */ r->phy = MT7612U_PHY_OFDM; r->mcs = 0; r->nss = 1; @@ -221,6 +232,7 @@ static int beacon_split(const void *buf, size_t len, const uint8_t **mpdu, *mpdu = p; *mpdu_len = len; } + /* Unconditionally, whatever the caller's radiotap said: a beacon is * broadcast, and mt_tx_build() turns a cleared no_ack into * MT_TXWI_ACK_CTL_REQ - an ACK request on a frame no one may ACK. Both @@ -270,6 +282,17 @@ static int mt_mac_set_bss_base(struct mt7612u_dev *d, const uint8_t *a) return mt_rmw(d, MT_MAC_BSSID_DW1, MT_MAC_BSSID_DW1_ADDR, dw1); } +/* Put the identity back if THIS call was what moved it. Shared by every + * failure path in beacon_start and by beacon_stop. */ +static void unwind_identity(struct mt7612u_dev *d, int took) +{ + if (!took) + return; + mt7612u_clear_ack_responder(d); + mt_mac_set_bss_base(d, d->macaddr); + d->beacon_took_identity = 0; +} + int mt7612u_beacon_start(struct mt7612u_dev *dev, const void *buf, size_t len, unsigned interval_tu) { @@ -278,6 +301,8 @@ int mt7612u_beacon_start(struct mt7612u_dev *dev, const void *buf, size_t len, size_t mpdu_len = 0; const uint8_t *ta, *bssid; uint8_t idx; + unsigned before; + int took = 0; if (!dev) return -1; if (beacon_split(buf, len, &mpdu, &mpdu_len, &rate)) return -1; @@ -326,8 +351,10 @@ int mt7612u_beacon_start(struct mt7612u_dev *dev, const void *buf, size_t len, /* Only claim ownership if nobody else already held the identity: a * caller who armed a responder first owns the saved factory address, * and restoring it on beacon stop would silently disarm them. */ - if (!was_taken) + if (!was_taken) { dev->beacon_took_identity = 1; + took = 1; + } } /* @@ -345,10 +372,44 @@ int mt7612u_beacon_start(struct mt7612u_dev *dev, const void *buf, size_t len, */ idx = (ta[0] & 0x02) ? 1 : 0; - if (mt_ap_set_bssid(dev, idx, bssid)) return -1; + /* + * From here the hardware is being changed, so every exit unwinds. Two + * things are being unwound, and they are different: + * + * - the identity, if THIS call took it. Leaving it retargeted after a + * failed start means the adapter answers for a BSS that does not exist. + * - the beacon engine, once mt_beacon_init() has disarmed it. That is why + * a failure at or after that point returns -2 and one before it returns + * -1: only the caller can know whether a PREVIOUS beacon is still on the + * air, and -1 means "nothing was touched, whatever was airing still is". + * + * And the io_err delta, because mt_beacon_set_enable()'s arming path is + * mt_rmw()/mt_set() - which report their read halves, not their writes - so + * without it a failed EP0 transfer returns 0 and the caller reports an AP + * that is ready and airing nothing. + */ + before = mt_io_errors(dev); + + if (mt_ap_set_bssid(dev, idx, bssid)) + goto fail_pre; mt_beacon_init(dev); - if (mt_beacon_write(dev, mpdu, mpdu_len, &rate)) return -1; - return mt_beacon_set_enable(dev, 1, interval_tu); + if (mt_beacon_write(dev, mpdu, mpdu_len, &rate)) + goto fail_post; + if (mt_beacon_set_enable(dev, 1, interval_tu)) + goto fail_post; + if (mt_io_errors(dev) != before) { + ERR("beacon: a USB transfer failed while arming"); + goto fail_post; + } + return 0; + +fail_post: + mt_beacon_set_enable(dev, 0, 0); + unwind_identity(dev, took); + return -2; +fail_pre: + unwind_identity(dev, took); + return -1; } int mt7612u_beacon_update(struct mt7612u_dev *dev, const void *buf, size_t len) @@ -369,7 +430,14 @@ int mt7612u_beacon_update(struct mt7612u_dev *dev, const void *buf, size_t len) * * Still no mt_beacon_init() and no set_enable(): the engine is armed, and * re-initialising it would clear the timer bits mid-flight. */ - mt_wr(dev, MT_BCN_BYPASS_MASK, 0xffff); + if (mt_wr_chk(dev, MT_BCN_BYPASS_MASK, 0xffff)) { + /* Checked: if the guard never lands, the copy below runs against a + * LIVE slot and a TBTT mid-copy airs a torn beacon - which is the one + * outcome this bracket exists to prevent, so failing to raise it must + * not be the quiet path. */ + ERR("beacon: could not suppress the slot for an in-place update"); + return -1; + } return mt_beacon_write(dev, mpdu, mpdu_len, &rate); } @@ -408,11 +476,7 @@ int mt7612u_beacon_stop(struct mt7612u_dev *dev) /* And the port MAC, if this call's opposite number was what retargeted * it. mt7612u_clear_ack_responder() is the restore. */ - if (dev->beacon_took_identity) { - mt7612u_clear_ack_responder(dev); - if (mt_mac_set_bss_base(dev, dev->macaddr)) rc = -1; - dev->beacon_took_identity = 0; - } + unwind_identity(dev, dev->beacon_took_identity); if (mt_io_errors(dev) != before) rc = -1; return rc; } diff --git a/src/mt7612u/caps.cpp b/src/mt7612u/caps.cpp index e3875601..cdab2157 100644 --- a/src/mt7612u/caps.cpp +++ b/src/mt7612u/caps.cpp @@ -81,6 +81,13 @@ int mt7612u_set_ack_responder(struct mt7612u_dev *d, const uint8_t mac[6]) memcpy(d->ack_saved_mac, d->macaddr, 6); d->ack_saved = 1; } + /* Ownership TRANSFERS to this caller. MT_MAC_ADDR is one register with two + * users - the beacon takes it too - and whoever wrote last owns what is + * there. Without this, a responder armed after StartBeacon would be + * silently disarmed by the matching StopBeacon restoring the factory + * address, and a stop after this call would put back an address the caller + * never asked for. */ + d->beacon_took_identity = 0; dw0 = (uint32_t)mac[0] | ((uint32_t)mac[1] << 8) | ((uint32_t)mac[2] << 16) | ((uint32_t)mac[3] << 24); diff --git a/tests/ap_responder.cpp b/tests/ap_responder.cpp index 4505d461..4f94b760 100644 --- a/tests/ap_responder.cpp +++ b/tests/ap_responder.cpp @@ -290,13 +290,17 @@ int main(int argc, char** argv) { (unsigned long long)g_probe.load(), (unsigned long long)g_auth.load(), (unsigned long long)g_assoc.load(), (unsigned long long)g_data.load(), (unsigned long long)g_sent.load()); - /* The chip beacons AUTONOMOUSLY once StartBeacon arms it, and _exit(0) - * below skips every destructor - so without this the beacon keeps airing - * after the process is gone, until the adapter is power-cycled, and it - * contaminates whatever runs next on that channel. Measured on MT7612U: - * a scan after exit still found the SSID live at 308 ms. Realtek has the - * same exposure - IRadio.h calls it bench-bitten. beacon_update_probe.cpp - * already did this; these did not. */ - if (g_dev) g_dev->StopBeacon(); + /* Retried, and the failure reported. StopBeacon can now genuinely fail (an + * EP0 stall during teardown), IRadio.h says such a failure "must be retried + * ... before its shared port is reused", and `_exit(0)` below means there is + * no destructor coming to try again. A beacon that survives here survives + * the process. */ + if (g_dev) { + bool silenced = false; + for (int i = 0; i < 3 && !silenced; ++i) silenced = g_dev->StopBeacon(); + if (!silenced) + fprintf(stderr, "WARNING: the beacon could not be stopped - it is still " + "airing; power-cycle the adapter\n"); + } _exit(0); } diff --git a/tests/ap_wpa2.cpp b/tests/ap_wpa2.cpp index 522616ad..5e7f3c87 100644 --- a/tests/ap_wpa2.cpp +++ b/tests/ap_wpa2.cpp @@ -423,13 +423,17 @@ int main(int argc, char** argv) { std::this_thread::sleep_for(std::chrono::milliseconds(1)); } fprintf(stderr, "sent=%llu 4way_state=%d\n", (unsigned long long)g_sent.load(), g_state); - /* The chip beacons AUTONOMOUSLY once StartBeacon arms it, and _exit(0) - * below skips every destructor - so without this the beacon keeps airing - * after the process is gone, until the adapter is power-cycled, and it - * contaminates whatever runs next on that channel. Measured on MT7612U: - * a scan after exit still found the SSID live at 308 ms. Realtek has the - * same exposure - IRadio.h calls it bench-bitten. beacon_update_probe.cpp - * already did this; these did not. */ - if (g_dev) g_dev->StopBeacon(); + /* Retried, and the failure reported. StopBeacon can now genuinely fail (an + * EP0 stall during teardown), IRadio.h says such a failure "must be retried + * ... before its shared port is reused", and `_exit(0)` below means there is + * no destructor coming to try again. A beacon that survives here survives + * the process. */ + if (g_dev) { + bool silenced = false; + for (int i = 0; i < 3 && !silenced; ++i) silenced = g_dev->StopBeacon(); + if (!silenced) + fprintf(stderr, "WARNING: the beacon could not be stopped - it is still " + "airing; power-cycle the adapter\n"); + } _exit(0); } diff --git a/tests/mt7612u_ap_onair.sh b/tests/mt7612u_ap_onair.sh index 2654b752..5b05d251 100755 --- a/tests/mt7612u_ap_onair.sh +++ b/tests/mt7612u_ap_onair.sh @@ -53,18 +53,35 @@ say() { printf '%s\n' "$*"; } ok() { pass=$((pass+1)); printf ' PASS %s\n' "$*"; } bad() { fail=$((fail+1)); printf ' FAIL %s\n' "$*"; } +# PIDs this script started, so cleanup kills those and nothing else. `pkill -x +# wpa_supplicant` would drop every wireless client on the host, and a name kill +# would reach a concurrent run of this same test. +KIDS="" +reap() { + local pid + for pid in $KIDS; do kill "$pid" 2>/dev/null; done + KIDS="" +} + cleanup() { - pkill -x apr_onair apw_onair bstop_onair 2>/dev/null - pkill -x wpa_supplicant 2>/dev/null + reap [ -n "${STA_IF:-}" ] && { ip addr flush dev "$STA_IF" 2>/dev/null iw dev "$STA_IF" disconnect 2>/dev/null; } # The MAC beacons autonomously. If a cell died before its teardown, only a # port power-cycle is certain to silence it - and leaving one airing poisons # the next run of this very script. - echo 0 > "/sys/bus/usb/devices/$AP_SYSFS/authorized" 2>/dev/null - sleep 2 - echo 1 > "/sys/bus/usb/devices/$AP_SYSFS/authorized" 2>/dev/null - sleep 3 + # + # Confirmed against the VID:PID first. This runs as root and writes a + # deauthorize to a path the caller supplied; a stale or mistyped AP_SYSFS + # would otherwise yank whatever else is plugged there - someone's keyboard, + # a disk mid-write. + if [ "$(cat "/sys/bus/usb/devices/$AP_SYSFS/idVendor" 2>/dev/null)" = "0e8d" ] && + [ "$(cat "/sys/bus/usb/devices/$AP_SYSFS/idProduct" 2>/dev/null)" = "7612" ]; then + echo 0 > "/sys/bus/usb/devices/$AP_SYSFS/authorized" 2>/dev/null + sleep 2 + echo 1 > "/sys/bus/usb/devices/$AP_SYSFS/authorized" 2>/dev/null + sleep 3 + fi } trap cleanup EXIT INT TERM @@ -89,10 +106,18 @@ say "AP $AP_SYSFS station $STA_SYSFS ($STA_IF) ch$CH ($FREQ MHz)" # a live beacon into a pass for "gone", and it stops a missed scan reporting a # live beacon as absent. Observed: a "beacon not scannable" FAIL in a run where # the station then associated, pinged, and got an auth at retry=0. -seen() { +seen() { # $1 = SSID, $2 = BSSID local i n best=0 for i in 1 2 3; do - n=$(iw dev "$STA_IF" scan flush freq "$FREQ" 2>/dev/null | grep -c "SSID: $1") + # Matched on BSSID *and* SSID: a neighbour running "devourerAP" would + # otherwise pass an arm check, fail a stop check, or break the exact-count + # comparison. awk keeps the pairing - grep -c on two patterns would count + # them independently. + n=$(iw dev "$STA_IF" scan flush freq "$FREQ" 2>/dev/null | + awk -v b="$2" -v ss="SSID: $1" ' + /^BSS / { cur = tolower($2); sub(/\(.*/, "", cur) } + index($0, ss) { if (cur == tolower(b)) c++ } + END { print c + 0 }') n=${n:-0} [ "$n" -gt "$best" ] && best=$n [ "$best" -gt 0 ] && break @@ -126,12 +151,12 @@ cell_open() { build ap_responder apr_onair || { bad "open: build"; return; } env $(apenv) timeout $((SECS + 20)) /tmp/apr_onair "$SECS" \ >"$OUT/open.jsonl" 2>"$OUT/open.log" & - local ap=$! + local ap=$!; KIDS="$KIDS $ap" sleep 12 came_up "$OUT/open.log" || { bad "open: AP did not come up (see $OUT/open.log)"; kill $ap 2>/dev/null; return; } ok "open: beacon armed" - [ "$(seen devourerAP)" = 1 ] && ok "open: beacon on air" || bad "open: beacon not scannable" + [ "$(seen devourerAP 02:42:75:05:d6:00)" = 1 ] && ok "open: beacon on air" || bad "open: beacon not scannable" ip addr flush dev "$STA_IF" 2>/dev/null if timeout 30 iw dev "$STA_IF" connect -w devourerAP >/dev/null 2>&1; then @@ -157,7 +182,7 @@ cell_open() { iw dev "$STA_IF" disconnect 2>/dev/null; ip addr flush dev "$STA_IF" 2>/dev/null wait $ap 2>/dev/null sleep 3 - [ "$(seen devourerAP)" = 0 ] \ + [ "$(seen devourerAP 02:42:75:05:d6:00)" = 0 ] \ && ok "open: nothing left airing after exit" \ || bad "open: beacon STILL AIRING after exit" } @@ -168,7 +193,7 @@ cell_wpa2() { build ap_wpa2 apw_onair -lcrypto || { bad "wpa2: build"; return; } env $(apenv) DEVOURER_WPA2_PSK="$PSK" timeout $((SECS + 20)) /tmp/apw_onair "$SECS" \ >"$OUT/wpa2.jsonl" 2>"$OUT/wpa2.log" & - local ap=$! + local ap=$!; KIDS="$KIDS $ap" sleep 12 came_up "$OUT/wpa2.log" || { bad "wpa2: AP did not come up (see $OUT/wpa2.log)"; kill $ap 2>/dev/null; return; } ok "wpa2: beacon armed" @@ -176,7 +201,8 @@ cell_wpa2() { local wpa="$OUT/wpa.conf" printf 'network={\n\tssid="devourerAP"\n\tpsk="%s"\n\tkey_mgmt=WPA-PSK\n\tproto=RSN\n\tpairwise=CCMP\n\tgroup=CCMP\n\tscan_ssid=1\n}\n' "$PSK" > "$wpa" ip addr flush dev "$STA_IF" 2>/dev/null - wpa_supplicant -i "$STA_IF" -c "$wpa" -B >/dev/null 2>&1 + wpa_supplicant -i "$STA_IF" -c "$wpa" -P "$OUT/wpa.pid" -B >/dev/null 2>&1 + KIDS="$KIDS $(cat "$OUT/wpa.pid" 2>/dev/null)" local i for i in $(seq 1 20); do grep -q "4-WAY HANDSHAKE COMPLETE" "$OUT/wpa2.log" && break @@ -185,7 +211,8 @@ cell_wpa2() { if grep -q "4-WAY HANDSHAKE COMPLETE" "$OUT/wpa2.log"; then ok "wpa2: 4-way complete (MIC verified, station keyed)" else - bad "wpa2: 4-way did not complete"; pkill -x wpa_supplicant 2>/dev/null + bad "wpa2: 4-way did not complete" + kill "$(cat "$OUT/wpa.pid" 2>/dev/null)" 2>/dev/null kill $ap 2>/dev/null; return fi @@ -197,11 +224,11 @@ cell_wpa2() { bad "wpa2: encrypted ping lost packets" fi - pkill -x wpa_supplicant 2>/dev/null + kill "$(cat "$OUT/wpa.pid" 2>/dev/null)" 2>/dev/null ip addr flush dev "$STA_IF" 2>/dev/null wait $ap 2>/dev/null sleep 3 - [ "$(seen devourerAP)" = 0 ] \ + [ "$(seen devourerAP 02:42:75:05:d6:00)" = 0 ] \ && ok "wpa2: nothing left airing after exit" \ || bad "wpa2: beacon STILL AIRING after exit" } @@ -213,7 +240,7 @@ cell_stop() { local phase=24 env $(apenv) timeout $((phase * 3 + 40)) /tmp/bstop_onair "$phase" \ >"$OUT/stop.log" 2>&1 & - local ap=$! + local ap=$!; KIDS="$KIDS $ap" # Wait for the ARM ITSELF, not for the phase banner. The banner prints # before StartBeacon, and the arm is not instant - it copies a 1600-byte @@ -240,19 +267,19 @@ cell_stop() { wait_arm 0 30 || { bad "stop: never armed"; kill $ap 2>/dev/null; return; } sleep 4 - [ "$(seen mtStopCheck)" = 1 ] && ok "stop: armed - beacon on air" || bad "stop: armed but not scannable" + [ "$(seen mtStopCheck 02:4d:54:53:54:50)" = 1 ] && ok "stop: armed - beacon on air" || bad "stop: armed but not scannable" local n_arms; n_arms=$(armed) local i for i in $(seq 1 60); do grep -q "PHASE 2" "$OUT/stop.log" && break; sleep 1; done sleep 6 - [ "$(seen mtStopCheck)" = 0 ] && ok "stop: stopped - beacon gone" || bad "stop: STILL AIRING after StopBeacon" + [ "$(seen mtStopCheck 02:4d:54:53:54:50)" = 0 ] && ok "stop: stopped - beacon gone" || bad "stop: STILL AIRING after StopBeacon" # The re-arm is the same non-instant operation: wait for the second # "beaconing every", not for the banner that precedes it. wait_arm "$n_arms" 60 || { bad "stop: re-arm never reported"; kill $ap 2>/dev/null; return; } sleep 4 - [ "$(seen mtStopCheck)" = 1 ] && ok "stop: re-armed - beacon back" || bad "stop: re-arm did not air" + [ "$(seen mtStopCheck 02:4d:54:53:54:50)" = 1 ] && ok "stop: re-armed - beacon back" || bad "stop: re-arm did not air" wait $ap 2>/dev/null grep -q "0 failure(s)" "$OUT/stop.log" \ diff --git a/tests/mt7612u_beacon_stop_check.cpp b/tests/mt7612u_beacon_stop_check.cpp index 17d4e3cf..5f463b26 100644 --- a/tests/mt7612u_beacon_stop_check.cpp +++ b/tests/mt7612u_beacon_stop_check.cpp @@ -154,7 +154,18 @@ int main(int argc, char **argv) { /* Explicit, not left to the destructor - the point of this program is that * the teardown path is the thing under test. */ - dev->StopBeacon(); + { + /* Retried like the AP harnesses: the point of this program is the teardown + * path, so leaving its own beacon airing would be the worst possible way + * to end it. */ + bool silenced = false; + for (int i = 0; i < 3 && !silenced; ++i) silenced = dev->StopBeacon(); + if (!silenced) { + std::fprintf(stderr, "FAIL: the final StopBeacon never succeeded - the " + "beacon is still airing\n"); + fails++; + } + } std::printf("\nlocal checks: %d failure(s). The SSID phases are the " "witness's call.\n", fails); return fails ? 1 : 0; diff --git a/tests/ul_trigger_ap.cpp b/tests/ul_trigger_ap.cpp index 3e6ab6ae..671ed475 100644 --- a/tests/ul_trigger_ap.cpp +++ b/tests/ul_trigger_ap.cpp @@ -446,13 +446,17 @@ int main(int argc, char** argv) { (unsigned long long)g_ul.load(), (unsigned long long)g_ul_tb.load(), (unsigned long long)g_data.load(), (unsigned long long)g_sent.load()); - /* The chip beacons AUTONOMOUSLY once StartBeacon arms it, and _exit(0) - * below skips every destructor - so without this the beacon keeps airing - * after the process is gone, until the adapter is power-cycled, and it - * contaminates whatever runs next on that channel. Measured on MT7612U: - * a scan after exit still found the SSID live at 308 ms. Realtek has the - * same exposure - IRadio.h calls it bench-bitten. beacon_update_probe.cpp - * already did this; these did not. */ - if (g_dev) g_dev->StopBeacon(); + /* Retried, and the failure reported. StopBeacon can now genuinely fail (an + * EP0 stall during teardown), IRadio.h says such a failure "must be retried + * ... before its shared port is reused", and `_exit(0)` below means there is + * no destructor coming to try again. A beacon that survives here survives + * the process. */ + if (g_dev) { + bool silenced = false; + for (int i = 0; i < 3 && !silenced; ++i) silenced = g_dev->StopBeacon(); + if (!silenced) + fprintf(stderr, "WARNING: the beacon could not be stopped - it is still " + "airing; power-cycle the adapter\n"); + } _exit(0); } From c5c0c7ed6cb622261dd59ef5eac4f15ff16cf50e Mon Sep 17 00:00:00 2001 From: snokvist Date: Fri, 11 Sep 2026 06:54:09 +0200 Subject: [PATCH 12/13] review: the unwind still leaked on the two paths that move the identity Maintainer review at 424170b. The blocking one is mine again, and it is the same class the previous two commits each claimed to have closed. unwind_identity() reached every failure path in beacon_start EXCEPT the two inside the retarget block itself: if (mt7612u_set_ack_responder(dev, ta)) return -1; if (mt_mac_set_bss_base(dev, ta)) return -1; set_ack_responder writes MT_MAC_ADDR and readback-verifies AFTERWARDS, so its failure returns with the identity already moved; set_bss_base is only reachable once that write landed. Both left the adapter answering for a BSS that was never armed. Compounded by the tri-state I added last round: -1 is contracted as "the hardware was never touched, whatever was airing still is", so StartBeacon leaves _beacon_active alone - reporting the previous beacon as live while MT_MAC_ADDR points at the new ta. Ownership is claimed BEFORE those writes now, so the unwind can reach them. Two more on the update path: a payload mt_beacon_write refuses on length left every slot suppressed with nothing to lower them again - the AP silently off the air while _beacon_active still said otherwise. The length rule is factored out and applied before the guard goes up, and every failure exit lowers it. update accepted a beacon carrying a different addr2. IRadio says the port registers keep the StartBeacon identity, so that airs a beacon matching neither the APC slot nor MT_MAC_ADDR - beacons perfectly, acknowledges nobody. It remembers what start programmed and refuses. The `authorized` toggle: measured rather than argued. CLAUDE.md is right that it is not a cold cycle - VBUS never drops - and the comment claiming "power-cycle" was wrong. But the specific concern, that an autonomously beaconing MAC rides through it, does not hold for this part: armed -> SSID seen host process killed -> SSID STILL seen (the beacon is autonomous) authorized toggle -> SSID gone So the comment now says what it is measured to guarantee, and AP_VBUS= : gives a real cold cycle through uhubctl for anyone who wants one - hub ports only, never an xhci root port, which is where both adapters on this bench sit. Nits: implemented the AP RX-filter DUP clear the doc concluded and the regs.h defines were added for - dropping duplicates hides a station's retransmission, which is the retry evidence that says whether the ACKs are landing. Removed the dead wait_gone(). The beacon-steer trio's comment claimed more than the override delivers: it still returns 0, which IRadio documents as the "no active beacon" answer, so a programmatic caller is no better off and only an operator reading a log is. Rewrote "The gap - the driver primitives to add" as current state; five of its six items ship in this PR and git is the changelog. Verification gap closed: the MBSS mask correction changes MT_MAC_BSSID_DW1 at init on the plain injector path from #422/#423, not just the AP path, so tests/regress.py on two MT7612U - 4/4 cells green, 9100/9231 hits devourer-TX both directions. AP harness after all of it: 14 passed, 0 failed. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj --- docs/mt7612u-ap-mode.md | 123 +++++++++-------------------------- src/mt7612u/Mt7612uRadio.cpp | 13 ++-- src/mt7612u/beacon.cpp | 107 ++++++++++++++++++++++-------- src/mt7612u/internal.h | 4 ++ tests/mt7612u_ap_onair.sh | 48 +++++++++----- 5 files changed, 152 insertions(+), 143 deletions(-) diff --git a/docs/mt7612u-ap-mode.md b/docs/mt7612u-ap-mode.md index 06c173c2..357997ac 100644 --- a/docs/mt7612u-ap-mode.md +++ b/docs/mt7612u-ap-mode.md @@ -139,99 +139,36 @@ beacon *timer* are already in place. The receiver runs (per the #414 tick), and `mt_tx_build()` already produces `[TXWI][802.11]` which is exactly the reserved- page beacon shape. -## The gap — the driver primitives to add - -Each has a direct mt76 recipe. Estimates are the C-library side only. - -1. **`StartBeacon` — load the beacon + arm the MAC beacon function (~70 LOC).** - - Add two register defines our `regs.h` lacks: `MT_BEACON_BASE` (0xc000) and - `MT_BCN_OFFSET(n)` (0x041c + n·4). Everything else is present. - - Program the 5 USB beacon slots: `slot_size = (8192/5) & ~63 = 1600`, - offsets via `MT_BCN_OFFSET` — mt76 `mt76x02_set_beacon_offsets` - (`mt76x02_beacon.c:10`, `N_BCN_SLOTS=5` in `mt76x02_usb_core.c:126`). - - Write `[TXWI][beacon MPDU]` into `MT_BEACON_BASE` with `mt_wr_copy()` — - mt76 `mt76x02_write_beacon`/`mt76x02_mac_set_beacon` - (`mt76x02_beacon.c:24,54`). `mt_tx_build()` already emits that shape. - - Enable: set `MT_BEACON_TIME_CFG` `BEACON_TX | TBTT_EN | TIMER_EN` with - `INTVAL = interval_tu` — mt76 `mt76x02_mac_set_beacon_enable` - (`mt76x02_beacon.c:69`). Point `MT_MAC_BSSID_*` at the AP BSSID, **unicast** - (`0x02…`) — `docs/ap-mode.md`'s hardest-won finding: an I/G-set BSSID makes - the station drop auth before it reaches the air. - - 802.11 sequence numbering: let the MAC number it (the HW-seq path mt76 - uses for beacons) or number it per update in software; `beacon_wire_check` - expects +1 per beacon. - - **Trap — a silent no-transmit.** `MT_BCN_BYPASS_MASK` (0x108c) is - **inverted**. `0xffff`, the value `mt76x02_init_beacon_config()` writes, is - "suppress every slot" — the guard mt76 raises *during* an update ("prevent - corrupt transmissions during update"). A slot only airs once its bit is - **cleared**, which mt76 does after loading the page: - `0xff00 | ~(0xff00 >> beacon_data_count)` (`mt76x02_usb_core.c:223`, - identically `mt76x02_mmio.c:43`); for one beacon in slot 0 that is - `0xffffff7f`. Leave it at `0xffff` and the beacon timer runs, the TSF - advances and every register reads correct — while nothing reaches the air. - - **Trap — the beacon TXWI needs two extra bits.** `MT_TXWI_FLAGS_TS` (BIT 3; - MAC inserts the TSF timestamp) and `MT_TXWI_ACK_CTL_NSEQ` (MAC assigns the - sequence number), exactly as `mt76x02_mac_write_txwi()` sets them for - beacon/probe-resp subtypes. Without them the beacon airs with a frozen - timestamp and a constant sequence number. - - **Trap — `regs.h` had the MBSS masks two bits high.** `MBSS_MODE`, - `MBEACON_N` and `MBSS_LOCAL_BIT` were transcribed as 19:18 / 22:20 / 23 - instead of mt76's 17:16 / 20:18 / 21. Harmless while the port only - injected (only `mac_setaddr` uses them, and beacons were never generated), - but it programmed MBSS_MODE=4 (invalid) and MBEACON_N=15. Corrected; - `MT_MAC_BSSID_DW1` now reads `0x003f____`, matching mt76 bit for bit. - -2. **`StopBeacon` (~10 LOC).** Clear `BEACON_TX | TBTT_EN | TIMER_EN`. Note the - `IRadio` contract: the chip beacons autonomously, so a session that ends - without a power-cycle **must** call this or the beacon contaminates the next - run (`src/IRadio.h:407-413`). - -3. **Per-station / group key install (~50 LOC).** `MT_WCID_KEY(idx)` + - `MT_WCID_ATTR` PKEY_MODE/PAIRWISE for pairwise, `MT_SKEY` + `MT_SKEY_MODE` - for the GTK — mt76 `mt76x02_mac_wcid_set_key` / `mac_shared_key_setup` - (`mt76x02_mac.c`). The cipher enum (`MT76X02_CIPHER_*`) is small. This buys - **hardware CCMP**; the software CCMP in `ap_wpa2.cpp` still works as the - fallback/portable path. - -4. **AP RX filter — one line, not ~15 LOC.** Measured: the managed default - `0x00015f97` already leaves `OTHER_BSS`, `BCAST` and `MCAST` **undropped**, - so a probe request with a wildcard BSSID and auth addressed to us both - arrive unchanged; mt76 clears `OTHER_BSS` for every mode too - (`mt76x02_configure_filter`), and defines no AP-specific filter. The single - change an AP wants is clearing `DUP` (`0x00015f17`) so a station's - retransmission stays visible — dropping duplicates hides exactly the - retry evidence that tells you whether your ACKs are landing. - -5. **`SetAckResponder` — covered by address match. OPEN ITEM NOW RESOLVED.** - On MT the MAC auto-ACKs frames matching the programmed `MT_MAC_ADDR`, so - pointing it at the BSSID (done in `mac_setaddr`) is the ACK responder. - Confirmed on hardware: **there is no AP op-mode/net-type register on this - part.** mt76 sets none — its only AP-specific work in `mt76x02_sta_add` is a - *software* PS flag (`MT_WCID_FLAG_CHECK_PS`) plus a GTK restriction — and a - real station's auth arrived at **retry=0** with nothing but address match + - beacon enable. Address match + beacon *is* the AP. - - **But one register the gap analysis missed:** `mac_setaddr()` zeroes all - eight APC BSSID slots, which is right for an injector and wrong for an AP. - The MAC matches the BSS against `MT_MAC_APC_BSSID_L/H(idx)` for the slot - the MBSS index selects, so an AP must publish its BSSID there — - `mt76x02_mac_set_bssid()` (`mt76x02_mac.c`), 4 lines. Without it the MAC - matches nothing for the BSS. - - The simplest consistent choice is to make the AP BSSID the device's own - MAC: `mac_setaddr()` already programs it into `MT_MAC_ADDR` (what the MAC - ACKs against) and `MT_MAC_BSSID`, so only the APC slot is left to write. - Advertising any *other* BSSID in the beacon leaves a station addressing - auth to an address the MAC does not answer for. - -6. **`UpdateBeaconPayload` (~10 LOC, optional).** Re-write the reserved page in - place. Only needed for dynamic beacon content (below). - -### The two integration layers - -- **The mt7612u C library** gains items 1–4 above (~150 LOC, all with recipes). -- **`Mt7612uRadio`** (the wrapper from the integration PR) exposes them as - `StartBeacon`/`StopBeacon`/`UpdateBeaconPayload`/`SetAckResponder` over the C - ABI, so the **existing** C++ AP harnesses in `tests/` run unchanged. No AP - logic is written — it already exists. +## What is still missing: hardware key install + +One item, and it is the only thing between this and a fully hardware-accelerated +AP. Everything else in the original gap list — the beacon load and arm, the APC +address match, the TSF/sequence offload, the RX filter — is implemented and on +air; git has the history. + +The MAC has real per-station key hardware, and none of it is reached: + +- `MT_WCID_KEY(idx)` is not even defined in this tree. `MT_WCID_ATTR`, + `MT_SKEY` and `MT_SKEY_MODE` are, and are zeroed at init + (`wcid_and_key_clear()` in `init.cpp`) — the "encrypt nothing" configuration + an injector wants, and the same registers a key install writes. +- mt76's recipe is small: `mt76x02_mac_wcid_set_key` is ~40 lines of + `wr_copy` + `rmw_field` over primitives this subtree already has, plus + `mt76x02_mac_shared_key_setup` for the GTK. +- **There is no per-frame encrypt flag.** TX encryption is selected entirely by + `txwi->wcid` pointing at a WCID whose `ATTR.PKEY_MODE` is set, and this + backend hardcodes `wcid = 0xff` (the no-station index). So it is all-or- + nothing per station: install a key and every frame to that WCID is encrypted + in hardware; you cannot mix with software CCMP on the same peer. +- On RX the hardware sets `MT_RXINFO_DECRYPT` and **strips IV, MIC and MMIC**, + so `mt_rx_parse()` would need to handle a changed frame layout, not just + report a flag. + +The blocker is not the driver. `IRadio` has no key surface at all — no install, +no cipher enum — because devourer does CCMP in software on every backend, which +is a reasonable choice when only Jaguar1 has the Realtek TX-descriptor security +field. Reaching MediaTek's crypto therefore means adding an interface member +most backends cannot implement, which is a design decision rather than a port. ## Limitations and shortfalls of a userspace AP on MediaTek — and workarounds diff --git a/src/mt7612u/Mt7612uRadio.cpp b/src/mt7612u/Mt7612uRadio.cpp index 37307d6b..a2acf9ab 100644 --- a/src/mt7612u/Mt7612uRadio.cpp +++ b/src/mt7612u/Mt7612uRadio.cpp @@ -906,11 +906,14 @@ bool Mt7612uRadio::UpdateBeaconPayload(const uint8_t *beacon, size_t len) { return mt7612u_beacon_update(_dev, beacon, len) == 0; } -/* The beacon-steer trio. Not implemented, and REFUSING QUIETLY IS THE PROBLEM: - * IRadio's default returns 0, which means "applied a 0 us shift" and is - * indistinguishable from "this backend cannot steer". tests/beacon_steer_check - * and the PTP harnesses run against any backend and would read a clean zero. - * Every other unsupported knob here says so out loud; these were the exception. +/* The beacon-steer trio. Not implemented. + * + * These log and still return 0, which is what IRadio documents as the "no + * active beacon" answer - so a PROGRAMMATIC caller cannot tell "cannot steer" + * from "steered by nothing" any better than before. The log is for the + * operator reading a harness run, and that is all it buys; saying so because + * every other unsupported knob here refuses in a way a caller can act on, and + * these three cannot without an interface change. * * Steering needs a pre-TBTT interrupt to re-time against, which this static * reserved-page path does not have - mt76's own steering lives in diff --git a/src/mt7612u/beacon.cpp b/src/mt7612u/beacon.cpp index 85143437..f19cc864 100644 --- a/src/mt7612u/beacon.cpp +++ b/src/mt7612u/beacon.cpp @@ -57,6 +57,20 @@ void mt_beacon_init(struct mt7612u_dev *d) mt_beacon_set_offsets(d); } +/* The slot has to hold the body, its TXWI and the DMA header. Factored out so + * mt7612u_beacon_update() can apply it BEFORE it suppresses the slot - a + * refusal after the guard is up leaves the AP off the air. Returns non-zero + * when the frame does not fit, and says so. */ +static int len_fits_slot(size_t len) +{ + if (len + MT_TXWI_LEN + MT_DMA_HDR_LEN > MT_BCN_SLOT_SIZE) { + ERR("beacon %zu B + TXWI exceeds the %d B slot", len, + (int)MT_BCN_SLOT_SIZE); + return -1; + } + return 0; +} + /* * mt76x02_mac_set_beacon(): write [TXWI][beacon MPDU] into slot 0. * @@ -72,14 +86,10 @@ int mt_beacon_write(struct mt7612u_dev *d, const void *frame, size_t len, uint8_t buf[MT_BCN_SLOT_SIZE]; int total; - /* The slot must hold the TXWI and the DMA header as well as the body; - * mt_tx_build() enforces the tighter bound anyway, but from here the - * error names the beacon rather than reporting a bad frame length. */ - if (len + MT_TXWI_LEN + MT_DMA_HDR_LEN > MT_BCN_SLOT_SIZE) { - ERR("beacon %zu B + TXWI exceeds the %d B slot", len, - (int)MT_BCN_SLOT_SIZE); + /* Same rule mt7612u_beacon_update() applies before it suppresses the slot; + * kept here too because beacon_start reaches this directly. */ + if (len_fits_slot(len)) return -1; - } total = mt_tx_build(d, buf, sizeof buf, frame, len, rate, 0xff, MT_TXOPT_BEACON, 0, 0); if (total < 0) @@ -343,18 +353,23 @@ int mt7612u_beacon_start(struct mt7612u_dev *dev, const void *buf, size_t len, * Both registers because the index below is derived from the MBSS base, * not from MT_MAC_ADDR. */ { + /* Ownership is claimed BEFORE the writes, not after them. Both calls + * below move MT_MAC_ADDR and can then fail - set_ack_responder writes + * the register and readback-verifies afterwards, and set_bss_base can + * only be reached once that write landed - so a failure here leaves + * the identity moved. Claiming first is what lets the unwind put it + * back; claiming after meant these two paths leaked it, and returned + * -1, which tells the caller the hardware was never touched. */ const int was_taken = dev->ack_saved; - if (mt7612u_set_ack_responder(dev, ta)) - return -1; - if (mt_mac_set_bss_base(dev, ta)) - return -1; - /* Only claim ownership if nobody else already held the identity: a - * caller who armed a responder first owns the saved factory address, - * and restoring it on beacon stop would silently disarm them. */ if (!was_taken) { dev->beacon_took_identity = 1; took = 1; } + if (mt7612u_set_ack_responder(dev, ta)) + goto fail_pre; + if (mt_mac_set_bss_base(dev, ta)) + goto fail_pre; + memcpy(dev->beacon_ident, ta, 6); } /* @@ -390,6 +405,18 @@ int mt7612u_beacon_start(struct mt7612u_dev *dev, const void *buf, size_t len, */ before = mt_io_errors(dev); + /* + * The one RX-filter change an AP wants, and the reason is evidence rather + * than throughput: DUP drops retransmissions, and a station's retry is + * exactly how you learn whether your ACKs are landing. An auth arriving + * with FC Retry set means the MAC did not acknowledge the first one; drop + * duplicates and that signal disappears. mt76 leaves OTHER_BSS, BCAST and + * MCAST undropped in every mode, so a wildcard probe request already + * reaches us and nothing else here needs changing. Cleared in place rather + * than rewriting a copied literal, so it cannot drift from the default. + */ + mt_clear(dev, MT_RX_FILTR_CFG, MT_RX_FILTR_CFG_DUP); + if (mt_ap_set_bssid(dev, idx, bssid)) goto fail_pre; mt_beacon_init(dev); @@ -417,28 +444,52 @@ int mt7612u_beacon_update(struct mt7612u_dev *dev, const void *buf, size_t len) struct mt7612u_tx_rate rate; const uint8_t *mpdu = NULL; size_t mpdu_len = 0; + int rc; if (!dev) return -1; if (beacon_split(buf, len, &mpdu, &mpdu_len, &rate)) return -1; - /* Suppress the slot for the duration of the copy. mt_wr_copy() spans many - * 64-byte EP0 transactions, so a TBTT landing mid-copy would air a TORN - * beacon - leading bytes new, trailing bytes old. mt76 brackets the same - * write for the same reason ("Prevent corrupt transmissions during - * update", mt76x02_usb_core.c). mt_beacon_write() lowers the guard again - * on its way out, which is why this is the only half needed here. - * - * Still no mt_beacon_init() and no set_enable(): the engine is armed, and - * re-initialising it would clear the timer bits mid-flight. */ + /* + * Refuse a beacon that would change the identity. IRadio says addr2/addr3 + * are not changeable mid-flight and that the port registers keep the + * StartBeacon identity - so loading one with a different BSSID airs a + * beacon that no longer matches the programmed APC slot or MT_MAC_ADDR. + * It beacons perfectly and acknowledges nobody, which is the failure the + * start path goes to some length to prevent. + */ + if (memcmp(mpdu + 10, dev->beacon_ident, 6) != 0) { + ERR("beacon: an in-place update cannot change addr2 - the port " + "identity keeps what beacon_start programmed"); + return -1; + } + + /* + * Everything that can refuse this payload runs BEFORE the slot is + * suppressed. mt_beacon_write() checks a caller-controlled length, and + * checking it after the guard was up meant a rejected payload left every + * slot suppressed with no path to lower them again - the AP silently off + * the air while _beacon_active still said otherwise. + */ + if (len_fits_slot(mpdu_len)) + return -1; + if (mt_wr_chk(dev, MT_BCN_BYPASS_MASK, 0xffff)) { - /* Checked: if the guard never lands, the copy below runs against a - * LIVE slot and a TBTT mid-copy airs a torn beacon - which is the one - * outcome this bracket exists to prevent, so failing to raise it must - * not be the quiet path. */ + /* If the guard never lands the copy below runs against a LIVE slot, + * and a TBTT mid-copy airs a torn beacon - the one outcome this + * bracket exists to prevent, so failing to raise it is not the quiet + * path. Nothing to unwind: the mask is whatever it already was. */ ERR("beacon: could not suppress the slot for an in-place update"); return -1; } - return mt_beacon_write(dev, mpdu, mpdu_len, &rate); + + rc = mt_beacon_write(dev, mpdu, mpdu_len, &rate); + if (rc) { + /* Lower the guard again rather than leaving the AP dark. A failed + * update should cost the update, not the beacon. */ + mt_wr(dev, MT_BCN_BYPASS_MASK, 0xff00u | ~(0xff00u >> 1)); + return rc; + } + return 0; } int mt7612u_beacon_stop(struct mt7612u_dev *dev) diff --git a/src/mt7612u/internal.h b/src/mt7612u/internal.h index d227ab34..839ca2a7 100644 --- a/src/mt7612u/internal.h +++ b/src/mt7612u/internal.h @@ -219,6 +219,10 @@ struct mt7612u_dev { * caller had already armed an ACK responder, because then the identity is * theirs and restoring would silently disarm it. */ int beacon_took_identity; + /* The addr2 mt7612u_beacon_start() programmed, so an in-place update can + * refuse a beacon that would change it - the port registers keep the + * start identity, so a changed BSSID airs and matches nothing. */ + uint8_t beacon_ident[6]; struct mt_async *a; FILE *wrlog; FILE *mculog; diff --git a/tests/mt7612u_ap_onair.sh b/tests/mt7612u_ap_onair.sh index 5b05d251..63c0e704 100755 --- a/tests/mt7612u_ap_onair.sh +++ b/tests/mt7612u_ap_onair.sh @@ -28,7 +28,8 @@ # sudo tests/mt7612u_ap_onair.sh # sudo AP_SYSFS=5-1 STA_SYSFS=2-1 CH=36 tests/mt7612u_ap_onair.sh open # -# Env: AP_SYSFS, STA_SYSFS, CH, PSK, FW_DIR, SECS. Cells: open|wpa2|stop|all. +# Env: AP_SYSFS, STA_SYSFS, CH, PSK, FW_DIR, SECS, AP_VBUS (hubloc:port for a +# real VBUS cold cycle via uhubctl; hub ports only). Cells: open|wpa2|stop|all. set -u ROOT="$(cd "$(dirname "$0")/.." && pwd)" @@ -67,20 +68,38 @@ cleanup() { reap [ -n "${STA_IF:-}" ] && { ip addr flush dev "$STA_IF" 2>/dev/null iw dev "$STA_IF" disconnect 2>/dev/null; } - # The MAC beacons autonomously. If a cell died before its teardown, only a - # port power-cycle is certain to silence it - and leaving one airing poisons - # the next run of this very script. + # The MAC beacons autonomously, so a cell that died before its teardown can + # leave one airing into the next cell. What silences it: # - # Confirmed against the VID:PID first. This runs as root and writes a - # deauthorize to a path the caller supplied; a stale or mistyped AP_SYSFS - # would otherwise yank whatever else is plugged there - someone's keyboard, - # a disk mid-write. + # An `authorized` toggle is NOT a power cycle - VBUS never drops and chip + # state survives, which is why this tree's CLAUDE.md warns against calling it + # cold. But on this part it does end the beacon, measured rather than assumed: + # armed -> SSID seen; host process killed -> SSID STILL seen (the beacon is + # autonomous); toggle -> SSID gone. The re-enumeration is what stops the + # timer. That is all this needs to guarantee between cells, and it is all it + # claims. + # + # For a genuine cold cycle, set AP_VBUS=: and it uses uhubctl + # the way tests/regress.py's REGRESS_VBUS_MAP does. Per-port-switchable HUB + # ports only - never an xhci root port, which has wedged a device here badly + # enough to need the machine powered off. + # + # Either way, confirmed against the VID:PID first: this runs as root and + # writes to a path the caller supplied, and a stale AP_SYSFS would otherwise + # yank whatever else is plugged there. if [ "$(cat "/sys/bus/usb/devices/$AP_SYSFS/idVendor" 2>/dev/null)" = "0e8d" ] && [ "$(cat "/sys/bus/usb/devices/$AP_SYSFS/idProduct" 2>/dev/null)" = "7612" ]; then - echo 0 > "/sys/bus/usb/devices/$AP_SYSFS/authorized" 2>/dev/null - sleep 2 - echo 1 > "/sys/bus/usb/devices/$AP_SYSFS/authorized" 2>/dev/null - sleep 3 + if [ -n "${AP_VBUS:-}" ]; then + uhubctl -l "${AP_VBUS%%:*}" -p "${AP_VBUS##*:}" -a off >/dev/null 2>&1 + sleep 4 + uhubctl -l "${AP_VBUS%%:*}" -p "${AP_VBUS##*:}" -a on >/dev/null 2>&1 + sleep 5 + else + echo 0 > "/sys/bus/usb/devices/$AP_SYSFS/authorized" 2>/dev/null + sleep 2 + echo 1 > "/sys/bus/usb/devices/$AP_SYSFS/authorized" 2>/dev/null + sleep 3 + fi fi } trap cleanup EXIT INT TERM @@ -259,11 +278,6 @@ cell_stop() { for i in $(seq 1 "$2"); do [ "$(armed)" -gt "$1" ] && return 0; sleep 1; done return 1 } - wait_gone() { # the log line that says StopBeacon ran, then a settle - local i - for i in $(seq 1 "$1"); do grep -q "PHASE 3" "$OUT/stop.log" && return 0; sleep 1; done - return 1 - } wait_arm 0 30 || { bad "stop: never armed"; kill $ap 2>/dev/null; return; } sleep 4 From 6465ab1fc3be7f97715340e264296a7cfe9fc6dd Mon Sep 17 00:00:00 2001 From: snokvist Date: Fri, 11 Sep 2026 12:46:48 +0200 Subject: [PATCH 13/13] review: close the retry the last fix could not reach, and guard addr3 Five fixes. Two of them are defects this branch introduced - one last round, one in the first draft of this one. 1. mt7612u_clear_ack_responder() keeps ack_saved when its own writes fail. unwind_identity()'s comment claimed the flags survive a failed restore "so Stop()'s documented retry has something left to retry". True for the MBSS half; false for MT_MAC_ADDR. clear_ack_responder() cleared ack_saved unconditionally and early-returns on !ack_saved, so attempts two and three were no-ops against a still-leaked port MAC - the retry the comment promised could not reach the register it was written for. It now keeps the flag on an mt_io_errors() delta, same idiom as the callers. Both halves of the retry have to agree before either works. 2. mt7612u_beacon_update() compares addr3 as well as addr2. IRadio.h:394-396 names both; only addr2 was checked. addr3 is the half programmed into the APC slot, so an update that moved only addr3 aired a BSSID the slot does not hold - the deaf AP the guard exists to prevent, through the guard. beacon_ident is 12 bytes now and one memcpy covers the adjacent pair; beacon_split() already requires a 24-byte header, so the range is in bounds by construction. 3. set_ack_responder() keeps its MT_AUTO_RSP_EN re-assert out of the I/O accumulator. Introduced by fix 1's sibling change - moving beacon_start's io_err snapshot above the identity writes, which was right for MT_MAC_ADDR_DW1 and wrong for this. mac_reset() already sets MT_AUTO_RSP_EN (init.cpp:174), so the mt_set() here is a documented no-op; but mt_set() is mt_rmw(), which on a failed READ bumps io_err and skips its write, and its return is ignored because the readback below is the real check. One transient EP0 read stall on MT_AUTO_RSP_CFG therefore tore down a verified-good arm: -2 from beacon_start, _beacon_active cleared, for a beacon that was on the air. 4. -1 no longer escapes from inside the identity block. The tri-state contracts -1 as "nothing was touched, whatever was airing still is", but the only two paths returning it were reachable *after* set_ack_responder had written MT_MAC_ADDR. The unwind restores the FACTORY address - the only one saved anywhere - so a failed re-arm over a live beacon retracted an identity that beacon never had while its page and timers kept airing it, and the caller was told nothing changed. There is no atomic re-arm to offer: one MT_MAC_ADDR, one save slot, and the previous occupant is not in it. Those exits go to fail_post now, so -1 belongs to the input refusals above the first hardware write and means what it says. 5. The RX-filter comment attributes DUP-clear correctly. It said "this backend's receive default is already what an AP wants". The default is 0x00015f97 (initvals.h, init.cpp:290) and MT_RX_FILTR_CFG_DUP is SET in it. mt7612u_set_monitor_rx() is what leaves it clear, deliberately, and every AP path reaches it via StartRxLoop; a TX-only InitWrite does not and has no receiver to count retries with. Also drops MT_RX_FILTR_CFG_MCAST and _BCAST, which this branch added and nothing uses. Tested: 63/63 ctest, subtree check green, and tests/mt7612u_ap_onair.sh 14/14 on ch36 and 14/14 on ch6. The update guard has a positive control plus two negative arms against a live beacon in mt7612u_beacon_stop_check.cpp - an unchanged payload must be accepted, so a guard that refused everything would not pass. Only the addr3 arm discriminates against the pre-fix code; the addr2 arm is a regression check and the test says so. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj --- docs/mt7612u-ap-mode.md | 199 +++++++++++++------------ src/mt7612u/beacon.cpp | 219 ++++++++++++++++++++++------ src/mt7612u/caps.cpp | 40 ++++- src/mt7612u/internal.h | 14 +- src/mt7612u/regs.h | 2 - tests/mt7612u_ap_onair.sh | 2 +- tests/mt7612u_beacon_stop_check.cpp | 43 ++++++ 7 files changed, 367 insertions(+), 152 deletions(-) diff --git a/docs/mt7612u-ap-mode.md b/docs/mt7612u-ap-mode.md index 357997ac..306a35b4 100644 --- a/docs/mt7612u-ap-mode.md +++ b/docs/mt7612u-ap-mode.md @@ -1,31 +1,29 @@ # MT7612U as a fully-userspace access point -**Status: it works.** devourer's own AP harnesses run against this part -unmodified — beacon, probe, auth, assoc, and the ARP/ICMP data plane — with a -real Linux station associated. `StartBeacon`, `UpdateBeaconPayload` and +**Status: it works.** devourer's own AP harnesses run against this part with no +change to their AP logic — beacon, probe, auth, assoc, and the ARP/ICMP data +plane — with a real Linux station associated. `StartBeacon`, `UpdateBeaconPayload` and `StopBeacon` are implemented on `Mt7612uRadio`, so nothing in -`tests/ap_responder.cpp` or `tests/ap_wpa2.cpp` branches on the backend. (Both -carry one MediaTek-specific *comment* now, explaining why they silence the -beacon before `_exit`; no code depends on it.) +`tests/ap_responder.cpp` or `tests/ap_wpa2.cpp` branches on the backend. Their AP +logic is untouched; each gained a dozen lines that silence the beacon before +`_exit`, and nothing in that is MediaTek-specific — the beacon is +hardware-autonomous on the Realtek parts too. -**WPA2-PSK works too.** `tests/ap_wpa2.cpp`, also unmodified, completes the +**WPA2-PSK works too.** `tests/ap_wpa2.cpp`, its AP logic equally untouched, completes the 4-way handshake against a real `wpa_supplicant` station and carries encrypted traffic. It needs the same five `IRadio` methods as the open-network harness - `InitWrite`, `StartBeacon`, `StartRxLoop`, `send_packet`, `StopBeacon` - and no others. CCMP is done in software in the harness, so no key API is involved; that is a separate point from the method count. -What is NOT done: **hardware** CCMP. `MT_WCID_KEY` and `MT_SKEY` are untouched, -so the claim below that hardware crypto is a capability *gain* on this part -remains unmeasured — what is measured is that the software path devourer -already had works here. Key install is the one item of the original gap list -that is still open. +What is NOT done: **hardware** CCMP. `MT_WCID_KEY` is absent and the key path +is unreached (`MT_SKEY` is defined, and zeroed at init), so whether hardware +crypto is a capability *gain* on this part is unmeasured — what is measured is +that the software path devourer already had works here. See "What is still +missing" below. File:line references below are to the merged subtree (`src/mt7612u/`, all -`.cpp` since the C++ migration) and to `reference/mt76 @ be5ce79`. Some of the -citations in "The gap" section still name the pre-migration `.c` filenames and -pre-merge line numbers; they are kept because the reasoning is still correct, -but do not expect them to resolve. +`.cpp` since the C++ migration) and to `reference/mt76 @ be5ce79`. ## Verified on hardware (2026-09-08, MT7612U at USB 2-1) @@ -50,7 +48,7 @@ driver work. ## Verified through IRadio (2026-09-10) — devourer as the AP The section above is the bring-up gates driving the C library directly. This -one is devourer itself: `tests/ap_responder.cpp`, unmodified, built against +one is devourer itself: `tests/ap_responder.cpp`, built unchanged against `libdevourer.a` and pointed at an MT7612U. A second MT7612U on the kernel `mt76x2u` driver is the station. ch36, `iw reg set SE`. @@ -93,82 +91,80 @@ one is devourer itself: `tests/ap_responder.cpp`, unmodified, built against explicitly now, and `tests/mt7612u_beacon_stop_check.cpp` is what actually exercises the transition. -## The claim, and why it holds - -The gap between "MT7612U injector" (what the subtree is) and "MT7612U userspace -AP" is small — a few hundred lines of C in the backend, and no new AP logic at -all. Two reasons: - -1. **devourer already has the AP brain, and it is backend-agnostic.** The - probe/auth/assoc responder, the DHCP/ARP/ICMP data plane, and the WPA2 4-way - handshake with software CCMP all live in `tests/` (`ap_responder.cpp`, - `ap_wpa2.cpp`, `probe_responder.cpp`, `beacon_*.cpp`), driven entirely - through the `IRadio` interface — `StartBeacon` + the RX callback + - `send_packet`. `docs/ap-mode.md` documents a complete open and WPA2-PSK AP - validated against real Linux stations on this stack. None of it is - Realtek-specific; it works against any backend that implements the beacon - and ACK primitives. - -2. **MediaTek's MAC offloads in hardware exactly what an AP needs most.** The - beacon is auto-transmitted from a reserved page at each TBTT, TSF-stamped by - the MAC; ACK is SIFS-timed by the MAC against the programmed address; - 802.11 sequence numbering is a MAC function; and CCMP has real per-station - key hardware (`MT_WCID_KEY`, `MT_SKEY`). On Realtek, devourer does CCMP in - **software** because the security TX-desc field is absent on most - generations (`docs/ap-mode.md`: "only Jaguar1 has - `SET_TX_DESC_SEC_TYPE_8812`"). So on MT the encrypted data plane, and GTK - rekey which is explicitly out of scope on Realtek, become **hardware** — - this part is a capability *gain*, not a gap. - -## What the MT7612U backend already has - -Verified in the merged subtree: - -| AP need | present today | where | -|---|---|---| -| Port MAC + BSSID programmed | yes — `MT_MAC_ADDR_DW0/1`, `MT_MAC_BSSID_DW0/1`, MBSS_MODE=3, MBEACON_N | `init.c:206‑216` (`mac_setaddr`) | -| Station table (WCID) | yes — `mt_wcid_setup(idx, mac)` writes `MT_WCID_ATTR` + address; all zeroed at init | `tx.c:95`, `init.c:236` | -| Crypto key slots | the shared-key store is present and zeroed at init (`MT_SKEY`, `MT_SKEY_MODE`, `src/mt7612u/init.cpp` `wcid_and_key_clear()`). The per-station key store is NOT defined in this tree - `MT_WCID_KEY` does not exist here, which is part of why hardware CCMP is unreached | -| ACKed unicast TX | yes — `no_ack=0` sets `MT_TXWI_ACK_CTL_REQ`; BA-window field present | `tx.c:164‑167` | -| Beacon-interval timer regs | defined — `MT_BEACON_TIME_CFG` INTVAL/TIMER_EN/TBTT_EN/BEACON_TX, `MBEACON_N` | `regs.h:176‑180,169` | -| RX filter control | yes — managed default `0x00015f97`, monitor clears to error-only | `init.c:278,494‑509` | -| Register block copy | yes — `mt_wr_copy()` for reserved-page writes | used in `init.c` | - -So the addressing, the station table, the crypto slots, the ACK path and the -beacon *timer* are already in place. The receiver runs (per the #414 tick), and -`mt_tx_build()` already produces `[TXWI][802.11]` which is exactly the reserved- -page beacon shape. +## Three findings that shaped the implementation + +Not a status list — these are the things that were not obvious from mt76 and +that the code now depends on. + +1. **On this MAC, "the AP" is an address match plus a beacon; there is no + responder register.** The immediate-response engine ACKs frames whose + address 1 matches `MT_MAC_ADDR_DW0/DW1`, gated by `MT_AUTO_RSP_EN` — which + init already leaves on (`mac_reset()` writes `MT_AUTO_RSP_CFG = 0x13`, + `init.cpp:174`, reached from `mt_init_hardware()` at `:408`). So arming + an ACK responder means *retargeting the port identity*, and closing the gate + does not stop a die that matches on identity — restoring the address does. + The consequence is that `MT_MAC_ADDR` has two users, the beacon and + `SetAckResponder`, sharing one register and one save slot; ownership belongs + to whoever wrote last, and both paths have to hand it over explicitly. + +2. **The APC BSSID slot index is derived from the address, and getting it wrong + is silent.** Under `MBSS_MODE=3` mt76 computes + `idx = 1 + (((macaddr[0] ^ addr[0]) >> 2) & 7)` for a locally-administered + address and 0 otherwise (`mt76x02_util.c:310`) — *after* `mt76x02_mac_setaddr` + has moved both the port MAC and the MBSS base, so its XOR is zero by + construction. A beacon that retargets only `MT_MAC_ADDR` leaves the base at + the factory address, the hardware derives a different slot, and the AP + beacons perfectly and acknowledges nobody. `mt_mac_set_bss_base()` + (`beacon.cpp`) is what makes the mt76 identity hold here. The masks + themselves were also transcribed two bits high in an earlier draft + (`regs.h:170-179`); `MBSS_MODE=4` is not a valid mode. + +3. **The RX filter's *default* is not what an AP wants — `set_monitor_rx` is.** + The init value is `0x00015f97` (`init.cpp:290`), and `MT_RX_FILTR_CFG_DUP` + is set in it. What leaves DUP clear is `mt7612u_set_monitor_rx()` + (`init.cpp:560-569`), deliberately, because duplicate suppression hides the + retransmissions an ACK-responder test counts — a station's retry with the FC + Retry bit set is exactly how you learn whether your ACKs are landing, and + `auth … retry=0` in the on-air harness is that evidence. Every AP path + reaches it (`StartRxLoop` calls it, and an AP must receive); a TX-only + consumer does not, and has no receiver to count retries with anyway. The + beacon path therefore touches the filter in neither direction. ## What is still missing: hardware key install -One item, and it is the only thing between this and a fully hardware-accelerated -AP. Everything else in the original gap list — the beacon load and arm, the APC -address match, the TSF/sequence offload, the RX filter — is implemented and on -air; git has the history. - -The MAC has real per-station key hardware, and none of it is reached: - -- `MT_WCID_KEY(idx)` is not even defined in this tree. `MT_WCID_ATTR`, - `MT_SKEY` and `MT_SKEY_MODE` are, and are zeroed at init - (`wcid_and_key_clear()` in `init.cpp`) — the "encrypt nothing" configuration - an injector wants, and the same registers a key install writes. -- mt76's recipe is small: `mt76x02_mac_wcid_set_key` is ~40 lines of - `wr_copy` + `rmw_field` over primitives this subtree already has, plus - `mt76x02_mac_shared_key_setup` for the GTK. -- **There is no per-frame encrypt flag.** TX encryption is selected entirely by - `txwi->wcid` pointing at a WCID whose `ATTR.PKEY_MODE` is set, and this - backend hardcodes `wcid = 0xff` (the no-station index). So it is all-or- - nothing per station: install a key and every frame to that WCID is encrypted - in hardware; you cannot mix with software CCMP on the same peer. -- On RX the hardware sets `MT_RXINFO_DECRYPT` and **strips IV, MIC and MMIC**, - so `mt_rx_parse()` would need to handle a changed frame layout, not just - report a flag. - -The blocker is not the driver. `IRadio` has no key surface at all — no install, -no cipher enum — because devourer does CCMP in software on every backend, which -is a reasonable choice when only Jaguar1 has the Realtek TX-descriptor security -field. Reaching MediaTek's crypto therefore means adding an interface member -most backends cannot implement, which is a design decision rather than a port. +The MAC has real per-station key hardware and none of it is reached. This is +the last item, but it is more than one register: + +- **Two registers are absent from this tree.** `MT_WCID_KEY` and `MT_WCID_IV` + are both undefined here; `mt76x02_mac_wcid_set_key` writes both + (`mt76x02_mac.c`). `MT_WCID_ATTR`, `MT_SKEY` and `MT_SKEY_MODE` *are* defined + and are zeroed at init by `wcid_and_key_clear()` — the "encrypt nothing" + configuration an injector wants, and the same registers a key install writes. +- **The per-frame encrypt gate is set the wrong way for crypto.** There IS such + a flag — `MT_TXD_INFO_WIV` (`regs.h`) — and `mt_tx_build()` sets it + unconditionally, meaning "no hardware IV insertion, this frame is not + encrypted". mt76 gates it on whether the WCID has a key + (`mt76x02_usb_core.c`, `mt76x02_txrx.c`: `!wcid || hw_key_idx == 0xff || + sw_iv`). A key install has to make WIV conditional too, not just fill the key + slots. +- **TX selects encryption by WCID, so it is per-station and all-or-nothing.** + `txwi->wcid` chooses the key; `mt_tx_build()` is called with `0xff` (the + no-station index) from every library path. `tools/bringup.cpp` does pass a + real WCID for its rate-LUT gate, so the plumbing exists — but no + `Mt7612uRadio` path or AP harness installs a station. +- **RX needs real work, not a flag.** The hardware strips the MIC and MMIC, but + **not** the IV/PN: mt76 removes that in the driver using `MT_RXINFO_PN_LEN`, + and deliberately does not on a fragment. `MT_RXINFO_PN_LEN` is already + defined here and unused; `MT_RXINFO_DECRYPT` is not defined at all. + +The blocker is still not the driver. `IRadio` has no key surface at all — no +install, no cipher enum — because devourer does CCMP in software on every +backend, which is reasonable when only Jaguar1 has the Realtek TX-descriptor +security field. The maintainer's guidance (PR #424) is to keep it that way for +now and design the key surface against two backends rather than one: a crypto +key interface is a much larger contract than a feature flag — key lifetime, GTK +vs PTK, rekey, who owns the replay counter — and expensive to undo once callers +exist. ## Limitations and shortfalls of a userspace AP on MediaTek — and workarounds @@ -223,19 +219,30 @@ clients and BlockAck reordering are where a userspace MT AP stops being worth it, and both have clean "don't support it" workarounds for the return-video use case. -## End-to-end verification (reuse, don't rebuild) +## End-to-end verification -devourer's existing AP checks are backend-agnostic and become the acceptance -suite once the MT backend implements the primitives: +devourer's existing AP checks are backend-agnostic, so they are the acceptance +suite for this backend with no AP-logic change: - `tests/beacon_wire_check.cpp` — beacon frame control, +1 seq per beacon, live TSF. -- `tests/beacon_kernel_scan.sh` — a real `rtw88` station's `iw scan` lists the AP. +- `tests/beacon_kernel_scan.sh` — a real station's `iw scan` lists the AP. - `tests/probe_responder.cpp` — active-scan probe response, no beacon. - `tests/ap_responder.cpp` + `tests/ap_ping_demo.sh` — open assoc → DHCP lease → ping 0% loss. - `tests/ap_wpa2.cpp` + `tests/ap_wpa2_demo.sh` — WPA2 4-way → encrypted DHCP → - encrypted ping (here, exercise the **hardware** CCMP path). + encrypted ping. Software CCMP; the **hardware** CCMP path is the open item. + +`tests/mt7612u_ap_onair.sh` is the one MT-specific piece. Its three cells drive +`ap_responder`, `ap_wpa2` and `tests/mt7612u_beacon_stop_check.cpp` against a +real station and grade them by `iw scan` (matched on BSSID *and* SSID) and by +the AP's own log, so a run is a pass/fail line rather than an operator reading +output. It is a root harness — `iw`, `wpa_supplicant`, and between cells a USB +power-cycle when `AP_VBUS` names a hub port, otherwise an `authorized` toggle, +which is not a cold cycle but does end this MAC's autonomous beacon (measured, +see the comment there). Environment: `CH`, `BUILD`, `FW_DIR`, `PSK`, `SECS`, +`AP_SYSFS`, `STA_SYSFS`, `AP_VBUS`, plus an optional cell argument (`open`, +`wpa2`, `stop`, `all`). Success = a real Linux station associates and passes IP traffic against the MT7612U backend, open and WPA2-PSK, on both 2.4 and 5 GHz, with the static -beacon. +beacon. Measured: 14/14 on ch36 and 14/14 on ch6. diff --git a/src/mt7612u/beacon.cpp b/src/mt7612u/beacon.cpp index f19cc864..2f3fa1a0 100644 --- a/src/mt7612u/beacon.cpp +++ b/src/mt7612u/beacon.cpp @@ -296,11 +296,38 @@ static int mt_mac_set_bss_base(struct mt7612u_dev *d, const uint8_t *a) * failure path in beacon_start and by beacon_stop. */ static void unwind_identity(struct mt7612u_dev *d, int took) { - if (!took) - return; - mt7612u_clear_ack_responder(d); + const unsigned before = mt_io_errors(d); + + /* + * The two registers have different ownership and cannot share one flag. + * + * MT_MAC_BSSID has exactly two writers in the whole backend - mac_setaddr + * at init, and mt_mac_set_bss_base() here - so a beacon that moved it + * always owns it, and it is restored unconditionally. Gating it on `took` + * left it pointing at the beacon's addr2 after any hand-off, and nothing + * else in the library ever writes it back: the hardware then derives the + * BSS index from an address the host no longer believes it is using. + * + * MT_MAC_ADDR is co-owned with the ACK responder, so it is restored only + * while the beacon still holds it. + */ mt_mac_set_bss_base(d, d->macaddr); - d->beacon_took_identity = 0; + if (took) + mt7612u_clear_ack_responder(d); + + /* Flags survive a restore that did not land, so Stop()'s documented retry + * has something left to retry. Clearing them regardless made the second + * and third attempts no-ops against a still-leaked identity. + * + * This only reaches MT_MAC_ADDR because mt7612u_clear_ack_responder() + * keeps `ack_saved` on its own failure for the same reason - it early- + * returns on !ack_saved, so a retry that found the flag cleared would do + * nothing at all no matter what this function decides. The two halves of + * the retry have to agree. */ + if (mt_io_errors(d) != before) + return; + if (took) + d->beacon_took_identity = 0; } int mt7612u_beacon_start(struct mt7612u_dev *dev, const void *buf, size_t len, @@ -309,6 +336,7 @@ int mt7612u_beacon_start(struct mt7612u_dev *dev, const void *buf, size_t len, struct mt7612u_tx_rate rate; const uint8_t *mpdu = NULL; size_t mpdu_len = 0; + static const uint8_t zero6[6] = { 0 }; const uint8_t *ta, *bssid; uint8_t idx; unsigned before; @@ -352,24 +380,76 @@ int mt7612u_beacon_start(struct mt7612u_dev *dev, const void *buf, size_t len, * * Both registers because the index below is derived from the MBSS base, * not from MT_MAC_ADDR. */ + + /* Snapshot before the FIRST hardware write, so the delta covers the + * identity writes too - MT_MAC_ADDR_DW1 goes out as a bare mt_wr and the + * readback checks DW0 only, so a failed DW1 would otherwise be invisible + * and the AP would beacon with half an address. */ + before = mt_io_errors(dev); { - /* Ownership is claimed BEFORE the writes, not after them. Both calls - * below move MT_MAC_ADDR and can then fail - set_ack_responder writes - * the register and readback-verifies afterwards, and set_bss_base can - * only be reached once that write landed - so a failure here leaves - * the identity moved. Claiming first is what lets the unwind put it - * back; claiming after meant these two paths leaked it, and returned - * -1, which tells the caller the hardware was never touched. */ - const int was_taken = dev->ack_saved; - if (!was_taken) { - dev->beacon_took_identity = 1; - took = 1; - } + /* + * `took` is decided BEFORE the writes and the device flag is set + * AFTER them, and the split is load-bearing in both directions. + * + * Before, because both calls move MT_MAC_ADDR and can then fail - + * set_ack_responder writes the register and readback-verifies + * afterwards, and set_bss_base is only reachable once that write + * landed - so the unwind needs to know we own it while those failures + * are still in flight. + * + * After, because mt7612u_set_ack_responder() CLEARS + * beacon_took_identity itself: that is how a caller arming a responder + * takes ownership away from a beacon. Setting the device flag first + * meant the call immediately below wiped it, and StopBeacon then never + * restored the identity on the success path - which is the previous + * round's fix for the failure paths breaking the success one. + */ + /* + * Always 1. The retarget below is unconditional, so this call always + * moves the identity and therefore always owns it at this instant. + * + * It used to be `!dev->ack_saved`, meaning "somebody else got here + * first, leave it to them" - but ack_saved is also set by THIS + * function's own call below, and is only cleared by a successful + * beacon_stop. So a re-arm over a live beacon, and any session whose + * config arms rx.ack_responder, both took the "somebody else" branch + * and disabled the restore for the rest of the session. And the branch + * protected nothing even when it fired: the retarget had already + * overwritten that responder's address in hardware, so declining to + * restore left MT_MAC_ADDR at the beacon's addr2 - neither the + * responder's address nor the factory one. + * + * Hand-off is the responder's job, not ours: mt7612u_set_ack_responder() + * clears beacon_took_identity, so a caller arming one AFTER the beacon + * takes ownership and beacon_stop then leaves it alone. + */ + took = 1; + /* + * fail_post, not fail_pre, and that is the whole point of there being + * only one failure label past this line. + * + * -1 is contracted as "nothing was touched, whatever was airing still + * is", and these two exits cannot honour it: set_ack_responder has + * already written MT_MAC_ADDR before it readback-verifies, and + * set_bss_base is only reachable once that write landed. The unwind + * puts the FACTORY address back - the only address saved anywhere - + * so over a live beacon a failed re-arm restored an identity that + * beacon never had, while its page and timers kept airing it. The + * caller, told -1, left _beacon_active true and went on believing in + * an AP that beacons perfectly and acknowledges nobody. + * + * There is no atomic re-arm to offer here: one MT_MAC_ADDR, one save + * slot, and the previous occupant's address is not in it. So a failure + * after the identity moves takes the beacon down deliberately - engine + * disarmed, APC slots zeroed, identity retracted, -2 - which the + * caller can act on. Silence is a worse outcome than a deaf AP only if + * you are not told about it. + */ if (mt7612u_set_ack_responder(dev, ta)) - goto fail_pre; + goto fail_post; if (mt_mac_set_bss_base(dev, ta)) - goto fail_pre; - memcpy(dev->beacon_ident, ta, 6); + goto fail_post; + dev->beacon_took_identity = took; } /* @@ -393,33 +473,57 @@ int mt7612u_beacon_start(struct mt7612u_dev *dev, const void *buf, size_t len, * * - the identity, if THIS call took it. Leaving it retargeted after a * failed start means the adapter answers for a BSS that does not exist. - * - the beacon engine, once mt_beacon_init() has disarmed it. That is why - * a failure at or after that point returns -2 and one before it returns - * -1: only the caller can know whether a PREVIOUS beacon is still on the - * air, and -1 means "nothing was touched, whatever was airing still is". + * - the beacon engine, once mt_beacon_init() has disarmed it. + * + * Both unwind through the single fail_post label and return -2. -1 is + * reserved for the refusals ABOVE the first hardware write - bad input, a + * malformed frame, a multicast addr2 - so it can keep meaning "nothing was + * touched, whatever was airing still is" without qualification. An earlier + * draft returned -1 from inside the identity block, where that promise was + * already false. * - * And the io_err delta, because mt_beacon_set_enable()'s arming path is - * mt_rmw()/mt_set() - which report their read halves, not their writes - so - * without it a failed EP0 transfer returns 0 and the caller reports an AP - * that is ready and airing nothing. + * And the io_err delta, because several writes on this path report only + * their read halves or nothing at all - mt_beacon_set_enable()'s + * mt_rmw()/mt_set(), and MT_MAC_ADDR_DW1, which set_ack_responder writes + * with a bare mt_wr and whose readback checks DW0 only. Without the delta + * a failed DW1 transfer returns 0 and the AP beacons with half an address + * and acknowledges nobody. The snapshot is taken before the identity + * writes, above, for exactly that reason. */ - before = mt_io_errors(dev); /* - * The one RX-filter change an AP wants, and the reason is evidence rather - * than throughput: DUP drops retransmissions, and a station's retry is - * exactly how you learn whether your ACKs are landing. An auth arriving - * with FC Retry set means the MAC did not acknowledge the first one; drop - * duplicates and that signal disappears. mt76 leaves OTHER_BSS, BCAST and - * MCAST undropped in every mode, so a wildcard probe request already - * reaches us and nothing else here needs changing. Cleared in place rather - * than rewriting a copied literal, so it cannot drift from the default. + * No RX-filter change, and the reason is mt7612u_set_monitor_rx(), NOT a + * default. The init value is 0x00015f97 (initvals.h, re-written at + * init.cpp:290) and BIT(7) - MT_RX_FILTR_CFG_DUP - is SET in it. What + * leaves DUP clear is mt7612u_set_monitor_rx(), which rewrites the + * register as PHY_ERR and nothing else - these are DROP bits, so CRC_ERR + * joins it when the caller does NOT want corrupted frames kept + * (init.cpp:566, `if (!keep_corrupted)`). init.cpp:557 says dropping DUP + * there is deliberate, because "duplicate suppression would hide the + * retransmissions an ACK-responder test counts". + * + * Every AP path goes through it: Mt7612uRadio::StartRxLoop() calls it, and + * an AP has to receive. A TX-only consumer - InitWrite() with no RX loop - + * does not, and there DUP stays set; that costs it nothing, because with + * no receiver there are no retransmissions to count. + * + * So touching the filter here would be wrong in both directions: clearing + * DUP is a no-op on every path that beacons, and RESTORING it on the way + * out would switch duplicate filtering on in a session that deliberately + * had it off - destroying the retry=0 evidence the AP harness measures. + * Both were here for one round; neither belongs. */ - mt_clear(dev, MT_RX_FILTR_CFG, MT_RX_FILTR_CFG_DUP); - - if (mt_ap_set_bssid(dev, idx, bssid)) - goto fail_pre; mt_beacon_init(dev); + /* + * Kept below mt_beacon_init(). It writes MT_MAC_APC_BSSID_L then _H and + * nothing in unwind_identity() touches the APC slots, so a failure between + * the two leaves a half-programmed slot: over a live beacon that + * half-overwrites the airing AP's own entry and it silently stops + * acknowledging. fail_post zeroes both slots, which is the only unwind + * that covers it, and everything from here down exits that way. + */ + if (mt_ap_set_bssid(dev, idx, bssid)) + goto fail_post; if (mt_beacon_write(dev, mpdu, mpdu_len, &rate)) goto fail_post; if (mt_beacon_set_enable(dev, 1, interval_tu)) @@ -428,15 +532,32 @@ int mt7612u_beacon_start(struct mt7612u_dev *dev, const void *buf, size_t len, ERR("beacon: a USB transfer failed while arming"); goto fail_post; } + /* Recorded only once the arm has succeeded. Written before the last + * failure exit, a -1 re-arm replaced it while the OLD beacon was still + * airing - and beacon_update then refused the live beacon's real addr2 and + * accepted one that was not on the air. Host state describing the hardware + * is part of what -1 promises not to change. + * + * addr2 and addr3 together: `ta` is mpdu + 10 and `bssid` is mpdu + 16, so + * twelve bytes from `ta` are exactly the pair, and both were programmed + * above - addr2 into the identity registers, addr3 into APC slot `idx`. */ + memcpy(dev->beacon_ident, ta, 12); return 0; +/* + * The only failure label past the first hardware write, so -2 covers every one + * of them and -1 is left to mean exactly what it says: refused on its input, + * nothing touched. Disarm, erase, retract, in that order. + */ fail_post: mt_beacon_set_enable(dev, 0, 0); + /* The APC slot was programmed above; leave no BSS the MAC still matches. + * After a -2 the caller clears _beacon_active, so StopBeacon early-returns + * and this residue would be unreachable for the rest of the session. */ + mt_ap_set_bssid(dev, 0, zero6); + mt_ap_set_bssid(dev, 1, zero6); unwind_identity(dev, took); return -2; -fail_pre: - unwind_identity(dev, took); - return -1; } int mt7612u_beacon_update(struct mt7612u_dev *dev, const void *buf, size_t len) @@ -456,10 +577,16 @@ int mt7612u_beacon_update(struct mt7612u_dev *dev, const void *buf, size_t len) * beacon that no longer matches the programmed APC slot or MT_MAC_ADDR. * It beacons perfectly and acknowledges nobody, which is the failure the * start path goes to some length to prevent. + * + * BOTH addresses, in one 12-byte compare over the adjacent addr2/addr3 + * pair. Checking addr2 alone still admitted a changed addr3, and addr3 is + * the half that goes into the APC slot - the update would air a BSSID the + * slot does not hold, producing precisely the deaf AP described above + * through the guard meant to stop it. */ - if (memcmp(mpdu + 10, dev->beacon_ident, 6) != 0) { - ERR("beacon: an in-place update cannot change addr2 - the port " - "identity keeps what beacon_start programmed"); + if (memcmp(mpdu + 10, dev->beacon_ident, 12) != 0) { + ERR("beacon: an in-place update cannot change addr2 or addr3 - the " + "port identity and the APC slot keep what beacon_start programmed"); return -1; } diff --git a/src/mt7612u/caps.cpp b/src/mt7612u/caps.cpp index cdab2157..fdba679a 100644 --- a/src/mt7612u/caps.cpp +++ b/src/mt7612u/caps.cpp @@ -94,7 +94,25 @@ int mt7612u_set_ack_responder(struct mt7612u_dev *d, const uint8_t mac[6]) mt_wr(d, MT_MAC_ADDR_DW0, dw0); mt_wr(d, MT_MAC_ADDR_DW1, (uint32_t)mac[4] | ((uint32_t)mac[5] << 8) | FIELD_PREP(MT_MAC_ADDR_DW1_U2ME_MASK, 0xff)); - mt_set(d, MT_AUTO_RSP_CFG, MT_AUTO_RSP_EN); + /* + * Kept out of the I/O-error accumulator, deliberately. + * + * The gate is already on: mac_reset() writes MT_AUTO_RSP_CFG = 0x13 + * (init.cpp:174, reached from mt_init_hardware() at :408) and + * MT_AUTO_RSP_EN is BIT(0), so this mt_set() is a re-assertion of a bit + * that is already set - the same no-op mt7612u_clear_ack_responder() + * relies on. But mt_set() is mt_rmw(), which on a failed READ bumps + * io_err and skips its write; a transient EP0 read stall here would then + * show up in mt7612u_beacon_start()'s io_err delta and tear down an arm + * this function just verified as good, returning -2 for a beacon that is + * on the air. The readback below is the check for this bit - if the gate + * really is closed, that is what fails, with a message that says so. + */ + { + const unsigned io = mt_io_errors(d); + mt_set(d, MT_AUTO_RSP_CFG, MT_AUTO_RSP_EN); + mt_io_restore(d, io); + } /* Verify the arm. The U2ME byte of DW1 is write-only on this silicon, * so only DW0 and the low half of DW1 can be read back. */ @@ -113,15 +131,20 @@ int mt7612u_set_ack_responder(struct mt7612u_dev *d, const uint8_t mac[6]) void mt7612u_clear_ack_responder(struct mt7612u_dev *d) { + unsigned before; + if (!d->ack_saved) return; + before = mt_io_errors(d); + /* Move the identity off the responder address first: on a MAC that * matches on address 1, clearing the gate alone leaves it answering * for whatever address is still programmed. * - * MT_AUTO_RSP_EN is deliberately NOT cleared here. mt_init_hardware() - * writes MT_AUTO_RSP_CFG = 0x13 (init.c), and MT_AUTO_RSP_EN is BIT(0), + * MT_AUTO_RSP_EN is deliberately NOT cleared here. mac_reset(), which + * mt_init_hardware() runs, writes MT_AUTO_RSP_CFG = 0x13 + * (init.cpp:174 from init.cpp:408), and MT_AUTO_RSP_EN is BIT(0), * so the gate is already on before any caller arms a responder - the * mt_set() in mt7612u_set_ack_responder() is a no-op on it. Clearing it * here would leave the device in a state its own init never produces; @@ -134,5 +157,16 @@ void mt7612u_clear_ack_responder(struct mt7612u_dev *d) mt_wr(d, MT_MAC_ADDR_DW1, (uint32_t)a[4] | ((uint32_t)a[5] << 8) | FIELD_PREP(MT_MAC_ADDR_DW1_U2ME_MASK, 0xff)); } + + /* Keep ack_saved when the restore did not land, so the caller's retry has + * something to retry. Both writes are bare mt_wr() - void, no readback - + * so a failed EP0 transfer is otherwise indistinguishable from a landed + * one. Clearing the flag regardless made the early-return above swallow + * every later attempt, including the three mt7612u_beacon_stop() drives + * through unwind_identity(), against an MT_MAC_ADDR still sitting on the + * responder address. The flag means "a restore is still owed" and nothing + * reads it as "a responder is armed", so leaving it set is safe. */ + if (mt_io_errors(d) != before) + return; d->ack_saved = 0; } diff --git a/src/mt7612u/internal.h b/src/mt7612u/internal.h index 839ca2a7..35a24a96 100644 --- a/src/mt7612u/internal.h +++ b/src/mt7612u/internal.h @@ -219,10 +219,16 @@ struct mt7612u_dev { * caller had already armed an ACK responder, because then the identity is * theirs and restoring would silently disarm it. */ int beacon_took_identity; - /* The addr2 mt7612u_beacon_start() programmed, so an in-place update can - * refuse a beacon that would change it - the port registers keep the - * start identity, so a changed BSSID airs and matches nothing. */ - uint8_t beacon_ident[6]; + /* The addr2 AND addr3 mt7612u_beacon_start() programmed, so an in-place + * update can refuse a beacon that would change either. Both, because they + * land in different registers: addr2 in MT_MAC_ADDR and the MBSS base, + * addr3 in the APC BSSID slot. Guarding addr2 alone let an update move the + * BSSID the beacon advertises while the slot still held the old one - the + * AP beacons perfectly and acknowledges nobody, which is the exact failure + * this guard exists to prevent. The two are adjacent in the 802.11 header + * (bytes 10 and 16 of the 24-byte management header beacon_split() + * requires), so one memcpy covers them. */ + uint8_t beacon_ident[12]; struct mt_async *a; FILE *wrlog; FILE *mculog; diff --git a/src/mt7612u/regs.h b/src/mt7612u/regs.h index 9fe9bfd3..e9538591 100644 --- a/src/mt7612u/regs.h +++ b/src/mt7612u/regs.h @@ -243,8 +243,6 @@ enum mt_mcu_cr_mode { MT_RF_CR, MT_BBP_CR, MT_RF_BBP_CR, MT_HL_TEMP_CR_UPDATE }; #define MT_RX_FILTR_CFG_PROMISC BIT(2) #define MT_RX_FILTR_CFG_OTHER_BSS BIT(3) #define MT_RX_FILTR_CFG_VER_ERR BIT(4) -#define MT_RX_FILTR_CFG_MCAST BIT(5) -#define MT_RX_FILTR_CFG_BCAST BIT(6) #define MT_RX_FILTR_CFG_DUP BIT(7) #define MT_RX_FILTR_CFG_CTRL_RSV BIT(16) #define MT_AUTO_RSP_CFG 0x1404 diff --git a/tests/mt7612u_ap_onair.sh b/tests/mt7612u_ap_onair.sh index 63c0e704..2e9d5585 100755 --- a/tests/mt7612u_ap_onair.sh +++ b/tests/mt7612u_ap_onair.sh @@ -297,7 +297,7 @@ cell_stop() { wait $ap 2>/dev/null grep -q "0 failure(s)" "$OUT/stop.log" \ - && ok "stop: local contract checks (2nd stop false, update-with-no-beacon false)" \ + && ok "stop: local contract checks (2nd stop false, update-with-no-beacon false, update accepts an unchanged payload and refuses a changed addr3 - and still refuses a changed addr2)" \ || bad "stop: local contract checks failed" } diff --git a/tests/mt7612u_beacon_stop_check.cpp b/tests/mt7612u_beacon_stop_check.cpp index 5f463b26..009a92db 100644 --- a/tests/mt7612u_beacon_stop_check.cpp +++ b/tests/mt7612u_beacon_stop_check.cpp @@ -125,6 +125,49 @@ int main(int argc, char **argv) { std::fprintf(stderr, "FAIL: StartBeacon returned false\n"); return 1; } + + /* + * The identity guard, exercised against a LIVE beacon. + * + * StartBeacon records addr2 AND addr3, and an in-place update may change + * neither: addr2 is in the port identity registers and the MBSS base, addr3 + * is in the APC slot, and an update rewrites none of them. A beacon that + * aired a different BSSID than the slot holds looks perfect on a scan and + * acknowledges nobody - it cannot be caught by watching the air. + * + * Only the addr3 arm discriminates. addr2 was already guarded, so that arm + * is a regression check, not evidence for this change; addr3 was accepted + * before and is refused now. Both are kept, and said apart, because a test + * whose arms are not distinguished reads as twice the coverage it has. + * + * The positive control carries as much weight as the two negatives: a guard + * that refused every payload would pass both refusal checks. The GUARD runs + * ahead of the suppress/copy bracket, so a refusal costs the update and not + * the beacon - the PHASE 1 scan below must still find it. The accepted one + * does go through the bracket and re-copies the page, which is two EP0 + * transfers for a 62-byte beacon, not the full slot. + */ + { + const size_t rtap = (size_t)bcn[2] | ((size_t)bcn[3] << 8); + const struct { size_t off; const char *what; } probes[] = { + { rtap + 10, "addr2" }, { rtap + 16, "addr3" } }; + + if (!dev->UpdateBeaconPayload(bcn.data(), bcn.size())) { + std::fprintf(stderr, "FAIL: UpdateBeaconPayload refused an unchanged " + "payload over a live beacon\n"); + fails++; + } + for (const auto &p : probes) { + std::vector bad = bcn; + bad[p.off] ^= 0x40; /* still unicast, still locally administered */ + if (dev->UpdateBeaconPayload(bad.data(), bad.size())) { + std::fprintf(stderr, "FAIL: UpdateBeaconPayload accepted a changed " + "%s\n", p.what); + fails++; + } + } + } + std::this_thread::sleep_for(std::chrono::seconds(secs)); banner("PHASE 2: stopped - the SSID MUST be gone");