cleanup: follow-ups from the station series reviews (#451-#464), part of #465

docs/jaguar3-tx-ring.md

@@ -343,7 +343,16 @@ Available with this PR:
   The verdict is the send failures, not `txdma_status`: on the faulting runs
   the periodic `tx.stats` still read `txdma_status` 0 up to its last sample
   (it is taken once per 500 frames, so a latch just before the stop would
-  not show). One adapter, one channel. On the 8822B (8812BU) this txdemo
+  not show). Nor is a nonzero `txdma_status` by itself the wedge: an 8812CU
+  on USB2 at `DEVOURER_TX_GAP_US=0` with 1400-byte QoS data read `0x2000`
+  (bit 13, `BIT_PAYLOAD_OVF_8822C`) from the first sample, on the fixed and
+  the control build alike, while TX completed 8051/8051; at the default 2 ms
+  gap it read 0, and one later USB2 run at gap 0 did not reproduce the
+  latch. So bit 13 can latch at max duty on USB2; bit 18
+  (`BIT_TXPKTBUF_REQ_ERR`) is the bit measured with the wedge
+  (`IRtlRadio::GetTxDmaStatus`). One adapter, one channel; a second bench
+  reproduced the defect and the fix clearing it on an 8812CU, and gave the
+  same 8812BU LLT result as below. On the 8822B (8812BU) this txdemo
   form does NOT reproduce - unfixed and fixed alike ran clean (item 3); the
   Jaguar2 verification is the LLT check below. The 8822E form is unmeasured
   (the `ap_wpa2` stress, station-mode PR, is its record). Those runs used a radiotap-prefixed beacon; the demo now passes
@@ -355,16 +364,6 @@ Available with this PR:
   submitted / 0 failed with the beacon armed and then stopped, the
   aggregated path 0 failed, and A-MPDU over QoS data 0 failed.
 
-  **The maintainer's bench (josephnef), 2026-09-27:** the reproducer
-  reproduces and the fix clears it on an 8812CU, and the 8812BU LLT check
-  gives the same result as above. And a counterpart for `txdma_status`: an
-  8812CU on USB2 at `DEVOURER_TX_GAP_US=0` with 1400-byte QoS data read
-  `0x2000` (bit 13, `BIT_PAYLOAD_OVF_8822C`) from the first sample, on the
-  fixed and the control build alike, while TX completed 8051/8051; at the
-  default 2 ms gap it read 0. So a nonzero `txdma_status` is not by itself
-  the wedge - bit 18 (`BIT_TXPKTBUF_REQ_ERR`) is the bit measured with it
-  (`IRtlRadio::GetTxDmaStatus`).
-
   **The canonical-frame form does not reproduce**: without
   `DEVOURER_TX_QOS_DATA`/`DEVOURER_TX_PAYLOAD_BYTES`/`DEVOURER_TX_WITH_RX`,
   the unfixed build ran 20051 submitted / 0 failed / `txdma_status` 0, the

docs/logging.md

@@ -110,7 +110,7 @@ Emitters: L = library, RX/TX/... = demo. Optional fields in [brackets];
 ### TX plane
 | ev | emitter | fields |
 |---|---|---|
-| `tx.frame` | TX | n, rc — precoder demo variant: n, ok |
+| `tx.frame` | TX | n, rc, t (the `tx.report` timebase: the first one dates the first submit) — precoder demo variant: n, ok |
 | `tx.stats` | TX | submitted, failed, was_timeout, last_rc; the `final:1` event also carries t (the `tx.report` timebase, so a harness can tell how long before the end a transmitter last reported); periodic events (not the `final:1` one) also carry `txdma_status` (the raw `REG_TXDMA_STATUS` latch; which bits mean a stopped transmitter: `IRtlRadio::GetTxDmaStatus`) where `IRtlRadio::HasTxDmaStatus()` (which backends: its declaration), or `txdma_read_failed:1` when that sample's register read failed |
 | `tx.agg` | L (`DEVOURER_TX_USB_AGG`, send_packets) | frames, bytes, shim, ok — one per multi-frame bulk-OUT URB. The sync-TX generations (Jaguar2/Jaguar3/RTL8733B) also emit `sent` — bytes actually transferred, OR the negative libusb rc on a transport error (deliberately raw: this event is the only machine-readable carrier of the aggregated-path error code) — and set `ok` only on a FULL write, so `ok=false` splits as `sent < 0` transport error vs `0 <= sent < bytes` short write. Jaguar1 TX is async: its `ok` means URB accepted by the transport and there is no `sent` field (bytes resolve at completion reaping) |
 | `tx.report` | L (`DEVOURER_TX_REPORT`, CCX decode) | t, state (0=delivered, 1=retry-drop), ok, retries, final_rate, queue_time_raw, bmc, macid, fmt ("8812"\|"halmac"); halmac adds tag (SW_DEFINE echo), rts_retries, missed (fw-stuffed constant on Jaguar3 — tag gaps are the drop signal; `tests/txrpt_coverage_attrib.py`) — t is the achieved-report-rate timebase (the CCX emission ceiling is reports/s) |

