mt7612u: an 802.11 station identity seam on IRadio, implemented for the MT7612U

[snokvist]

What changed

• IRadio::SetStationIdentity(own, bssid) / ClearStationIdentity(): a
  vendor-neutral seam for the STATION half of an infrastructure BSS. The
  contract lives at the declaration in src/IRadio.h. It covers:

  • why this is not SetAckResponder(bssid);
  • ORDERING: call after the RX loop is running;
  • ClearStationIdentity() returns whether the rollback was verified;
  • a later port-0 claimant (ACK responder, beacon) is backend-specific;
  • transmission is not part of the arm. A station's unicast must request
    an ACK (build_stream_radiotap(mode, false)). It also needs a nonzero
    tx.retry_limit: the default is 0, which sends each unicast frame once.

  The not-ported defaults are false for SetStationIdentity and true for
  ClearStationIdentity.

• IRadio::StartRxLoop doc: which thread runs the callback, and the lock
  rule with its libusb mechanism. A synchronous transfer completes only when
  some thread handles libusb events. In the Async RX mode that thread is the
  RX thread, which is inside the callback waiting on the caller's lock.

• AdapterCaps::station_mode_ok (the station_mode field of the
  adapter.caps event). TRUE on the MT7612U, FALSE (not ported) on every
  other backend.

• The MT7612U station arm, src/mt7612u/station.cpp:

  • mt7612u_set_station_identity() writes no register. It refuses unless
    own already is MT_MAC_ADDR and MT_AUTO_RSP_EN is set, and both reads
    fail closed. It records the BSSID for the host.

  • The decision is the pure src/mt7612u/StationIdentity.h.

  • After every write to MT_MAC_ADDR (ACK responder, beacon, responder
    clear) the arm is re-checked against what the register holds
    (mt7612u_station_identity_check), tri-state:

    • a verified move drops the arm with a WARN;
    • an unreadable register keeps it, with a WARN;
    • a beacon start that fails and unwinds the identity back restores the
      arm it dropped.

    Headless cells in mt7612u_station_identity cover this.

  • Mt7612uRadio::SetStationIdentity warns when tx.retry_limit is 0.

  • The new C entry points are mt7612u_set_station_identity,
    mt7612u_clear_station_identity and mt7612u_station_bssid; api_link
    now counts 39.

• Hardware gates and harnesses: mt7612uprobe gains staid, sta,
  staack, norsp and bssen. Three harnesses drive them and the existing
  txs gate: tests/mt7612u_sta_identity.sh, tests/mt7612u_sta_autoack.sh
  and tests/mt7612u_sta_uplink.sh, plus the helper
  tests/sta_unicast_inject.py.

  • tests/mt7612u_sta_uplink.sh sets the DUT's retry limit explicitly
    (RETRY_LIMIT, default 15) and checks it was read back. It refuses 0.
    Its header states the runtime: about 9 minutes per arm at FRAMES=200.

  • The harnesses share tests/mt7612u_sta_lib.sh, and all run as root:

    • they kill only PIDs this run recorded (no pattern kills, stale PID files
      removed first);
    • they refuse a DUT_SYSFS that is not 0e8d:7612;
    • they hand the DUT back to mt76x2u on exit with a VID:PID-guarded
      authorized toggle.

  • The identity harness takes hostapd's PID from -P. It accepts an AP
    only if the adapter:

    • is a USB device and not a hub;
    • is not the DUT;
    • carries a wireless netdev with no IPv4/IPv6 default route;
    • supports AP mode.

    It verifies that the unicast injector actually injected.

• Record: docs/mt7612u-station-identity.md, plus a section in
  src/mt7612u/CLAUDE.md.

Why

A station has to receive the AP's unicast addressed to it and auto-ACK it. On
the MT7612U, the obvious way to arm that (SetAckResponder(bssid)) is the one
thing that breaks it. The interface needed a separate call whose contract says
so. The backend's behaviour had to be measured, not read off registers.
station_mode_ok lets a caller refuse up front instead of starting a
handshake it cannot finish.

What is measured

The source is docs/mt7612u-station-identity.md. Every cell used one MT7612U
on channel 6, near field, driven by mt7612uprobe rather than through
IRadio.

• Auto-ACK with nothing armed. Asked of the transmitter: a Realtek
  RTL8812CU's own CCX tx.report, retry limit 12.
  • A (nothing armed): 100.0% acknowledged, 0.45 mean retries, 1279 reports.
  • B (destination nobody holds), C (DUT not running) and D
    (MT_AUTO_RSP_EN cleared): 0% at 12.00 retries each.
  • A and D differ by one bit on one code path and one filter.
  • Against it: one peer, one run per arm. A monitor-filter run shows the
    same shape, but it was not single-variable.
• MT_MAC_BSSID, and the AP's BSSID in the station's APC slot, do not
  gate a managed station's receive.
  • MT_MAC_BSSID programmed WRONG: 5877 unicast frames against 6250 with
    nothing programmed (re-run: 5909 vs 6478).
  • The AP in slot 0: 5878 (5857).
  • A wrong BSSID in the enabled station slot does not gate
    acknowledgement: autoack arm E on 6e99f43 was acknowledged 867/867.
    The DUT log reads WRONG BSSID 02:00:00:de:ad:02 in station APC slot 0, BIT(16) SET (high reg 000102ad), other slots empty, MT_MAC_BSSID = own address (verified).
  • Against it:
    • arm A's 6-10% excess is unexplained beyond a first-arm pattern;
    • arm E is one run, one peer, one DUT, and measures acknowledgement only;
    • the earlier "derived slot" rows and arm E runs wrote slot 1 by the
      AP-side rule (mt76 keys a station's slot on its own address, slot 0
      here) and are withdrawn;
    • on the station-slot gate (new rig, one run per arm), unicast to the
      DUT was A 6277, B 5857, C 5861, D 5846, E 5852 and F (both WRONG, base
      and slot 0) 6003. Every write was verified, BIT(16) was clear, and the
      managed filter 0x00015f97 was in force;
    • so neither register decides acceptance, and F would collapse if one
      did. It does not show the registers are without effect: B-F sit 4-7%
      below A, mostly B-E, which is the unexplained first-arm excess every
      run shows, and a few-percent effect would hide in it. One unit, one AP,
      one run.
• Moving MT_MAC_ADDR under the managed filter drops reception of the
  AP's unicast from 103 frames to 0.
