From 4f94f546a5e87df65cb8bbe62be2d901228711f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Agust=C3=ADn=20Gonz=C3=A1lez?= Date: Wed, 14 Oct 2026 08:29:11 -0300 Subject: [PATCH 1/3] kestrel: ACK responder on its own macid / ADDR_CAM entry Follow-up to #487 (review point 5). The responder registered its station at macid 0 / ADDR_CAM entry 0, the context InitWrite programs for injection and that every injected frame's WD points at; arming rewrote it and clearing re-ran the bring-up registration over it. The station now lives at macid 1 / ADDR_CAM entry 1 (BSSID CAM 1), and the injection context is never touched: entry 0 reads back byte-identical before the arm, while armed and after the clear. Clearing changed with it. The 8852B firmware leaves a disconnected station's entry VALID: join_info(disconnect) + a NO_LINK addr_cam, a role REMOVE, and role CREATE + NO_LINK all left entry 1 valid with the responder's address (the old layout hid this, because the bring-up re-registration rewrote entry 0 wholesale). Clear now disconnects the station in the firmware and clears VALID from the host, the way the vendor's no-firmware mac_upd_addr_cam path writes the CAM, repeating the clear until the entry has read back empty for 25 ms so a late firmware rewrite cannot re-validate it. Measured, TX20U Nano (8852B) responder, TX50UH injector counting ACKs, ch161, MCS1, one attempt per frame, 3 runs per row (AdapterCaps.h): 14400/14415 armed at Init, 14369/14427 armed at InitWrite, 14363/14421 retargeted, 0/6485 at the old address, 0/6449 never armed, 0/6519 after a live disarm; arm -> retarget -> clear -> re-arm in one session, 3 runs, every phase as expected. Injection delivery is unchanged armed vs unarmed (>= 2900/3000 at a witness, on this layout and the old one), so this is about the injection context, not a delivery fix. Adversarial: one armed cell in about 55 across development read entry 1 back committed and answered no ACKs; not reproduced in the following cells (base layout: 0 such cells in about 40). The responder-transmitting row read 80.6% once, with the injector's own USB sends stalling (up to 1 s, 7 failures); 98.7% and 99.2% in the other two runs. The 8852C path is unchanged: its arm still refuses. --- docs/kestrel-ack-responder.md | 16 +++++++- src/AdapterCaps.h | 12 +++--- src/kestrel/HalKestrel.h | 34 +++++++++++++++- src/kestrel/RtlKestrelDevice.cpp | 67 +++++++++++++++++++++++--------- src/kestrel/RtlKestrelDevice.h | 24 ++++++------ 5 files changed, 113 insertions(+), 40 deletions(-) diff --git a/docs/kestrel-ack-responder.md b/docs/kestrel-ack-responder.md index c383d893..1bc07b2a 100644 --- a/docs/kestrel-ack-responder.md +++ b/docs/kestrel-ack-responder.md @@ -26,7 +26,21 @@ was measured to get there. On ch149, shared with a neighbour AP, one of three runs without the clear dropped to 82.4% single-shot; three with it stayed at >= 99.6%. On a quiet ch161 it made no difference. -- **Clear** is exercised live by rxdemo's `DEVOURER_ACK_DISARM_AFTER_MS`. +- **Own macid.** The station sits at macid 1 / ADDR_CAM entry 1, apart from + the injection context at macid 0 / entry 0: the firmware commits entry 1 in + the same ~2.2 ms, and entry 0 reads back byte-identical before the arm, + after it and after the clear. Sharing entry 0 (the previous layout) had no + measurable cost either: a witness counted the same >= 2900 of 3000 injected + frames armed and unarmed, on both layouts. The separation is for the + injection context's integrity, not a delivery fix. +- **Unexplained cell.** In about 55 armed cells across development, one read + entry 1 back committed (after 10 ms rather than ~2.2 ms) and answered no + ACKs; other slow commits were fine and it has not reproduced. +- **Clear needs the host.** After `join_info(disconnect)` and a NO_LINK + addr_cam, entry 1 stays VALID with the responder's address; a role REMOVE, + or role CREATE plus NO_LINK, leaves it valid too. Clearing VALID from the + host takes, and re-read for 2 s it stays clear. rxdemo's + `DEVOURER_ACK_DISARM_AFTER_MS` exercises it live. - **8852C:** its firmware accepts both commands and leaves the entry invalid, so the arm refuses. The vendor's 8852C association sends far more before the addr_cam (BB/RF offload, port configuration); which of it the C needs is open. diff --git a/src/AdapterCaps.h b/src/AdapterCaps.h index 42e6dc5f..5dd84f63 100644 --- a/src/AdapterCaps.h +++ b/src/AdapterCaps.h @@ -194,11 +194,13 @@ struct AdapterCaps { * tests/ack_txreport_matrix.sh run with the 8733B as the responder), * 8852B (Kestrel has no CCX report, so the soliciting TX is an 8832CU * counting the ACK frames it receives, one attempt per frame, MCS1 on - * ch161, 3 runs per row: 14230/14430 armed at Init, 14391/14431 armed at - * InitWrite, 14347/14425 retargeted, 0/6440 at the old address after the - * retarget, 0/6194 never armed, 0/6603 after a live disarm). The 8852C - * reads false: its firmware does not commit the station entry, and the - * arm refuses on the read-back. + * ch161, 3 runs per row, responder on its own macid 1 / ADDR_CAM entry 1: + * 14400/14415 armed at Init, 14369/14427 armed at InitWrite, 14363/14421 + * retargeted, 0/6485 at the old address after the retarget, 0/6449 never + * armed, 0/6519 after a live disarm; one cell in about 55 across the + * development runs read the entry back committed and answered no ACKs, + * not reproduced since). The 8852C reads false: its firmware does not + * commit the station entry, and the arm refuses on the read-back. * * The `on`, `retarget`, and legacy `off` rows establish arming, retargeting, * and a never-armed control. A backend-owned `disarmed` row supports only diff --git a/src/kestrel/HalKestrel.h b/src/kestrel/HalKestrel.h index 4fea87e5..d02cd0f5 100644 --- a/src/kestrel/HalKestrel.h +++ b/src/kestrel/HalKestrel.h @@ -279,17 +279,47 @@ class HalKestrel { * with addr_cam_holds(). The 8852C firmware accepts both commands and does * not commit. */ bool register_infra_sta(const uint8_t own[6], const uint8_t bssid[6], - uint8_t macid = 0, uint8_t band = 0, uint8_t port = 0) { + uint8_t macid, uint8_t addr_cam_idx, + uint8_t bssid_cam_idx, uint8_t band = 0, + uint8_t port = 0) { bool ok = _fw.fw_join_info(macid, reg::MAC_AX_NET_TYPE_INFRA, reg::MAC_AX_SELF_ROLE_CLIENT, reg::MAC_AX_WIFI_ROLE_STATION, port, band, /*connect=*/true); ok = _fw.fw_upd_addr_cam_sta(macid, own, bssid, reg::MAC_AX_NET_TYPE_INFRA, - /*addr_cam_idx=*/0, /*bssid_cam_idx=*/1) && + addr_cam_idx, bssid_cam_idx) && ok; return ok; } + /* Undo register_infra_sta in the firmware: join_info(disconnect), then a + * NO_LINK addr_cam for `macid`. On the 8852B this does NOT invalidate the + * physical entry (it stays VALID with the station's SMA), so callers also + * clear_addr_cam_entry() and verify with addr_cam_holds(). */ + bool unregister_infra_sta(const uint8_t own[6], uint8_t macid, + uint8_t addr_cam_idx, uint8_t bssid_cam_idx, + uint8_t band = 0, uint8_t port = 0) { + bool ok = _fw.fw_join_info(macid, reg::MAC_AX_NET_TYPE_INFRA, + reg::MAC_AX_SELF_ROLE_CLIENT, + reg::MAC_AX_WIFI_ROLE_STATION, port, band, + /*connect=*/false); + ok = _fw.fw_upd_addr_cam(macid, own, reg::MAC_AX_NET_TYPE_NO_LINK, + addr_cam_idx, bssid_cam_idx) && + ok; + return ok; + } + + /* Write one dword of the MAC's internal table SRAM (the write half of + * read_mac_mem32). */ + void write_mac_mem32(uint32_t internal_addr, uint32_t value) { + _device.rtw_write32(reg::R_AX_FILTER_MODEL_ADDR, internal_addr); + _device.rtw_write32_wide(reg::R_AX_INDIR_ACCESS_ENTRY, value); + } + /* Clear VALID on address-CAM entry `idx`. */ + void clear_addr_cam_entry(uint8_t idx) { + write_mac_mem32(reg::ADDR_CAM_MEM_BASE + idx * reg::ADDR_CAM_ENT_LONG_SIZE, 0u); + } + /* One dword of the MAC's internal table SRAM (rtw89 mac_mem_read): the * internal address goes to R_AX_FILTER_MODEL_ADDR, the data comes back * through the R_AX_INDIR_ACCESS_ENTRY window (0x40000, a 32-bit-address diff --git a/src/kestrel/RtlKestrelDevice.cpp b/src/kestrel/RtlKestrelDevice.cpp index 62e6cf49..f8db1d02 100644 --- a/src/kestrel/RtlKestrelDevice.cpp +++ b/src/kestrel/RtlKestrelDevice.cpp @@ -536,6 +536,13 @@ namespace { * on it (A1 alone selects the response), it only has to be a unicast address * distinct from the responder's own. */ constexpr uint8_t kAckResponderBssid[6] = {0x02, 0x00, 0x00, 0x00, 0x00, 0x01}; +/* The responder's own station: a macid and ADDR_CAM entry apart from the + * injection context InitWrite programs at macid 0 / entry 0 (which the WD of + * every injected frame keeps pointing at), and a BSSID CAM entry apart from + * that context's index 0. */ +constexpr uint8_t kAckMacid = 1; +constexpr uint8_t kAckAddrCamIdx = 1; +constexpr uint8_t kAckBssidCamIdx = 1; std::string mac_text(const uint8_t *m) { char b[18]; @@ -569,7 +576,8 @@ bool RtlKestrelDevice::SetAckResponder(const devourer::MacAddr &mac) { const uint32_t resp0 = _device.rtw_read32(r::R_AX_TRXPTCL_RESP_0); if (!_ack_responder) _ack_resp0_saved = resp0; - const bool sent = _hal.register_infra_sta(m, bssid); + const bool sent = _hal.register_infra_sta(m, bssid, kAckMacid, kAckAddrCamIdx, + kAckBssidCamIdx); _device.rtw_write32(r::R_AX_TRXPTCL_RESP_0, resp0 & ~r::B_AX_RSP_CHK_ALL); /* The H2Cs going out says nothing about the firmware committing them: read * the physical CAM entry the RMAC matches against. The firmware applies the @@ -577,7 +585,7 @@ bool RtlKestrelDevice::SetAckResponder(const devourer::MacAddr &mac) { const auto t0 = std::chrono::steady_clock::now(); bool committed = false; int polls = 0; - while (sent && !(committed = _hal.addr_cam_holds(/*idx=*/0, m)) && + while (sent && !(committed = _hal.addr_cam_holds(kAckAddrCamIdx, m)) && std::chrono::steady_clock::now() - t0 < std::chrono::milliseconds(100)) { ++polls; std::this_thread::sleep_for(std::chrono::milliseconds(1)); @@ -604,9 +612,9 @@ bool RtlKestrelDevice::SetAckResponder(const devourer::MacAddr &mac) { return false; } _ack_responder = mac; - _logger->info("Kestrel: hardware ACK responder armed for {} (ADDR_CAM " - "entry 0 committed, read back after {} us)", mac_text(m), - waited_us); + _logger->info("Kestrel: hardware ACK responder armed for {} (macid {}, " + "ADDR_CAM entry {} committed, read back after {} us)", + mac_text(m), kAckMacid, kAckAddrCamIdx, waited_us); return true; } @@ -623,31 +631,52 @@ void RtlKestrelDevice::ClearAckResponder() { std::optional RtlKestrelDevice::disarm_ack_locked(std::span held) { namespace r = kestrel::reg; - /* Back to what InitWrite registers for macid 0: the NO_LINK self entry, - * which the firmware does not commit -- so the RMAC stops answering. */ - _hal.register_sta_role(0, 0, 0); - _hal.add_self_sta(kInjectSA, /*macid=*/0); + /* Disconnect the responder's station in the firmware; the injection + * context at macid 0 is never touched. */ + _hal.unregister_infra_sta(held.front().bytes.data(), kAckMacid, + kAckAddrCamIdx, kAckBssidCamIdx); _device.rtw_write32(r::R_AX_TRXPTCL_RESP_0, _ack_resp0_saved); - /* The rewrite is as asynchronous as the arm: give it the same bound. */ + /* The 8852B firmware keeps a disconnected station's entry VALID (measured: + * neither join_info(disconnect) + a NO_LINK addr_cam, nor a role REMOVE, + * nor role CREATE + NO_LINK clears it), so the host clears VALID itself, as + * the vendor's no-firmware mac_upd_addr_cam path writes the CAM. The H2Cs + * above land asynchronously (2-20 ms measured for the arm) and rewrite the + * entry, so the clear repeats until the entry has read back empty for 25 ms + * straight, within a 150 ms bound. */ const auto still_held = [&]() -> const devourer::MacAddr * { for (const auto &a : held) - if (_hal.addr_cam_holds(/*idx=*/0, a.bytes.data())) + if (_hal.addr_cam_holds(kAckAddrCamIdx, a.bytes.data())) return &a; return nullptr; }; - const auto t0 = std::chrono::steady_clock::now(); - const devourer::MacAddr *left; - while ((left = still_held()) && - std::chrono::steady_clock::now() - t0 < std::chrono::milliseconds(100)) + using clock = std::chrono::steady_clock; + const auto t0 = clock::now(); + auto clean_since = t0; + const devourer::MacAddr *left = nullptr; + bool first = true; + for (;;) { + if (first || left) { + _hal.clear_addr_cam_entry(kAckAddrCamIdx); + clean_since = clock::now(); + first = false; + } + left = still_held(); + if (!left && clock::now() - clean_since >= std::chrono::milliseconds(25)) + break; + if (clock::now() - t0 >= std::chrono::milliseconds(150)) + break; std::this_thread::sleep_for(std::chrono::milliseconds(1)); + } if (left) { _logger->warn("Kestrel: ACK responder clear did not take: ADDR_CAM entry " - "0 still holds {}, the MAC may keep answering for it " - "(ClearAckResponder retries)", mac_text(left->bytes.data())); + "{} still holds {}, the MAC may keep answering for it " + "(ClearAckResponder retries)", kAckAddrCamIdx, + mac_text(left->bytes.data())); return *left; } - _logger->info("Kestrel: hardware ACK responder disarmed (ADDR_CAM entry 0 " - "no longer holds {})", mac_text(held.front().bytes.data())); + _logger->info("Kestrel: hardware ACK responder disarmed (ADDR_CAM entry {} " + "no longer holds {})", kAckAddrCamIdx, + mac_text(held.front().bytes.data())); return std::nullopt; } diff --git a/src/kestrel/RtlKestrelDevice.h b/src/kestrel/RtlKestrelDevice.h index 76982311..6aadb95c 100644 --- a/src/kestrel/RtlKestrelDevice.h +++ b/src/kestrel/RtlKestrelDevice.h @@ -91,19 +91,17 @@ class RtlKestrelDevice : public IRtlRadio { * once the firmware has COMMITTED a valid ADDR_CAM entry for it, and it does * that only for a station registered the way a real association registers * one (HalKestrel::register_infra_sta). The arm programs that station with - * `mac` as its own address, clears the response-abort CCA gates, and is - * verified by reading the physical CAM entry back -- false when the entry - * did not commit, which is the 8852C today. Post-bring-up only. Clear puts - * macid 0 back to the bring-up NO_LINK entry and verifies the CAM no - * longer holds `mac`; an address still held after a failed clear (or a - * failed retarget, which checks the old address too) stays recorded, so a - * later ClearAckResponder retries it. Arm and clear are serialized. - * - * The responder shares macid 0 / ADDR_CAM entry 0 with injection: while - * armed, injected frames (whose WD carries macid 0) go out under the - * station context the arm registered, and clear restores the bring-up - * entry. A second macid for the responder is untested: whether the - * firmware commits an entry other than 0 has not been measured. */ + * `mac` as its own address, on its own macid 1 / ADDR_CAM entry 1 so the + * injection context at macid 0 / entry 0 (what every injected frame's WD + * points at) is left as bring-up programmed it; clears the response-abort + * CCA gates; and is verified by reading the physical CAM entry back -- + * false when the entry did not commit, which is the 8852C today. + * Post-bring-up only. Clear disconnects that station and clears the + * entry's VALID bit from the host (the 8852B firmware leaves a + * disconnected station's entry valid), and verifies the CAM no longer + * holds `mac`; an address still held after a failed clear (or a failed + * retarget, which checks the old address too) stays recorded, so a later + * ClearAckResponder retries it. Arm and clear are serialized. */ bool SetAckResponder(const devourer::MacAddr &mac) override; void ClearAckResponder() override; /* rxdemo's live-disarm measurement hook (DEVOURER_ACK_DISARM_AFTER_MS): the From 60d79c74a3378f41c8484d97caa9b2eef9f72b81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Agust=C3=ADn=20Gonz=C3=A1lez?= Date: Wed, 14 Oct 2026 10:20:27 -0300 Subject: [PATCH 2/3] kestrel: ACK responder slot apart from peers; clear only when stable Review follow-ups on the own-macid responder: - Peer stations take macid 1 / ADDR_CAM entry 1 by convention (tests/ul_trigger_ap.cpp, kestrelprobe trig), the slot the responder had just moved to, so a peer registration and the responder could replace each other and a clear could invalidate the peer's entry. The responder now uses macid 127 / ADDR_CAM entry 127, the last entry of the 0x7f search range, and RegisterPeerSta / RegisterBeamformee refuse that slot (logged). Entry 127 commits in the same ~2.2 ms. - The clear's 150 ms deadline could end the loop on an empty read that came right after re-clearing a late firmware rewrite, and an empty read was taken as success. Success now requires the entry to have read back empty for the full 25 ms; on a timeout the clear is reported as not taken and the last address the entry held stays recorded for a retry. Re-measured on this version, same bench and rows as before (TX20U Nano responder, TX50UH injector, ch161, MCS1, one attempt per frame, 3 runs): 14414/14422 armed at Init, 14376/14417 armed at InitWrite, 14419/14432 retargeted, 0/6597 at the old address, 0/6575 never armed, 0/6561 after a live disarm, responder transmitting 14308/14437; arm -> retarget -> clear -> re-arm 3/3 runs as expected; injection delivery >= 2900/3000 armed and unarmed. One unarmed delivery cell heard none of the 3179 injected frames at the witness (txdemo sent them all, code path untouched by this change); three re-runs read >= 2900/3000. Full MSVC build and ctest pass (78, 4 skipped as on master). --- docs/kestrel-ack-responder.md | 34 +++++++++++++---------- src/AdapterCaps.h | 15 +++++----- src/kestrel/RtlKestrelDevice.cpp | 47 ++++++++++++++++++++++++++------ src/kestrel/RtlKestrelDevice.h | 28 +++++++++++-------- 4 files changed, 83 insertions(+), 41 deletions(-) diff --git a/docs/kestrel-ack-responder.md b/docs/kestrel-ack-responder.md index 1bc07b2a..086dd560 100644 --- a/docs/kestrel-ack-responder.md +++ b/docs/kestrel-ack-responder.md @@ -26,20 +26,26 @@ was measured to get there. On ch149, shared with a neighbour AP, one of three runs without the clear dropped to 82.4% single-shot; three with it stayed at >= 99.6%. On a quiet ch161 it made no difference. -- **Own macid.** The station sits at macid 1 / ADDR_CAM entry 1, apart from - the injection context at macid 0 / entry 0: the firmware commits entry 1 in - the same ~2.2 ms, and entry 0 reads back byte-identical before the arm, - after it and after the clear. Sharing entry 0 (the previous layout) had no - measurable cost either: a witness counted the same >= 2900 of 3000 injected - frames armed and unarmed, on both layouts. The separation is for the - injection context's integrity, not a delivery fix. -- **Unexplained cell.** In about 55 armed cells across development, one read - entry 1 back committed (after 10 ms rather than ~2.2 ms) and answered no - ACKs; other slow commits were fine and it has not reproduced. -- **Clear needs the host.** After `join_info(disconnect)` and a NO_LINK - addr_cam, entry 1 stays VALID with the responder's address; a role REMOVE, - or role CREATE plus NO_LINK, leaves it valid too. Clearing VALID from the - host takes, and re-read for 2 s it stays clear. rxdemo's +- **Own macid.** The station sits at macid 127 / ADDR_CAM entry 127 (the last + entry of the 0x7f search range), apart from the injection context at macid 0 + / entry 0 and from the low slots peer registrations take (`RegisterPeerSta` + callers use 1, 2, ...; it and `RegisterBeamformee` now refuse 127). The + firmware commits entry 127 in the same ~2.2 ms as entry 1 or 0. With the + station at entry 1 during development, entry 0 read back byte-identical + before the arm, after it and after the clear. Sharing entry 0 (the previous + layout) had no measurable cost: a witness counted the same >= 2900 of 3000 + injected frames armed and unarmed, on both layouts. The separation is for + the injection context's integrity, not a delivery fix. +- **Unexplained cell.** In about 55 armed cells across development (station at + entry 1), one read the entry back committed (after 10 ms rather than ~2.2 + ms) and answered no ACKs; other slow commits were fine and it has not + reproduced. +- **Clear needs the host.** Measured at entry 1: after `join_info(disconnect)` + and a NO_LINK addr_cam, the entry stays VALID with the responder's address; + a role REMOVE, or role CREATE plus NO_LINK, leaves it valid too. Clearing + VALID from the host takes, and re-read for 2 s it stays clear. Because those + H2Cs land asynchronously, the clear repeats until the entry has read back + empty for 25 ms, and only that counts as disarmed. rxdemo's `DEVOURER_ACK_DISARM_AFTER_MS` exercises it live. - **8852C:** its firmware accepts both commands and leaves the entry invalid, so the arm refuses. The vendor's 8852C association sends far more before the diff --git a/src/AdapterCaps.h b/src/AdapterCaps.h index 5dd84f63..e69bd25c 100644 --- a/src/AdapterCaps.h +++ b/src/AdapterCaps.h @@ -194,13 +194,14 @@ struct AdapterCaps { * tests/ack_txreport_matrix.sh run with the 8733B as the responder), * 8852B (Kestrel has no CCX report, so the soliciting TX is an 8832CU * counting the ACK frames it receives, one attempt per frame, MCS1 on - * ch161, 3 runs per row, responder on its own macid 1 / ADDR_CAM entry 1: - * 14400/14415 armed at Init, 14369/14427 armed at InitWrite, 14363/14421 - * retargeted, 0/6485 at the old address after the retarget, 0/6449 never - * armed, 0/6519 after a live disarm; one cell in about 55 across the - * development runs read the entry back committed and answered no ACKs, - * not reproduced since). The 8852C reads false: its firmware does not - * commit the station entry, and the arm refuses on the read-back. + * ch161, 3 runs per row, responder on its own macid 127 / ADDR_CAM entry + * 127: 14414/14422 armed at Init, 14376/14417 armed at InitWrite, + * 14419/14432 retargeted, 0/6597 at the old address after the retarget, + * 0/6575 never armed, 0/6561 after a live disarm; one cell in about 55 + * across the development runs, at entry 1, read the entry back committed + * and answered no ACKs, not reproduced since). The 8852C reads false: its + * firmware does not commit the station entry, and the arm refuses on the + * read-back. * * The `on`, `retarget`, and legacy `off` rows establish arming, retargeting, * and a never-armed control. A backend-owned `disarmed` row supports only diff --git a/src/kestrel/RtlKestrelDevice.cpp b/src/kestrel/RtlKestrelDevice.cpp index f8db1d02..01285730 100644 --- a/src/kestrel/RtlKestrelDevice.cpp +++ b/src/kestrel/RtlKestrelDevice.cpp @@ -538,10 +538,14 @@ namespace { constexpr uint8_t kAckResponderBssid[6] = {0x02, 0x00, 0x00, 0x00, 0x00, 0x01}; /* The responder's own station: a macid and ADDR_CAM entry apart from the * injection context InitWrite programs at macid 0 / entry 0 (which the WD of - * every injected frame keeps pointing at), and a BSSID CAM entry apart from - * that context's index 0. */ -constexpr uint8_t kAckMacid = 1; -constexpr uint8_t kAckAddrCamIdx = 1; + * every injected frame keeps pointing at), and from the low slots peer + * registrations take by convention (RegisterPeerSta callers use 1, 2, ...): + * the last entry of the 0x7f ADDR_CAM search range, reserved against + * RegisterPeerSta / RegisterBeamformee. BSSID CAM 1, apart from the injection + * context's index 0 (peers use 0 too, and only the addr entry selects the + * response). */ +constexpr uint8_t kAckMacid = 127; +constexpr uint8_t kAckAddrCamIdx = 127; constexpr uint8_t kAckBssidCamIdx = 1; std::string mac_text(const uint8_t *m) { @@ -653,7 +657,11 @@ RtlKestrelDevice::disarm_ack_locked(std::span held) { const auto t0 = clock::now(); auto clean_since = t0; const devourer::MacAddr *left = nullptr; + /* The address the entry last held; kept on record if the clear is not + * confirmed, so the next ClearAckResponder retries it. */ + const devourer::MacAddr *last_held = &held.front(); bool first = true; + bool stable = false; for (;;) { if (first || left) { _hal.clear_addr_cam_entry(kAckAddrCamIdx); @@ -661,18 +669,25 @@ RtlKestrelDevice::disarm_ack_locked(std::span held) { first = false; } left = still_held(); - if (!left && clock::now() - clean_since >= std::chrono::milliseconds(25)) + if (left) + last_held = left; + else if (clock::now() - clean_since >= std::chrono::milliseconds(25)) { + stable = true; break; + } if (clock::now() - t0 >= std::chrono::milliseconds(150)) break; std::this_thread::sleep_for(std::chrono::milliseconds(1)); } - if (left) { + /* Success only once the entry has stayed empty for the whole interval: an + * empty read right after a late rewrite was cleared is not a confirmation. */ + if (!stable) { _logger->warn("Kestrel: ACK responder clear did not take: ADDR_CAM entry " - "{} still holds {}, the MAC may keep answering for it " + "{} {} {}, the MAC may keep answering for it " "(ClearAckResponder retries)", kAckAddrCamIdx, - mac_text(left->bytes.data())); - return *left; + left ? "still holds" : "did not stay clear of", + mac_text(last_held->bytes.data())); + return *last_held; } _logger->info("Kestrel: hardware ACK responder disarmed (ADDR_CAM entry {} " "no longer holds {})", kAckAddrCamIdx, @@ -1071,12 +1086,24 @@ bool RtlKestrelDevice::ConfigureUlOfdma(const devourer::UlOfdmaConfig &cfg) { return _hal.configure_ul_ofdma(cfg); } +bool RtlKestrelDevice::peer_slot_reserved(const char *who, uint8_t macid, + uint8_t addr_cam_idx) const { + if (macid != kAckMacid && addr_cam_idx != kAckAddrCamIdx) + return false; + _logger->error("Kestrel: {} refused: macid {} / ADDR_CAM entry {} overlaps " + "the ACK responder's reserved macid {} / entry {}", + who, macid, addr_cam_idx, kAckMacid, kAckAddrCamIdx); + return true; +} + bool RtlKestrelDevice::RegisterPeerSta(const uint8_t peer_mac[6], uint8_t macid, uint8_t addr_cam_idx) { if (!_tx_up) { _logger->error("Kestrel: RegisterPeerSta before InitWrite"); return false; } + if (peer_slot_reserved("RegisterPeerSta", macid, addr_cam_idx)) + return false; return _hal.register_peer_sta(peer_mac, macid, addr_cam_idx); } @@ -1087,6 +1114,8 @@ bool RtlKestrelDevice::RegisterBeamformee(const uint8_t peer_mac[6], _logger->error("Kestrel: RegisterBeamformee before InitWrite"); return false; } + if (peer_slot_reserved("RegisterBeamformee", macid, addr_cam_idx)) + return false; /* The peer must exist as a fw role/ADDR_CAM entry first (same as a scheduled- * UL peer); then layer the CSI/bf params + hw sounding maps on top. */ bool ok = _hal.register_peer_sta(peer_mac, macid, addr_cam_idx); diff --git a/src/kestrel/RtlKestrelDevice.h b/src/kestrel/RtlKestrelDevice.h index 6aadb95c..16bcb6ce 100644 --- a/src/kestrel/RtlKestrelDevice.h +++ b/src/kestrel/RtlKestrelDevice.h @@ -91,17 +91,19 @@ class RtlKestrelDevice : public IRtlRadio { * once the firmware has COMMITTED a valid ADDR_CAM entry for it, and it does * that only for a station registered the way a real association registers * one (HalKestrel::register_infra_sta). The arm programs that station with - * `mac` as its own address, on its own macid 1 / ADDR_CAM entry 1 so the - * injection context at macid 0 / entry 0 (what every injected frame's WD - * points at) is left as bring-up programmed it; clears the response-abort - * CCA gates; and is verified by reading the physical CAM entry back -- - * false when the entry did not commit, which is the 8852C today. - * Post-bring-up only. Clear disconnects that station and clears the - * entry's VALID bit from the host (the 8852B firmware leaves a - * disconnected station's entry valid), and verifies the CAM no longer - * holds `mac`; an address still held after a failed clear (or a failed - * retarget, which checks the old address too) stays recorded, so a later - * ClearAckResponder retries it. Arm and clear are serialized. */ + * `mac` as its own address, on its own macid 127 / ADDR_CAM entry 127 so + * the injection context at macid 0 / entry 0 (what every injected frame's + * WD points at) is left as bring-up programmed it, and RegisterPeerSta / + * RegisterBeamformee refuse that slot so a peer cannot take it; clears the + * response-abort CCA gates; and is verified by reading the physical CAM + * entry back -- false when the entry did not commit, which is the 8852C + * today. Post-bring-up only. Clear disconnects that station and clears + * the entry's VALID bit from the host (the 8852B firmware leaves a + * disconnected station's entry valid), and succeeds only once the entry + * has read back empty for 25 ms; an address still held, or not confirmed + * gone, after a failed clear (or a failed retarget, which checks the old + * address too) stays recorded, so a later ClearAckResponder retries it. + * Arm and clear are serialized. */ bool SetAckResponder(const devourer::MacAddr &mac) override; void ClearAckResponder() override; /* rxdemo's live-disarm measurement hook (DEVOURER_ACK_DISARM_AFTER_MS): the @@ -314,6 +316,10 @@ class RtlKestrelDevice : public IRtlRadio { * timeout, nullopt once gone. Caller holds _ack_mutex. */ std::optional disarm_ack_locked(std::span held); + /* True (and logged) when a peer registration would take the ACK + * responder's reserved macid or ADDR_CAM entry. */ + bool peer_slot_reserved(const char *who, uint8_t macid, + uint8_t addr_cam_idx) const; std::optional _ack_disarm_after_ms; std::jthread _ack_disarm_thread; /* DEVOURER_ACK_RESPONDER: arm at the end of bring-up, throw if refused. */ From 9d9d9b8c4300006c53b18b836263b6411599d0ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Agust=C3=ADn=20Gonz=C3=A1lez?= Date: Wed, 14 Oct 2026 11:13:34 -0300 Subject: [PATCH 3/3] kestrel: ACK responder clear waits 50 ms stable; deadline only while held Maintainer review notes on 60d79c7: - The clear counted as taken after 25 ms of empty read-backs, while the arm's firmware commit was measured at 2-20 ms: ~5 ms of margin over the tail. A late NO_LINK rewrite after the window would leave the entry valid with nothing on record to retry. The window is now 50 ms. - The 150 ms bound was checked after the stability check, so an entry that first read empty late could be reported "did not stay clear" while clean. The bound now ends the wait only while the entry is still held; an entry that reads empty always gets its full window (worst case about 200 ms), and a timeout always means the entry still holds the address. - docs: "refuse 127", current state rather than changelog wording. TX20U Nano responder, TX50UH injector, ch161, MCS1: 3 live disarms 0 ACKs each (0/2215, 0/2217, 0/2223) with "disarmed (ADDR_CAM entry 127 ...)"; 2 arm -> retarget -> clear -> re-arm cycles, every phase as expected; armed 4809/4811. Full MSVC build and ctest pass (78, 4 skipped). --- docs/kestrel-ack-responder.md | 4 ++-- src/kestrel/RtlKestrelDevice.cpp | 17 +++++++++-------- src/kestrel/RtlKestrelDevice.h | 2 +- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/docs/kestrel-ack-responder.md b/docs/kestrel-ack-responder.md index 086dd560..7a1e55c2 100644 --- a/docs/kestrel-ack-responder.md +++ b/docs/kestrel-ack-responder.md @@ -29,7 +29,7 @@ was measured to get there. - **Own macid.** The station sits at macid 127 / ADDR_CAM entry 127 (the last entry of the 0x7f search range), apart from the injection context at macid 0 / entry 0 and from the low slots peer registrations take (`RegisterPeerSta` - callers use 1, 2, ...; it and `RegisterBeamformee` now refuse 127). The + callers use 1, 2, ...; it and `RegisterBeamformee` refuse 127). The firmware commits entry 127 in the same ~2.2 ms as entry 1 or 0. With the station at entry 1 during development, entry 0 read back byte-identical before the arm, after it and after the clear. Sharing entry 0 (the previous @@ -45,7 +45,7 @@ was measured to get there. a role REMOVE, or role CREATE plus NO_LINK, leaves it valid too. Clearing VALID from the host takes, and re-read for 2 s it stays clear. Because those H2Cs land asynchronously, the clear repeats until the entry has read back - empty for 25 ms, and only that counts as disarmed. rxdemo's + empty for 50 ms, and only that counts as disarmed. rxdemo's `DEVOURER_ACK_DISARM_AFTER_MS` exercises it live. - **8852C:** its firmware accepts both commands and leaves the entry invalid, so the arm refuses. The vendor's 8852C association sends far more before the diff --git a/src/kestrel/RtlKestrelDevice.cpp b/src/kestrel/RtlKestrelDevice.cpp index 01285730..cab9eb20 100644 --- a/src/kestrel/RtlKestrelDevice.cpp +++ b/src/kestrel/RtlKestrelDevice.cpp @@ -645,8 +645,10 @@ RtlKestrelDevice::disarm_ack_locked(std::span held) { * nor role CREATE + NO_LINK clears it), so the host clears VALID itself, as * the vendor's no-firmware mac_upd_addr_cam path writes the CAM. The H2Cs * above land asynchronously (2-20 ms measured for the arm) and rewrite the - * entry, so the clear repeats until the entry has read back empty for 25 ms - * straight, within a 150 ms bound. */ + * entry, so the clear repeats until the entry has read back empty for 50 ms + * straight, well past that tail. The 150 ms bound only ends the wait while + * the entry is still held; an entry that reads empty is always given its + * full 50 ms, so the worst case is about 200 ms. */ const auto still_held = [&]() -> const devourer::MacAddr * { for (const auto &a : held) if (_hal.addr_cam_holds(kAckAddrCamIdx, a.bytes.data())) @@ -669,23 +671,22 @@ RtlKestrelDevice::disarm_ack_locked(std::span held) { first = false; } left = still_held(); - if (left) + if (left) { last_held = left; - else if (clock::now() - clean_since >= std::chrono::milliseconds(25)) { + if (clock::now() - t0 >= std::chrono::milliseconds(150)) + break; + } else if (clock::now() - clean_since >= std::chrono::milliseconds(50)) { stable = true; break; } - if (clock::now() - t0 >= std::chrono::milliseconds(150)) - break; std::this_thread::sleep_for(std::chrono::milliseconds(1)); } /* Success only once the entry has stayed empty for the whole interval: an * empty read right after a late rewrite was cleared is not a confirmation. */ if (!stable) { _logger->warn("Kestrel: ACK responder clear did not take: ADDR_CAM entry " - "{} {} {}, the MAC may keep answering for it " + "{} still holds {}, the MAC may keep answering for it " "(ClearAckResponder retries)", kAckAddrCamIdx, - left ? "still holds" : "did not stay clear of", mac_text(last_held->bytes.data())); return *last_held; } diff --git a/src/kestrel/RtlKestrelDevice.h b/src/kestrel/RtlKestrelDevice.h index 16bcb6ce..95ae505d 100644 --- a/src/kestrel/RtlKestrelDevice.h +++ b/src/kestrel/RtlKestrelDevice.h @@ -100,7 +100,7 @@ class RtlKestrelDevice : public IRtlRadio { * today. Post-bring-up only. Clear disconnects that station and clears * the entry's VALID bit from the host (the 8852B firmware leaves a * disconnected station's entry valid), and succeeds only once the entry - * has read back empty for 25 ms; an address still held, or not confirmed + * has read back empty for 50 ms; an address still held, or not confirmed * gone, after a failed clear (or a failed retarget, which checks the old * address too) stays recorded, so a later ClearAckResponder retries it. * Arm and clear are serialized. */