docs/mt7612u-tx-retry.md

@@ -158,19 +158,32 @@ What it shows:
   fps, the same per-frame cost as the No-Ack rows. At limit 5 and on the
   initvals they time out on every per-frame wait (T40), and three of the
   eight such rows are 39/40 with the lag below.
-- **Characterised, unexplained: a one-step status lag.** In some arms EVERY
-  per-frame wait times out (T40), yet the entries do arrive - one step
-  behind: a frame's status becomes visible only after the next frame is
-  submitted. Such an arm ends 39/40 and its last entry lands in the next arm
-  as late (or, for the very first arm, as one foreign entry). Arm a lags in
-  every pass; which other arms lag varies from pass to pass (c, e, f and g
-  are each clean in some passes and lagging in others), so it tracks chip
-  state, not arm configuration. Counting 39/40 rows, it hit five of sixteen
-  at limit 0, eleven at limit 5 and seven on the initvals - one run each,
-  too few to call a trend. The two candidate explanations
-  (status posted only on the next TX; the EXT/FIFO pairing off by one)
-  produce identical signatures in this gate and are not distinguishable
-  here. The retry and success columns exclude every late and foreign entry.
+- **A one-step status lag - a stale EXT read on the arm's first entry.** In
+  some arms EVERY per-frame wait times out (T40) and the arm ends 39/40, with
+  one late entry (or, for the very first arm, one foreign entry) in that
+  SAME arm's row. Counting 39/40 rows, it hit five of sixteen at limit 0,
+  eleven at limit 5 and seven on the initvals - one run each. Arm a lags in
+  every pass; which other arms lag varies from pass to pass. The table rules
+  out "status posted only on the next TX": at limit 0, receiver ON, arm f
+  lags with L1 although arm e before it settled 40/40 and owed nothing, and
+  arm g after it shows no late entry. What fits is the two-transfer read:
+  when the FIFO is empty at the EXT read and an entry is filed before the
+  main read, the popped entry is paired with the stale EXT word of the
+  previous entry. On an arm's first entry that is the previous arm's pktid,
+  so the entry was counted late, the arm stayed one short, and every
+  per-frame wait timed out. A race on the poll timing, which is why it
+  varies from pass to pass and with the host. The gate now claims that
+  entry back (`txs_drain`), under all of: it is the arm's first popped entry
+  (no own entry yet), the arm has submitted a frame, and its pktid is that
+  of the last arm that sent a frame, which must have settled with no entry
+  owed - or, on the session's first arm, any pktid. A claimed entry counts
+  in entries, and in success when its SUCCESS bit is set; it stays out of
+  the retry columns, and is reported as "stale-EXT entries claimed". These
+  tables were taken before that change. With the claim keyed to the
+  previous arm, the author's unit then settled 16/16 arms at limits 5 and 0
+  (recorded on issue #461); keying it to the last arm that sent a frame,
+  so an arm with every submit failed is skipped, has not been run on
+  hardware.
 - **Arms e-h**: e, f and g read like c whenever they are clean; nothing
   distinguishes them. h (broadcast, WCID 1) lagged in five of six passes.
 
@@ -244,9 +257,11 @@ why it is worth an issue of its own.
   tables; the delivery table above shows ACK-requesting retries working
   against a real AP.
 - UNSETTLED rows are read for the retry value, never for the counts.
-- The one-step status lag is unexplained, and its two candidate causes are
-  indistinguishable in this gate. It moves an arm's last entry into the next
-  arm's late count, never into another arm's statistics.
+- The one-step status lag is attributed to the stale-EXT race above from
+  this table's pattern, not from a bus trace. The claim recovers at most one
+  entry per arm - the first - and only on the conditions above; after an
+  UNSETTLED arm a previous-pktid entry is still reported as late. A deficit
+  of more than one entry is not this race.
 - `mt7612uprobe txs` reads two registers per status poll, so its `fps` is
   per-frame submit-to-status time, not comparable with any steady-state
   injection figure.

examples/tx/main.cpp

@@ -2,6 +2,7 @@
 #include <cassert>
 #include <chrono>
 #include <climits>
+#include <cstdio>
 #include <cstdlib>
 #include <cstring>
 #include <iomanip>
@@ -783,6 +784,24 @@ int main(int argc, char **argv) {
     });
     logger->info("DEVOURER_POLL_INTR_IN — EP 0x85 interrupt-IN poller running");
   }
+  /* Stops and joins the optional IN drainers on every exit from here: a
+   * still-joinable std::thread terminates the process when destroyed, and
+   * both threads poll `handle`, so the normal path joins them explicitly
+   * before session.close(). (The DEVOURER_TX_WITH_RX fork child never runs
+   * this: it leaves through std::_Exit - see there.) */
+  struct DrainerJoin {
+    std::atomic<bool> &bulk_running, &intr_running;
+    std::thread &bulk, &intr;
+    void join() {
+      bulk_running = false;
+      intr_running = false;
+      if (bulk.joinable())
+        bulk.join();
+      if (intr.joinable())
+        intr.join();
+    }
+    ~DrainerJoin() { join(); }
+  } drainers{bulk_in_running, intr_running, bulk_in_thread, intr_in_thread};
 
   WiFiDriver wifi_driver{logger};
   std::unique_ptr<IRadio> owned_device;
@@ -1007,13 +1026,36 @@ int main(int argc, char **argv) {
   if (tx_with_rx && !rx_thread_mode) {
     pid_t fpid = fork();
     if (fpid == 0) {
+#if !defined(_MSC_VER) /* fork() is a real fork here, not the (0) stub */
+      /* The post-fork rule: the child holds copies of the parent's objects -
+       * the IN-drainer std::threads among them, joinable copies of threads
+       * that do not exist here - so it must run no destructor. It flushes
+       * stdio and leaves through std::_Exit, never through a return; an
+       * exception from Init must not unwind it either. */
+      try {
+        rtlDevice->Init(packetProcessor,
+                        SelectedChannel{
+                            .Channel = static_cast<uint8_t>(channel),
+                            .ChannelOffset = 0,
+                            .ChannelWidth = CHANNEL_WIDTH_20,
+                        });
+      } catch (const std::exception &e) {
+        logger->error("RX child: {}", e.what());
+      } catch (...) {
+        logger->error("RX child: unknown exception");
+      }
+      std::fflush(nullptr);
+      std::_Exit(1);
+#else
+      /* The stub: this IS the only process, so it tears down normally. */
       rtlDevice->Init(packetProcessor,
                       SelectedChannel{
                           .Channel = static_cast<uint8_t>(channel),
                           .ChannelOffset = 0,
                           .ChannelWidth = CHANNEL_WIDTH_20,
                       });
       return 1;
+#endif
     }
   }
 
@@ -1031,16 +1073,9 @@ int main(int argc, char **argv) {
   } catch (const std::exception &e) {
     /* InitWrite returns void, so a refused bring-up (e.g. a channel/width/
      * offset combination the chip rejects) surfaces as an exception. The
-     * device already tore itself down; exit cleanly instead of aborting —
-     * which means the optional IN-drainer threads above must be joined
-     * first, or their still-joinable std::thread destructors terminate. */
+     * device already tore itself down; exit cleanly instead of aborting
+     * (the drainers guard joins the IN-drainer threads). */
     logger->error("TX bring-up failed: {}", e.what());
-    bulk_in_running = false;
-    intr_running = false;
-    if (bulk_in_thread.joinable())
-      bulk_in_thread.join();
-    if (intr_in_thread.joinable())
-      intr_in_thread.join();
     return 1;
   }
 
@@ -1072,6 +1107,30 @@ int main(int argc, char **argv) {
     });
     logger->info("DEVOURER_TX_WITH_RX=thread: RX loop started alongside TX");
   }
+  /* Stops and joins the RX and USB event threads on every exit from here,
+   * the early returns below included - a joinable std::thread destructor
+   * terminates the process. The normal teardown's order: StopRxLoop and the
+   * RX join, then the event pump (which polls g_devourer_should_stop). It is
+   * declared after the session, so on an early return it runs while the
+   * device and libusb are still alive. Both threads start after the
+   * DEVOURER_TX_WITH_RX fork, so a fork child never reaches here. */
+  struct IoThreadsJoin {
+    IRadio *dev;
+    std::thread &rx, &usb;
+    bool done = false;
+    void join() {
+      if (done)
+        return;
+      done = true;
+      dev->StopRxLoop();
+      if (rx.joinable())
+        rx.join();
+      g_devourer_should_stop = true;
+      if (usb.joinable())
+        usb.join();
+    }
+    ~IoThreadsJoin() { join(); }
+  } io_threads{rtlDevice, rx_thread, usb_thread};
 
   /* DEVOURER_STA_IDENTITY: arm before the first frame, once the RX loop is
    * shown running - its first received frame (3 s cap: a silent channel still
@@ -1946,9 +2005,10 @@ int main(int argc, char **argv) {
    * explicitly, before Stop() powers the chip down; the destructor covers an
    * exception or an early return. It is declared after the DeviceSession, so
    * it runs before the device is destroyed. `attempted` is cleared only by a
-   * StopBeacon that returned (true, or a clean false = nothing active, per
-   * its contract); after three throws it stays set, so a later stop() - the
-   * destructor on an exception path - tries again. detach() drops the device
+   * StopBeacon that returned true, or a clean false when nothing was armed;
+   * after three failed attempts (throws, or a refused disable of an armed
+   * beacon) it stays set, so a later stop() - the destructor on an exception
+   * path - tries again. detach() drops the device
    * before the normal path destroys it. */
   struct TxBeaconGuard {
     IRadio *dev;
@@ -1960,17 +2020,18 @@ int main(int argc, char **argv) {
         return;
       for (int i = 0; i < 3; i++) {
         try {
-          /* true = stopped; a clean false = nothing active (StopBeacon
-           * contract), expected after a refused StartBeacon - either way
-           * there is nothing to retry. */
-          dev->StopBeacon();
+          /* After a successful arm, false means the disable was refused
+           * (Jaguar2/3), so retry. Otherwise a clean false is nothing
+           * active - expected after a refused StartBeacon. */
+          if (!dev->StopBeacon() && armed)
+            continue;
           attempted = false;
           return;
         } catch (const std::exception &e) {
           log->warn("DEVOURER_TX_BEACON_TU: StopBeacon threw: {}", e.what());
         }
       }
-      /* Three throws: `attempted` stays set so a later stop() tries again. */
+      /* Three failures: `attempted` stays set so a later stop() tries again. */
       log->error("DEVOURER_TX_BEACON_TU: StopBeacon failed 3 times - the "
                  "beacon may keep airing until the adapter is re-enumerated "
                  "or powered down (Jaguar2 has no teardown power-down)");
@@ -2481,7 +2542,8 @@ int main(int argc, char **argv) {
         ++frames_in_dwell >= hop_dwell)
       frames_in_dwell = 0;
     if (tx_count <= 10 || tx_count % 500 == 0) {
-      devourer::Ev(*g_ev, "tx.frame").f("n", tx_count).f("rc", rc);
+      /* t: the tx.report timebase, so a harness can date the first submit. */
+      devourer::Ev(*g_ev, "tx.frame").f("n", tx_count).f("rc", rc).t();
       /* TX submission health — the driver-drop / congestion feed (xtx). A
        * climbing failed with was_timeout=1 is a full TX FIFO (recoverable
        * back-pressure); a hard rc is a broken path. */
@@ -2657,11 +2719,7 @@ int main(int argc, char **argv) {
   sta_stop = true;
   if (sta_clear_thread.joinable())
     sta_clear_thread.join();
-  rtlDevice->StopRxLoop();
-  if (rx_thread.joinable())
-    rx_thread.join();
-  if (usb_thread.joinable())
-    usb_thread.join();
+  io_threads.join();
 
   /* Clean chip de-init before releasing the interface: card-disable PWR_SEQ on
    * the HalMAC families, TX quiesce on Jaguar1 — so the adapter re-enumerates
@@ -2673,6 +2731,7 @@ int main(int argc, char **argv) {
    * does exactly the same on every other exit path. */
   /* The beacon guard must not call into the device once it is gone. */
   tx_beacon.detach();
+  drainers.join();   /* they poll the handle session.close() releases */
   session.close();
   /* A truncated caller stream is a producer fault, and a harness that scored
    * the run as if it had ended cleanly would be scoring a short measurement. */

src/IRtlRadio.h

@@ -167,10 +167,11 @@ class IRtlRadio : public IRadio {
    *     armed it latched and the part transmitted nothing more for the life
    *     of the process, while the receiver worked on (the 8812BU's wedge
    *     read 0x10, then 0x15 - bits not decoded);
-   *   - bit 13, BIT_PAYLOAD_OVF_8822C (0x00002000), latches under host-side
-   *     max-duty backpressure while TX continues - an 8812CU on USB2 at
-   *     DEVOURER_TX_GAP_US=0 read it from the first sample and still
-   *     completed every frame. A poller must not treat it as the wedge.
+   *   - bit 13, BIT_PAYLOAD_OVF_8822C (0x00002000), can latch under
+   *     host-side max-duty backpressure while TX continues - an 8812CU on
+   *     USB2 at DEVOURER_TX_GAP_US=0 read it from the first sample and still
+   *     completed every frame (a later such run did not latch it). A poller
+   *     must not treat it as the wedge.
    * Other bits are undecoded. Records: docs/jaguar3-tx-ring.md.
    *
    * NOT FOR THE SEND PATH. This is a register read over USB - see the

src/StationArm.h

@@ -57,7 +57,9 @@
  * (Jaguar2's does not; Jaguar1's is optional) and a port left on MACID = own
  * / Infra goes on acknowledging for a station whose process has gone. A
  * (re-)bring-up clears a held arm first (retire()); a clear that does not
- * verify keeps the record for ClearStationIdentity to retry. */
+ * verify keeps the record for ClearStationIdentity to retry. Stop() also
+ * clears _station_ready, so an arm after Stop() is refused until the next
+ * bring-up. */
 
 #include <algorithm>
 #include <cstdint>

src/jaguar1/RtlJaguarDevice.cpp

@@ -2487,9 +2487,10 @@ void RtlJaguarDevice::Stop() {
   /* A station arm ends with the session: restored before the optional
    * power-down (best effort; a failure is logged by the clear), so a chip
    * left powered (tuning.teardown_power_down=0) does not keep answering for
-   * the station. */
+   * the station. A new arm is refused until the next bring-up. */
   {
     std::lock_guard<std::recursive_mutex> lock(_port0_mu);
+    _station_ready = false;
     if (_station.armed())
       (void)_station.clear(_device, _logger, "Jaguar1");
   }

src/jaguar2/RtlJaguar2Device.cpp

@@ -2326,9 +2326,11 @@ void RtlJaguar2Device::Stop() {
   /* This Stop leaves the chip powered, so a station arm would outlive the
    * session: port 0 on MACID = own / Infra keeps acknowledging for the
    * station after the process has gone. Clear it here (best effort; a
-   * failure is logged by the clear). */
+   * failure is logged by the clear). A new arm is refused until the next
+   * bring-up. */
   {
     std::lock_guard<std::mutex> lk(_reg_mu);
+    _station_ready = false;
     if (_station.armed())
       (void)_station.clear(_device, _logger, "Jaguar2");
   }

src/jaguar3/RtlJaguar3Device.cpp

@@ -858,9 +858,11 @@ void RtlJaguar3Device::Stop() {
   /* A station arm ends with the session, not with whatever the de-init
    * below leaves: restored first (best effort; a failure is logged by the
    * clear), so the port stops answering for the station even where the
-   * power-down does not complete. */
+   * power-down does not complete. A new arm is refused until the next
+   * bring-up. */
   {
     std::lock_guard<std::mutex> lk(_reg_mu);
+    _station_ready = false;
     if (_station.armed())
       (void)_station.clear(_device, _logger, "Jaguar3");
   }