• Uplink (MT_TX_STAT_FIFO, the peer's ACK responder armed):
  • Answering: 200/200 at 0.0 mean retries (6e99f43: 200/200, 0.0, max 1).
  • Control: 0/200 at the full ladder (6e99f43: 0/157 settled, 16.0).
  • Against it: the control is UNSETTLED by construction and is accepted
    only as a floor. The first run used the initvals' retry limit of 15; the
    later runs programmed 15 and confirmed it by read-back.
  • Not a default library session, which sends NOACK with retry limit 0.

Withdrawn (kept in the doc): the BSSID gate's first run, which had the
monitor filter installed. Also the probe-response auto-ACK method
(staack's verdict), whose control cannot move against hostapd.

What it can't do

• The library's own RX path is promiscuous. Mt7612uRadio::StartRxLoop
  installs the monitor filter unconditionally, so a station driven through
  IRadio does not run the managed filter the cells measured.
  • Acknowledgement holds either way.
  • "Moving MT_MAC_ADDR makes it deaf" is a managed-filter property.
• No cell armed through IRadio. The seam writes no register, so the
  measured state is the state a successful arm leaves. The
  arm-then-measure path is still unexercised, and no in-tree demo calls
  SetStationIdentity, so the retry-limit warning is not bench-checked
  either.
• Every cell is an unassociated station. Power save, TIM, cross-BSS
  duplicate detection and hardware key lookup are untested.
• No data plane here, and no src/sta caller. Frame building, the
  supplicant, CCMP and the per-transmitter DupDetector belong to whatever
  drives the seam (a later PR).
• The MT7612U only. Every other backend returns the not-ported default.
• Scale: one DUT, one peer or AP, one channel, near field, no soak.

Follow-ups

• The Realtek station arm (SetStationIdentity on Jaguar1/2/3).
• The station harness that associates through IRadio over the src/sta
  core, and the condensed station doc.
• A role-selected managed receive filter on the MT7612U.

Verification

• Review record:

  • pre-open pass on aff83bb: two Flash reviewers, then an Opus checker that
    verified each finding against the tree;
  • 13 findings: 9 fixed after dedupe, 2 not real (the
    ClearStationIdentity default, the station_mode_ok peer sentence), 2
    wording;
  • mt7612uprobe staid now also checks that arming and clearing write no
    register, so it expects 12 passed, 0 failed (was 10).
  • qodo's first pass on mt7612u: an 802.11 station identity seam on IRadio, implemented for the MT7612U #460, 9 findings, all verified against the code
    and all 9 fixed:
    • src/mt7612u/CLAUDE.md reduced to pointers;
    • the station gates tear RX down in the established order
      (rx_teardown(): receiver off, then the EP4 ring; bringup.cpp
rx_teardown comment, Mt7612uRadio.cpp mt7612u_rx_quiesce);
    • sta_dut_take verifies that no driver is left bound;
    • staack verifies its restores;
    • bssen and sta verify their final register reset;
    • strict argument parsing for sta, staack, norsp and bssen;
    • one run per OUT (lock);
    • the AP re-enumerated only while it is still the accepted device;
    • bssen (and norsp) read the receive filter back.
  • Found in the same pass: the receiving dwells did not tick the PHY once a
    second, which the public header requires (mt7612u_phy_tick); they do
    now.
  • A bench run after that pass found the Realtek peer left without a
    driver: txdemo/rxdemo's libusb open detaches it and the harness only
    handed back the DUT. autoack and uplink now record the peer's
    idVendor:idProduct:serial and re-enumerate it on exit while the path
    still reports that ID (sta_peer_record / sta_peer_handback in
    tests/mt7612u_sta_lib.sh). This is harness-only; the gates are
    unchanged.
  • qodo's second pass on mt7612u: an 802.11 station identity seam on IRadio, implemented for the MT7612U #460, 4 findings, all verified and fixed:
    • a private OUT: an unset OUT gets a fresh mktemp -d (0700), and a
      given one is refused if it is a symlink or not owned by root or the
      invoking user;
    • the station gates stop the MAC when the first mt_mac_start() fails,
      since it sets ENABLE_TX before its WPDMA poll. This is an error path
      only; the measured path is unchanged;
    • the DUT is handed back only while its path still reports the
      VID:PID:serial recorded at take;
    • the peer must match PEER_VID:PEER_PID, not be a hub, and not be the
      DUT's path, or the run refuses to start.

• cmake -S . -B build -DDEVOURER_MT7612U=ON -DDEVOURER_REQUIRE_STA_CRYPTO_TESTS=ON && cmake --build build -j4 && ctest

  • 80/80 pass (2 skipped: mt7612u_usb_ids_vs_mt76 and
    mt7612u_initvals_generated, which skip in every configure here).
  • Includes the new mt7612u_station_identity and the station crypto
    cells.

• Default configure: 78/78 pass. mt7612u_station_identity is not
  registered there, because it is gated on DEVOURER_MT7612U.

• RTL8733B-only subset: 73/73 pass.

• MT7612U-only subset: 70/70 pass, and mt7612u_station_identity is
  registered.

• make -C src/mt7612u check: passes, "api_link: 39 public entry points
  resolved".

• Hardware (root, mt76x2u unbound, run from the repo root; the scripts
  link ./firmware to /lib/firmware/mediatek):

  • mt7612uprobe staid: the contract on the chip.
  • tests/mt7612u_sta_autoack.sh: needs a Realtek CCX peer.
  • tests/mt7612u_sta_identity.sh: needs a hostapd-capable AP adapter.
  • tests/mt7612u_sta_uplink.sh: needs a Realtek ACK-responder peer.

Bench record.

Rig, same for both heads:

• MT7612U at 7-1, own 40:a5:ef:5a:32:f8;
• peer RTL8812CU at 5-1;
• AP RTL8812BU at 1-1.1 on rtw_8822bu with hostapd;
• ch6, near field.

a1873ee:

• staid: 10/10.
• autoack:
  • A 845/845, 0.12 retries;
  • B, C, D 0% at 12.00;
  • E 877/877, but slot 1, so not evidence.
• uplink: A 200/200. B was cut off by an outer timeout.
• identity:
  • sta read 5.9-6.5k unicast per arm, filtr=00015f97;
  • D-F wrote slot 1;
  • staack arm C "received NOTHING".

6e99f43:

• staid: 10/10.
• autoack: "3 passed".
  • A 858/858, 0.04 retries;
  • B, C, D 0% at 12.00;
  • E 867/867 in the verified station slot.
• uplink (FRAMES=200): "1 passed".
  • A 200/200, 0.0 retries, max 1;
  • B 0/157 at 16.0.
• identity: reproduced twice.
  • staid passes, and staack arm C "received NOTHING".
  • The sta gate's bring-up then logged 8 MCU timeouts, and every arm read
    0-8 frames and no beacons. The gate said INCONCLUSIVE, correctly.
  • The unicast injector reached about 30 frames/s of 300 (230/s in the
    a1873ee run).

Adversarial sides:

• one unit, one peer, one AP, one run per arm;
• staack's verdict is not evidence.

b94e119 (gates in their final form), same new rig; peer RTL8812CU at 5-1:

• autoack: rc 0, "3 passed".
  • A 860/860, 0.06 mean retries.
  • B, C, D 0% at 12.00.
  • E 873/873 at 0.11, with a wrong BSSID in the verified, enabled station
    slot 0.
• identity: rc 0, 0 MCU timeouts, DUT and AP handed back.
  • staid 12/12; staack arm C received nothing.
  • sta verified, to_us: A 6268, B 5871, C 5881, D 5885, E 5877,
    F 6018.
  • The injector achieved 35742 frames in 123 s.
• Against it:
  • one unit, one peer, one AP, one run per arm;
  • B-F sit 4-6% below A, the unexplained first-arm excess again;
  • arm E is acknowledgement only;
  • staack's verdict is not evidence.
• That run found the peer left driverless after autoack; fixed as above.
  Confirmed with no manual hand-back on 87aef53 and again on cc48be4
  (after the qodo pass-2 harness guards): autoack "3 passed" both times,
  and afterwards the DUT (1-1) was on mt76x2u and the peer (5-1) on
  rtl88x2cu, both with their netdevs.

523a324, new rig:

• DUT MT7612U at 1-1 (480 Mbit/s); AP RTL8812BU at 8-1 (USB3).
• identity: rc 0, 0 MCU timeouts, both adapters handed back.
  • staid 10/10; staack arm C deaf as recorded (verdict INCONCLUSIVE by
    design).
  • sta "measured", with the table above.
  • The injector achieved 35964 frames in 124 s, about 290/s.

The two failing identity runs (inferred, not proven).

• The AP sat on a hub port that enumerated at FULL speed (12 Mbit/s).
• Its transmit path stalled at about 30 frames/s of the 300 asked, and its
  beacons stopped.
• The DUT's channel calibrations timed out during the flood, the late-reply
  failure mcu.cpp documents.
• On high-speed ports the same harness measured cleanly.
• Not proven: the good a1873ee run had the AP at the same full-speed
  position.
• Guards kept:
  • the stimulus starts only after the gate reports its bring-up done;
  • a table fed at under half the asked rate is refused as an AP stall;
  • the harness header states the rig requirement (both adapters at high
    speed or better, and no full-speed hub).
• The code-level analysis stands: the timeouts precede every line
  gate_sta changed, and no kernel driver touched the DUT mid-run.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VNC8xhn1rNCi5t6uLvE6M3

[snokvist]

@josephnef PR4 of the station series, now that #452 and #454 are in. It is one commit on 101fdc9: the IRadio station identity seam, with the MT7612U as its first implementation. It builds on #452's retry-limit semantics: a station session has to set tx.retry_limit explicitly and send unicast with build_stream_radiotap(mode, false), and the arm WARNs when the limit is 0.

Before opening it went through two independent review passes, each checked against the tree, and it was re-verified on an MT7612U: staid 12/12 including the new no-write readback, autoack, identity and uplink. The bench record in the description covers each head. The Realtek station arm (PR5) and the station harnesses (PR6) will be separate PRs.

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Add MT7612U infrastructure-station identity support to IRadio

✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

AI Description

• Add a station-identity API and capability flag so callers can identify supported radios before
 association.
• Arm MT7612U station identity without changing MAC registers, and track competing port-identity
 claims.
• Add headless tests, hardware measurement harnesses, and a record of evidence and limitations.

Diagram

graph TD
  Caller["Station caller"] --> Caps["Capability check"] --> API["IRadio seam"] --> Radio["MT7612U wrapper"] --> Core["Station core"] --> Registers["MAC readback"]
  Claimants["Port claimants"] --> Core
  Core --> State["Station arm"]

Loading

High-Level Assessment

Keep the separate, read-only station seam. Reusing SetAckResponder(bssid) would move the MT7612U port identity away from the station's address; programming BSSID registers adds another writer without a demonstrated receive benefit. A later PR should exercise the seam through IRadio with an associated station and address the library's promiscuous RX filter.

Files changed (24) +3325 / -11

Enhancement (11) +700 / -3

caps_event.hEmit station-mode support in adapter capabilities +1/-0

Emit station-mode support in adapter capabilities

• Includes AdapterCaps::station_mode_ok as station_mode in adapter.caps events.

examples/common/caps_event.h

AdapterCaps.hDefine the measured station-mode capability +46/-0

Define the measured station-mode capability

• Adds station_mode_ok, defaulting to false for unported backends. Its contract specifies the on-air evidence required to enable it and the limits of the MT7612U measurements.

src/AdapterCaps.h

IRadio.hIntroduce the vendor-neutral station-identity contract +104/-1

Introduce the vendor-neutral station-identity contract

• Adds SetStationIdentity and ClearStationIdentity defaults, with argument, ordering, rollback, port-ownership, and transmit requirements. Expands StartRxLoop documentation to explain callback threading and the libusb lock-order deadlock.

src/IRadio.h

Mt7612uRadio.cppExpose station identity through the MT7612U radio +46/-0

Expose station identity through the MT7612U radio

• Delegates station arm and clear operations to the C backend and warns when an armed session has a zero TX retry limit. Reports station_mode_ok as supported on this backend.

src/mt7612u/Mt7612uRadio.cpp

Mt7612uRadio.hDeclare MT7612U station-identity overrides +8/-0

Declare MT7612U station-identity overrides

• Declares the arm and clear overrides and notes that this read-only implementation cannot detect an arm attempted before the RX loop.

src/mt7612u/Mt7612uRadio.h

StationIdentity.hSeparate station validation and ownership policy from I/O +186/-0

Separate station validation and ownership policy from I/O

• Adds pure decisions for argument validation, fail-closed register reads, and tri-state port comparison. Tracks whether a later port claim drops an arm or a failed beacon start restores it.

src/mt7612u/StationIdentity.h

beacon.cppRestore eligible station arms after failed beacon starts +9/-1

Restore eligible station arms after failed beacon starts

• Names beacon identity claims when arming the ACK responder. On failed start, rechecks the unwound port identity and restores only a station arm dropped by that start.

src/mt7612u/beacon.cpp

caps.cppRecheck station ownership after ACK-responder changes +27/-1

Recheck station ownership after ACK-responder changes

• Checks the station arm against register readback after responder arm and clear operations, dropping it only on a verified port move. Provides a caller-aware responder entry point for beacon diagnostics.

src/mt7612u/caps.cpp

mt7612u.hPublish the MT7612U station C API +32/-0

Publish the MT7612U station C API

• Declares station arm, clear, and recorded-BSSID retrieval functions. Documents why the implementation verifies existing identity and auto-response state rather than writing MAC or BSSID registers.

src/mt7612u/include/mt7612u/mt7612u.h

internal.hStore station state on the device and expose recheck hooks +18/-0

Store station state on the device and expose recheck hooks

• Adds station arm state to mt7612u_dev and declares internal helpers used by beacon and ACK-responder port-identity changes.

src/mt7612u/internal.h

station.cppImplement the read-only MT7612U station arm +223/-0

Implement the read-only MT7612U station arm

• Refuses an arm unless the requested own address matches MT_MAC_ADDR and MT_AUTO_RSP_EN reads as enabled; failed reads refuse the arm. Records the BSSID for the host, clears state without hardware writes, and warns when later port changes drop or leave an arm unverified.

src/mt7612u/station.cpp

Tests (8) +2323 / -0

api_link.cLink-check three new station C entry points +3/-0

Link-check three new station C entry points

• Adds arm, clear, and BSSID retrieval to the C-compiled public API link test.

src/mt7612u/tests/api_link.c

bringup.cppAdd station hardware gates to mt7612uprobe +1052/-0

Add station hardware gates to mt7612uprobe

• Adds staid, sta, staack, norsp, and bssen gates for register-contract, managed-receive, and auto-ACK experiments. Includes readback and inconclusive-result checks so invalid setup is not treated as evidence.

src/mt7612u/tools/bringup.cpp

mt7612u_sta_autoack.shMeasure downlink auto-ACK from the transmitting peer +294/-0

Measure downlink auto-ACK from the transmitting peer

• Runs DUT and control arms while a Realtek transmitter reports ACK outcomes through per-frame TX reports. Refuses missing reports, interrupted arms, and dead-device controls.

tests/mt7612u_sta_autoack.sh

mt7612u_sta_identity.shDrive managed station-receive experiments against an AP +287/-0

Drive managed station-receive experiments against an AP

• Sets up a guarded hostapd AP, runs the station contract and port-move gates, and injects unicast for the BSSID receive comparison. Checks gate status and injector throughput before accepting a measurement.

tests/mt7612u_sta_identity.sh

mt7612u_sta_lib.shShare guarded USB and process cleanup for station harnesses +74/-0

Share guarded USB and process cleanup for station harnesses

• Provides run-specific PID tracking, MT7612U VID:PID validation, driver unbinding, and guarded USB re-enumeration to hand the DUT back after tests.

tests/mt7612u_sta_lib.sh

mt7612u_sta_uplink.shVerify acknowledged station uplink with a controlled peer +258/-0

Verify acknowledged station uplink with a controlled peer

• Compares MT7612U TX-status retries when a Realtek peer answers for the target or another address. Requires an explicitly nonzero, read-back-confirmed retry limit and rejects unreliable success attribution.

tests/mt7612u_sta_uplink.sh

mt7612u_station_selftest.cppExercise station refusal and ownership decisions headlessly +232/-0

Exercise station refusal and ownership decisions headlessly

• Tests malformed arguments, both failed-read paths, port matching, and auto-response gating. Covers arm loss, unknown readback, conditional restoration, and clearing recorded addresses.

tests/mt7612u_station_selftest.cpp

sta_unicast_inject.pyGenerate bounded unicast stimulus for the receive gate +123/-0

Generate bounded unicast stimulus for the receive gate

• Injects FromDS unicast frames through a monitor interface so the identity harness can measure delivery to the DUT rather than beacons alone. Validates inputs, caps the send rate, and reports the injected count.

tests/sta_unicast_inject.py

Documentation (4) +284 / -8

logging.mdDocument the station capability event field +1/-1

Document the station capability event field

• Adds station_mode to the documented adapter.caps event fields.

docs/logging.md

mt7612u-station-identity.mdRecord station-identity measurements and their limits +258/-0

Record station-identity measurements and their limits

• Documents controlled receive, auto-ACK, and uplink measurements supporting the MT7612U implementation. Retains withdrawn results and explains rig failures, unassociated-station coverage, and the managed-versus-monitor filter limitation.

docs/mt7612u-station-identity.md

mt7612u.mdList the new station test and API entry points +9/-7

List the new station test and API entry points

• Adds the gated station-identity ctest to the MT7612U testing inventory and updates the public C API link-test count to 39.

docs/mt7612u.md

CLAUDE.mdDocument MT7612U station-identity hazards +16/-0

Document MT7612U station-identity hazards

• Warns against using the ACK-responder API for a station and distinguishes measured managed-filter behavior from the library's monitor-filter RX path.

src/mt7612u/CLAUDE.md

Other (1) +18 / -0

CMakeLists.txtBuild the station backend and register its headless test +18/-0

Build the station backend and register its headless test

• Adds the MT7612U station implementation and policy header to the optional backend. Registers the policy self-test when DEVOURER_MT7612U is enabled.

CMakeLists.txt

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Local users can redirect root test writes ✓ Resolved

Description

The harnesses accept a predictable /tmp output path with mkdir -p, and sta_lock_take() then
writes through that path without rejecting a pre-existing symlink. If a local user creates the path
before a root-run harness starts, lock and test-output writes can be redirected to another
directory.

Code

tests/mt7612u_sta_lib.sh[42]

+  echo "$$" > "$OUT/.lock/pid"

Evidence

All three scripts default OUT to a fixed path under /tmp and run as root. They use mkdir -p
before the shared lock helper writes $OUT/.lock/pid; neither operation rejects an OUT symlink.

tests/mt7612u_sta_identity.sh[49-57]
tests/mt7612u_sta_autoack.sh[47-58]
tests/mt7612u_sta_uplink.sh[62-76]
tests/mt7612u_sta_lib.sh[27-43]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The root-run harnesses use predictable output paths under `/tmp`; a pre-existing symlink can redirect their writes.
## Fix Focus Areas
- tests/mt7612u_sta_lib.sh[27-50]
- tests/mt7612u_sta_identity.sh[49-57]
- tests/mt7612u_sta_autoack.sh[47-58]
- tests/mt7612u_sta_uplink.sh[62-76]
## Recommended Fix
Create or validate a non-symlink output directory owned by the invoking root process with restrictive permissions before creating the lock or any output files. Reject unsafe existing paths rather than following them.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. A replacement radio can be reset ✓ Resolved

Description

sta_dut_take() verifies the DUT only by vendor and product IDs and does not save an instance
identity before setting STA_DUT_TAKEN. sta_dut_handback() repeats only that same check before
toggling authorized, so unplugging the original device and attaching another MT7612U at the same
sysfs path resets the replacement during cleanup.

Code

tests/mt7612u_sta_lib.sh[R81-87]

+sta_dut_handback() {
+  [ "$STA_DUT_TAKEN" = yes ] || return 0
+  STA_DUT_TAKEN=no
+  sta_is_mt7612u "$DUT_SYSFS" || return 0
+  echo 0 > "/sys/bus/usb/devices/$DUT_SYSFS/authorized" 2>/dev/null
+  sleep 2
+  echo 1 > "/sys/bus/usb/devices/$DUT_SYSFS/authorized" 2>/dev/null

Evidence

The helper defining the current DUT guard reads only idVendor and idProduct; unlike the peer
helper, acquisition stores no identity for comparison. The cleanup path performs the destructive
authorization toggle after that same VID:PID-only guard.

tests/mt7612u_sta_lib.sh[53-56]
tests/mt7612u_sta_lib.sh[63-87]
tests/mt7612u_sta_lib.sh[111-121]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
Issue description
The destructive DUT handback operation identifies a device solely by VID:PID. A replacement MT7612U at the same sysfs path therefore passes the check and is deauthorized/re-authorized even though it was never acquired by this test run.
Fix Focus Areas
- tests/mt7612u_sta_lib.sh[63-87]
Recommended Fix
Capture the DUT identity from `sta_usb_id "$DUT_SYSFS"` after accepting it in `sta_dut_take()`. Only perform the `authorized` toggle in `sta_dut_handback()` when the current identity exactly matches the recorded one; otherwise log and leave the device untouched.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. An unrelated USB device can be reset ✓ Resolved

Description

sta_peer_record() accepts any sysfs USB device with readable identity fields, without comparing it
to PEER_VID, PEER_PID, or checking that it is a wireless adapter. The auto-ACK and uplink
harnesses then call sta_peer_handback(), so a mistaken PEER_SYSFS can deauthorize and
re-enumerate an unrelated device or hub during cleanup.

Code

tests/mt7612u_sta_lib.sh[R130-135]

+sta_peer_record() {
+  STA_PEER_ID=$(sta_usb_id "$PEER_SYSFS")
+  [ -n "$STA_PEER_ID" ] || {
+    echo "refusing PEER_SYSFS=$PEER_SYSFS - no USB device there"
+    return 1
+  }

Evidence

The shared peer recorder obtains a generic USB identity and accepts any nonempty value, while the
handback code toggles authorization once that identity remains present. Both peer-based harnesses
expose configurable VID/PID and sysfs path but invoke the generic recorder without cross-validating
those values.

tests/mt7612u_sta_lib.sh[123-150]
tests/mt7612u_sta_autoack.sh[40-60]
tests/mt7612u_sta_uplink.sh[55-78]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
Issue description
Peer recording accepts any USB sysfs node and subsequent cleanup toggles that node's `authorized` attribute. A typo or incorrect `PEER_SYSFS` can therefore reset unrelated USB hardware, including a hub.
Fix Focus Areas
- tests/mt7612u_sta_lib.sh[123-150]
- tests/mt7612u_sta_autoack.sh[40-60]
- tests/mt7612u_sta_uplink.sh[55-78]
Recommended Fix
Pass the configured peer VID and PID into the shared validation or validate them before `sta_peer_record()`. Refuse devices whose IDs do not match, reject hubs, and require the expected wireless interface before recording an identity eligible for handback.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

View action required (2)

4. Station guidance repeats header rules ✓ Resolved

Description

The new CLAUDE.md guidance restates the header’s warning against SetAckResponder(bssid) and its
explanation that SetStationIdentity does not write the port register. When that contract changes,
readers must reconcile two versions even though this guidance already points them to the header.

Code

src/mt7612u/CLAUDE.md[R70-73]

+- **Never arm a station with `SetAckResponder(bssid)`.** On this part it
+  retargets `MT_MAC_ADDR`, the register the auto-response engine matches
+  address 1 against, so the station stops being acknowledged - and under the
+  managed filter stops receiving too. `SetStationIdentity` writes no register

Evidence

Rule 1 requires agent documentation to reference existing header contracts rather than duplicate
them. The added subtree guidance repeats the warning and register behavior documented in the public
station identity header.

CLAUDE.md: Keep Agent Documentation Narrow and Avoid Duplicated Header Contracts: CLAUDE.md: Keep Agent Documentation Narrow and Avoid Duplicated Header Contracts: CLAUDE.md: Keep Agent Documentation Narrow and Avoid Duplicated Header Contracts: CLAUDE.md: Keep Agent Documentation Narrow and Avoid Duplicated Header Contracts: CLAUDE.md: Keep Agent Documentation Narrow and Avoid Duplicated Header Contracts: CLAUDE.md: Keep Agent Documentation Narrow and Avoid Duplicated Header Contracts: CLAUDE.md: Keep Agent Documentation Narrow and Avoid Duplicated Header Contracts: CLAUDE.md: Keep Agent Documentation Narrow and Avoid Duplicated Header Contracts
src/mt7612u/CLAUDE.md[70-75]
src/mt7612u/include/mt7612u/mt7612u.h[304-310]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new station guidance duplicates a contract documented in the station identity header.
## Fix Focus Areas
- src/mt7612u/CLAUDE.md[70-75]
## Recommended Fix
Replace the repeated refusal and register-write explanation with a concise pointer to the relevant header contract, retaining only guidance not documented there.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

5. Station probes can wedge receive DMA ✓ Resolved

Description

The new receive probes call mt_async_stop() before disabling the MAC receiver. On a busy channel,
cancelling the EP4 ring leaves RX enabled through mt_mac_stop()'s flush and TX-idle wait, creating
an undrained interval that can stop receive DMA.

Code

src/mt7612u/tools/bringup.cpp[R4513-4514]

+		mt_async_stop(&dev);
+		mt_mac_stop(&dev);

Evidence

The probe's normal teardown has the unsafe order, as do the new staack, norsp, and bssen
paths. The existing API contract explains why RX must be disabled first, and mt_mac_stop() clears
RX only after other work.

src/mt7612u/tools/bringup.cpp[4510-4515]
src/mt7612u/tools/bringup.cpp[4774-4779]
src/mt7612u/tools/bringup.cpp[5098-5102]
src/mt7612u/tools/bringup.cpp[5203-5206]
src/mt7612u/internal.h[378-385]
src/mt7612u/init.cpp[301-339]
src/mt7612u/tools/bringup.cpp[83-98]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new station probes cancel the RX ring while the MAC receiver is still enabled, risking an undrained EP4 and stalled RX DMA.
## Fix Focus Areas
- src/mt7612u/tools/bringup.cpp[4462-4475]
- src/mt7612u/tools/bringup.cpp[4510-4515]
- src/mt7612u/tools/bringup.cpp[4730-4779]
- src/mt7612u/tools/bringup.cpp[5044-5102]
- src/mt7612u/tools/bringup.cpp[5147-5206]
## Recommended Fix
At every normal and error-path teardown of these RX probes, disable MAC RX before cancelling the ring; use the existing `rx_teardown()` helper, then call `mt_mac_stop()`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

6. A failed probe leaves transmission enabled ✓ Resolved

Description

The new gates return directly when their first mt_mac_start() call fails, without calling
mt_mac_stop(). If its WPDMA poll times out, mt_mac_start() has already enabled MAC transmission,
and the probe exits without disabling it.

Code

src/mt7612u/tools/bringup.cpp[R4544-4546]

+		if (mt_mac_start(&dev, MT_RX_DRAIN_NONE)) {
+			sta_reset_bss(&dev, dev.macaddr); return 1;
+		}

Evidence

The new gates skip MAC teardown on initial start failure. mt_mac_start() enables TX before a poll
that can fail, while mt_mac_stop() clears the TX and RX enable bits.

src/mt7612u/tools/bringup.cpp[4542-4549]
src/mt7612u/tools/bringup.cpp[5122-5131]
src/mt7612u/init.cpp[269-290]
src/mt7612u/init.cpp[319-347]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The initial MAC-start failure branches in the new station gates skip teardown even though MAC transmission may already be enabled.
## Fix Focus Areas
- src/mt7612u/tools/bringup.cpp[4544-4546]
- src/mt7612u/tools/bringup.cpp[4812-4813]
- src/mt7612u/tools/bringup.cpp[5122-5125]
- src/mt7612u/tools/bringup.cpp[5227-5230]
## Recommended Fix
Call `mt_mac_stop()` on each initial `mt_mac_start()` failure path before returning, while retaining any gate-specific register cleanup.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

7. A failed unbind still starts the DUT tests ✓ Resolved

Description

sta_dut_take() discards the result of writing to the kernel driver's unbind file and returns
success unconditionally. If unbinding fails while the driver still owns the DUT, all three harnesses
proceed with the probe and cleanup later toggles USB authorization as though acquisition succeeded.

Code

tests/mt7612u_sta_lib.sh[R32-35]

+  STA_DUT_TAKEN=yes
+  echo "$DUT_SYSFS:1.0" > /sys/bus/usb/drivers/mt76x2u/unbind 2>/dev/null
+  sleep 2
+  return 0

Evidence

The helper has no set -e protection, marks the DUT taken before the unchecked write, and
explicitly returns zero. Each harness trusts that result; handback relies on the prematurely set
flag.

tests/mt7612u_sta_lib.sh[25-45]
tests/mt7612u_sta_identity.sh[178-180]
tests/mt7612u_sta_autoack.sh[90-93]
tests/mt7612u_sta_uplink.sh[104-106]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A failed kernel-driver unbind is treated as successful DUT acquisition, so the harnesses proceed and later re-enumerate a device they did not acquire.
## Fix Focus Areas
- tests/mt7612u_sta_lib.sh[27-45]
## Recommended Fix
Check whether the DUT is bound to `mt76x2u`; when it is, require the unbind to succeed and verify ownership was released. Set `STA_DUT_TAKEN` only after successful acquisition, and return failure otherwise.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

8. Probe arms can retain the wrong identity ✓ Resolved

Description

gate_staack() ignores whether restoring MT_AUTO_RSP_EN succeeded and cannot tell whether
mt7612u_clear_ack_responder() restored MT_MAC_ADDR. If either operation fails after its arm, the
command continues into the next arm or its verdict with auto-response disabled or the port identity
still retargeted.

Code

src/mt7612u/tools/bringup.cpp[R4774-4777]

+		if (armi == 1)
+			mt_rmw(&dev, MT_AUTO_RSP_CFG, MT_AUTO_RSP_EN, MT_AUTO_RSP_EN);
+		else if (armi == 2)
+			mt7612u_clear_ack_responder(&dev);

Evidence

Arm B clears the response bit and arm C retargets the MAC, but neither restoration is verified. The
register read-modify-write helper can report success despite a failed write, while responder
clearing uses void writes and retains its saved state if an I/O failure is recorded.

src/mt7612u/tools/bringup.cpp[4742-4753]
src/mt7612u/tools/bringup.cpp[4774-4792]
src/mt7612u/usb.cpp[452-461]
src/mt7612u/caps.cpp[217-260]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The station ACK probe ignores restoration failures, allowing later arms to run with a changed response bit or port identity.
## Fix Focus Areas
- src/mt7612u/tools/bringup.cpp[4742-4753]
- src/mt7612u/tools/bringup.cpp[4774-4779]
## Recommended Fix
Read back and verify `MT_AUTO_RSP_EN` and `MT_MAC_ADDR` after their respective restoration paths. Abort with a failed or inconclusive result rather than running another arm when either cannot be restored.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

View review recommended (5)

9. A BSSID probe can leave its slot enabled ✓ Resolved

Description

gate_bssen() ignores the result of sta_reset_bss() during final cleanup and prints `GATE BSSEN:
done` regardless. If a cleanup register write fails, the wrong BSSID or its enable bit can remain in
hardware across probe processes without the completed gate reporting the failed restoration.

Code

src/mt7612u/tools/bringup.cpp[R5203-5207]

+	/* Leave the registers as init does: the chip keeps them across runs. */
+	sta_reset_bss(&dev, dev.macaddr);
+	mt_async_stop(&dev);
+	mt_mac_stop(&dev);
+	printf("GATE BSSEN: done\n");

Evidence

The gate explicitly sets the wrong APC BSSID and bit 16, then discards the reset result.
sta_reset_bss() returns failure for writes it cannot complete, and its comment states that the bit
persists across processes; gate_sta() also discards its final reset result.

src/mt7612u/tools/bringup.cpp[4358-4376]
src/mt7612u/tools/bringup.cpp[5154-5158]
src/mt7612u/tools/bringup.cpp[5203-5208]
src/mt7612u/tools/bringup.cpp[4553-4555]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The BSSID probe reports completion even when it fails to restore registers that persist across probe runs.
## Fix Focus Areas
- src/mt7612u/tools/bringup.cpp[4358-4376]
- src/mt7612u/tools/bringup.cpp[5186-5189]
- src/mt7612u/tools/bringup.cpp[5203-5208]
- src/mt7612u/tools/bringup.cpp[4553-4555]
## Recommended Fix
Check cleanup results on both normal and early exits, report a failed or inconclusive gate if restoration fails, and verify the BSSID base and APC slot state after cleanup. Apply the same check to `gate_sta()`'s final reset.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

10. Invalid dwell times appear to pass ✓ Resolved

Description

The new command dispatch parses probe durations with atoi(), which converts a malformed duration
to zero without rejecting it. For norsp or bssen, such an argument skips the receive dwell
entirely yet reaches GATE NORSP: done or GATE BSSEN: done with a successful exit.

Code

src/mt7612u/tools/bringup.cpp[R5388-5394]

+	} else if (!strcmp(cmd, "norsp")) {
+		rc = gate_norsp(argc > 2 ? (uint8_t)atoi(argv[2]) : 6,
+		                argc > 3 ? atoi(argv[3]) : 25,
+		                argc > 4 ? atoi(argv[4]) : 1);
+	} else if (!strcmp(cmd, "bssen")) {
+		rc = gate_bssen(argc > 2 ? (uint8_t)atoi(argv[2]) : 6,
+		                argc > 3 ? atoi(argv[3]) : 25);

Evidence

The dispatch uses unchecked atoi() for the new durations. Both probes' dwell loops are bypassed
for zero, after which their successful completion paths remain reachable.

src/mt7612u/tools/bringup.cpp[5378-5394]
src/mt7612u/tools/bringup.cpp[5090-5106]
src/mt7612u/tools/bringup.cpp[5199-5208]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Malformed probe durations silently become zero, causing receive probes to report completion without spending time on air.
## Fix Focus Areas
- src/mt7612u/tools/bringup.cpp[5378-5394]
- src/mt7612u/tools/bringup.cpp[5090-5106]
- src/mt7612u/tools/bringup.cpp[5199-5208]
## Recommended Fix
Parse all new probe numeric arguments with full-string and range validation, including a positive dwell time, before initializing or changing the device. Return an argument error for invalid values.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

11. Concurrent test runs kill each other ✓ Resolved

Description

sta_pid_init() unconditionally removes shared $OUT/.pid_ files, while sta_pid_record() and
sta_pid_kill() subsequently use those same fixed paths without any run ownership or lock. When two
harness invocations use the default or same OUT, the later invocation overwrites the first run's
records and the first cleanup can signal the later run's DUT, peer, injector, or hostapd process.

Code

tests/mt7612u_sta_lib.sh[R50-52]

+sta_pid_init() {
+  for _sta_n in "$@"; do rm -f "$OUT/.pid_$_sta_n"; done
+}

Evidence

The helper deletes and rewrites PID files identified only by $OUT and a role name, and all three
new harnesses default to deterministic output directories before calling it. Consequently,
overlapping runs share the same PID-file namespace rather than tracking only their own children.

tests/mt7612u_sta_lib.sh[47-66]
tests/mt7612u_sta_autoack.sh[47-58]
tests/mt7612u_sta_identity.sh[49-57]
tests/mt7612u_sta_uplink.sh[62-76]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
Issue description
The shared PID files under `$OUT` have no per-run ownership. Concurrent harness executions can overwrite each other's recorded PIDs and terminate processes they did not start.
Fix Focus Areas
- tests/mt7612u_sta_lib.sh[47-66]
- tests/mt7612u_sta_autoack.sh[47-58]
- tests/mt7612u_sta_identity.sh[49-57]
- tests/mt7612u_sta_uplink.sh[62-76]
Recommended Fix
Create a unique run-owned PID directory (or acquire an exclusive lock for the selected `OUT`) before removing or recording PID files. Make all PID record, kill, and cleanup operations use that directory, and refuse a concurrent invocation when ownership cannot be acquired.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

12. Cleanup can reset a replacement device ✓ Resolved

Description

The identity harness's cleanup writes the authorized toggle for AP_SYSFS after checking only
that the path still has an authorized attribute. If the accepted AP is unplugged and another USB
device appears at that sysfs path before cleanup, the trap resets that replacement even though the
initial USB, wireless, routing, and AP-mode checks no longer describe it.

Code

tests/mt7612u_sta_identity.sh[R94-98]

+  if [ -n "${AP_SYSFS:-}" ] && [ -e "/sys/bus/usb/devices/$AP_SYSFS/authorized" ]; then
+    echo 0 > "/sys/bus/usb/devices/$AP_SYSFS/authorized" 2>/dev/null
+    sleep 3
+    echo 1 > "/sys/bus/usb/devices/$AP_SYSFS/authorized" 2>/dev/null
+    sleep 8

Evidence

The script carefully validates the AP before use, but its later privileged reset has only an
attribute-existence check. The DUT helper demonstrates the required immediate VID:PID revalidation
pattern before its own authorization toggle.

tests/mt7612u_sta_identity.sh[94-105]
tests/mt7612u_sta_identity.sh[113-146]
tests/mt7612u_sta_lib.sh[38-45]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
Issue description
Cleanup can toggle USB authorization for a device that replaced the AP at the same sysfs path. The initial AP safety checks are not repeated immediately before the destructive authorization writes.
Fix Focus Areas
- tests/mt7612u_sta_identity.sh[94-105]
- tests/mt7612u_sta_identity.sh[113-146]
Recommended Fix
Persist a stable identity for the accepted AP, such as vendor, product, and serial where available, and revalidate it immediately before writing `authorized`. If identity cannot be verified or differs, skip the re-enumeration and emit a warning rather than resetting the current device at that path.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

13. The station test can certify a false result ✓ Resolved

Description

gate_bssen() labels the state as a managed receive filter after mt_mac_start() but never reads
MT_RX_FILTR_CFG back or examines the I/O failure caused by that write. Since mt_mac_start() uses
an unchecked write for that register, a failed write or retained monitor filter can leave the
enabled-BSSID experiment running outside its stated premise while the peer result is still reported
as evidence that the BSSID does not gate the station.

Code

src/mt7612u/tools/bringup.cpp[R5149-5152]

+	if (mt_mac_start(&dev, MT_RX_DRAIN_RING)) {
+		mt_async_stop(&dev); mt_mac_stop(&dev); return 1;
+	}
+	/* Managed filter as mt_mac_start() left it. */

Evidence

The common MAC-start routine writes the receive filter with bare mt_wr(), so its return value does
not establish that the filter landed. The new sta gate explicitly reads that register and treats
an unexpected value as unverified because the filter is necessary to evaluate the BSSID plane,
whereas bssen omits that control.

src/mt7612u/tools/bringup.cpp[5147-5152]
src/mt7612u/tools/bringup.cpp[4470-4486]
src/mt7612u/init.cpp[288-291]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
Issue description
The `bssen` gate assumes that `mt_mac_start()` installed the managed receive filter, but does not verify it. An unchecked filter-register write can leave the experiment in monitor/promiscuous mode and make its BSSID conclusion invalid.
Fix Focus Areas
- src/mt7612u/tools/bringup.cpp[5147-5152]
- src/mt7612u/tools/bringup.cpp[4470-4486]
- src/mt7612u/init.cpp[288-291]
Recommended Fix
After the second `mt_mac_start()` in `gate_bssen`, read `MT_RX_FILTR_CFG` with `mt_rr_chk()` and verify the managed-filter bit pattern used by `gate_sta`. Mark the gate inconclusive and clean up if the read fails or the expected filter is not present.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can add REVIEW.md to your repo root and Qodo follows it on every PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[snokvist]

/review

[qodo-free-for-open-source-projects]

Code review by qodo was updated up to the latest commit 87aef53

[josephnef]

Reviewed at cc48be4.

Verified here: configure with -DDEVOURER_MT7612U=ON -DDEVOURER_REQUIRE_STA_CRYPTO_TESTS=ON, build clean (only the pre-existing basic_types.h / Radiotap.c warnings), ctest 80/80 with mt7612u_station_identity registered and passing, make -C src/mt7612u check resolves 39 entry points. CI green on every job.

Not verified here: the on-air gates. The rig's CF-922AC (INVENTORY #17) is not plugged at the moment, so staid / autoack / identity / uplink could not be re-run on independent hardware. The bench record in the description is the only hardware evidence; it is honest about that (one unit, one peer, one run per arm, no arm through IRadio).

Library side: no findings. Checked the tri-state hand-off end to end (fail_post → unwind_identity → clear_ack_responder's own check at allow_restore=0 → the beacon's check at !sta_lost_before; idempotent, so the double call is correct), new mt7612u_dev{} zero-initialises sta, mt_rr_chk leaves *val untouched so the failed-read-vs-zero-address case really is caught, mt_io_restore keeps a beacon start's io-error delta honest, and the StartRxLoop thread/lock text matches what UsbTransport actually does in Async vs SpscFat mode.

One thing to fix before merge (test tool, not the library):

• bringup.cpp gate_sta: the arm loop's if (g_stop) break; falls straight into the verdict block, which only tests any_beacon / any_to_us / unverified. A SIGTERM during arm B, after arm A already saw beacons and unicast, prints GATE STA: measured and returns 0 with B–F never run. gate_txs and gate_tsfwrap return 3 INTERRUPTED - no verdict for this; gate_sta should too. tests/mt7612u_sta_identity.sh folds the gate's rc into its exit, so today a gate cut short by anything other than the harness's own Ctrl-C reads as a pass.

Nits, take or leave:

• gate_norsp / gate_bssen discard wait_ticking()'s return, so an interrupted 1 s dwell exits 0 with "done". Same rc-3 convention applies.
• gate_bssen's single bad path returns restored ? 1 : 2 for both a failed register write (a defect) and the "BIT(16) would not stay set" case (genuinely inconclusive). Split them.
• mt7612u_sta_autoack.sh / _uplink.sh cleanup: sta_pid_kill peer INT then sta_peer_handback — the peer PID was started inside a command substitution, so the lib's wait returns at once and the authorized toggle can land while txdemo/rxdemo is still inside chip de-init. A bounded kill -0 poll before re-enumerating closes it.
• Same two scripts: sta_peer_record sets STA_PEER_ID before the peer is ever opened, so a failing sta_dut_take (exit 2) still bounces the driver of a Realtek adapter this run never touched.
• All three harnesses: [ ! -e "$ROOT/firmware" ] follows symlinks, so a dangling operator link is treated as absent, overwritten, and then deleted by cleanup as "ours". && [ ! -L "$ROOT/firmware" ] keeps the stated rule.
• mt7612u_sta_lib.sh lock: kill -0 on a PID read from a reused OUT can match a recycled PID of an unrelated process and refuse the run until it exits. Failure mode is a refusal, never a wrong number, so this is cosmetic.

Interface shape looks right to me: the seam is separate from SetAckResponder for a measured reason, the not-ported defaults are the trivially-true ones, and station_mode_ok is gated on a bench cell rather than a code reading. Happy to approve once the gate_sta rc is in.

[josephnef]

Hardware follow-up at cc48be4, second unit and second rig: MT7612U Comfast CF-922AC (40:a5:ef:5f:65:51, USB3 hub port 4-2.3.2), peer RTL8812CU (0bda:c812, high-speed port 3-2.4), ch6, near field, one run per arm.

• mt7612uprobe staid: 12 passed, 0 failed.
• tests/mt7612u_sta_autoack.sh: 3 passed. A 1706/1706 at 0.01 mean retries; B, C, D 0% at 12.00; E (wrong BSSID, enabled station slot 0) 1695/1695 at 0.02.
• tests/mt7612u_sta_uplink.sh (FRAMES=60, RETRY_LIMIT=15 read back): 1 passed. A 60/60 at 1.9 mean retries (max 3), B 0/60 at 16.0 (UNSETTLED floor, as the script expects). The 1.9 vs your 0.0 is the only number that moved between units.
• tests/mt7612u_sta_identity.sh, AP = TP-Link T3U (8812BU) on rtw88 at a SuperSpeed root port, hostapd 2.11: staid 12/12; staack arm C received nothing with MT_MAC_ADDR moved (A and B both 100/100 responses, 1.0% retried, INCONCLUSIVE by design). The sta table: A 6141 / B 5747 unicast to us (B −6.4%, your first-arm excess again), then the AP stalled at arm C — rtw88_8822bu: failed to get tx report from firmware in the kernel log, injector 12289 frames in 123 s, beacons down to ~120 per arm, to_us 0 in D–F. The harness refused the table (under half rate, exit 1), which is the right call, so the BSSID half is not reproduced here; that is the rtw88 stall your doc describes, on a high-speed port this time, so the full-speed-hub inference is not the whole story. This rig has no vendor 8822bu module for its kernel, so a non-rtw88 AP was not available.
• Hand-back verified after each harness: DUT back on mt76x2u with its netdev.

Two harness portability notes from this rig, both worth a line in the script header rather than code:

• mt7612u_sta_identity.sh reads AP-ENABLED from hostapd's -f log. This host's hostapd (Arch 2.11) accepts -f and never creates the file, so the harness reported "hostapd did not come up" with hostapd running fine. I ran it through a shim that captures stdout into the -f path. A hostapd_cli/iw dev check would be host-independent.
• The AP re-enumerates to a different sysfs path when a driver first binds it (the T3U moved from bus 9 to bus 10 when rtw88 probed it, and the 8812BU from 3-2.3.3 to 4-2.3.3). AP_SYSFS taken before the driver load is stale; the guard catches it ("not a USB device") but the message does not say why.

Neither changes the verdict above: still a comment pending the gate_sta interrupt rc.

[josephnef]

Changing the status to changes requested: the gate_sta interrupt exit code (falls into the verdict block and reports measured, rc 0, when cut short mid-arm; convention is rc 3 INTERRUPTED) needs to land before merge. Details in my two comments above.