[josephnef]

Part of #254. This re-measures the four M0 contracts (#261) on the RTL8852CE over PCIe/vfio, on the epic-254 rig: three J6412 boxes with two 8852CEs each, PTP-locked. M0 was originally measured on Jaguar USB and the 8821CE only, and every radio on the rig is Kestrel. The run found three Kestrel API gaps and one RX bug, all fixed here.

Fixes

• UpdateBeaconPayload was not implemented on Kestrel. It is now the BCN_UPD_V1 H2C with the new content; port and TBTT timing are not touched.
• StopBeacon was not implemented, so a beacon kept airing after its process exited. It now clears BCNTX_EN, matching vendor MAC_AX_PCFG_TX_SW / rtw_hal_beacon_stop.
• StartBeacon didn't strip a leading radiotap header, which the IRadio contract requires. A beacon with one (e.g. beacon_update_probe's) aired as a garbage frame and the witness heard nothing.
• RX PPDU-status pairing was off by one PPDU.
  • The RX loop attached each PPDU status to the WIFI frames that followed it.
  • The MAC delivers a PPDU's frames first and its status after: a trace shows the status with the same ppdu_cnt in the next RX buffer, ~140 µs later. So every frame carried the previous PPDU's RSSI/SNR/EVM.
  • Frames are now held until their status arrives, using the vendor phl_rx.c pairing (ppdu_cnt plus rate/bw/gi_ltf/ppdu_type).
  • A frame whose status never arrives is delivered without one (RSSI 0 = no sample): on the next PPDU, after 2 ms idle, or at loop exit. Nothing is dropped or reordered.
  • This fixes per-UE attribution, GetRxQuality and GetActiveRxPaths on Kestrel.

Tooling

• examples/common/radio_open.h: dl_departure_tx, txegress_witness, beacon_update_probe and ue_rx_probe now open USB or PCIe (DEVOURER_PCIE_BDF). They are also CMake targets now.
• dl_departure_tx takes DL_CSMA=1 to keep carrier sense on.

Measured (results in docs/scheduled-mac.md)

contract	8852CE PCIe result
DL send_packet→air guard	floor 1.3–2.0 µs RMS (8821CE: 11 µs); p99.9 0.46 ms on clean ch149, 2.7 ms on ch36 next to a foreign AP, whose beacons the late frames phase-lock to; max ~5 ms on both, cause not established
dynamic beacon update	30/30 aired, 0 skips, 0 torn, 0 regressions, update→air 13/16 ms p50/p99 (next beacon)
unicast ACK	responder armed: 1036/1036 aired once; unarmed: 9 copies (retry 8). Witness copy-count, since Kestrel has no CCX report
per-UE RX attribution	counts separate (3.90 vs 4); signal stats bled before the pairing fix, and after it, attenuating one UE moves only that UE (−55 → −71.5 dBm)

Re-checked after the RX change: beacon_phase_observer egress timing is unchanged (276/276 at 22 µs, 0.43 µs RMS), and ctest passes 79/79 on j6412-1.

Not covered: Kestrel USB (8852BU/CU, 8832xU) shares the RX loop but is not on this rig, so the pairing change is unverified there. The vendor pairing is bus-independent.

🤖 Generated with Claude Code

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

PR Summary by Qodo

Fix Kestrel beacon controls and RX pairing; enable PCIe M0 probes

🐞 Bug fix ✨ Enhancement 📝 Documentation 🕐 40+ Minutes

AI Description

• Implement Kestrel beacon updates and stops, and strip radiotap headers before transmission.
• Pair RX frames with their own PPDU status to restore accurate per-UE signal metrics.
• Enable USB/PCIe M0 probes and document RTL8852CE on-air contract measurements.

Diagram

graph TD
  Probes["M0 Probes"] --> Open["Radio Open"] --> Device["Kestrel Device"] --> Hal["Beacon HAL"] --> Chip["RTL8852CE"] --> Pair["RX Pairing"] --> Callback["Packet Consumers"]
  Pair --> Quality["Quality Metrics"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Buffer multiple PPDUs by descriptor identity

• ➕ Could pair statuses that arrive after frames from a newer PPDU.
• ➖ Adds buffering, expiry, and ordering complexity to a latency-sensitive RX path.

Recommendation: Keep the single-in-flight PPDU buffer for the observed frame-then-status ordering: it limits latency and delivers frames without invented measurements when status is missing. Consider multi-PPDU buffering only if testing reveals interleaved or later-arriving statuses, particularly on unverified Kestrel USB devices.

Files changed (11) +298 / -101

Enhancement (2) +82 / -0

radio_open.hShare USB and PCIe radio opening across probes +75/-0

Share USB and PCIe radio opening across probes

• Adds an environment-driven opener that selects vfio PCIe by BDF or USB by VID/PID and applies the same radio configuration to either transport.

examples/common/radio_open.h

HalKestrel.hDeclare Kestrel beacon update and stop controls +7/-0

Declare Kestrel beacon update and stop controls

• Exposes the HAL operations and documents that updates preserve port timing while stopping leaves the port and TSF running.

src/kestrel/HalKestrel.h

Bug fix (3) +131 / -33

HalKestrel.cppAdd Kestrel beacon update and stop operations +12/-0

Add Kestrel beacon update and stop operations

• Reuses the beacon firmware command to replace content without reprogramming timing, and clears the port's beacon-transmit enable bit to stop transmission.

src/kestrel/HalKestrel.cpp

RtlKestrelDevice.cppCorrect PPDU attribution and complete beacon APIs +115/-28

Correct PPDU attribution and complete beacon APIs

• Holds copied RX frames for matching PPDU status, delivering them without signal samples on mismatch, timeout, or exit; matched samples feed RX quality and path metrics. Also strips optional beacon radiotap headers and implements beacon update and stop through the HAL.

src/kestrel/RtlKestrelDevice.cpp

RtlKestrelDevice.hExpose beacon lifecycle methods and remove stale status cache +4/-5

Expose beacon lifecycle methods and remove stale status cache

• Declares the Kestrel IRadio update and stop overrides and removes the previous-status field that caused misattributed RX measurements.

src/kestrel/RtlKestrelDevice.h

Tests (4) +25 / -65

beacon_update_probe.cppRun the beacon-update probe over USB or PCIe +4/-14

Run the beacon-update probe over USB or PCIe

• Replaces probe-local USB initialization with the shared environment-driven radio opener.

tests/beacon_update_probe.cpp

dl_departure_tx.cppAdd PCIe opening and optional carrier sense to departure probe +13/-23

Add PCIe opening and optional carrier sense to departure probe

• Uses the shared USB/PCIe opener and adds DL_CSMA=1 to retain carrier sense for comparison runs. Reports the selected transport and carrier-sense setting.

tests/dl_departure_tx.cpp

txegress_witness.cppRun the TX-egress witness over USB or PCIe +4/-14

Run the TX-egress witness over USB or PCIe

• Replaces its USB-only setup with the shared opener while retaining the witness behavior.

tests/txegress_witness.cpp

ue_rx_probe.cppRun the per-UE RX probe over USB or PCIe +4/-14

Run the per-UE RX probe over USB or PCIe

• Switches radio initialization to the shared opener so attribution measurements can run on the PCIe rig.

tests/ue_rx_probe.cpp

Documentation (1) +48 / -3

scheduled-mac.mdRecord RTL8852CE PCIe contract measurements +48/-3

Record RTL8852CE PCIe contract measurements

• Documents departure timing, beacon-update reliability and latency, ACK copy counts, and per-UE RX attribution. Notes channel-dependent tails, measurement limits, and the PPDU pairing diagnosis.

docs/scheduled-mac.md

Other (1) +12 / -0

CMakeLists.txtBuild all four M0 contract probes +12/-0

Build all four M0 contract probes

• Adds CMake targets for the departure, witness, beacon-update, and UE-RX probes with their shared environment configuration.

CMakeLists.txt

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

Code Review by Qodo

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

1. Kestrel RX frames carry garbage metadata fields ✓ Resolved

Description

The WIFI branch now builds each packet as HeldRx h;, which default-initializes
Packet/rx_pkt_attrib. The old code used Packet p{}, which zeroed them. Every rx_pkt_attrib
field without a default member initializer that this branch never sets (pkt_rpt_type, physt,
seq_num, stbc, ldpc, sgi, qos, encrypt, ...) is therefore indeterminate on every
delivered Kestrel frame. Consumers such as examples/rx/main.cpp and examples/tx/main.cpp branch
on pkt_rpt_type == C2H_PACKET and log seq/stbc/ldpc/sgi, so frames can be misclassified as
C2H and dropped, or reported with random values, depending on stack contents.

---

2. Beacon header stripping has no regression test ✓ Resolved

Description

kestrel_beacon_radiotap_len is a new pure helper that decodes the radiotap version byte and
little-endian it_len and bounds-checks them, but the PR adds no headless selftest for it. It sits
in front of both StartBeacon and UpdateBeaconPayload, so a later bounds change (for example
accepting it_len == len) would air a garbage or empty beacon with nothing in CI to catch it.

---

3. RX frames can inherit another signal ✓ Resolved

Description

The WIFI branch flushes held frames only when ppdu_cnt changes, although the status branch uses
rate, bandwidth, GI/LTF and PPDU type to distinguish PPDUs too. If a new PPDU reuses the three-bit
count with different descriptor fields, its frames join the held batch and receive the earlier
PPDU's signal measurements.

---

4. USB frames can wait 100 ms for delivery ✓ Resolved

Description

The 2 ms held-frame timeout is checked only when the transport calls should_stop(), rather than on
a timer that wakes the RX loop. With no further USB completion or PPDU, the transport can spend up
to 100 ms in its event wait before delivering a frame whose status never arrived.

---

View review recommended (1)

5. Stopped beacons leave the port in AP mode ✓ Resolved

Description

HalKestrel::stop_beacon() clears BCNTX_EN but leaves the port's NET_TYPE set to AP. After a
successful StopBeacon(), the shared port therefore does not return to the No Link state required
for ordinary injection by the radio contract.

---

Tip of the day

💡 Did you know, you can commit Qodo's fix in one click with committable suggestions (GitHub & GitLab)

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[josephnef]

Addressed all five Qodo findings, plus the Windows build failure, in 2fe7abd:

1. Garbage metadata. HeldRx now holds Packet p{} and is created as HeldRx h{}, value-initialized like the old Packet p{}.
2. Radiotap helper test. beacon_radiotap_len moved to FrameParserKestrel.h. kestrel_rxparse now covers: a bare MPDU, LE it_len, it_len ≥ len, it_len < 8, version ≠ 0, a short buffer, and null.
3. Inherited signal. A WIFI frame joins the held batch only if same_ppdu() matches (count + rate/bw/gi_ltf/type), the same test the status pairing uses. same_ppdu also has selftest cases.
4. USB 100 ms. New ITransport::set_rx_max_wait_us. USB caps its event wait (and the sync read) with it; PCIe already polls every 200 µs. The Kestrel RX loop sets 1 ms and resets it on exit.
   • The review also surfaced a race: in USB spsc mode, on_data runs on a consumer thread while the predicate's flush runs on the event thread. The held state is now under a mutex.
5. StopBeacon AP mode. stop_beacon also returns NET_TYPE to NO_LINK, and the port stays enabled as the injection setup leaves it.

• Windows (cl). The M0 probes include unistd.h. They are POSIX bench tools, so they are now built only when NOT WIN32.

Re-verified on RTL8852CE:

• ctest 79/79 on all three rig boxes.
• With UE2 attenuated, UE1 reads −55.0 dBm and UE2 −71.2 dBm (no bleed).
• Beacon update 30/30, 0 skips; 0 beacons after StopBeacon.
• beacon_phase_observer egress: 275 of 276 beacons at 22 µs.

[josephnef]

Applying skip-qodo-gate. Qodo posted its review as an issue comment, not a PR review object, so the gate has no threads to check. In that comment all five findings are ✓ Resolved, with 0 new bugs, rule violations or requirement gaps. All 25 other checks pass on 917e090, including the macOS and Windows builds.