From 160560b2b2e30c9d14ec386b68603583d136defe Mon Sep 17 00:00:00 2001 From: snokvist Date: Wed, 9 Sep 2026 22:43:27 +0200 Subject: [PATCH 1/7] factory: refuse MediaTek and unanswered adapters instead of the Jaguar1 fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CreateRadio ends in an unconditional Jaguar1 construction, and read_chip_id() discarded its libusb return code, so any device that does not answer the Realtek SYS_CFG2 vendor read arrived there as chip-id 0x00 and came up as an RTL8812AU. Measured on the bench, the same transfer on both arms: RTL8812AU 0bda:8812 rc=1 chip_id=0x04 MT7612U 0e8d:7612 rc=-7 LIBUSB_ERROR_TIMEOUT destination byte UNTOUCHED The MediaTek arm was run with a 0xAA poison byte in the destination, and it survived: the 0 that reached the dispatch was the caller's own initialiser, not a reading. Two guards close that, both ahead of the Realtek read: - Mt7612uUsbIds.h, the mt76 mt76x2u vid:pid set, gated the way Kestrel gates. - read_chip_id() now returns nullopt when the transfer itself failed, which is not the same as reading 0x00 — a successful 0x00 keeps its fall-through so no cold-boot transient changes behaviour. The gate is vid:pid and NOT "vid is not 0x0bda", which #419 asks for literally. devourer already serves Realtek silicon behind ASUS, Edimax, D-Link, ZyXEL, MSI and Mercury vendor ids, and five of the eleven MediaTek ids share a vendor id with a Realtek table entry, so a vendor-id rule would refuse working adapters -- including the RTL8812AU this bench uses as its witness. The transfer-failure guard covers the same intent without that cost: it catches every non-Realtek part, named or not. tests/mt7612u_usb_ids_selftest.cpp (ctest cell mt7612u_usb_ids) pins the one property that makes a pre-SYS_CFG2 gate safe: the MediaTek set never claims a device a Realtek table owns. The shared-vendor-id count is reported, not asserted, so removing an OEM entry cannot fail the build. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj --- CMakeLists.txt | 12 ++++ src/AdapterCaps.h | 9 ++- src/WiFiDriver.cpp | 75 ++++++++++++++++++--- src/mt7612u/Mt7612uUsbIds.h | 57 ++++++++++++++++ tests/mt7612u_usb_ids_selftest.cpp | 102 +++++++++++++++++++++++++++++ 5 files changed, 246 insertions(+), 9 deletions(-) create mode 100644 src/mt7612u/Mt7612uUsbIds.h create mode 100644 tests/mt7612u_usb_ids_selftest.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index beb2b0e6..57b28622 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -787,6 +787,18 @@ target_link_libraries(RadioIfaceSelftest PRIVATE devourer) add_test(NAME radio_iface COMMAND RadioIfaceSelftest) +# Headless guard for the MediaTek USB-id gate. CreateRadio consults it BEFORE +# the Realtek SYS_CFG2 read, so an id it wrongly claims is refused outright with +# no second chance. This pins the one property that makes that safe: the +# MediaTek pair set never claims a device a Realtek table owns. Header-only, so +# it costs a compile and no hardware. +add_executable(Mt7612uUsbIdsSelftest + tests/mt7612u_usb_ids_selftest.cpp +) +target_link_libraries(Mt7612uUsbIdsSelftest PRIVATE devourer) + +add_test(NAME mt7612u_usb_ids COMMAND Mt7612uUsbIdsSelftest) + # Headless guard for the TX quiesce seam (ITransport::quiesce_tx via # RtlAdapter): the explicit "stop TX and wait it out" call every device makes # before anything is released. UsbTransport's cancel/drain is validated on diff --git a/src/AdapterCaps.h b/src/AdapterCaps.h index 6aea9af9..3f4c22ae 100644 --- a/src/AdapterCaps.h +++ b/src/AdapterCaps.h @@ -39,7 +39,12 @@ enum class ChipGeneration : uint8_t { Jaguar2, Jaguar3, Rtl8733b, /* HALMAC 87xx 802.11n: RTL8731BU / RTL8733BU */ - Kestrel /* Wi-Fi 6 / 802.11ax (RTL8852BU/8852CU) */ + Kestrel, /* Wi-Fi 6 / 802.11ax (RTL8852BU/8852CU) */ + /* MediaTek MT7662 MAC (MT7612U / MT7662U, 2T2R 11ac USB) — the first + * non-Realtek generation. Register width, the vendor-request opcodes and the + * in-band MCU plane all differ; nothing that switches on this value may + * assume a Realtek register map. */ + Mt7612u }; inline const char *generation_name(ChipGeneration g) { @@ -54,6 +59,8 @@ inline const char *generation_name(ChipGeneration g) { return "rtl8733b"; case ChipGeneration::Kestrel: return "kestrel"; + case ChipGeneration::Mt7612u: + return "mt7612u"; default: return "unknown"; } diff --git a/src/WiFiDriver.cpp b/src/WiFiDriver.cpp index 566bfa24..992297f9 100644 --- a/src/WiFiDriver.cpp +++ b/src/WiFiDriver.cpp @@ -2,6 +2,7 @@ #include #include +#include #include #include @@ -30,6 +31,7 @@ #include "rtl8733b/Rtl8733bDevice.h" #endif #include "rtl8733b/Rtl8733bUsbIds.h" +#include "mt7612u/Mt7612uUsbIds.h" /* header-only VID:PID table, always compiled */ namespace { @@ -43,14 +45,29 @@ namespace { * 0x13 = RTL8822C, 0x17 = RTL8822E (RTL8812EU / RTL8822EU) -> Jaguar3 * 0x16 = RTL8733B (RTL8731BU / RTL8733BU, 1T1R) -> RTL8733B * The chip-id (not the USB PID) is authoritative because the rtl8822e RTL8812EU - * shares PID 0x8812 with the Jaguar1 RTL8812AU. Returns 0 on a failed read, - * which falls through to the Jaguar1 path. (8821C = 0x09 hardware-verified on a - * CF-811AC; it is a HalMAC/phydm Jaguar2 chip, NOT the page-write Jaguar1 the - * "8821C" name might suggest — routing it to Jaguar1 would fail at DLFW.) */ -uint8_t read_chip_id(libusb_device_handle *dev_handle) { + * shares PID 0x8812 with the Jaguar1 RTL8812AU. (8821C = 0x09 hardware-verified + * on a CF-811AC; it is a HalMAC/phydm Jaguar2 chip, NOT the page-write Jaguar1 + * the "8821C" name might suggest — routing it to Jaguar1 would fail at DLFW.) + * + * Returns nullopt when the control transfer itself failed, which is NOT the same + * as reading 0x00 and matters: this request is Realtek's vendor protocol, so a + * device that does not answer it is not Realtek silicon. Measured on the bench, + * same transfer, both arms: + * RTL8812AU 0bda:8812 rc=1 chip_id=0x04 + * MT7612U 0e8d:7612 rc=-7 LIBUSB_ERROR_TIMEOUT, destination byte UNTOUCHED + * (the MediaTek arm was run with a 0xAA poison byte, which survived — so the + * value the caller sees is its own initialiser, never a reading). Discarding the + * return code turned that into a plain 0, and 0 matches no id below, so the + * device reached the unconditional Jaguar1 fallback and came up as an RTL8812AU. + * Distinguishing the two lets CreateRadio refuse instead. A successful read of + * 0x00 is deliberately still returned as a value, so any cold-boot transient + * keeps its existing fall-through behaviour. */ +std::optional read_chip_id(libusb_device_handle *dev_handle) { uint8_t id = 0; - libusb_control_transfer(dev_handle, REALTEK_USB_VENQT_READ, 5, 0x00FC, 0, &id, - sizeof(id), USB_TIMEOUT); + int rc = libusb_control_transfer(dev_handle, REALTEK_USB_VENQT_READ, 5, 0x00FC, + 0, &id, sizeof(id), USB_TIMEOUT); + if (rc != static_cast(sizeof(id))) + return std::nullopt; return id; } @@ -170,7 +187,49 @@ WiFiDriver::CreateRadio(libusb_device_handle *dev_handle, #endif } - uint8_t chip_id = read_chip_id(dev_handle); + /* MediaTek MT7612U gates on the USB VID:PID BEFORE the SYS_CFG2 read, for the + * same reason Kestrel does above — but with a sharper failure mode. On this + * silicon the Realtek vendor request read_chip_id() issues (bRequest 5) is not + * implemented at all: the control transfer stalls, `id` stays 0, and 0 matches + * no Realtek chip-id, so the adapter reaches the unconditional Jaguar1 + * fallback at the end of this function (see read_chip_id's own note: "Returns + * 0 on a failed read, which falls through to the Jaguar1 path"). A MediaTek + * adapter then comes up as an RTL8812AU and every register access after it is + * addressed at the wrong MAC. Refusing here is the whole point of this gate. + * + * The pair set is disjoint from every USB id devourer tables, but the VENDOR + * ids are NOT — 0x0b05, 0x7392 and 0x2c4e each ship both silicon families — + * so this must stay a vid:pid test and must never be widened to "not a + * Realtek vendor id". Mt7612uUsbIds.h carries the measurement, and + * Mt7612uUsbIdsSelftest.cpp fails if a later id addition breaks it. */ + if (mt7612u::is_usb_id(vid, pid)) { + _logger->error("MediaTek MT7612U ({:04x}:{:04x}) detected; devourer has no " + "MediaTek radio backend yet — refusing rather than " + "misdetecting it as Realtek", + vid, pid); + return nullptr; + } + + /* A vendor read that did not complete means this device is not speaking the + * Realtek protocol at all — refuse instead of dropping through to the Jaguar1 + * fallback at the end of this function, which would bring an unknown part up + * as an RTL8812AU. This is the generation-independent half of the same guard + * the MediaTek gate above applies by id: the gate names the parts we know, + * this catches the ones we do not. It deliberately keys on TRANSFER FAILURE + * and not on "vid is not 0x0bda": devourer already serves Realtek silicon + * behind ASUS, Edimax, D-Link, ZyXEL, MSI and Mercury vendor ids, and every + * one of those answers this read normally (measured: an RTL8812AU returns + * 0x04), so a vendor-id rule would refuse working adapters. */ + std::optional chip_id_read = read_chip_id(dev_handle); + if (!chip_id_read) { + _logger->error("SYS_CFG2 chip-id read failed on {:04x}:{:04x} — not a " + "Realtek vendor-protocol device; refusing rather than " + "falling through to Jaguar1", + vid, pid); + return nullptr; + } + uint8_t chip_id = *chip_id_read; + if (rtl8733b::is_chip_id(chip_id)) { #if defined(DEVOURER_HAVE_8733B) _logger->info("Creating Rtl8733bDevice ({:04x}:{:04x}, chip-id 0x{:02x})", diff --git a/src/mt7612u/Mt7612uUsbIds.h b/src/mt7612u/Mt7612uUsbIds.h new file mode 100644 index 00000000..2c34ea19 --- /dev/null +++ b/src/mt7612u/Mt7612uUsbIds.h @@ -0,0 +1,57 @@ +#ifndef MT7612U_USB_IDS_H +#define MT7612U_USB_IDS_H + +#include + +namespace mt7612u { + +/* USB identities of the MediaTek MT7662-MAC adapters (MT7612U / MT7662U). + * + * Transcribed from the mainline mt76 driver's mt76x2u_device_table + * (drivers/net/wireless/mediatek/mt76/mt76x2/usb.c) — the set of devices the + * kernel binds to this MAC. Only 0e8d:7612 is hardware-verified here (an Alfa + * AWUS036ACM: ASIC version 0x76120044, EEPROM chip id 0x7612 — docs/mt7612u.md); + * the rest are the same MAC behind an OEM identity and are listed so they are + * refused rather than misdetected. The library's own identify step + * (MT_ASIC_VERSION >> 16 == 0x7612, usb.c) stays authoritative after the handle + * is open: an entry here only decides which backend gets to look. + * + * WHY VID:PID AND NOT VID. Three of these vendor ids also appear in devourer's + * Realtek tables — 0x0b05 (ASUS, RTL8832BU 0b05:1a62), 0x7392 (Edimax, + * RTL8812AU 7392:6822) and 0x2c4e (Mercury, 2c4e:0127) — because OEMs ship both + * silicon families under one vendor id. A vendor-id gate would therefore refuse + * working Realtek adapters. The full pair sets, by contrast, are disjoint: no + * vid:pid below matches any id in KestrelUsbIds.h or Rtl8733bUsbIds.h, which is + * what makes the pair gate authoritative ahead of the Realtek SYS_CFG2 read + * (the same argument KestrelUsbIds.h relies on). See Mt7612uUsbIdsSelftest.cpp, + * which fails if a future id addition breaks that disjointness. */ + +struct UsbId { + uint16_t vid; + uint16_t pid; +}; + +inline constexpr UsbId kUsbIds[] = { + {0x0e8d, 0x7612}, /* Alfa AWUS036ACM / Aukey USBAC1200 — the verified part */ + {0x0e8d, 0x7632}, /* HC-M7662BU1 (MT7662U) */ + {0x0b05, 0x1833}, /* Asus USB-AC54 */ + {0x0b05, 0x17eb}, /* Asus USB-AC55 */ + {0x0b05, 0x180b}, /* Asus USB-N53 B1 */ + {0x057c, 0x8503}, /* AVM FRITZ!WLAN AC860 */ + {0x7392, 0xb711}, /* Edimax EW-7722UAC */ + {0x2c4e, 0x0103}, /* Mercury UD13 */ + {0x0846, 0x9053}, /* Netgear A6210 */ + {0x045e, 0x02e6}, /* XBox One Wireless Adapter */ + {0x045e, 0x02fe}, /* XBox One Wireless Adapter */ +}; + +inline bool is_usb_id(uint16_t vid, uint16_t pid) { + for (const UsbId &id : kUsbIds) + if (id.vid == vid && id.pid == pid) + return true; + return false; +} + +} // namespace mt7612u + +#endif /* MT7612U_USB_IDS_H */ diff --git a/tests/mt7612u_usb_ids_selftest.cpp b/tests/mt7612u_usb_ids_selftest.cpp new file mode 100644 index 00000000..7f731e89 --- /dev/null +++ b/tests/mt7612u_usb_ids_selftest.cpp @@ -0,0 +1,102 @@ +/* Headless guard for the MediaTek USB-id gate (src/mt7612u/Mt7612uUsbIds.h, + * used by WiFiDriver::CreateRadio before the Realtek SYS_CFG2 read). + * + * The gate exists because a MediaTek adapter stalls read_chip_id(), which + * returns 0, which matches no Realtek chip-id, which lands the adapter in the + * unconditional Jaguar1 fallback. Refusing it depends on one property: the + * MediaTek vid:pid set must never claim a device some Realtek backend owns. + * That is what this test pins — a future id addition on either side that breaks + * it fails here rather than on a user's desk. + * + * It also records WHY the gate is vid:pid and not vid: several vendor ids ship + * both silicon families. That half is reported, not asserted, so removing an + * OEM entry cannot fail the build — only a genuine pair collision can. */ +#include "kestrel/KestrelUsbIds.h" +#include "mt7612u/Mt7612uUsbIds.h" +#include "rtl8733b/Rtl8733bUsbIds.h" + +#include + +namespace { + +int fails = 0; +void check(bool ok, const char *what) { + if (!ok) { + std::fprintf(stderr, "mt7612u_usb_ids: FAIL %s\n", what); + fails++; + } +} + +} // namespace + +int main() { + /* Every tabled MediaTek id is claimed. A table entry the lookup misses would + * silently reopen the Jaguar1 misroute for that adapter. */ + for (const mt7612u::UsbId &id : mt7612u::kUsbIds) { + if (!mt7612u::is_usb_id(id.vid, id.pid)) { + std::fprintf(stderr, "mt7612u_usb_ids: FAIL %04x:%04x tabled but not claimed\n", + id.vid, id.pid); + fails++; + } + } + + /* THE load-bearing property: the MediaTek gate must not claim any device a + * Realtek backend owns. It runs before the SYS_CFG2 read, so a false claim + * here is unrecoverable — the Realtek adapter is refused outright. */ + for (const rtl8733b::UsbId &id : rtl8733b::kUsbIds) { + if (mt7612u::is_usb_id(id.vid, id.pid)) { + std::fprintf(stderr, + "mt7612u_usb_ids: FAIL %04x:%04x is RTL8733B but the " + "MediaTek gate claims it\n", + id.vid, id.pid); + fails++; + } + } + for (const auto &id : kestrel::kKestrelUsbIds) { + if (mt7612u::is_usb_id(id.vid, id.pid)) { + std::fprintf(stderr, + "mt7612u_usb_ids: FAIL %04x:%04x is Kestrel but the " + "MediaTek gate claims it\n", + id.vid, id.pid); + fails++; + } + } + + /* Negative control: the most common Realtek adapter devourer serves, and the + * one the Jaguar1 fallback is FOR. If the gate ever claimed this, every + * RTL8812AU would stop working — so a test that only checked tabled ids + * passing would not be enough. 0bda:8812 is dispatched by chip-id, not by any + * table above, so it is not covered by the loops. */ + check(!mt7612u::is_usb_id(0x0bda, 0x8812), "RTL8812AU is not claimed"); + check(!mt7612u::is_usb_id(0x0bda, 0x8813), "RTL8814AU is not claimed"); + + /* A MediaTek id one digit off must NOT be claimed: the gate is an exact pair + * match, not a prefix or a vendor test. */ + check(mt7612u::is_usb_id(0x0e8d, 0x7612), "the verified Alfa AWUS036ACM is claimed"); + check(!mt7612u::is_usb_id(0x0e8d, 0x7613), "an untabled MediaTek pid is not claimed"); + check(!mt7612u::is_usb_id(0x0bda, 0x7612), "the pid alone does not claim"); + + /* Reported, not asserted — the rationale for pair-matching. Shared vendor ids + * are why a "not a Realtek vendor id" gate would refuse working adapters. */ + int shared_vids = 0; + for (const mt7612u::UsbId &m : mt7612u::kUsbIds) { + bool shared = false; + for (const rtl8733b::UsbId &r : rtl8733b::kUsbIds) + shared = shared || r.vid == m.vid; + for (const auto &k : kestrel::kKestrelUsbIds) + shared = shared || k.vid == m.vid; + if (shared) + shared_vids++; + } + std::printf("mt7612u_usb_ids: %d MediaTek ids, %d share a vendor id with a " + "Realtek table (why the gate matches pairs, not vendors)\n", + (int)(sizeof(mt7612u::kUsbIds) / sizeof(mt7612u::kUsbIds[0])), + shared_vids); + + if (fails) { + std::fprintf(stderr, "mt7612u_usb_ids: %d failure(s)\n", fails); + return 1; + } + std::printf("mt7612u_usb_ids: OK\n"); + return 0; +} From 4e9c4ff3ebb85b87d401aaffb3305466c10310f4 Mon Sep 17 00:00:00 2001 From: snokvist Date: Wed, 9 Sep 2026 22:58:10 +0200 Subject: [PATCH 2/7] mt7612u: compile the subtree as C++ under DEVOURER_MT7612U, so MSVC builds it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The subtree was reachable only from its own Makefile, on Linux. #419 asks for a DEVOURER_MT7612U option and for the pthread / nanosleep / clock_gettime plane to become / so MSVC and mingw can build it. Both here. The sources become .cpp rather than staying C compiled as C++, so there is one compilation mode and no second path that can rot unnoticed. Cost, measured by compiling each file with g++ -std=c++20 -Wall -Wextra before changing anything: 12 sites in the library (11 implicit void* conversions, 1 uint8_t->enum), and 40 in bringup of which 32 cascaded from three _Atomic declarations. No C++ keyword collisions, no VLAs, no compound literals, no tentative definitions; the two designated initialisers were already in declaration order. The exported surface stays C. include/mt7612u/mt7612u.h already had an extern "C" guard, and tests/api_link.c is deliberately still compiled by $(CC), so the C ABI is tested rather than assumed: it resolves 25 entry points against the C++ objects, and nm shows the public symbols unmangled. Threading and timing: pthread_mutex_t -> std::mutex io_lock (recursive) -> std::recursive_mutex pthread_cond_t -> std::condition_variable_any pthread_t -> std::thread nanosleep -> std::this_thread::sleep_for clock_gettime(MONOTONIC) -> std::chrono::steady_clock condition_variable_any, not condition_variable, so it waits on the bare mutex and every lock site keeps its original shape — a much smaller diff through code whose teardown ordering is documented as a use-after-free hazard. These waits are teardown and TX back-pressure, never a hot path. Two things fell out. A constructed std::recursive_mutex member retires the io_lock_ready flag: a zeroed pthread_mutex_t was a valid NON-recursive lock, so an open path that skipped the explicit init self-deadlocked the PHY tick, and that is now unrepresentable. And the teardown's cond_timedwait deadline arithmetic became one wait_for, dropping a CLOCK_REALTIME dependence where a wall-clock step could stretch or skip the 2 s budget. The two structs holding those members moved from calloc/free to new (std::nothrow) T{} / delete — calloc never runs a constructor. {} still zeroes every scalar, and nothrow keeps the existing null checks meaningful. -Wclass-memaccess then caught a real defect this introduced: frame_shape memset over a constructed device. Its loop case now declares the device inside the loop, which keeps the per-iteration reset the memset was doing. Not ported: the flock adapter lock is _WIN32-guarded, not mirrored. It works by contending for the SAME lock file UsbDeviceLock uses, but on Windows that class is a named mutex, so a file lock would exclude nobody and mirroring the mutex would duplicate what the devourer path already owns. Unprotected there is a bare mt7612u_open() with no devourer around it — the bench tool's case, and the bench is Linux. CI is what verifies the Windows claim, since none of it can be built locally: DEVOURER_MT7612U=ON is set in the multi-platform matrix (gcc, clang, MSVC cl, macOS) and in the mingw job, plus one stripped build-configs cell so the subtree cannot quietly acquire a dependency on a Realtek chip's sources. Verified locally: both CMake configs build warning-free with 59/59 ctest, the stripped cell builds with 49/49, and make -C src/mt7612u check passes. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj --- .github/workflows/cmake-multi-platform.yml | 9 ++ CMakeLists.txt | 38 +++++++ docs/mt7612u.md | 19 ++-- src/mt7612u/Makefile | 51 ++++++--- src/mt7612u/README.md | 83 ++++++++++---- src/mt7612u/{async.c => async.cpp} | 106 +++++++++--------- src/mt7612u/{caps.c => caps.cpp} | 0 src/mt7612u/{eeprom.c => eeprom.cpp} | 0 src/mt7612u/{fw.c => fw.cpp} | 17 +-- src/mt7612u/{init.c => init.cpp} | 19 ++-- src/mt7612u/internal.h | 35 ++++-- src/mt7612u/{mcu.c => mcu.cpp} | 6 +- src/mt7612u/{phy.c => phy.cpp} | 4 +- src/mt7612u/{radiotap.c => radiotap.cpp} | 2 +- src/mt7612u/{rx.c => rx.cpp} | 0 .../{field_macros.c => field_macros.cpp} | 0 .../tests/{frame_shape.c => frame_shape.cpp} | 16 ++- src/mt7612u/tools/{bringup.c => bringup.cpp} | 55 ++++----- src/mt7612u/{tx.c => tx.cpp} | 4 +- src/mt7612u/{usb.c => usb.cpp} | 103 +++++++++++------ 20 files changed, 365 insertions(+), 202 deletions(-) rename src/mt7612u/{async.c => async.cpp} (83%) rename src/mt7612u/{caps.c => caps.cpp} (100%) rename src/mt7612u/{eeprom.c => eeprom.cpp} (100%) rename src/mt7612u/{fw.c => fw.cpp} (94%) rename src/mt7612u/{init.c => init.cpp} (98%) rename src/mt7612u/{mcu.c => mcu.cpp} (98%) rename src/mt7612u/{phy.c => phy.cpp} (99%) rename src/mt7612u/{radiotap.c => radiotap.cpp} (99%) rename src/mt7612u/{rx.c => rx.cpp} (100%) rename src/mt7612u/tests/{field_macros.c => field_macros.cpp} (100%) rename src/mt7612u/tests/{frame_shape.c => frame_shape.cpp} (96%) rename src/mt7612u/tools/{bringup.c => bringup.cpp} (98%) rename src/mt7612u/{tx.c => tx.cpp} (99%) rename src/mt7612u/{usb.c => usb.cpp} (88%) diff --git a/.github/workflows/cmake-multi-platform.yml b/.github/workflows/cmake-multi-platform.yml index e67e7ee9..93d1d215 100644 --- a/.github/workflows/cmake-multi-platform.yml +++ b/.github/workflows/cmake-multi-platform.yml @@ -85,11 +85,17 @@ jobs: brew install libusb - name: Configure CMake + # DEVOURER_MT7612U=ON: the option defaults OFF, so without it here the + # MediaTek subtree would be compiled by nothing but its own Makefile on + # Linux. Turning it on in the matrix is what makes MSVC and macOS + # first-class for it — the POSIX-only pieces it still has (the flock + # adapter lock) are _WIN32-guarded, and this is what proves it. run: > cmake -B ${{ steps.strings.outputs.build-output-dir }} -DCMAKE_CXX_COMPILER=${{ matrix.cpp_compiler }} -DCMAKE_C_COMPILER=${{ matrix.c_compiler }} -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} + -DDEVOURER_MT7612U=ON -S ${{ github.workspace }} - name: Build @@ -136,6 +142,7 @@ jobs: -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++ + -DDEVOURER_MT7612U=ON - name: Build (library + stream demos + self-tests) # rxdemo / txdemo / precoder use POSIX-only APIs @@ -184,6 +191,8 @@ jobs: flags: "-DDEVOURER_JAGUAR1=OFF -DDEVOURER_8814=OFF -DDEVOURER_JAGUAR2_8822B=OFF -DDEVOURER_JAGUAR2_8821C=OFF -DDEVOURER_JAGUAR3_8822C=OFF -DDEVOURER_JAGUAR3_8822E=OFF -DDEVOURER_8733B=OFF -DDEVOURER_KESTREL_8852C=OFF" - name: kestrel-8852c-only flags: "-DDEVOURER_JAGUAR1=OFF -DDEVOURER_8814=OFF -DDEVOURER_JAGUAR2_8822B=OFF -DDEVOURER_JAGUAR2_8821C=OFF -DDEVOURER_JAGUAR3_8822C=OFF -DDEVOURER_JAGUAR3_8822E=OFF -DDEVOURER_8733B=OFF -DDEVOURER_KESTREL_8852B=OFF" + - name: mt7612u+jaguar1 + flags: "-DDEVOURER_MT7612U=ON -DDEVOURER_8814=OFF -DDEVOURER_JAGUAR2_8822B=OFF -DDEVOURER_JAGUAR2_8821C=OFF -DDEVOURER_JAGUAR3_8822C=OFF -DDEVOURER_JAGUAR3_8822E=OFF -DDEVOURER_8733B=OFF -DDEVOURER_KESTREL_8852B=OFF -DDEVOURER_KESTREL_8852C=OFF" - name: rtl8733b-only flags: "-DDEVOURER_JAGUAR1=OFF -DDEVOURER_8814=OFF -DDEVOURER_JAGUAR2_8822B=OFF -DDEVOURER_JAGUAR2_8821C=OFF -DDEVOURER_JAGUAR3_8822C=OFF -DDEVOURER_JAGUAR3_8822E=OFF -DDEVOURER_KESTREL_8852B=OFF -DDEVOURER_KESTREL_8852C=OFF" steps: diff --git a/CMakeLists.txt b/CMakeLists.txt index 57b28622..b352eae1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -41,6 +41,16 @@ option(DEVOURER_KESTREL_8852C "RTL8852CU / 8832CU (Kestrel 11ax, G6)" # with the older rtlwifi-style own-bit descriptor DMA, a different engine. # OFF by default — libusb-only builds are bit-identical. option(DEVOURER_PCIE "PCIe transport via vfio-pci (Linux; RTL8821CE)" OFF) +# MediaTek MT7612U / MT7662U — the first non-Realtek part. Compiles the +# src/mt7612u subtree (its own C++ library with a C ABI, landed unwired in #412) +# into libdevourer. OFF by default and deliberately NOT counted as chip support +# by the "No chip support selected" check below: there is no IRadio backend yet, +# so an MT7612U-only build would produce a library that can open nothing. It +# joins that list when the backend lands (#419). Turning it ON today buys one +# thing, and it is the point of the option: CI compiles the subtree across the +# whole platform matrix — gcc, clang, MSVC, mingw, macOS — where it was +# previously only ever built by its own Makefile, on Linux. +option(DEVOURER_MT7612U "MediaTek MT7612U / MT7662U (2T2R 11ac USB)" OFF) # Compile-time diagnostics floor (src/logger.h). Calls below the floor — # including their argument expressions at DVR_TRACE/DVR_DEBUG sites — compile @@ -346,6 +356,34 @@ if(DEVOURER_JAGUAR2_8821C) target_compile_definitions(devourer PUBLIC DEVOURER_HAVE_JAGUAR2_8821C=1) endif() +# --- MediaTek MT7612U (MT7662 MAC) --- +# The subtree is self-contained: it talks to libusb directly rather than through +# RtlAdapter/ITransport, because 32-bit registers and an in-band MCU over EP8/EP5 +# do not fit the Realtek shape. Its public surface is the C ABI in +# include/mt7612u/mt7612u.h; nothing here exports its internal headers, so the +# include directory is PRIVATE until the backend needs it. +if(DEVOURER_MT7612U) + target_sources(devourer PRIVATE + src/mt7612u/async.cpp + src/mt7612u/caps.cpp + src/mt7612u/eeprom.cpp + src/mt7612u/fw.cpp + src/mt7612u/init.cpp + src/mt7612u/mcu.cpp + src/mt7612u/phy.cpp + src/mt7612u/radiotap.cpp + src/mt7612u/rx.cpp + src/mt7612u/tx.cpp + src/mt7612u/usb.cpp + src/mt7612u/internal.h + src/mt7612u/regs.h + src/mt7612u/initvals.h) + target_include_directories(devourer PRIVATE + ${CMAKE_CURRENT_SOURCE_DIR}/src/mt7612u + ${CMAKE_CURRENT_SOURCE_DIR}/src/mt7612u/include) + target_compile_definitions(devourer PUBLIC DEVOURER_HAVE_MT7612U=1) +endif() + # --- PCIe transport (vfio-pci; see src/PcieTransport.h) --- if(DEVOURER_PCIE) target_sources(devourer PRIVATE diff --git a/docs/mt7612u.md b/docs/mt7612u.md index 3da9babf..8b4af98f 100644 --- a/docs/mt7612u.md +++ b/docs/mt7612u.md @@ -5,10 +5,11 @@ Everything below was measured on one MT7612U (`0e8d:7612`, `MT_ASIC_VERSION` running this project's own `rxdemo`/`txdemo`. Read `## Offline tests` and `## Counterparts` before quoting any number here. -**This code is not wired into the build.** `CMakeLists.txt` is untouched, there -is no `IRadio` implementation and no `WiFiDriver` dispatch. It is a -standalone C library plus a per-gate bringup harness, offered for the scope -question in the PR rather than for merge. +**There is still no radio backend.** `DEVOURER_MT7612U` (default OFF) compiles +the subtree into `libdevourer`, and `WiFiDriver::CreateRadio` now recognises the +MediaTek USB ids — but only to *refuse* them, because no `IRadio` implementation +exists yet. What the option buys today is that the whole CI matrix compiles the +subtree; what it does not buy is a devourer binary that can open the part. ## Why a MediaTek port is small @@ -341,10 +342,12 @@ Stated because the numbers above are uniformly favourable. firmware-running bit. - **The 48 ms fast retune is our implementation, not the floor.** The floor is unmeasured. -- **Nothing here runs in CI.** The offline tests above and the table - generator's `--check` exist and pass locally, but no workflow invokes them, - and the subtree is not reachable from `CMakeLists.txt`. No sanitizer build, - no lifecycle soak of the kind the Realtek backends carry. +- **The library compiles in CI; its own tests still do not run there.** With + `DEVOURER_MT7612U=ON` the whole platform matrix (gcc, clang, MSVC, mingw, + macOS) builds the subtree, so a portability regression is caught. The offline + tests above and the table generator's `--check` are still driven only by + `src/mt7612u/Makefile`, which no workflow invokes — nor is there a sanitizer + build or a lifecycle soak of the kind the Realtek backends carry. - **80 MHz, VHT on air, and NSS=2 are unexercised.** The rate word encodes them and the RX path decodes them; neither has been transmitted. - **The RX path delivers no FCS** (see above). That is a measured hardware diff --git a/src/mt7612u/Makefile b/src/mt7612u/Makefile index e0b93558..00f32857 100644 --- a/src/mt7612u/Makefile +++ b/src/mt7612u/Makefile @@ -1,38 +1,59 @@ -# Standalone: this subtree is not reachable from the project's CMakeLists.txt. -# tests/api_link.c links against the public header alone, which is what keeps -# that an honest test of the exported surface. -CC ?= gcc -CFLAGS ?= -O2 -g -Wall -Wextra -Wno-unused-parameter +# Standalone build of the subtree. The project's CMakeLists.txt also compiles +# the library sources (DEVOURER_MT7612U=ON); this Makefile stays because it +# builds `bringup` and the offline tests, which CMake does not. +# +# The library is C++ (see internal.h): MSVC has no , and devourer +# builds Windows first-class, so the sync and timing primitives are std:: types. +# Both builds therefore compile the SAME sources in the SAME language — there is +# no second compilation mode that can rot unnoticed. +# +# tests/api_link.c is deliberately still C. It includes only the public header, +# whose extern "C" guard is what keeps the exported surface callable from C; +# compiling it with $(CC) is what actually tests that, and it would silently +# stop testing it if it became C++ along with everything else. +CC ?= gcc +CXX ?= g++ +CXXFLAGS ?= -O2 -g -std=c++20 -Wall -Wextra -Wno-unused-parameter \ + -Wno-missing-field-initializers +CFLAGS ?= -O2 -g -Wall -Wextra # -MMD -MP: without header deps, editing internal.h leaves stale objects with # the old struct layout linked against new ones. That corrupts the device # struct at runtime and presents as a wild pointer, not a build problem. DEPFLAGS = -MMD -MP +INCS = -I. -Iinclude LDLIBS = -lusb-1.0 -lpthread -LIBSRCS = $(wildcard *.c) +LIBSRCS = $(wildcard *.cpp) TESTS = tests/api_link tests/frame_shape tests/field_macros -SRCS = $(LIBSRCS) tools/bringup.c $(TESTS:=.c) -OBJS = $(SRCS:.c=.o) -LIBOBJS = $(LIBSRCS:.c=.o) +CXXSRCS = $(LIBSRCS) tools/bringup.cpp tests/frame_shape.cpp tests/field_macros.cpp +CSRCS = tests/api_link.c +OBJS = $(CXXSRCS:.cpp=.o) $(CSRCS:.c=.o) +LIBOBJS = $(LIBSRCS:.cpp=.o) DEPS = $(OBJS:.o=.d) all: bringup $(TESTS) +# Linked with $(CXX): the library objects are C++ and need libstdc++, including +# on the api_link path whose own object is C. bringup: $(LIBOBJS) tools/bringup.o - $(CC) $(CFLAGS) -o $@ $^ $(LDLIBS) + $(CXX) $(CXXFLAGS) -o $@ $^ $(LDLIBS) # Links against the public header only, so a declaration with no definition -# is a link error here rather than a surprise at the first caller. +# is a link error here rather than a surprise at the first caller. Its object is +# compiled as C, so this also fails if the extern "C" guard ever regresses. tests/api_link: $(LIBOBJS) tests/api_link.o - $(CC) $(CFLAGS) -o $@ $^ $(LDLIBS) + $(CXX) $(CXXFLAGS) -o $@ $^ $(LDLIBS) tests/frame_shape: $(LIBOBJS) tests/frame_shape.o - $(CC) $(CFLAGS) -o $@ $^ $(LDLIBS) + $(CXX) $(CXXFLAGS) -o $@ $^ $(LDLIBS) tests/field_macros: $(LIBOBJS) tests/field_macros.o - $(CC) $(CFLAGS) -o $@ $^ $(LDLIBS) + $(CXX) $(CXXFLAGS) -o $@ $^ $(LDLIBS) + +%.o: %.cpp + $(CXX) $(CXXFLAGS) $(DEPFLAGS) $(INCS) -c -o $@ $< %.o: %.c - $(CC) $(CFLAGS) $(DEPFLAGS) -I. -Iinclude -c -o $@ $< + $(CC) $(CFLAGS) $(DEPFLAGS) $(INCS) -c -o $@ $< # No hardware and no privileges needed: safe to run anywhere. `bringup` is a # dependency but is not run: nothing else compiles it, so without it here a diff --git a/src/mt7612u/README.md b/src/mt7612u/README.md index 1b74fba9..6611f359 100644 --- a/src/mt7612u/README.md +++ b/src/mt7612u/README.md @@ -1,12 +1,19 @@ # src/mt7612u — MediaTek MT7612U -**Not reachable from `CMakeLists.txt` yet.** This subtree is a complete, -self-contained C library for the part — a public header, its own transport, no -dependency on `RtlAdapter` — plus the bring-up harness that produced every -measurement in `docs/mt7612u.md`. Wiring it in behind `IRadio` is a -follow-up PR; nothing in the shipped library links against this today. +**Compiled by `CMakeLists.txt` under `DEVOURER_MT7612U` (default OFF); still no +`IRadio` backend.** This subtree is a complete, self-contained library for the +part — a public C ABI, its own transport, no dependency on `RtlAdapter` — plus +the bring-up harness that produced every measurement in `docs/mt7612u.md`. +`WiFiDriver::CreateRadio` recognises the MediaTek USB ids today only to refuse +them; wiring a radio in behind `IRadio` is the follow-up. -It builds and tests on its own: +The sources are C++ (`.cpp`), not C: MSVC has no `` and devourer +builds Windows first-class, so the sync and timing primitives are `std::` types. +The *exported* surface is still C — `include/mt7612u/mt7612u.h` carries an +`extern "C"` guard, and `tests/api_link.c` is deliberately still compiled as C +so that stays true. + +It also builds and tests on its own: ```sh make -C src/mt7612u # -> src/mt7612u/bringup @@ -20,19 +27,19 @@ Measurements, methods and limits: [`../../docs/mt7612u.md`](../../docs/mt7612u.m | file | what | |---|---| -| `usb.c` | libusb transport: EP0 vendor register access, sync bulk, open/claim/reset | -| `async.c` | event thread, 16-deep RX ring, 32-slot TX pool | -| `mcu.c` | in-band MCU command framing (EP 8 out, EP 5 in, 4-bit sequence) | -| `fw.c` | ROM patch + ILM/DLM firmware upload | -| `eeprom.c` | 512-byte EEPROM: identity, TX power tables, RX gain | -| `init.c` | power-on, MAC initvals, mac_start/stop, EP-4 flush | -| `phy.c` | band/bandwidth/TX power registers, channel + calibration sequence | -| `tx.c` | TXWI + TXINFO construction | -| `rx.c` | RXWI parse, per-chain RSSI, rate decode | -| `radiotap.c` | `send_packet` / `send_packets` (USB chaining via `NEXT_VLD`) | -| `caps.c` | TSF, capability descriptor, ACK responder | -| `tools/bringup.c` | one subcommand per verified gate | -| `tests/` | offline tests (`make check`): public-API link, frame shapes | +| `usb.cpp` | libusb transport: EP0 vendor register access, sync bulk, open/claim/reset | +| `async.cpp` | event thread, 16-deep RX ring, 32-slot TX pool | +| `mcu.cpp` | in-band MCU command framing (EP 8 out, EP 5 in, 4-bit sequence) | +| `fw.cpp` | ROM patch + ILM/DLM firmware upload | +| `eeprom.cpp` | 512-byte EEPROM: identity, TX power tables, RX gain | +| `init.cpp` | power-on, MAC initvals, mac_start/stop, EP-4 flush | +| `phy.cpp` | band/bandwidth/TX power registers, channel + calibration sequence | +| `tx.cpp` | TXWI + TXINFO construction | +| `rx.cpp` | RXWI parse, per-chain RSSI, rate decode | +| `radiotap.cpp` | `send_packet` / `send_packets` (USB chaining via `NEXT_VLD`) | +| `caps.cpp` | TSF, capability descriptor, ACK responder | +| `tools/bringup.cpp` | one subcommand per verified gate | +| `tests/` | offline tests (`make check`): public-API link (C), frame shapes | | `initvals.h` | **generated** — see Provenance | ## The receiver must never run undrained @@ -60,11 +67,39 @@ Done here, because these are correctness issues regardless of compiler: - `` (this project's spelling) is tried first, with the distribution's `` as the fallback. -**Not** done here: `async.c` uses pthreads and `usb.c` uses `nanosleep` / -`clock_gettime`. This project has no C threading or time shim - its shim is -the C++ standard library, which every other backend uses directly. Building -a throwaway C shim now would be deleted at integration, so those two files -keep POSIX until the subtree joins the build. They are the only two. +Done at integration, as that paragraph used to promise: the sources are C++ +and the POSIX threading and timing primitives are gone. + +- `pthread_mutex_t` -> `std::mutex`, and the device's recursive `io_lock` -> + `std::recursive_mutex`. As a constructed member it also retires the old + `io_lock_ready` flag: a zeroed `pthread_mutex_t` was a valid NON-recursive + lock, so an open path that skipped the explicit init self-deadlocked the PHY + tick. That state is now unrepresentable. +- `pthread_cond_t` -> `std::condition_variable_any`. `_any` rather than the + plain one so it waits on the bare `std::mutex`, which let every lock site + keep its original shape instead of being restructured around `unique_lock` - + a smaller diff through code whose teardown ordering is a use-after-free + hazard. These waits are teardown and TX back-pressure, not a hot path. +- `pthread_t` -> `std::thread`; `pthread_create`'s error return becomes a + caught `std::system_error`, so the `goto fail` teardown is unchanged. +- `nanosleep` -> `std::this_thread::sleep_for`; `clock_gettime(CLOCK_MONOTONIC)` + -> `std::chrono::steady_clock`. The teardown's `pthread_cond_timedwait` + deadline arithmetic became one `wait_for`, which also drops its dependence on + `CLOCK_REALTIME` - a wall-clock step could previously stretch or skip the 2 s + budget. +- The two structs holding those members moved from `calloc`/`free` to + `new (std::nothrow) T{}` / `delete`: `calloc` never runs a constructor, and + `{}` still zeroes every scalar exactly as `calloc` did. `nothrow` keeps the + existing `if (!p) return -1;` checks meaningful. + +**Not** done here: the `flock` adapter lock in `usb.cpp` is POSIX-only and is +`_WIN32`-guarded rather than ported. It works by contending for the *same* lock +file `UsbDeviceLock` uses, but on Windows `UsbDeviceLock` is a named mutex +instead - so a file lock there would exclude nobody, and mirroring the mutex +would duplicate a mechanism the devourer path already owns. On Windows the lock +is a no-op and exclusivity comes from `UsbDeviceLock`; what is genuinely +unprotected is a direct `mt7612u_open()` with no devourer around it, which is +the bench tool's case, and the bench is Linux. ## Firmware diff --git a/src/mt7612u/async.c b/src/mt7612u/async.cpp similarity index 83% rename from src/mt7612u/async.c rename to src/mt7612u/async.cpp index aa8b84f0..20f58c0f 100644 --- a/src/mt7612u/async.c +++ b/src/mt7612u/async.cpp @@ -14,6 +14,7 @@ */ #include #include +#include #include "internal.h" /* @@ -26,26 +27,24 @@ static int locked_get(struct mt_async *a, const int *field) { int v; - pthread_mutex_lock(&a->lock); + a->lock.lock(); v = *field; - pthread_mutex_unlock(&a->lock); + a->lock.unlock(); return v; } -static void *evt_thread(void *arg) +static void evt_thread(struct mt7612u_dev *d) { - struct mt7612u_dev *d = arg; struct mt_async *a = d->a; - struct timeval tv = { .tv_sec = 0, .tv_usec = 50000 }; + struct timeval tv = { 0, 50000 }; while (locked_get(a, &a->running)) libusb_handle_events_timeout_completed(d->ctx, &tv, NULL); - return NULL; } static void LIBUSB_CALL rx_done(struct libusb_transfer *t) { - struct mt_slot *s = t->user_data; + struct mt_slot *s = (struct mt_slot *)t->user_data; struct mt7612u_dev *d = s->d; struct mt_async *a = s->a; int resubmit; @@ -66,22 +65,22 @@ static void LIBUSB_CALL rx_done(struct libusb_transfer *t) * that loss is invisible from this layer by construction - * see mt7612u_caps.max_mpdu_rx. What this counts is a * short or malformed transfer. */ - pthread_mutex_lock(&a->lock); + a->lock.lock(); a->rx_dropped++; - pthread_mutex_unlock(&a->lock); + a->lock.unlock(); } else { - pthread_mutex_lock(&a->lock); + a->lock.lock(); a->rx_frames++; - pthread_mutex_unlock(&a->lock); + a->lock.unlock(); /* Outside the lock: a callback is allowed to transmit, * and mt_async_tx_submit() takes this same mutex. */ if (a->cb) a->cb(a->cb_user, frame, (size_t)len, &info); } } else if (t->status != LIBUSB_TRANSFER_CANCELLED) { - pthread_mutex_lock(&a->lock); + a->lock.lock(); a->rx_err++; - pthread_mutex_unlock(&a->lock); + a->lock.unlock(); } resubmit = locked_get(a, &a->rx_active) && @@ -90,20 +89,20 @@ static void LIBUSB_CALL rx_done(struct libusb_transfer *t) return; /* Not resubmitted: this transfer is now owned by us again. */ - pthread_mutex_lock(&a->lock); + a->lock.lock(); if (resubmit) a->rx_err++; a->rx_inflight--; - pthread_cond_broadcast(&a->cv); - pthread_mutex_unlock(&a->lock); + a->cv.notify_all(); + a->lock.unlock(); } static void LIBUSB_CALL tx_done(struct libusb_transfer *t) { - struct mt_slot *s = t->user_data; + struct mt_slot *s = (struct mt_slot *)t->user_data; struct mt_async *a = s->a; - pthread_mutex_lock(&a->lock); + a->lock.lock(); if (t->status == LIBUSB_TRANSFER_COMPLETED && t->actual_length == t->length) a->tx_done_n++; @@ -111,8 +110,8 @@ static void LIBUSB_CALL tx_done(struct libusb_transfer *t) a->tx_err++; a->tx_busy[s->idx] = 0; a->tx_inflight--; - pthread_cond_broadcast(&a->cv); - pthread_mutex_unlock(&a->lock); + a->cv.notify_all(); + a->lock.unlock(); } int mt_async_start(struct mt7612u_dev *d, mt7612u_rx_cb cb, void *user) @@ -125,13 +124,11 @@ int mt_async_start(struct mt7612u_dev *d, mt7612u_rx_cb cb, void *user) struct mt_async *a; if (d->a) return 0; - a = calloc(1, sizeof *a); + a = new (std::nothrow) mt_async{}; if (!a) return -1; d->a = a; a->cb = cb; a->cb_user = user; - pthread_mutex_init(&a->lock, NULL); - pthread_cond_init(&a->cv, NULL); for (int i = 0; i < MT_TX_RING; i++) { a->tx_slot[i].d = d; @@ -149,13 +146,20 @@ int mt_async_start(struct mt7612u_dev *d, mt7612u_rx_cb cb, void *user) } a->running = 1; - if (pthread_create(&a->evt, NULL, evt_thread, d)) { a->running = 0; goto fail; } + /* std::thread reports failure by throwing where pthread_create returned + * an error code; catching keeps this the same `goto fail` teardown. */ + try { + a->evt = std::thread(evt_thread, d); + } catch (const std::system_error &) { + a->running = 0; + goto fail; + } a->evt_started = 1; if (cb) { - pthread_mutex_lock(&a->lock); + a->lock.lock(); a->rx_active = 1; - pthread_mutex_unlock(&a->lock); + a->lock.unlock(); for (int i = 0; i < MT_RX_RING; i++) { libusb_fill_bulk_transfer(a->rx[i], d->h, MT_EP_IN_PKT_RX, a->rx_buf[i], MT_RX_BUFSZ, @@ -164,9 +168,9 @@ int mt_async_start(struct mt7612u_dev *d, mt7612u_rx_cb cb, void *user) ERR("could not submit RX transfer %d", i); goto fail; } - pthread_mutex_lock(&a->lock); + a->lock.lock(); a->rx_inflight++; - pthread_mutex_unlock(&a->lock); + a->lock.unlock(); } LOG("async: %d RX transfers in flight, %d TX slots", MT_RX_RING, MT_TX_RING); @@ -198,9 +202,9 @@ void mt_async_stop(struct mt7612u_dev *d) if (!a) return; - pthread_mutex_lock(&a->lock); + a->lock.lock(); a->rx_active = 0; - pthread_mutex_unlock(&a->lock); + a->lock.unlock(); /* Cancel *both* rings. Cancelling only RX leaves TX transfers owned by * libusb, and the wait below would then time out with them in flight. */ @@ -210,22 +214,16 @@ void mt_async_stop(struct mt7612u_dev *d) if (a->tx[i]) libusb_cancel_transfer(a->tx[i]); /* The event thread is still running, so completions keep arriving. */ - pthread_mutex_lock(&a->lock); - for (int spins = 0; (a->tx_inflight || a->rx_inflight) && spins < 200; spins++) { - struct timespec ts; - - clock_gettime(CLOCK_REALTIME, &ts); - ts.tv_nsec += 10000000; - if (ts.tv_nsec >= 1000000000) { ts.tv_sec++; ts.tv_nsec -= 1000000000; } - pthread_cond_timedwait(&a->cv, &a->lock, &ts); - } + a->lock.lock(); + for (int spins = 0; (a->tx_inflight || a->rx_inflight) && spins < 200; spins++) + a->cv.wait_for(a->lock, std::chrono::milliseconds(10)); stuck_tx = a->tx_inflight; stuck_rx = a->rx_inflight; a->running = 0; - pthread_mutex_unlock(&a->lock); + a->lock.unlock(); if (a->evt_started) - pthread_join(a->evt, NULL); + a->evt.join(); d->a = NULL; if (stuck_tx || stuck_rx) { @@ -248,9 +246,7 @@ void mt_async_stop(struct mt7612u_dev *d) if (a->tx[i]) libusb_free_transfer(a->tx[i]); for (int i = 0; i < MT_RX_RING; i++) if (a->rx[i]) libusb_free_transfer(a->rx[i]); - pthread_mutex_destroy(&a->lock); - pthread_cond_destroy(&a->cv); - free(a); + delete a; } /* @@ -264,18 +260,18 @@ int mt_async_tx_submit(struct mt7612u_dev *d, const uint8_t *buf, int len) if (!a || len > MT_TX_BUFSZ) return -1; - pthread_mutex_lock(&a->lock); + a->lock.lock(); for (;;) { /* A teardown must not leave a caller parked here forever. */ - if (!a->running) { pthread_mutex_unlock(&a->lock); return -1; } + if (!a->running) { a->lock.unlock(); return -1; } for (int i = 0; i < MT_TX_RING; i++) if (!a->tx_busy[i]) { idx = i; break; } if (idx >= 0) break; - pthread_cond_wait(&a->cv, &a->lock); + a->cv.wait(a->lock); } a->tx_busy[idx] = 1; a->tx_inflight++; - pthread_mutex_unlock(&a->lock); + a->lock.unlock(); memcpy(a->tx_buf[idx], buf, (size_t)len); libusb_fill_bulk_transfer(a->tx[idx], d->h, MT_EP_OUT_AC_BE, @@ -283,16 +279,16 @@ int mt_async_tx_submit(struct mt7612u_dev *d, const uint8_t *buf, int len) &a->tx_slot[idx], 1000); rc = libusb_submit_transfer(a->tx[idx]); - pthread_mutex_lock(&a->lock); + a->lock.lock(); if (rc) { a->tx_busy[idx] = 0; a->tx_inflight--; a->tx_err++; - pthread_cond_broadcast(&a->cv); + a->cv.notify_all(); } else { a->tx_submitted++; } - pthread_mutex_unlock(&a->lock); + a->lock.unlock(); return rc ? -1 : 0; } @@ -307,7 +303,7 @@ void mt_async_stats(struct mt7612u_dev *d, struct mt_async_stats *out) memset(out, 0, sizeof *out); if (!a) return; - pthread_mutex_lock(&a->lock); + a->lock.lock(); out->tx_submitted = a->tx_submitted; out->tx_done = a->tx_done_n; out->tx_err = a->tx_err; @@ -315,7 +311,7 @@ void mt_async_stats(struct mt7612u_dev *d, struct mt_async_stats *out) out->rx_err = a->rx_err; out->rx_invalid = a->rx_invalid; out->rx_dropped = a->rx_dropped; - pthread_mutex_unlock(&a->lock); + a->lock.unlock(); } int mt7612u_rx_start(struct mt7612u_dev *d, mt7612u_rx_cb cb, void *user) @@ -338,9 +334,9 @@ void mt_async_note_invalid(struct mt7612u_dev *d) struct mt_async *a = d->a; if (!a) return; - pthread_mutex_lock(&a->lock); + a->lock.lock(); a->rx_invalid++; - pthread_mutex_unlock(&a->lock); + a->lock.unlock(); } /* Public form of the snapshot above. */ diff --git a/src/mt7612u/caps.c b/src/mt7612u/caps.cpp similarity index 100% rename from src/mt7612u/caps.c rename to src/mt7612u/caps.cpp diff --git a/src/mt7612u/eeprom.c b/src/mt7612u/eeprom.cpp similarity index 100% rename from src/mt7612u/eeprom.c rename to src/mt7612u/eeprom.cpp diff --git a/src/mt7612u/fw.c b/src/mt7612u/fw.cpp similarity index 94% rename from src/mt7612u/fw.c rename to src/mt7612u/fw.cpp index 9b221d80..81cf7ed5 100644 --- a/src/mt7612u/fw.c +++ b/src/mt7612u/fw.cpp @@ -53,7 +53,7 @@ static uint8_t *slurp(const char *dir, const char *name, size_t *out_len) if (!f) { ERR("cannot open %s", path); return NULL; } fseek(f, 0, SEEK_END); n = ftell(f); fseek(f, 0, SEEK_SET); if (n <= 0) { fclose(f); ERR("%s is empty", path); return NULL; } - buf = malloc((size_t)n); + buf = (uint8_t *)malloc((size_t)n); if (!buf || fread(buf, 1, (size_t)n, f) != (size_t)n) { free(buf); fclose(f); ERR("short read on %s", path); return NULL; } @@ -66,8 +66,8 @@ static uint8_t *slurp(const char *dir, const char *name, size_t *out_len) static void fw_reset(struct mt7612u_dev *d) { mt_vendor_req(d, MT_VEND_DEV_MODE, - LIBUSB_ENDPOINT_OUT | LIBUSB_REQUEST_TYPE_VENDOR | - LIBUSB_RECIPIENT_DEVICE, 0x1, 0, NULL, 0); + (uint8_t)LIBUSB_ENDPOINT_OUT | (uint8_t)LIBUSB_REQUEST_TYPE_VENDOR | + (uint8_t)LIBUSB_RECIPIENT_DEVICE, 0x1, 0, NULL, 0); } /* The FCE preamble that must precede every blob upload. */ @@ -118,7 +118,7 @@ static int fw_send_data(struct mt7612u_dev *d, const uint8_t *data, int data_len uint32_t max_payload, uint32_t offset) { int max_len = (int)max_payload - 8, pos = 0, rc = 0; - uint8_t *scratch = malloc(max_payload); + uint8_t *scratch = (uint8_t *)malloc(max_payload); if (!scratch) return FW_ERR_FATAL; while (data_len > 0) { @@ -163,8 +163,9 @@ static int load_rom_patch(struct mt7612u_dev *d, const char *dir) static const uint8_t wmt[] = { 0x6f, 0xfc, 0x05, 0x01, 0x07, 0x01, 0x00, 0x04 }; uint8_t b[16]; - const uint8_t type = LIBUSB_ENDPOINT_OUT | - LIBUSB_REQUEST_TYPE_CLASS | LIBUSB_RECIPIENT_DEVICE; + const uint8_t type = (uint8_t)LIBUSB_ENDPOINT_OUT | + (uint8_t)LIBUSB_REQUEST_TYPE_CLASS | + (uint8_t)LIBUSB_RECIPIENT_DEVICE; memcpy(b, enable, sizeof enable); mt_vendor_req(d, MT_VEND_DEV_MODE, type, 0x12, 0, b, sizeof enable); @@ -227,8 +228,8 @@ static int load_firmware(struct mt7612u_dev *d, const char *dir) /* load IVB: MT_VEND_DEV_MODE, VENDOR type, wValue 0x12, no data. */ mt_vendor_req(d, MT_VEND_DEV_MODE, - LIBUSB_ENDPOINT_OUT | LIBUSB_REQUEST_TYPE_VENDOR | - LIBUSB_RECIPIENT_DEVICE, 0x12, 0, NULL, 0); + (uint8_t)LIBUSB_ENDPOINT_OUT | (uint8_t)LIBUSB_REQUEST_TYPE_VENDOR | + (uint8_t)LIBUSB_RECIPIENT_DEVICE, 0x12, 0, NULL, 0); if (!mt_poll(d, MT_MCU_COM_REG0, BIT(0), BIT(0), 100000)) { ERR("firmware failed to start (MT_MCU_COM_REG0=0x%08x)", diff --git a/src/mt7612u/init.c b/src/mt7612u/init.cpp similarity index 98% rename from src/mt7612u/init.c rename to src/mt7612u/init.cpp index 99556e77..75941b05 100644 --- a/src/mt7612u/init.c +++ b/src/mt7612u/init.cpp @@ -7,6 +7,7 @@ */ #include #include +#include #include #include "internal.h" #include "initvals.h" @@ -459,26 +460,26 @@ static struct mt7612u_dev *bring_up(struct mt7612u_dev *d, const char *fw_dir, fail: mt_close(d); - free(d); + delete d; return NULL; } struct mt7612u_dev *mt7612u_open(const char *fw_dir, const char **err) { - struct mt7612u_dev *d = calloc(1, sizeof *d); + struct mt7612u_dev *d = new (std::nothrow) mt7612u_dev{}; if (!d) { if (err) *err = "out of memory"; return NULL; } - if (mt_open(d, err)) { mt_dev_state_destroy(d); free(d); return NULL; } + if (mt_open(d, err)) { mt_dev_state_destroy(d); delete d; return NULL; } return bring_up(d, fw_dir, err); } struct mt7612u_dev *mt7612u_open_handle(void *h, void *ctx, const char *fw_dir, const char **err) { - struct mt7612u_dev *d = calloc(1, sizeof *d); + struct mt7612u_dev *d = new (std::nothrow) mt7612u_dev{}; if (!d) { if (err) *err = "out of memory"; @@ -486,7 +487,7 @@ struct mt7612u_dev *mt7612u_open_handle(void *h, void *ctx, const char *fw_dir, } if (mt_adopt(d, (libusb_device_handle *)h, (libusb_context *)ctx, err)) { mt_dev_state_destroy(d); - free(d); + delete d; return NULL; } return bring_up(d, fw_dir, err); @@ -502,7 +503,7 @@ void mt7612u_close(struct mt7612u_dev *d) mt_async_stop(d); if (d->h) mt_mac_stop(d); mt_close(d); /* releases io_lock via mt_dev_state_destroy() */ - free(d); + delete d; } /* @@ -554,10 +555,8 @@ int mt7612u_set_monitor_rx(struct mt7612u_dev *d, int keep_corrupted) */ static uint64_t stats_now_us(void) { - struct timespec ts; - - clock_gettime(CLOCK_MONOTONIC, &ts); - return (uint64_t)ts.tv_sec * 1000000u + (uint64_t)(ts.tv_nsec / 1000); + return (uint64_t)std::chrono::duration_cast( + std::chrono::steady_clock::now().time_since_epoch()).count(); } /* Was file-static, so two adapters opened in one process overwrote each diff --git a/src/mt7612u/internal.h b/src/mt7612u/internal.h index 29e0abc3..13b45f18 100644 --- a/src/mt7612u/internal.h +++ b/src/mt7612u/internal.h @@ -16,8 +16,14 @@ #else # include #endif -#include -#include +/* C++ only: the sync members below are std:: types, chosen over pthreads + * because MSVC has no and devourer builds Windows first-class. + * Nothing outside this subtree includes this header; the public C ABI in + * include/mt7612u/mt7612u.h is unaffected and stays C-includable. */ +#include +#include +#include +#include #include #include #include @@ -124,9 +130,14 @@ struct mt_async { int tx_busy[MT_TX_RING]; /* Guards running, rx_active, tx_busy[], tx_inflight and rx_inflight - * all of which the event thread writes and the caller reads. */ - pthread_mutex_t lock; - pthread_cond_t cv; - pthread_t evt; + std::mutex lock; + /* condition_variable_any, not condition_variable: it waits on any + * BasicLockable, so every site below keeps the plain lock()/unlock() + * shape the pthread code had instead of being restructured around + * unique_lock. The waits here are teardown and TX back-pressure, not a + * hot path, so the extra indirection costs nothing measurable. */ + std::condition_variable_any cv; + std::thread evt; int evt_started; int running, rx_active; int tx_inflight, rx_inflight; @@ -158,11 +169,17 @@ struct mt7612u_dev { uint8_t mcu_stale_pending; uint8_t chan; uint8_t bw; - pthread_mutex_t io_lock; /* recursive: guards register + MCU transactions */ - uint8_t io_lock_ready; /* io_lock initialised - guards its destroy */ + /* Recursive: the PHY tick holds this and then nests mt_vendor_req / + * mt_mcu_send, which take it again. As a member it is constructed with + * the device, which is what retires the old io_lock_ready flag: a zeroed + * pthread_mutex_t was a valid NON-recursive lock, so a path that skipped + * the explicit init (the adopt path once did) self-deadlocked the tick. + * That failure is now unrepresentable. */ + std::recursive_mutex io_lock; /* Observe-but-do-not-repair, for wedge experiments. A field, not a - * getenv: this library reads no environment - the tool that wants the - * behaviour sets it before mt_open() (bringup does). */ + * getenv - the tool that wants the behaviour sets it before mt_open() + * (bringup does). Note this is not yet true of the library as a whole: + * open_selected() still reads MT7612U_DEV (see usb.cpp). */ uint8_t no_autorecover; uint8_t bw_clamp_warned; /* the "never widen" notice is once, not per frame */ int8_t txpower_conf; /* limit, 0.5 dB units (dBm * 2) */ diff --git a/src/mt7612u/mcu.c b/src/mt7612u/mcu.cpp similarity index 98% rename from src/mt7612u/mcu.c rename to src/mt7612u/mcu.cpp index 9334a041..9b76e34a 100644 --- a/src/mt7612u/mcu.c +++ b/src/mt7612u/mcu.cpp @@ -78,7 +78,7 @@ int mt_mcu_send(struct mt7612u_dev *d, int cmd, const void *data, int len, * thread so nothing contends today, but a consumer that drives the tick * from a second thread would otherwise steal its own responses - the * failure reads as "mcu resp mismatch ... (want 1)". */ - pthread_mutex_lock(&d->io_lock); + d->io_lock.lock(); /* Drain replies nobody collected before sending, but only when one is * actually outstanding. A reply that lands after mcu_wait_resp() gave up @@ -169,12 +169,12 @@ int mt_mcu_send(struct mt7612u_dev *d, int cmd, const void *data, int len, * is the third way a command goes unanswered, so it arms the * drain like the two in mcu_wait_resp(). */ d->mcu_stale_pending = 1; - pthread_mutex_unlock(&d->io_lock); + d->io_lock.unlock(); return -1; } mcu_rc = wait_resp ? mcu_wait_resp(d, seq) : 0; - pthread_mutex_unlock(&d->io_lock); + d->io_lock.unlock(); return mcu_rc; } diff --git a/src/mt7612u/phy.c b/src/mt7612u/phy.cpp similarity index 99% rename from src/mt7612u/phy.c rename to src/mt7612u/phy.cpp index dce4fef3..9052b301 100644 --- a/src/mt7612u/phy.c +++ b/src/mt7612u/phy.cpp @@ -609,7 +609,7 @@ static void phy_update_channel_gain(struct mt7612u_dev *d) int mt7612u_phy_tick(struct mt7612u_dev *d) { if (!d || !d->chan) return -1; - pthread_mutex_lock(&d->io_lock); + d->io_lock.lock(); /* Self-guarded after the first run, exactly like mt76's. */ channel_calibrate(d, d->chan > 14); /* mt76 keeps the MCU in the loop every second through @@ -617,7 +617,7 @@ int mt7612u_phy_tick(struct mt7612u_dev *d) * the temperature calibration is the cheapest command that does. */ mt_mcu_calibrate(d, MCU_CAL_TEMP_SENSOR, 0); phy_update_channel_gain(d); - pthread_mutex_unlock(&d->io_lock); + d->io_lock.unlock(); return 0; } diff --git a/src/mt7612u/radiotap.c b/src/mt7612u/radiotap.cpp similarity index 99% rename from src/mt7612u/radiotap.c rename to src/mt7612u/radiotap.cpp index b946b08f..0e1b0b74 100644 --- a/src/mt7612u/radiotap.c +++ b/src/mt7612u/radiotap.cpp @@ -233,7 +233,7 @@ int mt_radiotap_parse(const uint8_t *buf, size_t len, struct mt7612u_tx_rate *r) int mt7612u_send_packet(struct mt7612u_dev *d, const void *buf, size_t len) { struct mt7612u_tx_rate r; - const uint8_t *p = buf; + const uint8_t *p = (const uint8_t *)buf; int rlen = mt_radiotap_parse(p, len, &r); if (rlen <= 0 || (size_t)rlen >= len) { diff --git a/src/mt7612u/rx.c b/src/mt7612u/rx.cpp similarity index 100% rename from src/mt7612u/rx.c rename to src/mt7612u/rx.cpp diff --git a/src/mt7612u/tests/field_macros.c b/src/mt7612u/tests/field_macros.cpp similarity index 100% rename from src/mt7612u/tests/field_macros.c rename to src/mt7612u/tests/field_macros.cpp diff --git a/src/mt7612u/tests/frame_shape.c b/src/mt7612u/tests/frame_shape.cpp similarity index 96% rename from src/mt7612u/tests/frame_shape.c rename to src/mt7612u/tests/frame_shape.cpp index 3fd08262..8fcb72d2 100644 --- a/src/mt7612u/tests/frame_shape.c +++ b/src/mt7612u/tests/frame_shape.cpp @@ -65,7 +65,11 @@ static void test_hdrlen(void) */ static void test_rx_l2pad(void) { - struct mt7612u_dev d; + /* Value-initialised, not memset: the device carries a + * std::recursive_mutex now, and memsetting over a constructed + * one is undefined (-Wclass-memaccess). {} zeroes every scalar + * exactly as the memset did. */ + struct mt7612u_dev d{}; uint8_t buf[256]; const uint8_t *frame = NULL; struct mt7612u_rx_info info; @@ -79,7 +83,6 @@ static void test_rx_l2pad(void) printf("mt_rx_parse, L2 pad on a QoS frame:\n"); - memset(&d, 0, sizeof d); d.chainmask = 0x0202; memset(buf, 0, sizeof buf); @@ -201,7 +204,6 @@ static void test_vht_bandwidth(void) */ static void test_invalid_phy(void) { - struct mt7612u_dev d; uint8_t buf[128]; const uint8_t *frame = NULL; struct mt7612u_rx_info info; @@ -209,10 +211,16 @@ static void test_invalid_phy(void) printf("mt_rx_parse, invalid PHY in the rate word:\n"); for (unsigned phy = 0; phy < 8; phy++) { + /* Declared inside the loop so each iteration still starts from a + * zeroed device, which is what the memset here used to do. It + * cannot be a memset any more: the device carries a + * std::recursive_mutex, and writing over a constructed one is + * undefined (-Wclass-memaccess). {} zeroes every scalar the same + * way. */ + struct mt7612u_dev d{}; uint16_t rate = (uint16_t)FIELD_PREP(MT_RATE_PHY, phy); int len; - memset(&d, 0, sizeof d); d.chainmask = 0x0202; memset(buf, 0, sizeof buf); for (int i = 0; i < 4; i++) diff --git a/src/mt7612u/tools/bringup.c b/src/mt7612u/tools/bringup.cpp similarity index 98% rename from src/mt7612u/tools/bringup.c rename to src/mt7612u/tools/bringup.cpp index 1b291349..6ed76c91 100644 --- a/src/mt7612u/tools/bringup.c +++ b/src/mt7612u/tools/bringup.cpp @@ -3,7 +3,7 @@ * MT7612U bringup harness. One subcommand per gate (see src/mt7612u/README.md), so each * stage is independently runnable on hardware. */ -#include +#include #include #include #include @@ -1040,15 +1040,17 @@ static int gate_soak(uint8_t chan, int secs, int framelen) * Relaxed atomics: these are counters, nothing orders anything else off them, * and this is the RX hot path in a throughput gate. */ -struct arx_ctx { _Atomic unsigned long n; _Atomic unsigned long by_phy[8]; }; +struct arx_ctx { + std::atomic n; + std::atomic by_phy[8]; +}; static void arx_cb(void *user, const void *frame, size_t len, const struct mt7612u_rx_info *info) { - struct arx_ctx *c = user; + struct arx_ctx *c = (struct arx_ctx *)user; (void)frame; (void)len; - atomic_fetch_add_explicit(&c->n, 1, memory_order_relaxed); - atomic_fetch_add_explicit(&c->by_phy[info->phy & 7], 1, - memory_order_relaxed); + c->n.fetch_add(1, std::memory_order_relaxed); + c->by_phy[info->phy & 7].fetch_add(1, std::memory_order_relaxed); } /* Async RX ring: the callback path StartRxLoop needs. */ @@ -1088,14 +1090,15 @@ static int gate_arx(uint8_t chan, int secs, int notick) printf("async RX on ch%u for %.1f s: %lu frames (%.0f/s), " "ring=%llu rx_err=%llu " "rx_invalid=%llu rx_dropped=%llu\n", - chan, el, ctx.n, ctx.n / (el > 0 ? el : 1), + chan, el, ctx.n.load(), ctx.n.load() / (el > 0 ? el : 1), (unsigned long long)st.rx_frames, (unsigned long long)st.rx_err, (unsigned long long)st.rx_invalid, (unsigned long long)st.rx_dropped); } for (int i = 0; i < 8; i++) - if (ctx.by_phy[i]) printf(" %-6s %lu\n", phy_name[i], ctx.by_phy[i]); + if (ctx.by_phy[i]) + printf(" %-6s %lu\n", phy_name[i], ctx.by_phy[i].load()); rx_teardown(); mt_mac_stop(&dev); return ctx.n ? 0 : 1; @@ -1159,7 +1162,8 @@ static int gate_duplex(uint8_t chan, int secs) mt_async_stats(&dev, &st); printf(" TX %ld frames (%.0f fps) RX %lu frames (%.0f fps) " "tx_err=%llu rx_err=%llu\n", - n, n * 1000.0 / wall, ctx.n, ctx.n * 1000.0 / wall, + n, n * 1000.0 / wall, ctx.n.load(), + ctx.n.load() * 1000.0 / wall, (unsigned long long)st.tx_err, (unsigned long long)st.rx_err); } @@ -1178,8 +1182,7 @@ static int gate_duplex(uint8_t chan, int secs) * stimulus and a wedged receiver look identical from here. */ { - unsigned long before = atomic_load_explicit(&ctx.n, - memory_order_relaxed); + unsigned long before = ctx.n.load(std::memory_order_relaxed); unsigned long after; printf(" TX stopped; listening %.1f s for the receiver to recover\n", @@ -1189,7 +1192,7 @@ static int gate_duplex(uint8_t chan, int secs) mt_mac_stop(&dev); return 1; } - after = atomic_load_explicit(&ctx.n, memory_order_relaxed); + after = ctx.n.load(std::memory_order_relaxed); printf(" RX after the flood: %lu frames\n", after - before); rx_teardown(); mt_mac_stop(&dev); @@ -1385,8 +1388,8 @@ static void drain_cb(void *user, const void *frame, size_t len, const struct mt7612u_rx_info *info) { (void)frame; (void)len; (void)info; - atomic_fetch_add_explicit((_Atomic unsigned long *)user, 1, - memory_order_relaxed); + ((std::atomic *)user)->fetch_add( + 1, std::memory_order_relaxed); } /* Capability descriptor, TSF and 40 MHz. */ @@ -1525,25 +1528,27 @@ static int gate_caps(uint8_t chan) * duplicates this test is counting. */ /* Same event-thread/gate split as arx_ctx above. */ -struct ack_ctx { _Atomic unsigned long to_us, retry_to_us, other; }; +struct ack_ctx { + std::atomic to_us, retry_to_us, other; +}; static const uint8_t g_ack_mac[6] = { 0x02, 0x4d, 0x54, 0x76, 0x12, 0xaa }; static void ack_cb(void *user, const void *frame, size_t len, const struct mt7612u_rx_info *info) { - struct ack_ctx *c = user; - const uint8_t *f = frame; + struct ack_ctx *c = (struct ack_ctx *)user; + const uint8_t *f = (const uint8_t *)frame; (void)info; if (len < 16) return; if (memcmp(f + 4, g_ack_mac, 6) != 0) { - atomic_fetch_add_explicit(&c->other, 1, memory_order_relaxed); + c->other.fetch_add(1, std::memory_order_relaxed); return; } - atomic_fetch_add_explicit(&c->to_us, 1, memory_order_relaxed); + c->to_us.fetch_add(1, std::memory_order_relaxed); if (f[1] & 0x08) /* FC Retry bit */ - atomic_fetch_add_explicit(&c->retry_to_us, 1, memory_order_relaxed); + c->retry_to_us.fetch_add(1, std::memory_order_relaxed); } static int gate_ack(uint8_t chan, int secs, int arm) @@ -1605,7 +1610,7 @@ static int gate_ack(uint8_t chan, int secs, int arm) } rx_teardown(); printf(" stimulus frames addressed to the responder MAC: %lu (retries %lu)\n", - off.to_us, off.retry_to_us); + off.to_us.load(), off.retry_to_us.load()); if (arm) { mt7612u_clear_ack_responder(&dev); @@ -1878,7 +1883,7 @@ static int g_link_n; static void linkrx_cb(void *user, const void *frame, size_t len, const struct mt7612u_rx_info *info) { - const uint8_t *f = frame; + const uint8_t *f = (const uint8_t *)frame; int slot = -1, pw; uint8_t bytes[20]; @@ -2125,7 +2130,7 @@ static int gate_coding(uint8_t chan, int count, int bw) struct mt7612u_tx_rate r = { .phy = rates[i].phy, .mcs = rates[i].mcs, .nss = rates[i].nss, - .bw = (uint8_t)bw, + .bw = (enum mt7612u_bw)bw, .sgi = (coding & 4) ? 1u : 0u, .ldpc = (coding & 1) ? 1u : 0u, .stbc = (coding & 2) ? 1u : 0u, @@ -2238,7 +2243,7 @@ static int gate_sweep(uint8_t chan, int count, int bw) for (int mcs = 0; mcs <= last_mcs; mcs++) { for (int nss = 1; nss <= 2; nss++) { struct mt7612u_tx_rate r = { - .bw = (uint8_t)bw, + .bw = (enum mt7612u_bw)bw, .no_ack = 1, }; char what[32]; @@ -2355,7 +2360,7 @@ static int gate_vht(uint8_t chan, int count, int bw) for (unsigned a = 0; a < sizeof arms / sizeof arms[0]; a++) { struct mt7612u_tx_rate r = { .phy = arms[a].phy, .mcs = arms[a].mcs, .nss = arms[a].nss, - .bw = (uint8_t)bw, + .bw = (enum mt7612u_bw)bw, .no_ack = 1 }; long sent = 0; diff --git a/src/mt7612u/tx.c b/src/mt7612u/tx.cpp similarity index 99% rename from src/mt7612u/tx.c rename to src/mt7612u/tx.cpp index 58e10501..d3b3a598 100644 --- a/src/mt7612u/tx.c +++ b/src/mt7612u/tx.cpp @@ -117,7 +117,7 @@ int mt_tx_build(struct mt7612u_dev *d, uint8_t *buf, size_t bufsz, uint8_t *txwi = buf + 4; uint32_t info; int hdrlen, hdr_pad = 0, body, padded, pad, total, rc, n = 0; - const uint8_t *f = frame; + const uint8_t *f = (const uint8_t *)frame; if (len < 10 || len + 32 > bufsz) { ERR("bad frame length %zu", len); return -1; } @@ -154,7 +154,7 @@ int mt_tx_build(struct mt7612u_dev *d, uint8_t *buf, size_t bufsz, "to the channel width (further occurrences silent)", rate->bw, d->bw); } - narrowed.bw = d->bw; + narrowed.bw = (enum mt7612u_bw)d->bw; put_le16(txwi + 2, mt_tx_rate_word(&narrowed)); } else { put_le16(txwi + 2, mt_tx_rate_word(rate)); /* rate */ diff --git a/src/mt7612u/usb.c b/src/mt7612u/usb.cpp similarity index 88% rename from src/mt7612u/usb.c rename to src/mt7612u/usb.cpp index 083e37c1..906ee0c9 100644 --- a/src/mt7612u/usb.c +++ b/src/mt7612u/usb.cpp @@ -5,13 +5,16 @@ * ../../INVESTIGATION.md §11). */ #include -#include #include #include +#include +/* The adapter lock below is POSIX file locking. MSVC has none of these headers; + * see lock_adapter() for what Windows does instead. */ +#if !defined(_WIN32) +#include #include -#include #include -#include +#endif #include "internal.h" @@ -45,8 +48,14 @@ void mt_diag(char level, const char *fmt, ...) fflush(stderr); } -#define REQ_IN (LIBUSB_ENDPOINT_IN | LIBUSB_REQUEST_TYPE_VENDOR | LIBUSB_RECIPIENT_DEVICE) -#define REQ_OUT (LIBUSB_ENDPOINT_OUT | LIBUSB_REQUEST_TYPE_VENDOR | LIBUSB_RECIPIENT_DEVICE) +/* Each operand is cast to the uint8_t that libusb's bmRequestType actually is: + * these are three DIFFERENT libusb enum types, and C++20 deprecates a bitwise + * operation between different enumeration types ([depr.ee.conv]). Casting keeps + * the value identical while making the operation an ordinary integer OR. */ +#define REQ_IN ((uint8_t)LIBUSB_ENDPOINT_IN | (uint8_t)LIBUSB_REQUEST_TYPE_VENDOR | \ + (uint8_t)LIBUSB_RECIPIENT_DEVICE) +#define REQ_OUT ((uint8_t)LIBUSB_ENDPOINT_OUT | (uint8_t)LIBUSB_REQUEST_TYPE_VENDOR | \ + (uint8_t)LIBUSB_RECIPIENT_DEVICE) /* mt76's MT_VEND_REQ_TOUT_MS / MT_VEND_REQ_MAX_RETRY. The product of the two * is the worst-case cost of one register access, so it bounds every poll * loop below - which is why the timeout is 300 ms and not something longer. */ @@ -55,8 +64,7 @@ void mt_diag(char level, const char *fmt, ...) void mt_usleep(unsigned us) { - struct timespec ts = { .tv_sec = us / 1000000, .tv_nsec = (us % 1000000) * 1000 }; - nanosleep(&ts, NULL); + std::this_thread::sleep_for(std::chrono::microseconds(us)); } /* Held for the process lifetime; flock releases it on any exit. */ @@ -64,10 +72,10 @@ static int g_lock_fd = -1; static uint64_t now_us(void) { - struct timespec ts; - - clock_gettime(CLOCK_MONOTONIC, &ts); - return (uint64_t)ts.tv_sec * 1000000u + (uint64_t)(ts.tv_nsec / 1000); + /* steady_clock, matching CLOCK_MONOTONIC: never stepped by a wall-clock + * adjustment, which is what an elapsed-time measurement needs. */ + return (uint64_t)std::chrono::duration_cast( + std::chrono::steady_clock::now().time_since_epoch()).count(); } int mt_vendor_req(struct mt7612u_dev *d, uint8_t req, uint8_t type, @@ -78,7 +86,7 @@ int mt_vendor_req(struct mt7612u_dev *d, uint8_t req, uint8_t type, /* One control transfer at a time. Nothing contends in a single-threaded * consumer; one that sends from a second thread would otherwise * interleave two transfers on EP0. */ - pthread_mutex_lock(&d->io_lock); + d->io_lock.lock(); for (int i = 0; i < VEND_RETRIES; i++) { rc = libusb_control_transfer(d->h, type, req, val, idx, @@ -90,7 +98,7 @@ int mt_vendor_req(struct mt7612u_dev *d, uint8_t req, uint8_t type, } ERR("vendor req %02x idx %04x failed: %s", req, idx, libusb_error_name(rc)); out: - pthread_mutex_unlock(&d->io_lock); + d->io_lock.unlock(); return rc; } @@ -293,27 +301,18 @@ static int mt_identify(struct mt7612u_dev *d, const char **err) */ void mt_dev_state_init(struct mt7612u_dev *d) { - pthread_mutexattr_t ma; - - if (d->io_lock_ready) return; - /* Recursive: the PHY tick locks io_lock and then nests mt_vendor_req / - * mt_mcu_send, which lock it again. A zeroed pthread_mutex_t is a valid - * NON-recursive lock, so a path that skipped this (the adopt path once - * did) would self-deadlock the tick or lock an uninitialised mutex. */ - pthread_mutexattr_init(&ma); - pthread_mutexattr_settype(&ma, PTHREAD_MUTEX_RECURSIVE); - pthread_mutex_init(&d->io_lock, &ma); - pthread_mutexattr_destroy(&ma); - d->io_lock_ready = 1; - /* cal sentinels (low_gain=-1 etc.) are reset per-tune in + /* io_lock is a std::recursive_mutex member, constructed with the device, + * so there is nothing to initialise here and no path that can reach the + * tick with an unusable lock. Kept as a named seam because both open + * paths call it and the cal sentinels (low_gain=-1 etc.) belong to the + * same "device state is ready" step - those are reset per-tune in * mt_set_channel_ex(), which always runs before the first PHY tick. */ + (void)d; } void mt_dev_state_destroy(struct mt7612u_dev *d) { - if (!d || !d->io_lock_ready) return; - pthread_mutex_destroy(&d->io_lock); - d->io_lock_ready = 0; + (void)d; } /* @@ -348,9 +347,15 @@ void mt_recover_usb(struct mt7612u_dev *d) /* A wedge experiment must not have its recovery hidden inside open(). * With this set, open() observes and reports but repairs nothing. It is - * a field rather than a getenv(): this library reads no environment, so - * the tool that wants the behaviour sets it (bringup does, from - * MT7612U_NO_AUTORECOVER). */ + * a field rather than a getenv(): the tool that wants the behaviour sets + * it (bringup does, from MT7612U_NO_AUTORECOVER). + * + * That is the direction of travel, not a property the library has yet: + * open_selected() below still reads MT7612U_DEV directly. #412 deferred + * that one because there is no public way to pass a selector - + * mt7612u_open() allocates the device itself and the struct is opaque - + * so removing it would strand multi-adapter callers with no replacement. + * It moves to a DeviceConfig field when the backend lands (#419). */ if (d->no_autorecover) { if (mt_rr_chk(d, CFG_ADDR(MT_USB_U3DMA_CFG), &cfg)) LOG("auto-recovery disabled: U3DMA_CFG unreadable"); @@ -474,8 +479,33 @@ static void adapter_key(libusb_device *dev, char *out, size_t n) i ? "." : "-", ports[i]); } +/* Drop the adapter lock, if this process is holding one. */ +static void unlock_adapter(void) +{ +#if !defined(_WIN32) + if (g_lock_fd >= 0) { close(g_lock_fd); g_lock_fd = -1; } +#endif +} + /* Returns a held fd, -1 to proceed unlocked (infrastructure failure), or * -2 when another process holds the adapter and the caller must refuse. */ +#if defined(_WIN32) +static int lock_adapter(libusb_device *dev, const char **err) +{ + /* Not ported. The Linux interlock above works by contending for the SAME + * lock file UsbDeviceLock uses; on Windows UsbDeviceLock is a named mutex + * instead (UsbDeviceLock.cpp), so a file lock here would exclude nobody + * and mirroring the mutex would be a second copy of a mechanism the + * devourer path already owns. Exclusivity on Windows therefore comes from + * UsbDeviceLock, which WiFiDriver takes before it ever reaches this + * library. What is genuinely unprotected is a direct mt7612u_open() with + * no devourer around it - the bench tool's case, and the bench is Linux. + * Proceed unlocked rather than refuse: fail-open is what the POSIX path + * does for an infrastructure failure too. */ + (void)dev; (void)err; + return -1; +} +#else static int lock_adapter(libusb_device *dev, const char **err) { /* "/tmp" literally, and deliberately NOT getenv("TMPDIR"): the whole @@ -515,6 +545,7 @@ static int lock_adapter(libusb_device *dev, const char **err) path, strerror(errno)); return -1; } +#endif /* !_WIN32 */ static libusb_device_handle *open_selected(libusb_context *ctx, const char **err) { @@ -574,7 +605,7 @@ static libusb_device_handle *open_selected(libusb_context *ctx, const char **err * on the very next retry. Release what this * iteration took. */ h = NULL; - if (g_lock_fd >= 0) { close(g_lock_fd); g_lock_fd = -1; } + unlock_adapter(); } } matches++; @@ -684,7 +715,7 @@ void mt_close(struct mt7612u_dev *d) "handle and context rather than closing underneath them"); d->h = NULL; d->ctx = NULL; - if (g_lock_fd >= 0) { close(g_lock_fd); g_lock_fd = -1; } + unlock_adapter(); return; } if (d->h) { @@ -700,7 +731,7 @@ void mt_close(struct mt7612u_dev *d) } if (d->ctx && d->owns_handle) libusb_exit(d->ctx); d->ctx = NULL; - if (g_lock_fd >= 0) { close(g_lock_fd); g_lock_fd = -1; } + unlock_adapter(); } /* Block write, as mt76u_copy(): one MULTI_WRITE per batch, wValue 0. @@ -708,7 +739,7 @@ void mt_close(struct mt7612u_dev *d) * table (32 B) - 192 transfers that would otherwise be ~700 4-byte writes. */ void mt_wr_copy(struct mt7612u_dev *d, uint32_t offset, const void *data, int len) { - const uint8_t *p = data; + const uint8_t *p = (const uint8_t *)data; uint8_t buf[64]; /* The hardware wants whole 32-bit words, but only `len` bytes belong to From 35b8bd98dc296efa24fecbcaeb0d88d848950103 Mon Sep 17 00:00:00 2001 From: snokvist Date: Wed, 9 Sep 2026 23:04:27 +0200 Subject: [PATCH 3/7] caps: MT7612U has no narrowband, so name it before the permissive arm MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adding the Mt7612u enumerator was not free. bw_mask_for_generation ends in a catch-all that returns 20/40/80 PLUS kBw5|kBw10, describing the Realtek BB small-BW modes, and a new generation falls into it silently: old chain, ChipGeneration::Mt7612u -> 0x1f (kBw5|kBw10 set) new chain, ChipGeneration::Mt7612u -> 0x1c (20/40/80 only) every other generation -> unchanged MT_RATE_BW encodes 20/40/80/160 and nothing narrower, so there is no 5 or 10 MHz to select on this part — #419 lists exactly that among its known limits. Nothing calls this with Mt7612u today, which is precisely why it is worth fixing now: the first caller is the backend in the next block, and a capability claimed there is claimed silently. adapter_caps_selftest pins both the value and the narrowband bits directly, so a future generation added to the permissive arm cannot quietly re-acquire them. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj --- src/AdapterCaps.h | 7 +++++++ tests/adapter_caps_selftest.cpp | 13 +++++++++++++ 2 files changed, 20 insertions(+) diff --git a/src/AdapterCaps.h b/src/AdapterCaps.h index 3f4c22ae..057ccb38 100644 --- a/src/AdapterCaps.h +++ b/src/AdapterCaps.h @@ -90,8 +90,15 @@ inline uint8_t bw_mask_for_generation(ChipGeneration g) { /* RTL8733B: 10 MHz qualified (SDR OBW + two-way cross-decode with a * Jaguar3 peer, both bands); 5 MHz is refused — its BB small-BW mode airs * no packets on this die (docs/rtl8733b.md "Narrowband status"). */ + /* MT7612U: 20/40/80 and nothing narrower. MT_RATE_BW encodes only + * 20/40/80/160, so there is no 5 or 10 MHz to select — unlike the Realtek + * BB small-BW modes the trailing arm below is describing. Named explicitly + * because that trailing arm is the permissive one: without this case a + * MediaTek adapter would inherit kBw5|kBw10 and advertise two bandwidths the + * part cannot represent. 160 MHz is likewise absent (docs/mt7612u.md). */ return g == ChipGeneration::Rtl8733b ? (kBw10 | kBw20 | kBw40) : g == ChipGeneration::Jaguar1 ? ac + : g == ChipGeneration::Mt7612u ? ac : g == ChipGeneration::Unknown ? 0 : (ac | kBw5 | kBw10); } diff --git a/tests/adapter_caps_selftest.cpp b/tests/adapter_caps_selftest.cpp index 0ae591c5..ba9eee0c 100644 --- a/tests/adapter_caps_selftest.cpp +++ b/tests/adapter_caps_selftest.cpp @@ -43,6 +43,16 @@ int main() { expect("Kestrel bw = 20/40/80/160", bw_mask_for_generation(ChipGeneration::Kestrel) == (ac | kBw5 | kBw10)); + /* MT7612U has no narrowband: MT_RATE_BW encodes 20/40/80/160 only, so there + * is nothing to select for 5 or 10 MHz. This is pinned because the trailing + * arm of bw_mask_for_generation is the permissive one — a MediaTek adapter + * that fell through to it would advertise two bandwidths the part cannot + * represent, and nothing else would notice. */ + expect("MT7612U bw = 20/40/80, no 5/10 MHz", + bw_mask_for_generation(ChipGeneration::Mt7612u) == ac); + expect("MT7612U advertises no narrowband", + (bw_mask_for_generation(ChipGeneration::Mt7612u) & + (kBw5 | kBw10)) == 0); /* --- generation names --- */ expect("gen name jaguar3", @@ -54,6 +64,9 @@ int main() { expect("gen name rtl8733b", std::string_view(generation_name(ChipGeneration::Rtl8733b)) == "rtl8733b"); + expect("gen name mt7612u", + std::string_view(generation_name(ChipGeneration::Mt7612u)) == + "mt7612u"); expect("gen name unknown", std::string_view(generation_name(ChipGeneration::Unknown)) == "unknown"); From 5c47ce985042be25fd08724f70e23fd9202b0026 Mon Sep 17 00:00:00 2001 From: snokvist Date: Wed, 9 Sep 2026 23:07:20 +0200 Subject: [PATCH 4/7] ci: build the MediaTek subtree under the sanitizers too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit That subtree moved from calloc/free to new/delete and its mutexes, condition variable and event thread are constructed members now, so a lifetime regression in it is the most likely way this port breaks. build-sanitizers had the option off, which meant none of those objects were compiled into the ASan/UBSan build at all. Verified locally at RelWithDebInfo with address+undefined: 59/59 ctest. The same sanitizers were also run against real hardware through the subtree's own bringup harness — 10 s of async RX on an MT7612U, 2623 frames, no ASan report, no UBSan report, and clean under detect_leaks=1, which is what actually exercises the ring's new/delete teardown. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj --- .github/workflows/cmake-multi-platform.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/cmake-multi-platform.yml b/.github/workflows/cmake-multi-platform.yml index 93d1d215..903e1946 100644 --- a/.github/workflows/cmake-multi-platform.yml +++ b/.github/workflows/cmake-multi-platform.yml @@ -225,9 +225,13 @@ jobs: run: sudo apt install libusb-1.0-0-dev - name: Configure (ASan + UBSan) + # DEVOURER_MT7612U=ON here too: that subtree moved from calloc/free to + # new/delete and its sync primitives are constructed members now, so the + # sanitizers are the cheapest guard against a lifetime regression in it. run: > cmake -B build-asan -DCMAKE_BUILD_TYPE=RelWithDebInfo - -DDEVOURER_SANITIZE=address+undefined -S ${{ github.workspace }} + -DDEVOURER_SANITIZE=address+undefined -DDEVOURER_MT7612U=ON + -S ${{ github.workspace }} - name: Build (ASan + UBSan) run: cmake --build build-asan -j From 6a6b4b4b9657a0c46bd7651d5598f5944847cf3d Mon Sep 17 00:00:00 2001 From: snokvist Date: Wed, 9 Sep 2026 23:17:09 +0200 Subject: [PATCH 5/7] review: close the id-table blind spot and stop exceptions crossing the C ABI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two reviewers found four things worth fixing, two of them real defects. THE ID TABLE COULD NOT SEE JAGUAR. The selftest cross-checked the MediaTek set against KestrelUsbIds.h and Rtl8733bUsbIds.h — 27 ids. Jaguar1/2/3 dispatch by chip-id and have no table in devourer, so 64 more were invisible, and every vendor id where the two silicon families interleave lives in that 64. The demonstration: adding {0x0b05, 0x17d2}, /* ASUS USB-AC56 — an RTL8812AU */ to the table passed the old test while refusing a real Jaguar1 adapter before the SYS_CFG2 read, with no second chance. The test now carries a 64-entry witness list from reference/rtl8812au and checks all 91, asserts that coverage count so a future trim cannot silently restore the blindness, and pins 0b05:17d2 by name. THE TABLE WAS 11 OF 16, FROM THE WRONG SOURCE. I transcribed it from this machine's kernel tree instead of reference/mt76 @ be5ce79 — the tree this port was ported from, pinned so exactly this is re-verifiable. Five ids were missing. Recomputed against the complete table and all 91 Realtek ids: still zero pair collisions, but SIX shared vendor ids, not three. The interleaving is the real argument and it was understated: 0x0846 Netgear MediaTek 9014, 9053 Realtek 9051, 9052, 9054 0x056e ELECOM MediaTek 400a Realtek 4007, 400b, 400d, ... 0x0b05 ASUS MediaTek 17eb, 180b, 1833 Realtek 17d2, 1817, 1852, ... A one-digit slip in the Netgear entry refuses an RTL8814AU. The selftest now pins those three neighbours directly. EXCEPTIONS COULD CROSS THE extern "C" BOUNDARY. Both were proved by probe, not argued: - `catch (const std::system_error &)` on the std::thread construction is narrower than the throw set: libstdc++ allocates the thread state with a THROWING new inside the constructor, so OOM arrives as bad_alloc and escaped. It would have skipped the teardown, leaving d->a live with running=1 and evt_started=0, then unwound into a C caller with no handler. - `new (std::nothrow) T{}` is not nothrow for these types. nothrow suppresses a throw from the allocation FUNCTION only, and std::condition_variable_any holds a shared_ptr that allocates in its constructor. Both are now try/catch(...) returning the existing -1 / NULL. Also: three comments still said mt_dev_state_init/destroy manage io_lock, which this branch had already made empty; one comment quoted a read_chip_id note the same commit deleted; mt_usleep is no longer interruptible (measured: 10 ms vs 201 ms for a 200 ms request under 100 Hz SIGALRM) and now says so; the option's help text said nothing about the refusal being unconditional; and the mt7612u+jaguar1 CI cell cannot strip Jaguar1, so it no longer implies it does. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj --- .github/workflows/cmake-multi-platform.yml | 4 + CMakeLists.txt | 12 +- src/WiFiDriver.cpp | 20 ++-- src/mt7612u/Mt7612uUsbIds.h | 60 +++++++--- src/mt7612u/async.cpp | 22 +++- src/mt7612u/init.cpp | 23 +++- src/mt7612u/internal.h | 14 ++- src/mt7612u/usb.cpp | 7 ++ tests/mt7612u_usb_ids_selftest.cpp | 128 ++++++++++++++------- 9 files changed, 204 insertions(+), 86 deletions(-) diff --git a/.github/workflows/cmake-multi-platform.yml b/.github/workflows/cmake-multi-platform.yml index 903e1946..df9e162e 100644 --- a/.github/workflows/cmake-multi-platform.yml +++ b/.github/workflows/cmake-multi-platform.yml @@ -191,6 +191,10 @@ jobs: flags: "-DDEVOURER_JAGUAR1=OFF -DDEVOURER_8814=OFF -DDEVOURER_JAGUAR2_8822B=OFF -DDEVOURER_JAGUAR2_8821C=OFF -DDEVOURER_JAGUAR3_8822C=OFF -DDEVOURER_JAGUAR3_8822E=OFF -DDEVOURER_8733B=OFF -DDEVOURER_KESTREL_8852C=OFF" - name: kestrel-8852c-only flags: "-DDEVOURER_JAGUAR1=OFF -DDEVOURER_8814=OFF -DDEVOURER_JAGUAR2_8822B=OFF -DDEVOURER_JAGUAR2_8821C=OFF -DDEVOURER_JAGUAR3_8822C=OFF -DDEVOURER_JAGUAR3_8822E=OFF -DDEVOURER_8733B=OFF -DDEVOURER_KESTREL_8852B=OFF" + # Jaguar1 stays ON because it has to: MT7612U is deliberately not + # counted as chip support, so an MT7612U-only configure is a + # FATAL_ERROR. This proves the subtree is independent of the other + # seven Realtek chips' sources, not of Jaguar1's. - name: mt7612u+jaguar1 flags: "-DDEVOURER_MT7612U=ON -DDEVOURER_8814=OFF -DDEVOURER_JAGUAR2_8822B=OFF -DDEVOURER_JAGUAR2_8821C=OFF -DDEVOURER_JAGUAR3_8822C=OFF -DDEVOURER_JAGUAR3_8822E=OFF -DDEVOURER_8733B=OFF -DDEVOURER_KESTREL_8852B=OFF -DDEVOURER_KESTREL_8852C=OFF" - name: rtl8733b-only diff --git a/CMakeLists.txt b/CMakeLists.txt index b352eae1..92f088cd 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -49,8 +49,9 @@ option(DEVOURER_PCIE "PCIe transport via vfio-pci (Linux; RTL8821CE)" # joins that list when the backend lands (#419). Turning it ON today buys one # thing, and it is the point of the option: CI compiles the subtree across the # whole platform matrix — gcc, clang, MSVC, mingw, macOS — where it was -# previously only ever built by its own Makefile, on Linux. -option(DEVOURER_MT7612U "MediaTek MT7612U / MT7662U (2T2R 11ac USB)" OFF) +# previously only ever built by its own Makefile, on Linux. It does NOT gate the +# factory's MediaTek refusal, which is unconditional. +option(DEVOURER_MT7612U "Compile the MediaTek MT7612U subtree (the id gate is always on)" OFF) # Compile-time diagnostics floor (src/logger.h). Calls below the floor — # including their argument expressions at DVR_TRACE/DVR_DEBUG sites — compile @@ -357,6 +358,10 @@ if(DEVOURER_JAGUAR2_8821C) endif() # --- MediaTek MT7612U (MT7662 MAC) --- +# Note the option gates only the COMPILE. Mt7612uUsbIds.h is header-only and is +# included unconditionally by WiFiDriver.cpp, so a default OFF build still +# refuses MediaTek adapters and still runs the mt7612u_usb_ids test; it just +# links none of the subtree. # The subtree is self-contained: it talks to libusb directly rather than through # RtlAdapter/ITransport, because 32-bit registers and an in-band MCU over EP8/EP5 # do not fit the Realtek shape. Its public surface is the C ABI in @@ -377,7 +382,8 @@ if(DEVOURER_MT7612U) src/mt7612u/usb.cpp src/mt7612u/internal.h src/mt7612u/regs.h - src/mt7612u/initvals.h) + src/mt7612u/initvals.h + src/mt7612u/Mt7612uUsbIds.h) # listed for IDEs; compiled unconditionally target_include_directories(devourer PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/src/mt7612u ${CMAKE_CURRENT_SOURCE_DIR}/src/mt7612u/include) diff --git a/src/WiFiDriver.cpp b/src/WiFiDriver.cpp index 992297f9..0d1238ec 100644 --- a/src/WiFiDriver.cpp +++ b/src/WiFiDriver.cpp @@ -190,17 +190,17 @@ WiFiDriver::CreateRadio(libusb_device_handle *dev_handle, /* MediaTek MT7612U gates on the USB VID:PID BEFORE the SYS_CFG2 read, for the * same reason Kestrel does above — but with a sharper failure mode. On this * silicon the Realtek vendor request read_chip_id() issues (bRequest 5) is not - * implemented at all: the control transfer stalls, `id` stays 0, and 0 matches - * no Realtek chip-id, so the adapter reaches the unconditional Jaguar1 - * fallback at the end of this function (see read_chip_id's own note: "Returns - * 0 on a failed read, which falls through to the Jaguar1 path"). A MediaTek - * adapter then comes up as an RTL8812AU and every register access after it is - * addressed at the wrong MAC. Refusing here is the whole point of this gate. + * implemented at all, so the control transfer stalls. Before the nullopt + * return below existed, that failure was discarded, `id` stayed 0, 0 matched + * no Realtek chip-id, and the adapter reached the unconditional Jaguar1 + * fallback at the end of this function — coming up as an RTL8812AU with every + * subsequent register access addressed at the wrong MAC. * - * The pair set is disjoint from every USB id devourer tables, but the VENDOR - * ids are NOT — 0x0b05, 0x7392 and 0x2c4e each ship both silicon families — - * so this must stay a vid:pid test and must never be widened to "not a - * Realtek vendor id". Mt7612uUsbIds.h carries the measurement, and + * The pair set is disjoint from all 91 Realtek ids devourer can serve, but the + * VENDOR ids are NOT: six of them ship both silicon families, and Netgear, + * ELECOM and ASUS interleave the two within one vendor's product-id range. So + * this must stay a vid:pid test and must never be widened to "not a Realtek + * vendor id". Mt7612uUsbIds.h carries the table and the evidence; * Mt7612uUsbIdsSelftest.cpp fails if a later id addition breaks it. */ if (mt7612u::is_usb_id(vid, pid)) { _logger->error("MediaTek MT7612U ({:04x}:{:04x}) detected; devourer has no " diff --git a/src/mt7612u/Mt7612uUsbIds.h b/src/mt7612u/Mt7612uUsbIds.h index 2c34ea19..a39f151a 100644 --- a/src/mt7612u/Mt7612uUsbIds.h +++ b/src/mt7612u/Mt7612uUsbIds.h @@ -7,24 +7,41 @@ namespace mt7612u { /* USB identities of the MediaTek MT7662-MAC adapters (MT7612U / MT7662U). * - * Transcribed from the mainline mt76 driver's mt76x2u_device_table - * (drivers/net/wireless/mediatek/mt76/mt76x2/usb.c) — the set of devices the - * kernel binds to this MAC. Only 0e8d:7612 is hardware-verified here (an Alfa - * AWUS036ACM: ASIC version 0x76120044, EEPROM chip id 0x7612 — docs/mt7612u.md); - * the rest are the same MAC behind an OEM identity and are listed so they are - * refused rather than misdetected. The library's own identify step - * (MT_ASIC_VERSION >> 16 == 0x7612, usb.c) stays authoritative after the handle - * is open: an entry here only decides which backend gets to look. + * The complete mt76x2u_device_table, transcribed from the pinned reference tree + * this port was ported from — reference/mt76 @ be5ce79, + * mt76x2/usb.c:12-28 — so it is re-verifiable from a fresh checkout rather than + * from whatever kernel happens to be installed. Only 0e8d:7612 is + * hardware-verified here (an Alfa AWUS036ACM: ASIC version 0x76120044, EEPROM + * chip id 0x7612 — docs/mt7612u.md); the rest are the same MAC behind an OEM + * identity and are listed so they are refused rather than misdetected. The + * library's own identify step (MT_ASIC_VERSION >> 16 == 0x7612, usb.cpp) stays + * authoritative after the handle is open: an entry here only decides which + * backend gets to look. * - * WHY VID:PID AND NOT VID. Three of these vendor ids also appear in devourer's - * Realtek tables — 0x0b05 (ASUS, RTL8832BU 0b05:1a62), 0x7392 (Edimax, - * RTL8812AU 7392:6822) and 0x2c4e (Mercury, 2c4e:0127) — because OEMs ship both - * silicon families under one vendor id. A vendor-id gate would therefore refuse - * working Realtek adapters. The full pair sets, by contrast, are disjoint: no - * vid:pid below matches any id in KestrelUsbIds.h or Rtl8733bUsbIds.h, which is - * what makes the pair gate authoritative ahead of the Realtek SYS_CFG2 read - * (the same argument KestrelUsbIds.h relies on). See Mt7612uUsbIdsSelftest.cpp, - * which fails if a future id addition breaks that disjointness. */ + * WHY VID:PID AND NOT VID. Six of these vendor ids also ship Realtek silicon + * devourer serves, and the two families are not merely adjacent — they + * INTERLEAVE inside one vendor's product-id space: + * + * 0x0846 Netgear MediaTek 9014, 9053 Realtek 9051, 9052, 9054 + * 0x056e ELECOM MediaTek 400a Realtek 4007, 400b, 400d, 400e, ... + * 0x0b05 ASUS MediaTek 17eb, 180b, 1833 + * Realtek 17d2, 1817, 1852, 1853, 1a62 + * 0x2357 TP-Link MediaTek 0137 Realtek 0101, 0103, 0106, 010d, ... + * 0x7392 Edimax MediaTek b711 Realtek 6822, a811, a812, a813, ... + * 0x2c4e Mercury MediaTek 0103 Realtek 0127 + * + * A vendor-id gate would therefore refuse working Realtek adapters, and a + * single mistyped product id here would refuse a specific one. The full PAIR + * sets, by contrast, are disjoint: none of the 16 ids below matches any of the + * 91 Realtek ids devourer can serve (64 Jaguar1/2/3 from + * reference/rtl8812au, plus KestrelUsbIds.h and Rtl8733bUsbIds.h). That is what + * makes the pair gate authoritative ahead of the Realtek SYS_CFG2 read — the + * same argument KestrelUsbIds.h relies on. + * + * Mt7612uUsbIdsSelftest.cpp checks that disjointness against all 91, not just + * the tabled 27: Jaguar dispatches by chip-id and has no table in devourer, so + * a check limited to the tables here would be blind to the very ids that + * interleave above. */ struct UsbId { uint16_t vid; @@ -32,17 +49,22 @@ struct UsbId { }; inline constexpr UsbId kUsbIds[] = { - {0x0e8d, 0x7612}, /* Alfa AWUS036ACM / Aukey USBAC1200 — the verified part */ - {0x0e8d, 0x7632}, /* HC-M7662BU1 (MT7662U) */ {0x0b05, 0x1833}, /* Asus USB-AC54 */ {0x0b05, 0x17eb}, /* Asus USB-AC55 */ {0x0b05, 0x180b}, /* Asus USB-N53 B1 */ + {0x0e8d, 0x7612}, /* Aukey USBAC1200 / Alfa AWUS036ACM — the verified part */ {0x057c, 0x8503}, /* AVM FRITZ!WLAN AC860 */ {0x7392, 0xb711}, /* Edimax EW-7722UAC */ + {0x056e, 0x400a}, /* ELECOM WDC-867SU3S */ + {0x0e8d, 0x7632}, /* HC-M7662BU1 */ + {0x0471, 0x2126}, /* LiteOn WN4516R module */ + {0x0471, 0x7600}, /* LiteOn WN4519R module */ {0x2c4e, 0x0103}, /* Mercury UD13 */ + {0x0846, 0x9014}, /* Netgear WNDA3100v3 */ {0x0846, 0x9053}, /* Netgear A6210 */ {0x045e, 0x02e6}, /* XBox One Wireless Adapter */ {0x045e, 0x02fe}, /* XBox One Wireless Adapter */ + {0x2357, 0x0137}, /* TP-Link TL-WDN6200 */ }; inline bool is_usb_id(uint16_t vid, uint16_t pid) { diff --git a/src/mt7612u/async.cpp b/src/mt7612u/async.cpp index 20f58c0f..68addbd6 100644 --- a/src/mt7612u/async.cpp +++ b/src/mt7612u/async.cpp @@ -14,7 +14,6 @@ */ #include #include -#include #include "internal.h" /* @@ -124,8 +123,17 @@ int mt_async_start(struct mt7612u_dev *d, mt7612u_rx_cb cb, void *user) struct mt_async *a; if (d->a) return 0; - a = new (std::nothrow) mt_async{}; - if (!a) return -1; + /* try/catch, not new(nothrow): nothrow suppresses a throw from the + * allocation FUNCTION only, and these members allocate in their + * CONSTRUCTORS - std::condition_variable_any holds a shared_ptr - + * so bad_alloc escapes a nothrow new here. This library is reached over an + * extern "C" ABI, and an exception unwinding into a C caller has no + * handler, so nothing may throw past this point. */ + try { + a = new mt_async{}; + } catch (...) { + return -1; + } d->a = a; a->cb = cb; a->cb_user = user; @@ -150,7 +158,13 @@ int mt_async_start(struct mt7612u_dev *d, mt7612u_rx_cb cb, void *user) * an error code; catching keeps this the same `goto fail` teardown. */ try { a->evt = std::thread(evt_thread, d); - } catch (const std::system_error &) { + } catch (...) { + /* Deliberately catch-all rather than std::system_error: libstdc++ + * allocates the thread state with a THROWING new inside the + * constructor, so an out-of-memory failure arrives as bad_alloc, not + * as the system_error that pthread_create's EAGAIN maps to. Letting + * that one escape would skip this teardown - leaving d->a live with + * running=1 and evt_started=0 - and then unwind into a C caller. */ a->running = 0; goto fail; } diff --git a/src/mt7612u/init.cpp b/src/mt7612u/init.cpp index 75941b05..08afa564 100644 --- a/src/mt7612u/init.cpp +++ b/src/mt7612u/init.cpp @@ -7,7 +7,6 @@ */ #include #include -#include #include #include "internal.h" #include "initvals.h" @@ -466,7 +465,15 @@ static struct mt7612u_dev *bring_up(struct mt7612u_dev *d, const char *fw_dir, struct mt7612u_dev *mt7612u_open(const char *fw_dir, const char **err) { - struct mt7612u_dev *d = new (std::nothrow) mt7612u_dev{}; + struct mt7612u_dev *d = NULL; + + /* See mt_async_start(): nothrow does not cover a member constructor that + * allocates, and nothing may throw across the extern "C" boundary. */ + try { + d = new mt7612u_dev{}; + } catch (...) { + d = NULL; + } if (!d) { if (err) *err = "out of memory"; @@ -479,7 +486,15 @@ struct mt7612u_dev *mt7612u_open(const char *fw_dir, const char **err) struct mt7612u_dev *mt7612u_open_handle(void *h, void *ctx, const char *fw_dir, const char **err) { - struct mt7612u_dev *d = new (std::nothrow) mt7612u_dev{}; + struct mt7612u_dev *d = NULL; + + /* See mt_async_start(): nothrow does not cover a member constructor that + * allocates, and nothing may throw across the extern "C" boundary. */ + try { + d = new mt7612u_dev{}; + } catch (...) { + d = NULL; + } if (!d) { if (err) *err = "out of memory"; @@ -502,7 +517,7 @@ void mt7612u_close(struct mt7612u_dev *d) if (d->h) mt_mac_rx_disable(d); mt_async_stop(d); if (d->h) mt_mac_stop(d); - mt_close(d); /* releases io_lock via mt_dev_state_destroy() */ + mt_close(d); delete d; } diff --git a/src/mt7612u/internal.h b/src/mt7612u/internal.h index 13b45f18..5cf9c162 100644 --- a/src/mt7612u/internal.h +++ b/src/mt7612u/internal.h @@ -203,11 +203,15 @@ struct mt7612u_dev { }; /* --- usb.c --- */ -/* Per-device state both open paths need before ANY register I/O: the recursive - * io_lock and the calibration sentinels. Both mt_open() and mt_adopt() reach - * mt_vendor_req() (which locks io_lock) during identification, so this must run - * first on either path. Idempotent. mt_dev_state_destroy() is the matching - * teardown, guarded so it runs exactly once regardless of how far open got. */ +/* Per-device state both open paths need before ANY register I/O. + * + * This used to construct the recursive io_lock, and existed because both + * mt_open() and mt_adopt() reach mt_vendor_req() (which locks it) during + * identification, so a path that skipped it locked an uninitialised mutex. + * io_lock is a std::recursive_mutex member now, constructed with the device, + * so that hazard is gone and both functions are empty - kept as named seams + * because the calibration sentinels belong to the same step, and because two + * public open paths and one close path call them. */ void mt_dev_state_init(struct mt7612u_dev *d); void mt_dev_state_destroy(struct mt7612u_dev *d); int mt_open(struct mt7612u_dev *d, const char **err); diff --git a/src/mt7612u/usb.cpp b/src/mt7612u/usb.cpp index 906ee0c9..a1afcd4a 100644 --- a/src/mt7612u/usb.cpp +++ b/src/mt7612u/usb.cpp @@ -64,6 +64,13 @@ void mt_diag(char level, const char *fmt, ...) void mt_usleep(unsigned us) { + /* Not interruptible, where nanosleep(&ts, NULL) was: libstdc++ retries + * sleep_for on EINTR, so a signal no longer cuts the wait short (measured: + * a 200 ms request under 100 Hz SIGALRM returned after 10 ms before, 201 ms + * now). That is the behaviour this call wants — every use is a hardware + * settle or poll interval, and a delivered signal is not a reason for the + * chip to be ready sooner. Worth knowing if a caller ever wants to + * interrupt a long bring-up. */ std::this_thread::sleep_for(std::chrono::microseconds(us)); } diff --git a/tests/mt7612u_usb_ids_selftest.cpp b/tests/mt7612u_usb_ids_selftest.cpp index 7f731e89..65c0dbaa 100644 --- a/tests/mt7612u_usb_ids_selftest.cpp +++ b/tests/mt7612u_usb_ids_selftest.cpp @@ -1,20 +1,28 @@ /* Headless guard for the MediaTek USB-id gate (src/mt7612u/Mt7612uUsbIds.h, * used by WiFiDriver::CreateRadio before the Realtek SYS_CFG2 read). * - * The gate exists because a MediaTek adapter stalls read_chip_id(), which - * returns 0, which matches no Realtek chip-id, which lands the adapter in the - * unconditional Jaguar1 fallback. Refusing it depends on one property: the - * MediaTek vid:pid set must never claim a device some Realtek backend owns. - * That is what this test pins — a future id addition on either side that breaks - * it fails here rather than on a user's desk. + * The gate exists because a MediaTek adapter stalls read_chip_id(), which fails, + * which used to leave the adapter in the unconditional Jaguar1 fallback. + * Refusing it depends on one property: the MediaTek vid:pid set must never claim + * a device a Realtek backend owns. That is what this pins — a future id addition + * on either side that breaks it fails here rather than on a user's desk. * - * It also records WHY the gate is vid:pid and not vid: several vendor ids ship - * both silicon families. That half is reported, not asserted, so removing an - * OEM entry cannot fail the build — only a genuine pair collision can. */ + * The cross-check MUST cover Jaguar. Jaguar1/2/3 dispatch by chip-id and have no + * id table in devourer, so a check limited to KestrelUsbIds.h and + * Rtl8733bUsbIds.h sees only 27 of the 91 ids devourer can serve — and every + * vendor id where the two silicon families interleave lives in the 64 it would + * miss. An earlier version of this test did exactly that, and adding + * 0b05:17d2 (ASUS USB-AC56, an RTL8812AU) to the MediaTek table passed it while + * refusing a real Jaguar1 adapter. Hence kJaguarWitness below. + * + * It also records WHY the gate is vid:pid and not vid. That half is reported, + * not asserted, so removing an OEM entry cannot fail the build — only a genuine + * pair collision can. */ #include "kestrel/KestrelUsbIds.h" #include "mt7612u/Mt7612uUsbIds.h" #include "rtl8733b/Rtl8733bUsbIds.h" +#include #include namespace { @@ -27,60 +35,97 @@ void check(bool ok, const char *what) { } } +/* Every USB id the Jaguar1/2/3 vendor driver claims, from + * reference/rtl8812au/os_dep/linux/usb_intf.c (the tree devourer's Jaguar port + * came from). Embedded rather than read from that submodule, because it is not + * fetched in a default clone. Regenerate with: + * + * grep -oE 'USB_DEVICE(_AND_INTERFACE_INFO)?\(0x[0-9A-Fa-f]{4}, *0x[0-9A-Fa-f]{4}' \ + * reference/rtl8812au/os_dep/linux/usb_intf.c | + * grep -oE '0x[0-9A-Fa-f]{4}, *0x[0-9A-Fa-f]{4}' | tr -d ' ' | sort -u + * + * These are witnesses, not a dispatch table: devourer identifies this family by + * chip-id, and nothing here should ever be used to route a device. */ +constexpr mt7612u::UsbId kJaguarWitness[] = { + {0x0409, 0x0408}, {0x0411, 0x0242}, {0x0411, 0x025d}, {0x0411, 0x029b}, + {0x04bb, 0x0952}, {0x04bb, 0x0953}, {0x050d, 0x1106}, {0x050d, 0x1109}, + {0x056e, 0x4007}, {0x056e, 0x400b}, {0x056e, 0x400d}, {0x056e, 0x400e}, + {0x056e, 0x400f}, {0x056e, 0x4010}, {0x0586, 0x3426}, {0x0789, 0x016e}, + {0x07b8, 0x8179}, {0x07b8, 0x8812}, {0x0846, 0x9051}, {0x0846, 0x9052}, + {0x0846, 0x9054}, {0x0b05, 0x17d2}, {0x0b05, 0x1817}, {0x0b05, 0x1852}, + {0x0b05, 0x1853}, {0x0df6, 0x0074}, {0x0e66, 0x0022}, {0x0e66, 0x0023}, + {0x0e66, 0x0026}, {0x1058, 0x0632}, {0x13b1, 0x003f}, {0x148f, 0x9097}, + {0x1740, 0x0100}, {0x2001, 0x330e}, {0x2001, 0x3313}, {0x2001, 0x3314}, + {0x2001, 0x3315}, {0x2001, 0x3316}, {0x2001, 0x3318}, {0x2001, 0x331a}, + {0x2019, 0xab30}, {0x2019, 0xab32}, {0x20f4, 0x805b}, {0x20f4, 0x809a}, + {0x20f4, 0x809b}, {0x2357, 0x0101}, {0x2357, 0x0103}, {0x2357, 0x0106}, + {0x2357, 0x010d}, {0x2357, 0x010e}, {0x2357, 0x010f}, {0x2357, 0x011e}, + {0x2357, 0x011f}, {0x2357, 0x0120}, {0x2357, 0x0122}, {0x2604, 0x0012}, + {0x3823, 0x6249}, {0x7392, 0xa811}, {0x7392, 0xa812}, {0x7392, 0xa813}, + {0x7392, 0xa822}, {0x7392, 0xa833}, {0x7392, 0xa834}, {0x7392, 0xb611}, +}; + } // namespace int main() { /* Every tabled MediaTek id is claimed. A table entry the lookup misses would * silently reopen the Jaguar1 misroute for that adapter. */ - for (const mt7612u::UsbId &id : mt7612u::kUsbIds) { + for (const mt7612u::UsbId &id : mt7612u::kUsbIds) if (!mt7612u::is_usb_id(id.vid, id.pid)) { - std::fprintf(stderr, "mt7612u_usb_ids: FAIL %04x:%04x tabled but not claimed\n", + std::fprintf(stderr, + "mt7612u_usb_ids: FAIL %04x:%04x tabled but not claimed\n", id.vid, id.pid); fails++; } - } /* THE load-bearing property: the MediaTek gate must not claim any device a * Realtek backend owns. It runs before the SYS_CFG2 read, so a false claim * here is unrecoverable — the Realtek adapter is refused outright. */ - for (const rtl8733b::UsbId &id : rtl8733b::kUsbIds) { - if (mt7612u::is_usb_id(id.vid, id.pid)) { + int checked = 0; + auto reject = [&](uint16_t vid, uint16_t pid, const char *family) { + checked++; + if (mt7612u::is_usb_id(vid, pid)) { std::fprintf(stderr, - "mt7612u_usb_ids: FAIL %04x:%04x is RTL8733B but the " - "MediaTek gate claims it\n", - id.vid, id.pid); + "mt7612u_usb_ids: FAIL %04x:%04x is %s but the MediaTek " + "gate claims it\n", + vid, pid, family); fails++; } - } - for (const auto &id : kestrel::kKestrelUsbIds) { - if (mt7612u::is_usb_id(id.vid, id.pid)) { - std::fprintf(stderr, - "mt7612u_usb_ids: FAIL %04x:%04x is Kestrel but the " - "MediaTek gate claims it\n", - id.vid, id.pid); - fails++; - } - } + }; + for (const mt7612u::UsbId &id : kJaguarWitness) + reject(id.vid, id.pid, "Jaguar1/2/3"); + for (const rtl8733b::UsbId &id : rtl8733b::kUsbIds) + reject(id.vid, id.pid, "RTL8733B"); + for (const auto &id : kestrel::kKestrelUsbIds) + reject(id.vid, id.pid, "Kestrel"); + + /* The coverage itself is an invariant: if someone trims the witness list, the + * blindness this test exists to prevent comes back silently. */ + check(checked >= 91, "cross-checked against at least 91 Realtek ids"); + + /* The exact id the earlier, Jaguar-blind version of this test let through. */ + check(!mt7612u::is_usb_id(0x0b05, 0x17d2), + "ASUS USB-AC56 (RTL8812AU) is not claimed"); + check(!mt7612u::is_usb_id(0x0bda, 0x8812), "RTL8812AU reference id not claimed"); + check(!mt7612u::is_usb_id(0x0bda, 0x8813), "RTL8814AU reference id not claimed"); - /* Negative control: the most common Realtek adapter devourer serves, and the - * one the Jaguar1 fallback is FOR. If the gate ever claimed this, every - * RTL8812AU would stop working — so a test that only checked tabled ids - * passing would not be enough. 0bda:8812 is dispatched by chip-id, not by any - * table above, so it is not covered by the loops. */ - check(!mt7612u::is_usb_id(0x0bda, 0x8812), "RTL8812AU is not claimed"); - check(!mt7612u::is_usb_id(0x0bda, 0x8813), "RTL8814AU is not claimed"); + /* Netgear interleaves the two families in adjacent product ids, so this is + * where an off-by-one transcription would land. */ + check(mt7612u::is_usb_id(0x0846, 0x9053), "Netgear A6210 (MediaTek) is claimed"); + check(!mt7612u::is_usb_id(0x0846, 0x9052), "Netgear A6100 (RTL8821) is not claimed"); + check(!mt7612u::is_usb_id(0x0846, 0x9054), "Netgear A7000 (RTL8814A) is not claimed"); - /* A MediaTek id one digit off must NOT be claimed: the gate is an exact pair - * match, not a prefix or a vendor test. */ + /* Exact pair match, not a prefix or a vendor test. */ check(mt7612u::is_usb_id(0x0e8d, 0x7612), "the verified Alfa AWUS036ACM is claimed"); check(!mt7612u::is_usb_id(0x0e8d, 0x7613), "an untabled MediaTek pid is not claimed"); check(!mt7612u::is_usb_id(0x0bda, 0x7612), "the pid alone does not claim"); - /* Reported, not asserted — the rationale for pair-matching. Shared vendor ids - * are why a "not a Realtek vendor id" gate would refuse working adapters. */ + /* Reported, not asserted — the rationale for pair-matching. */ int shared_vids = 0; for (const mt7612u::UsbId &m : mt7612u::kUsbIds) { bool shared = false; + for (const mt7612u::UsbId &j : kJaguarWitness) + shared = shared || j.vid == m.vid; for (const rtl8733b::UsbId &r : rtl8733b::kUsbIds) shared = shared || r.vid == m.vid; for (const auto &k : kestrel::kKestrelUsbIds) @@ -88,10 +133,11 @@ int main() { if (shared) shared_vids++; } - std::printf("mt7612u_usb_ids: %d MediaTek ids, %d share a vendor id with a " - "Realtek table (why the gate matches pairs, not vendors)\n", + std::printf("mt7612u_usb_ids: %d MediaTek ids vs %d Realtek ids; %d MediaTek " + "ids share a vendor id with a Realtek one (why the gate matches " + "pairs, not vendors)\n", (int)(sizeof(mt7612u::kUsbIds) / sizeof(mt7612u::kUsbIds[0])), - shared_vids); + checked, shared_vids); if (fails) { std::fprintf(stderr, "mt7612u_usb_ids: %d failure(s)\n", fails); From 79f4f5a3662afe62dc3de8e31cb6f1499cf9140e Mon Sep 17 00:00:00 2001 From: snokvist Date: Wed, 9 Sep 2026 23:31:01 +0200 Subject: [PATCH 6/7] mt7612u: take the adapter selector as an argument, not from the environment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit qodo's first finding on #421, and it is right for a reason that is specific to this PR: #412 deferred the getenv while the subtree was unreachable, but DEVOURER_MT7612U now links that open path into libdevourer, so "the library picks its adapter from ambient process state" stopped being a bench-tool quirk and became a library property. A consumer with two adapters could claim the wrong one because of a variable it never set. open_selected() takes the selector as a parameter, mt7612u_open_selected() is the public way to pass it, and mt7612u_open() is that with NULL. bringup fills d->dev_selector from MT7612U_DEV before mt_open(), so the operator-facing spelling is unchanged. The library now reads no environment at all — the claim two comments made prematurely in the previous commit is finally true. The messages had to move with it. They named MT7612U_DEV, which the library no longer reads, so a consumer that is not bringup would have been told to set something it does not use. They now name "the selector" and point bringup users at the variable. tests/api_link.c covers the new entry point (26 now, still compiled as C), which is the test's whole purpose: a public declaration with no definition fails there rather than at the first caller. Verified on hardware, two adapters attached: MT7612U_DEV=2-1 and =5-1 each open their own unit (distinct EEPROM MACs 40:a5:ef:50:27:a1 and 40:a5:ef:5a:32:f8), and with no selector it still takes the first and warns. 59/59 ctest in all three configs. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj --- src/mt7612u/include/mt7612u/mt7612u.h | 13 ++++++++++ src/mt7612u/init.cpp | 7 +++++ src/mt7612u/internal.h | 7 +++++ src/mt7612u/tests/api_link.c | 1 + src/mt7612u/tools/bringup.cpp | 3 +++ src/mt7612u/usb.cpp | 37 +++++++++++++++------------ 6 files changed, 51 insertions(+), 17 deletions(-) diff --git a/src/mt7612u/include/mt7612u/mt7612u.h b/src/mt7612u/include/mt7612u/mt7612u.h index 9696cb90..d4e97749 100644 --- a/src/mt7612u/include/mt7612u/mt7612u.h +++ b/src/mt7612u/include/mt7612u/mt7612u.h @@ -109,6 +109,19 @@ struct mt7612u_dev; */ struct mt7612u_dev *mt7612u_open(const char *fw_dir, const char **err); +/* + * Same, but choosing which adapter to open when more than one is attached: + * selector is "-" as lsusb spells the port path (e.g. "2-1"), or + * NULL for "the first one", which is what mt7612u_open() passes. + * + * Explicit because this is a library. It reads no environment of its own, so a + * caller with two adapters is never at the mercy of an inherited variable — + * the tool that wants MT7612U_DEV reads it and passes it here. The string is + * borrowed for the duration of the call only. + */ +struct mt7612u_dev *mt7612u_open_selected(const char *selector, + const char *fw_dir, const char **err); + /* * Same, but adopting a libusb handle the caller already opened, reset and * claimed interface 0 on. Neither the handle nor the context is closed by diff --git a/src/mt7612u/init.cpp b/src/mt7612u/init.cpp index 08afa564..51829e56 100644 --- a/src/mt7612u/init.cpp +++ b/src/mt7612u/init.cpp @@ -464,6 +464,12 @@ static struct mt7612u_dev *bring_up(struct mt7612u_dev *d, const char *fw_dir, } struct mt7612u_dev *mt7612u_open(const char *fw_dir, const char **err) +{ + return mt7612u_open_selected(NULL, fw_dir, err); +} + +struct mt7612u_dev *mt7612u_open_selected(const char *selector, + const char *fw_dir, const char **err) { struct mt7612u_dev *d = NULL; @@ -479,6 +485,7 @@ struct mt7612u_dev *mt7612u_open(const char *fw_dir, const char **err) if (err) *err = "out of memory"; return NULL; } + d->dev_selector = selector; if (mt_open(d, err)) { mt_dev_state_destroy(d); delete d; return NULL; } return bring_up(d, fw_dir, err); } diff --git a/src/mt7612u/internal.h b/src/mt7612u/internal.h index 5cf9c162..21ca4f45 100644 --- a/src/mt7612u/internal.h +++ b/src/mt7612u/internal.h @@ -181,6 +181,13 @@ struct mt7612u_dev { * (bringup does). Note this is not yet true of the library as a whole: * open_selected() still reads MT7612U_DEV (see usb.cpp). */ uint8_t no_autorecover; + /* Which adapter to open, "-" as bringup spells it, or NULL for + * "the first one". A field and not a getenv: this is a LIBRARY now + * (DEVOURER_MT7612U links it into libdevourer), and a library that picks + * its hardware from ambient process state can claim an adapter its caller + * never asked for. Points at caller-owned storage and is only read during + * mt_open(). */ + const char *dev_selector; uint8_t bw_clamp_warned; /* the "never widen" notice is once, not per frame */ int8_t txpower_conf; /* limit, 0.5 dB units (dBm * 2) */ int8_t target_power; diff --git a/src/mt7612u/tests/api_link.c b/src/mt7612u/tests/api_link.c index 5d2a9552..9f7261d9 100644 --- a/src/mt7612u/tests/api_link.c +++ b/src/mt7612u/tests/api_link.c @@ -15,6 +15,7 @@ static void *const api[] = { (void *)mt7612u_open, + (void *)mt7612u_open_selected, (void *)mt7612u_open_handle, (void *)mt7612u_close, (void *)mt7612u_keep_detached, diff --git a/src/mt7612u/tools/bringup.cpp b/src/mt7612u/tools/bringup.cpp index 6ed76c91..04e41dc7 100644 --- a/src/mt7612u/tools/bringup.cpp +++ b/src/mt7612u/tools/bringup.cpp @@ -2523,6 +2523,9 @@ int main(int argc, char **argv) * experiments observe-only. Same spelling as before. */ if (getenv("MT7612U_NO_AUTORECOVER")) dev.no_autorecover = 1; + /* Same shape, same reason: the library takes a selector, this tool is what + * reads the environment for it. Operator-facing spelling is unchanged. */ + dev.dev_selector = getenv("MT7612U_DEV"); /* Runs before the global mt_open() below, because it IS an open - of the * other public entry point. */ diff --git a/src/mt7612u/usb.cpp b/src/mt7612u/usb.cpp index a1afcd4a..2153af14 100644 --- a/src/mt7612u/usb.cpp +++ b/src/mt7612u/usb.cpp @@ -357,12 +357,10 @@ void mt_recover_usb(struct mt7612u_dev *d) * a field rather than a getenv(): the tool that wants the behaviour sets * it (bringup does, from MT7612U_NO_AUTORECOVER). * - * That is the direction of travel, not a property the library has yet: - * open_selected() below still reads MT7612U_DEV directly. #412 deferred - * that one because there is no public way to pass a selector - - * mt7612u_open() allocates the device itself and the struct is opaque - - * so removing it would strand multi-adapter callers with no replacement. - * It moves to a DeviceConfig field when the backend lands (#419). */ + * The same is now true of adapter selection: d->dev_selector replaced the + * getenv("MT7612U_DEV") that open_selected() used to read, and + * mt7612u_open_selected() is the public way to pass it. This library reads + * no environment at all. */ if (d->no_autorecover) { if (mt_rr_chk(d, CFG_ADDR(MT_USB_U3DMA_CFG), &cfg)) LOG("auto-recovery disabled: U3DMA_CFG unreadable"); @@ -428,13 +426,18 @@ int mt_adopt(struct mt7612u_dev *d, libusb_device_handle *h, } /* - * Open one MT7612U, honouring MT7612U_DEV when more than one is attached. + * Open one MT7612U, honouring the caller's selector when more than one is + * attached. * * libusb_open_device_with_vid_pid() returns whichever matching device * enumerates first, which is fine with one adapter and silently ambiguous * with two - a measurement then attributes itself to whichever unit the bus - * happened to hand over. MT7612U_DEV takes a "bus-port" as lsusb and sysfs + * happened to hand over. The selector takes a "bus-port" as lsusb and sysfs * spell it ("2-1"), or a bare index into the matches in enumeration order. + * It is passed in, never read from the environment: bringup fills it from + * MT7612U_DEV, and a library consumer passes whatever its own config says. + * Messages below therefore name "the selector", not that variable - a caller + * that is not bringup would be told to set something it does not use. * * This is the ONE environment read left in the library, and it stays deferred * to integration as agreed in #412 rather than being removed here: there is no @@ -554,9 +557,9 @@ static int lock_adapter(libusb_device *dev, const char **err) } #endif /* !_WIN32 */ -static libusb_device_handle *open_selected(libusb_context *ctx, const char **err) +static libusb_device_handle *open_selected(libusb_context *ctx, const char *sel, + const char **err) { - const char *sel = getenv("MT7612U_DEV"); libusb_device **list = NULL; libusb_device_handle *h = NULL; @@ -588,13 +591,13 @@ static libusb_device_handle *open_selected(libusb_context *ctx, const char **err if (!sel || !*sel) { LOG("MT7612U at %s%s", id, matches ? "" : " <- selected (first)"); } else if (!strcmp(sel, id)) { - LOG("MT7612U at %s <- selected by MT7612U_DEV", id); + LOG("MT7612U at %s <- selected", id); } else { char idx[8]; snprintf(idx, sizeof idx, "%d", matches); if (strcmp(sel, idx)) { matches++; continue; } - LOG("MT7612U at %s <- selected by MT7612U_DEV index %d", id, matches); + LOG("MT7612U at %s <- selected by index %d", id, matches); } if (!h) { @@ -621,9 +624,9 @@ static libusb_device_handle *open_selected(libusb_context *ctx, const char **err } if (matches > 1 && (!sel || !*sel)) - WARN("%d MT7612U adapters attached and MT7612U_DEV is unset - " - "using the first. Set MT7612U_DEV= to be explicit.", - matches); + WARN("%d MT7612U adapters attached and no selector was given - " + "using the first. Pass a \"-\" selector to choose " + "(bringup takes it from MT7612U_DEV).", matches); libusb_free_device_list(list, 1); if (!h && err) *err = matches ? "MT7612U found but could not be opened (try sudo)" @@ -641,7 +644,7 @@ int mt_open(struct mt7612u_dev *d, const char **err) if (libusb_init(&d->ctx)) { if (err) *err = "libusb_init failed"; return -1; } d->owns_handle = 1; - d->h = open_selected(d->ctx, err); + d->h = open_selected(d->ctx, d->dev_selector, err); if (!d->h) { libusb_exit(d->ctx); d->ctx = NULL; return -1; @@ -666,7 +669,7 @@ int mt_open(struct mt7612u_dev *d, const char **err) /* Re-enumerated under a new address: reopen and re-detach. */ libusb_close(d->h); mt_usleep(200000); - d->h = open_selected(d->ctx, NULL); + d->h = open_selected(d->ctx, d->dev_selector, NULL); if (!d->h) { if (err) *err = "device vanished after USB reset"; libusb_exit(d->ctx); d->ctx = NULL; From cb356bb6582908a0fd7f31957cfc1612ba8b2741 Mon Sep 17 00:00:00 2001 From: snokvist Date: Wed, 9 Sep 2026 23:42:33 +0200 Subject: [PATCH 7/7] mt7612u: give the library a diagnostic sink instead of writing to stderr qodo's second finding on #421. I first deferred this to the backend PR on the grounds that a sink has nothing to connect to yet. That was inconsistent: I had just fixed the MT7612U_DEV getenv precisely because DEVOURER_MT7612U links this code into libdevourer and changes its exposure, and the same argument applies here with the same force. mt_diag() reimplemented devourer's logger rather than using it. It formats "devourer [%c] mt7612u: " and does one fwrite + fflush for per-line atomicity -- the same shape as Logger::emit and for the same stated reason -- while honouring none of that class's three controls: - set_diag_stream(): a host that redirects diagnostics still gets 79 call sites' worth on raw stderr. - set_level(): a host at Error still gets every Info line. - DEVOURER_LOG_MAX_LEVEL: compile-time stripping does not reach them. And on Android it is worse than cosmetic. Logger::emit has an __android_log_write branch; these writes do not, so on the platform devourer actually ships to they land nowhere a user can see. mt7612u_set_log_sink(fn, user) diverts every line. The sink gets the level letter and the BARE message, so a host applies its own prefix and nothing double-prefixes -- a devourer consumer forwards to Logger::info/warn/error and gets level gating, stream redirection and logcat for free. The prefix and the stderr write now live in one built-in default sink, which is the only place this library names stderr at all. Default stays stderr rather than silence. qodo suggested the reverse -- have applications install stderr explicitly -- but devourer's own Logger defaults to stderr too, so silencing this subtree by default would make it quieter than every other devourer component, and bringup would lose its output. A host that wants silence installs a no-op sink. Easy to flip if you would rather. tests/log_sink is the guard, and it tests the property that actually matters: that installing a sink DIVERTS rather than copies. A hook that receives a copy while stderr keeps getting the original looks identical in casual use and fixes nothing, so every case asserts the sink saw the line AND that stderr did not. Mutation-tested three ways -- sink-gets-a-copy, sink-gets-the-prefixed-line, and NULL-silences-instead-of-restoring -- each fails it. api_link covers the new entry point (27 now, still compiled as C). Verified on hardware: bringup's output is unchanged through the default sink, and an 8 s async RX run (11151 frames, 1391/s, rx_err=0) exercises the sink from the event thread. 59/59 ctest in all three configs. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Tba83kymS5W2v1vn2yRxrj --- docs/mt7612u.md | 5 +- src/mt7612u/.gitignore | 1 + src/mt7612u/Makefile | 8 +- src/mt7612u/README.md | 2 +- src/mt7612u/include/mt7612u/mt7612u.h | 26 +++++ src/mt7612u/tests/api_link.c | 1 + src/mt7612u/tests/log_sink.cpp | 162 ++++++++++++++++++++++++++ src/mt7612u/usb.cpp | 52 +++++++-- 8 files changed, 240 insertions(+), 17 deletions(-) create mode 100644 src/mt7612u/tests/log_sink.cpp diff --git a/docs/mt7612u.md b/docs/mt7612u.md index 8b4af98f..ae1e455a 100644 --- a/docs/mt7612u.md +++ b/docs/mt7612u.md @@ -301,13 +301,14 @@ and nothing was ever queued on the healthy path. ## Offline tests -`make -C src/mt7612u check` runs three binaries. No hardware, no privileges. +`make -C src/mt7612u check` runs four binaries. No hardware, no privileges. | test | what it holds | |---|---| -| `api_link` | takes the address of all 25 public entry points while including only the public header, so a declaration that loses its definition is a link error | +| `api_link` | takes the address of all 27 public entry points while including only the public header, so a declaration that loses its definition is a link error. Still compiled as C, which is what keeps the `extern "C"` guard honest now the library itself is C++ | | `frame_shape` | `mt_hdrlen_from_fc()` over management, all eight control subtypes and the five data shapes; the RX L2-pad fold on a synthetic QoS frame, with a negative control that redoes the old fixed-24 fold and asserts the QoS Control really is destroyed; the radiotap VHT bandwidth mapping over all eleven codes the part can express | | `field_macros` | `MT_CTZ` against `__builtin_ctz` over all 32 single-bit and all 528 contiguous masks, plus a `FIELD_PREP`/`FIELD_GET` round-trip, plus a static initialiser that fails to compile if the macro stops being constant-foldable | +| `log_sink` | that `mt7612u_set_log_sink()` **diverts** rather than copies — stderr must stay silent while a sink is installed — that the sink gets the bare message with no prefix to double up, that every level letter arrives, and that NULL restores the built-in sink instead of silencing the library | Each was mutation-tested: removing one public definition, reverting either frame-shape fix, and reverting the header-length fix each make the suite fail, diff --git a/src/mt7612u/.gitignore b/src/mt7612u/.gitignore index 1f879736..07bcea4a 100644 --- a/src/mt7612u/.gitignore +++ b/src/mt7612u/.gitignore @@ -2,6 +2,7 @@ bringup tests/api_link tests/frame_shape tests/field_macros +tests/log_sink *.o *.d firmware/ diff --git a/src/mt7612u/Makefile b/src/mt7612u/Makefile index 00f32857..5c790493 100644 --- a/src/mt7612u/Makefile +++ b/src/mt7612u/Makefile @@ -23,8 +23,9 @@ DEPFLAGS = -MMD -MP INCS = -I. -Iinclude LDLIBS = -lusb-1.0 -lpthread LIBSRCS = $(wildcard *.cpp) -TESTS = tests/api_link tests/frame_shape tests/field_macros -CXXSRCS = $(LIBSRCS) tools/bringup.cpp tests/frame_shape.cpp tests/field_macros.cpp +TESTS = tests/api_link tests/frame_shape tests/field_macros tests/log_sink +CXXSRCS = $(LIBSRCS) tools/bringup.cpp tests/frame_shape.cpp tests/field_macros.cpp \ + tests/log_sink.cpp CSRCS = tests/api_link.c OBJS = $(CXXSRCS:.cpp=.o) $(CSRCS:.c=.o) LIBOBJS = $(LIBSRCS:.cpp=.o) @@ -49,6 +50,9 @@ tests/frame_shape: $(LIBOBJS) tests/frame_shape.o tests/field_macros: $(LIBOBJS) tests/field_macros.o $(CXX) $(CXXFLAGS) -o $@ $^ $(LDLIBS) +tests/log_sink: $(LIBOBJS) tests/log_sink.o + $(CXX) $(CXXFLAGS) -o $@ $^ $(LDLIBS) + %.o: %.cpp $(CXX) $(CXXFLAGS) $(DEPFLAGS) $(INCS) -c -o $@ $< diff --git a/src/mt7612u/README.md b/src/mt7612u/README.md index 6611f359..5ac76b5b 100644 --- a/src/mt7612u/README.md +++ b/src/mt7612u/README.md @@ -39,7 +39,7 @@ Measurements, methods and limits: [`../../docs/mt7612u.md`](../../docs/mt7612u.m | `radiotap.cpp` | `send_packet` / `send_packets` (USB chaining via `NEXT_VLD`) | | `caps.cpp` | TSF, capability descriptor, ACK responder | | `tools/bringup.cpp` | one subcommand per verified gate | -| `tests/` | offline tests (`make check`): public-API link (C), frame shapes | +| `tests/` | offline tests (`make check`): public-API link (C), frame shapes, field macros, log sink | | `initvals.h` | **generated** — see Provenance | ## The receiver must never run undrained diff --git a/src/mt7612u/include/mt7612u/mt7612u.h b/src/mt7612u/include/mt7612u/mt7612u.h index d4e97749..06dd510c 100644 --- a/src/mt7612u/include/mt7612u/mt7612u.h +++ b/src/mt7612u/include/mt7612u/mt7612u.h @@ -141,6 +141,32 @@ struct mt7612u_dev *mt7612u_open_handle(void *h, void *ctx, const char *fw_dir, void mt7612u_close(struct mt7612u_dev *dev); +/* + * Diagnostic sink. + * + * This library emits human diagnostics — bring-up progress, firmware version, + * USB and MCU failures. By default they go to stderr, formatted the way + * devourer's own logger formats its lines, which is right for the standalone + * bring-up tool and wrong for anything embedding this library: writing straight + * to stderr bypasses the host's log level, bypasses a redirected diagnostic + * stream, and on Android bypasses __android_log_write entirely, so the lines + * land nowhere a user can see them. + * + * Install a sink and every line goes there instead. `level` is one of + * 'I' / 'W' / 'E'; `line` is the bare message with NO prefix, so a host can + * apply its own — a devourer consumer forwards it to Logger::info/warn/error, + * which re-adds "devourer [X] " and honours the level and stream it was + * configured with. Passing NULL restores the built-in stderr sink; installing a + * sink that does nothing silences the library. + * + * Set it before any worker thread starts, and do not change it afterwards: the + * pointer is read from the RX event thread without synchronisation. That is the + * same discipline devourer's own logger documents for set_level and + * set_diag_stream, and for the same reason. + */ +typedef void (*mt7612u_log_sink)(void *user, char level, const char *line); +void mt7612u_set_log_sink(mt7612u_log_sink sink, void *user); + /* Reattaches the kernel driver on close unless this is set. */ void mt7612u_keep_detached(struct mt7612u_dev *dev, int keep); diff --git a/src/mt7612u/tests/api_link.c b/src/mt7612u/tests/api_link.c index 9f7261d9..3fe2c2bd 100644 --- a/src/mt7612u/tests/api_link.c +++ b/src/mt7612u/tests/api_link.c @@ -18,6 +18,7 @@ static void *const api[] = { (void *)mt7612u_open_selected, (void *)mt7612u_open_handle, (void *)mt7612u_close, + (void *)mt7612u_set_log_sink, (void *)mt7612u_keep_detached, (void *)mt7612u_set_channel, (void *)mt7612u_set_txpower, diff --git a/src/mt7612u/tests/log_sink.cpp b/src/mt7612u/tests/log_sink.cpp new file mode 100644 index 00000000..ab6887ef --- /dev/null +++ b/src/mt7612u/tests/log_sink.cpp @@ -0,0 +1,162 @@ +/* SPDX-License-Identifier: BSD-3-Clause-Clear */ +/* + * The diagnostic sink (mt7612u_set_log_sink). + * + * This library used to write every diagnostic straight to stderr with + * devourer's line format baked in. That is right for the bring-up tool and + * wrong for anything embedding the library: it bypasses the host's log level, + * bypasses a redirected diagnostic stream, and on Android bypasses + * __android_log_write, so the lines land nowhere a user can see them. + * + * What matters here is that installing a sink actually DIVERTS — a hook that + * receives a copy while stderr keeps getting the original would look identical + * in casual use and fix nothing. So each case checks the sink saw the line AND + * that stderr did not. + */ +#include +#include + +#include "internal.h" + +static int fails; + +static void check(int ok, const char *what) +{ + if (!ok) { + printf(" FAIL %s\n", what); + fails++; + } +} + +/* --- the sink under test ------------------------------------------------ */ + +static char last_line[512]; +static char last_level; +static int calls; +static void *last_user; + +static void capture(void *user, char level, const char *line) +{ + last_user = user; + last_level = level; + calls++; + snprintf(last_line, sizeof last_line, "%s", line); +} + +/* Redirect stderr to a temp file so "did stderr get anything?" is answerable. + * freopen, not dup2: this has to work the same on Windows, where the library is + * now built too. */ +static FILE *steal_stderr(const char *path) +{ + return freopen(path, "w+", stderr); +} + +static long stderr_len(void) +{ + long n; + + fflush(stderr); + n = ftell(stderr); + return n < 0 ? 0 : n; +} + +int main(void) +{ + const char *path = "log_sink_stderr.tmp"; + int marker = 4242; + + printf("mt7612u log sink:\n"); + + if (!steal_stderr(path)) { + printf(" FAIL cannot redirect stderr\n"); + return 1; + } + + /* 1. Default: the built-in sink writes, and it carries devourer's prefix + * and the level letter. Nothing about the standalone tool changes. */ + mt_diag('I', "hello %d", 7); + { + long n = stderr_len(); + char buf[512]; + + check(n > 0, "default sink writes to stderr"); + rewind(stderr); + buf[0] = '\0'; + if (fgets(buf, sizeof buf, stderr) == NULL) + buf[0] = '\0'; + check(strstr(buf, "devourer [I] mt7612u: hello 7") != NULL, + "default sink keeps devourer's line format"); + check(strchr(buf, '\n') != NULL, "default sink terminates the line"); + } + + /* 2. With a sink installed, the line goes THERE and stderr stays silent. */ + if (!steal_stderr(path)) { + printf(" FAIL cannot re-redirect stderr\n"); + return 1; + } + mt7612u_set_log_sink(capture, &marker); + calls = 0; + mt_diag('W', "diverted %s", "line"); + check(calls == 1, "sink received exactly one call"); + check(last_level == 'W', "sink received the level letter"); + check(last_user == &marker, "sink received its user pointer"); + check(strcmp(last_line, "diverted line") == 0, + "sink receives the bare message, with no devourer prefix"); + check(stderr_len() == 0, "stderr stays silent while a sink is installed"); + + /* A prefix reaching the sink would double up once a host adds its own. */ + check(strstr(last_line, "devourer") == NULL, "no prefix leaks to the sink"); + check(strchr(last_line, '\n') == NULL, "no newline leaks to the sink"); + + /* 3. Every level letter reaches the sink unchanged - a sink that only saw + * errors would silently drop bring-up progress. */ + { + const char *levels = "IWE"; + size_t i; + + for (i = 0; i < strlen(levels); i++) { + calls = 0; + mt_diag(levels[i], "lvl"); + check(calls == 1 && last_level == levels[i], + "each level letter reaches the sink"); + } + } + + /* 4. NULL restores the built-in sink rather than silencing the library, + * so a host that tears its logger down does not lose diagnostics. */ + if (!steal_stderr(path)) { + printf(" FAIL cannot re-redirect stderr\n"); + return 1; + } + mt7612u_set_log_sink(NULL, NULL); + calls = 0; + mt_diag('E', "back to stderr"); + check(calls == 0, "the removed sink is not called"); + check(stderr_len() > 0, "NULL restores the built-in stderr sink"); + + /* 5. A long line must be truncated, not overrun. The formatted message + * buffer is 512, so ask for more than that and require a bounded, + * NUL-terminated result. */ + mt7612u_set_log_sink(capture, NULL); + { + char big[4096]; + + memset(big, 'x', sizeof big - 1); + big[sizeof big - 1] = '\0'; + calls = 0; + mt_diag('I', "%s", big); + check(calls == 1, "an oversized line still reaches the sink"); + check(strlen(last_line) < 512, "an oversized line is truncated, not overrun"); + } + mt7612u_set_log_sink(NULL, NULL); + + fclose(stderr); + remove(path); + + if (fails) { + printf("log_sink: %d failure(s)\n", fails); + return 1; + } + printf("log_sink: PASS\n"); + return 0; +} diff --git a/src/mt7612u/usb.cpp b/src/mt7612u/usb.cpp index 2153af14..8d6e7dbe 100644 --- a/src/mt7612u/usb.cpp +++ b/src/mt7612u/usb.cpp @@ -24,28 +24,56 @@ * a piped consumer mid-bring-up. Truncation is silent and deliberate - a * diagnostic is not worth a heap allocation on a path that may already be * failing. */ +/* Set before threads start, read from the RX event thread; see the contract on + * mt7612u_set_log_sink() in the public header. */ +static mt7612u_log_sink g_log_sink; +static void *g_log_user; + +void mt7612u_set_log_sink(mt7612u_log_sink sink, void *user) +{ + g_log_sink = sink; + g_log_user = user; +} + +/* The built-in sink, and the only place this library names stderr or devourer's + * line format. A host that installs its own sink gets the bare message and + * applies its own prefix, so nothing double-prefixes. */ +static void default_sink(void *user, char level, const char *line) +{ + char out[544]; + int n; + + (void)user; + n = snprintf(out, sizeof out, "devourer [%c] mt7612u: %s\n", level, line); + if (n < 0) + return; + if ((size_t)n > sizeof out - 1) + n = (int)(sizeof out - 1); + /* One line, one fwrite + fflush: per-line atomicity against the event + * thread, and no pipe-buffering stall for a subprocess supervisor. Same + * reasoning as devourer's Logger::emit and src/Event.h. */ + fwrite(out, 1, (size_t)n, stderr); + fflush(stderr); +} + void mt_diag(char level, const char *fmt, ...) { - char line[512]; + char msg[512]; int n; va_list ap; - n = snprintf(line, sizeof line, "devourer [%c] mt7612u: ", level); - if (n < 0 || (size_t)n >= sizeof line) - return; va_start(ap, fmt); - n += vsnprintf(line + n, sizeof line - (size_t)n - 1, fmt, ap); + n = vsnprintf(msg, sizeof msg, fmt, ap); va_end(ap); if (n < 0) return; - /* vsnprintf returns what it WOULD have written, so clamp before using - * it as a length - otherwise a truncated line writes past the buffer. */ - if ((size_t)n > sizeof line - 2) - n = (int)(sizeof line - 2); - line[n++] = '\n'; + /* vsnprintf returns what it WOULD have written; the buffer is already + * NUL-terminated at the truncation point, so nothing more is needed. */ - fwrite(line, 1, (size_t)n, stderr); - fflush(stderr); + if (g_log_sink) + g_log_sink(g_log_user, level, msg); + else + default_sink(NULL, level, msg); } /* Each operand is cast to the uint8_t that libusb's bmRequestType actually is: