mt7612u: access-point support — hardware beacon behind IRadio, open and WPA2-PSK verified on air - #424
Open
snokvist wants to merge 11 commits into
Open
mt7612u: access-point support — hardware beacon behind IRadio, open and WPA2-PSK verified on air#424snokvist wants to merge 11 commits into
snokvist wants to merge 11 commits into
Conversation
PR Summary by QodoAdd MT7612U hardware-beacon access point support
AI Description
Diagram
High-Level Assessment
Files changed (17)
|
Code Review by Qodo
1. Access points report ready without airing
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Comment on lines
+162
to
+164
| 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); |
There was a problem hiding this comment.
1. Access points report ready without airing 📘 Rule violation ≡ Correctness
mt_beacon_set_enable() discards the interval RMW and timer-enable register results and returns zero unconditionally, while mt7612u_beacon_start() does not check the accumulated I/O-error count across initialization and enablement. If an EP0 USB transfer fails, StartBeacon() can still set _beacon_active = true and report that beaconing is active to AP harnesses and callers even though the interval is stale or the beacon engine was never armed.
Agent Prompt
## Issue description
Beacon enablement reports success even when USB register operations used to initialize beaconing, configure the interval, or arm the timer fail, allowing the public start path to mark an inactive or incorrectly configured beacon as active.
## Fix Focus Areas
- src/mt7612u/beacon.cpp[150-169]
- src/mt7612u/beacon.cpp[348-351]
- src/mt7612u/usb.cpp[206-249]
## Recommended Fix
Capture the I/O-error count before the complete beacon initialization and enable sequence, propagate each `mt_rmw()` failure immediately, and return failure if any initialization, interval, or timer-enable write increments the error counter. If enablement fails after partially configuring the hardware, best-effort disarm or suppress the beacon slot before returning so callers cannot receive success for an inactive or incorrectly configured beacon.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Collaborator
Author
There was a problem hiding this comment.
Fixed. Correct, and it is the counterpart to a fix I made last round and did not carry across: mt7612u_beacon_stop got an io_err bracket and mt7612u_beacon_start did not. mt_beacon_set_enable's arming path is mt_rmw/mt_set, which report their READ halves, so a failed EP0 write returned 0 and StartBeacon set _beacon_active = true for an engine that was never armed.
mt7612u_beacon_start now snapshots mt_io_errors() before it touches the hardware and fails if the count moved, alongside checking the returns it was already ignoring.
Comment on lines
+884
to
+887
| _beacon_active = false; | ||
| if (mt7612u_beacon_start(_dev, beacon, len, | ||
| static_cast<unsigned>(interval_tu)) != 0) | ||
| return false; |
There was a problem hiding this comment.
2. A replacement can orphan a live beacon 🐞 Bug ☼ Reliability
StartBeacon clears _beacon_active before the replacement has either validated its input or disarmed the existing engine. When a repeated start fails before mt_beacon_init, the previous autonomous beacon remains on air while update, stop, and destructor cleanup all treat it as inactive.
Agent Prompt
## Issue description
A failed repeated StartBeacon can leave the previous hardware beacon running while `_beacon_active` is false, preventing subsequent cleanup.
## Fix Focus Areas
- src/mt7612u/Mt7612uRadio.cpp[873-890]
- src/mt7612u/beacon.cpp[282-351]
## Recommended Fix
Make replacement transactional: retain the prior active state until the old engine is definitely disarmed, and distinguish failures occurring before versus after disarm so `_beacon_active` always reflects the actual hardware state.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Collaborator
Author
There was a problem hiding this comment.
Fixed, and this one is the sharpest finding in the set - it is a defect I introduced last round while fixing a different one.
I cleared _beacon_active before the call to make a failed re-arm failure-atomic. But mt7612u_beacon_start has failure paths BEFORE it touches the engine (input refused, identity retarget failed), and on those the previous beacon is still airing while the flag now says otherwise. UpdateBeaconPayload, StopBeacon and the destructor all then treat a live autonomous beacon as inactive, so nothing ever silences it - the exact outcome the whole change exists to prevent.
It needed three outcomes rather than two, because only the library knows which side of mt_beacon_init a failure landed on:
0 armed
-1 refused before the hardware was touched - the flag is NOT cleared,
whatever was airing still is
-2 failed after the engine was disarmed; the library unwound the identity
too, so nothing is airing and the flag is cleared
Comment on lines
+326
to
+330
| /* 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; |
There was a problem hiding this comment.
6. Stopping a beacon can erase a responder 🐞 Bug ≡ Correctness
Beacon start records identity ownership only from the responder state that existed at that moment, while a later SetAckResponder does not transfer that ownership. If a caller arms or changes the responder after starting the beacon, mt7612u_beacon_stop restores the factory identity and silently disarms that newer responder.
Agent Prompt
## Issue description
A responder configured after beacon start is overwritten when beacon stop restores the factory identity under stale ownership metadata.
## Fix Focus Areas
- src/mt7612u/beacon.cpp[320-331]
- src/mt7612u/beacon.cpp[409-415]
- src/mt7612u/Mt7612uRadio.cpp[850-860]
## Recommended Fix
Model beacon and responder ownership explicitly, update ownership whenever `SetAckResponder` or `ClearAckResponder` runs, and restore only the identity that immediately preceded the beacon-owned value.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Collaborator
Author
There was a problem hiding this comment.
Fixed, together with #11 - the two are the same defect seen from either end.
MT_MAC_ADDR is one register with two users and there was no arbitration: ownership was recorded once, at StartBeacon, from whatever ack_saved happened to be. So SetAckResponder after StartBeacon was silently disarmed by the matching StopBeacon (this thread), and the mirror case left the responder programmed for the beacon's addr2 instead of the caller's address (#11).
The rule is now "whoever wrote last owns it": mt7612u_set_ack_responder clears beacon_took_identity, so a responder armed after the beacon is not restored out from under the caller, and a stop after that call leaves the address alone. It is one line in caps.cpp with the reasoning next to it, and it is stated in the public header rather than left as behaviour.
Comment on lines
300
to
301
| if (g_dev) g_dev->StopBeacon(); | ||
| _exit(0); |
There was a problem hiding this comment.
3. Test exits can leave beacons airing 🐞 Bug ☼ Reliability
The AP harnesses call StopBeacon once, ignore its failure result, and immediately use _exit, while the lifecycle harness similarly discards its final stop result. A transient or persistent USB failure therefore bypasses destructor retries and can leave the autonomous beacon active even though these additions are intended to guarantee silence.
Agent Prompt
## Issue description
Hardware tests ignore StopBeacon failures and can exit while the autonomous MAC is still transmitting.
## Fix Focus Areas
- tests/ap_responder.cpp[293-301]
- tests/ap_wpa2.cpp[426-434]
- tests/ul_trigger_ap.cpp[449-457]
- tests/mt7612u_beacon_stop_check.cpp[155-160]
## Recommended Fix
Retry `StopBeacon` with a bounded policy, return a failing process status if it cannot be stopped, and avoid `_exit` until cleanup succeeds or the harness has explicitly power-cycled the target device.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Collaborator
Author
There was a problem hiding this comment.
Fixed. And the timing is pointed: StopBeacon only became capable of failing in the previous commit, and #411 amended IRadio.h the same day to say such a failure "must be retried (or followed by hardware shutdown) before its shared port is reused". These harnesses called it once, discarded the result, and _exit(0)ed - so there is no destructor coming to try again, and a transient EP0 stall leaves an autonomous beacon airing until the adapter is power-cycled.
All four (ap_responder, ap_wpa2, ul_trigger_ap, and the lifecycle harness's own final stop) now retry three times and print a warning naming what is still on the air. The lifecycle harness counts it as a failure, since teardown is the thing it exists to test.
Comment on lines
+210
to
+215
| 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 |
There was a problem hiding this comment.
7. A header-only frame becomes a beacon 🐞 Bug ≡ Correctness
beacon_split treats rlen == len as a bare MPDU even though the parser has identified a valid radiotap header consuming the entire buffer. A header-only input of sufficient length is consequently interpreted as an 802.11 frame, and its bytes can be used as the port addresses and reserved-page contents.
Agent Prompt
## Issue description
A valid radiotap header that consumes the entire input is incorrectly reclassified as a bare 802.11 frame.
## Fix Focus Areas
- src/mt7612u/beacon.cpp[201-223]
- src/mt7612u/radiotap.cpp[233-243]
## Recommended Fix
Accept the bare-MPDU branch only when parsing returns zero; reject `rlen >= len` when a radiotap header was recognized, matching the existing packet-send validation.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Collaborator
Author
There was a problem hiding this comment.
Fixed, along with #12 - they are two symptoms of the same weak classification.
beacon_split decided "radiotap or bare MPDU" from the parser's return value, which cannot carry that distinction: mt_radiotap_parse returns 0 both for "this is not a radiotap header" and for "it is one and it is malformed", and rlen == len (a header with no frame after it) fell to the bare branch too. Either way, radiotap bytes end up parsed as an 802.11 header - supplying the port addresses and the reserved-page contents, which then air.
Byte 0 decides now, and it is unambiguous: an 802.11 beacon's frame control is 0x80 and a radiotap header's version must be 0. A buffer starting 0x00 is held to the radiotap rules (parse must succeed, and must leave a frame after it); anything else is a bare MPDU. No fallback between the two.
Comment on lines
+92
to
+98
| seen() { | ||
| 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 |
There was a problem hiding this comment.
9. Nearby access points can pass the test 🐞 Bug ≡ Correctness
The seen helper counts scan output only by SSID and never verifies the expected BSSID of the adapter under test. Another access point using devourerAP or mtStopCheck can make arm and re-arm checks pass, make stop checks fail, or cause the exact-count comparison to reject an otherwise valid run.
Agent Prompt
## Issue description
On-air verdicts identify beacons by shared SSID strings rather than the unique BSSID under test.
## Fix Focus Areas
- tests/mt7612u_ap_onair.sh[82-102]
- tests/mt7612u_ap_onair.sh[134-162]
- tests/mt7612u_ap_onair.sh[241-255]
## Recommended Fix
Determine the expected BSSID for each cell and parse each scan BSS block so only an entry with both that BSSID and SSID counts; use presence rather than an exact global SSID count.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Collaborator
Author
There was a problem hiding this comment.
Fixed. seen() matched on SSID alone, so a neighbour running "devourerAP" could satisfy an arm check, break a stop check, or upset the exact-count comparison - and this bench sits in range of several APs.
It matches BSSID and SSID together now, via awk so the pairing is preserved (two grep -c patterns would count independently and pass on a coincidence of both appearing anywhere in the scan). Both harnesses use fixed, distinctive BSSIDs, so the check is exact.
Comment on lines
+348
to
+351
| 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); |
There was a problem hiding this comment.
5. Failed beacon setup changes later traffic 🐞 Bug ≡ Correctness
mt7612u_beacon_start() retargets MT_MAC_ADDR and the BSS base before programming the APC slot, reserved page, and beacon timer, but its later failure returns do not restore the previous identity or APC state. When any post-retarget operation fails, StartBeacon() leaves _beacon_active false so Stop() skips the only restoration path, and subsequent injector or monitor activity continues matching and auto-acknowledging the attempted AP identity.
Agent Prompt
## Issue description
`mt7612u_beacon_start()` changes the MAC/BSS identity before several fallible operations, but its early returns do not restore the prior identity or APC state. The C++ wrapper then treats the beacon as inactive, preventing teardown from invoking the existing restoration path.
## Fix Focus Areas
- src/mt7612u/beacon.cpp[320-351]
- src/mt7612u/Mt7612uRadio.cpp[878-887]
- src/mt7612u/caps.cpp[71-104]
## Recommended Fix
Capture the pre-start identity ownership and relevant register state, then funnel every failure after identity retargeting through a single rollback path. On failure, disable or suppress the beacon engine as needed, clear programmed APC state, restore the previous MAC/BSSID and ACK identity only when this call acquired it, and clear `beacon_took_identity` without disturbing a responder already owned by the caller; ensure returning `false` cannot leave altered device state behind.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Collaborator
Author
There was a problem hiding this comment.
Fixed. mt7612u_beacon_start retargeted MT_MAC_ADDR and the MT_MAC_BSSID base up front and then had four return -1s after it, none of which put the identity back - so a failed start left the adapter answering for a BSS that does not exist, with no beacon.
There is now a single unwind_identity() used by every failure path in beacon_start and by beacon_stop, and it restores only if THIS call was what moved the identity (see the ownership thread).
Comment on lines
+363
to
+373
| /* 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); |
There was a problem hiding this comment.
10. Beacon updates can air corrupted frames 🐞 Bug ☼ Reliability
mt7612u_beacon_update() performs the active-slot suppression with unchecked mt_wr() and starts the multipart reserved-page copy, while mt_beacon_write() snapshots io_err only after that guard write. If the suppression write fails, a TBTT can read mixed old and new page content during the copy, and the later copy and re-enable operations can still allow the update to report success.
Agent Prompt
## Issue description
Beacon updates rely on `MT_BCN_BYPASS_MASK` to prevent TBTT from reading the reserved page during a multipart USB copy, but the suppression write is not checked. Because the copy helper snapshots the error count only after that write, the update can overwrite the page and report success even when suppression failed.
## Fix Focus Areas
- src/mt7612u/beacon.cpp[354-374]
- src/mt7612u/beacon.cpp[88-118]
- src/mt7612u/usb.cpp[182-238]
## Recommended Fix
Use `mt_wr_chk()` for the bypass-mask suppression write, or snapshot `mt_io_errors()` before it and abort before copying if the error count changes. Include suppression, copying, and unsuppression in the same error boundary; keep the slot suppressed after a failed copy and only unsuppress it after the reserved-page write completes successfully.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Collaborator
Author
There was a problem hiding this comment.
Fixed. The guard write was the one part of the update path that could fail silently, and it is the part the whole bracket exists for: if the suppression never lands, the multi-transaction page copy runs against a live slot and a TBTT mid-copy airs a torn beacon.
It is mt_wr_chk now and the update refuses when it fails, rather than proceeding to a copy it knows is unguarded. mt_beacon_write's own io_err snapshot still covers the copy itself.
Comment on lines
+321
to
+330
| 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; |
There was a problem hiding this comment.
11. Ack responders retain the beacon address 🐞 Bug ≡ Correctness
mt7612u_beacon_start() overwrites an already armed responder's MAC address but deliberately does not claim restoration ownership when ack_saved was set. mt7612u_beacon_stop() consequently leaves the responder programmed for the beacon's addr2 rather than the address supplied by the earlier SetAckResponder() call.
Agent Prompt
## Issue description
Starting a beacon after `SetAckResponder()` overwrites the single hardware responder identity, but stopping the beacon does not restore the previously armed responder address. The current saved state only retains the factory address, so the prior responder target is lost.
## Fix Focus Areas
- src/mt7612u/beacon.cpp[321-331]
- src/mt7612u/beacon.cpp[409-415]
- src/mt7612u/caps.cpp[71-130]
## Recommended Fix
Either reject beacon start while an independent ACK responder owns the identity, or add explicit nested ownership state that saves and restores the prior responder target after beacon stop. Keep the factory-address restore behavior for the case where beaconing is the first identity owner.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Collaborator
Author
There was a problem hiding this comment.
Fixed - same change as #6, from the other side. See that thread: ownership of MT_MAC_ADDR now transfers to whoever wrote it last, so mt7612u_set_ack_responder takes it back from the beacon and a later stop does not restore over it.
Comment on lines
+201
to
+223
| 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; | ||
| *mpdu = p; | ||
| *mpdu_len = len; | ||
| } |
There was a problem hiding this comment.
12. Malformed beacon input can be transmitted 🐞 Bug ≡ Correctness
beacon_split() treats a zero return from mt_radiotap_parse() as a bare MPDU, although that parser also returns zero for malformed radiotap version, length, and present-map headers. A caller supplying such a radiotap-framed beacon can therefore have its header bytes interpreted as 802.11 data and loaded into the autonomous beacon page rather than receiving a failed start.
Agent Prompt
## Issue description
`beacon_split()` assumes `mt_radiotap_parse() == 0` always means no radiotap is present, but the parser uses the same result for several malformed-header cases. This violates the beacon API's radiotap-or-bare-MPDU input contract and can produce an invalid stored beacon.
## Fix Focus Areas
- src/mt7612u/beacon.cpp[201-223]
- src/mt7612u/radiotap.cpp[104-124]
## Recommended Fix
Distinguish a clearly bare MPDU from a buffer that begins as a radiotap header but fails radiotap validation. Reject the latter, ideally by making the parser return a negative error for malformed fixed-header and present-map cases while preserving zero exclusively for absent radiotap.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Collaborator
Author
There was a problem hiding this comment.
Fixed - same change as #7, see that thread. A malformed radiotap header is no longer reinterpreted as an 802.11 frame; the shape is decided by byte 0 and each is held to its own rules.
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj
…ged 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 (OpenIPC#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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj
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* - OpenIPC#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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj
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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj
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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj
The status block said Stages C-E "still need the IRtlDevice wrapper". Stale twice over: the interface was renamed to IRadio in OpenIPC#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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj
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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj
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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj
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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj
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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj
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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj
snokvist
force-pushed
the
feat/mt7612u-ap-mode
branch
from
September 10, 2026 20:10
7b1e446 to
424170b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follows #419's integration (#420-#423). That gave devourer an MT7612U it can
open, receive and transmit on. This gives it one it can be an access point
on — open and WPA2-PSK — with no MediaTek-specific code in the AP layer.
It works, and devourer's own harnesses are what prove it
tests/ap_responder.cppandtests/ap_wpa2.cpp, unmodified, against anMT7612U. Both already took an
IRadio*— #415 made them genuinelybackend-agnostic — so nothing in them knows this is MediaTek.
retry=0on the auth is the load-bearing one: an un-ACKed frame isretransmitted with FC Retry set, so retry=0 is the hardware ACK, and it is
the only evidence that the port identity and the APC slot are both right.
That output is
tests/mt7612u_ap_onair.sh, added here, so the numbers are acommand rather than a story.
What was actually missing
Nothing in the AP layer, and no new driver primitives.
StartBeacon,UpdateBeaconPayloadandStopBeaconfell through toIRadio's base-classreturn false, so a MediaTek adapter beaconed nothing. Everything underneathwas already device-verified (
mt_beacon_init/_write/_set_enable,mt_ap_set_bssid), andSetAckResponder,ReadTsfandWriteTsfwerealready implemented.
Three public entry points, because the backend reaches the subtree through the
public header and
api_linkholds that line (31 now):The identity is the whole trick.
IRadiosays addr2/addr3 set the portMAC/BSSID,
RtlJaguarDeviceimplements exactly that, andap_responder's owncomment says "MACID = BSSID, set by StartBeacon". My first draft instead
refused a BSSID that was not the adapter's own MAC — which would have made
both shipped harnesses unusable on this part, since both hardcode
02:42:75:05:d6:00. It now followsmt76x02_mac_setaddr— moveMT_MAC_ADDRand the
MT_MAC_BSSIDbase together, then derive the APC slot the waymt76x02_util.c:310does. Getting that wrong is the review finding below,and it is the reason the two registers move together rather than one of them.
Also fixed, not MediaTek-specific
ap_responder,ap_wpa2andul_trigger_apall arm a hardware beacon andthen
_exit(0), skipping every destructor — soStopBeaconnever ran and thebeacon kept airing after the process was gone, until the adapter was
power-cycled. Measured: a scan after exit found the SSID live at
last seen: 308 ms ago. The beacon is hardware-autonomous on the Realtek parts too, andIRadio.hhas warned about this since it was written;beacon_update_probe.cppalready called StopBeacon before its own
_exit, these three did not. One lineeach.
What review changed
Three adversarial rounds. The last one returned DO-NOT-MERGE and was right to;
its headline finding is the one worth reading:
The APC slot index rested on a premise that is false in this codebase. I
derived it as
(ta[0] & 2) ? 1 : 0, arguing mt76's1 + (((macaddr[0] ^ addr[0]) >> 2) & 7)collapses because the identity hadjust been retargeted. It does not.
dev->macaddris written once, from theEEPROM, and
MT_MAC_BSSID— the register carryingMBSS_MODE, and the basethe hardware derives the index from — is written once at init from the factory
MAC; the ACK-responder retarget moves only
MT_MAC_ADDR. So the hardware wasindexing off a different address than the host assumed, and the constant was
correct only where
1 + (((factory[0] ^ ta[0]) >> 2) & 7)happens to be 1.This bench is exactly such a case (
40:a5:ef:…, so the XOR term is 0), whichmeans every on-air run agreed with the wrong reasoning. On an adapter whose
first byte is
0xe8the AP would beacon perfectly and acknowledge nobody. Thebring-up gate had refused that case; the ABI flattened it to a constant. Fixed
by making the retarget a real
setaddr, so the premise is true by construction.Second:
mt7612u_beacon_stopcould not fail —mt_clearreports only itsread half and the
mt_ap_set_bssidreturns were dropped — which madeStopBeacon's failure branch unreachable and the new harness's assertion forit vacuous, while the real hazard (an EP0 stall leaving the MAC beaconing)
reported success. Also fixed: the reserved-page copy could fail mid-frame and
report success (a torn beacon airs, worse than none);
UpdateBeaconPayloaddid not raise the suppression guard mt76 explicitly brackets for that reason;
Stop()discarded the result while promising a retry that did not exist; thepublic header still described the pre-fix behaviour; and the beacon-steer trio
silently returned "applied a 0 µs shift".
The earlier round found eight more, one reproduced with a probe against the
real object file:
beacon_splitleft four of ninemt7612u_tx_ratefieldsindeterminate, and
sgi/ldpc/stbcgo straight into the rate word the MACtransmits verbatim while
power_adjshort-circuits the derived TX power.Rebase notes
The AP work predates the C++ migration (#421) and the backend (#422). Rebasing
surfaced that
beacon.cwas compiled by nothing — the subtree Makefileglobs
*.cppand the CMake list is explicit — plus C11_Atomic, implicitvoid*conversions, and amemsetover a no-longer-trivially-copyable type.The original four AP commits and that repair are squashed into one, because
the first three do not build on today's master and a bisect through them
returns build failures rather than the commit you are looking for.
What this does not show
In
docs/mt7612u-ap-mode.mdat the same length as the numbers:CCMP, the same code the Realtek backends use.
MT_WCID_KEY/MT_SKEYarenot wired up, so "crypto becomes hardware on MediaTek" remains a claim. It
looks reachable — mt76's key install is ~40 lines over primitives we have,
and we already zero those exact registers at init — but there is no key API
on
IRadioat all, so it is an interface question rather than a driver one.Happy to open that as its own issue.
Protected bit; "encrypted" rests on the verified MIC plus traffic reaching a
CCMP-only station.
mt76x2u), so it is not anindependent-generation witness. The RTL8812AU witness in the earlier Stage
A/B section is.
station, no rekey, no roaming, no channel change while beaconing.
Verification
ctestwithDEVOURER_MT7612U=ON, and with it OFF;make -C src/mt7612u checkgreen (4 binaries,api_linkat 31 entry points); nowarnings in either build.
original series did not (the first three predate the C++ migration and their
repair came fourth; they are squashed into one).
tests/mt7612u_ap_onair.sh— open 6/6,wpa2 4/4, stop 4/4.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj