Skip to content

jaguar3 8822E: match SVPcom monitor injection retry policy - #334

Open
etiaro wants to merge 1 commit into
OpenIPC:masterfrom
etiaro:master
Open

jaguar3 8822E: match SVPcom monitor injection retry policy#334
etiaro wants to merge 1 commit into
OpenIPC:masterfrom
etiaro:master

Conversation

@etiaro

@etiaro etiaro commented Jul 24, 2026

Copy link
Copy Markdown

These changes tested with rtl8812eu wifi card and PixelPilot android app, allowed me to successfully enable Adaptive link (10Hz TX) without distupting monitor mode RX performance.

Honor RADIOTAP_F_TX_NOACK, disable rate fallback, and preserve injected 802.11 sequence numbers. This prevents TX frames from expanding into repeated TX bursts that disrupt video RX.

Assisted-by: OpenAI GPT-5.6 Terra

Honor RADIOTAP_F_TX_NOACK, disable rate fallback, and preserve injected
802.11 sequence numbers. This prevents TX frames from expanding
into repeated TX bursts that disrupt video RX.

Assisted-by: OpenAI GPT-5.6 Terra

@josephnef josephnef left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks — this is a clean, faithful port of the right source, and it fixes a real problem. I verified it line-by-line against both svpcom/rtl8812eu (core/rtw_xmit.c NOACK→retry_ctrl mapping + the hal/rtl8822e/usb/rtl8822eu_xmit.c inject branch) and the same-lineage fork already vendored in this repo (reference/rtl88x2eu-5mhz, the pattrib->inject == 0xa5 branch) — the descriptor policy matches exactly.

What I ran against this branch:

  • Build clean, 37/37 ctest pass.

  • On-air 4-cell regression (tests/regress.py, ch 6) with an RTL8812CU as TX — the jaguar3 die this PR wasn't tested on, same fill_data_tx_desc_8822c — all cells pass, devourer↔devourer 7200/7000 hits. Basic injection is intact on both dies.

  • A/B of the unanswered-unicast retry pinning (QoS-Data to a silent MAC, DEVOURER_TX_REPORT=1, 8812CU):

    master: reports=1281  ack_rate=0.00  retries_mean=12.00  retries_max=12
    PR:     reports=1287  ack_rate=0.00  retries_mean= 0.00  retries_max= 0
    

That A/B confirms the mechanism works exactly as designed — but it also surfaces the two blocking issues (inline): the NOACK→no-retry and EN_HWSEQ→SW_SEQ changes apply to all injected frames, and devourer's own consumers (unlike wfb-ng) rely on the old behavior. wfb-ng opts into NOACK deliberately and manages its own 802.11 sequence numbers; devourer's demos stamp NOACK unconditionally and hardcode seq 0, and the hardware-ACK/ARQ + A-MPDU/BlockAck features were measured under the old descriptor.

Concretely: tests/ack_txreport_matrix.sh (the scheduled-MAC ARQ contract) now fails its off-phase check on jaguar3, and every demo frame airs seq 0.

None of this needs the port itself to change — it needs the new behavior scoped to frames that ask for it (details inline), or the demos + ack matrix + docs updated in the same PR so the behavior change is deliberate rather than silent. One small define nit inline as well.

Also worth a line in the PR description: this fill serves the 8822C/8812CU too, not just the 8822E — validated above, just so the scope is explicit.

SET_TX_DESC_RTS_DATA_RTY_LMT_8822C(d, 12);
/* RADIOTAP_F_TX_NOACK means zero MAC retries; other injected frames use
* SVPcom's six-attempt limit. */
SET_TX_DESC_RTS_DATA_RTY_LMT_8822C(d, retry ? 6 : 0);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: this silently disables the jaguar3 hardware-ARQ loop for every devourer consumer.

All in-tree radiotap producers stamp TX_FLAGS NOACK unconditionally (RadiotapBuilder's kTxFlagsNoAck, and examples/tx's hand-rolled headers with 0x08) — including the QoS-Data soliciting path that SetAckResponder + tests/ack_txreport_matrix.sh build on, where the MAC is supposed to retransmit until the hardware ACK arrives. Measured on an 8812CU: master pins retries_max=12 on unanswered unicast, this branch reads retries_max=0, and the matrix's off-phase check (retries_max >= 8) now fails.

The NOACK semantics are right for wfb-ng (it sets the flag deliberately per link). The problem is only that devourer's demos treat NOACK as boilerplate. Suggested resolution — either:

  1. Update the in-tree producers to stop stamping NOACK on frames that solicit ACKs, and re-baseline ack_txreport_matrix.sh in this PR, or
  2. Only honor NOACK→0 when the frame actually carried a radiotap TX_FLAGS field (keep the current 12, or 6, when the field is absent), so existing consumers keep their contract and wfb-ng gets the new one.

Either is fine — it just has to be a deliberate change, not a side effect.

SET_TX_DESC_TXPWR_OFSET_TYPE_8822C(d, pwr_ofset_type & 0x3);
SET_TX_DESC_EN_HWSEQ_8822C(d, 1);
SET_TX_DESC_EN_HWSEQ_8822C(d, 0);
SET_TX_DESC_SW_SEQ_8822C(d, seqnum & 0x0fff);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: EN_HWSEQ=0 here applies to all injected frames, not just wfb-ng's.

wfb-ng manages its own 802.11 sequence numbers, so copying the header seq is correct there. But devourer's demos hardcode seq 0x0000 in their headers — examples/tx's QoS builder even comments it /* seq (EN_HWSEQ overwrites) */ — so after this change every demo frame airs seq 0 on jaguar3. That breaks anything ordering on SSN: the A-MPDU/BlockAck path (SetAmpduMode — the measured +30% goodput in docs/aggregation.md was taken under hwseq, and a BA reorder window needs ascending SSNs) and receiver-side dedup heuristics.

Suggestion: fall back to the old hwseq behavior when the caller's header seq is 0 (seqnum == 0EN_HWSEQ=1, else SW_SEQ) — wfb-ng's incrementing counter passes through untouched, callers that never managed seq keep hardware sequencing, and no new knob is needed. Gating on the same TX_FLAGS-present signal as the retry change would also work.

pwr_type = txpkt_type_for_idx(txpkt_idx_for_qdb(radiotap_pkt_pwr_db * 4));
/* SVPcom monitor injection maps RADIOTAP_F_TX_NOACK to no MAC retry.
* wfb-ng sets this for raw-link packets, avoiding a repeated TX burst. */
constexpr uint16_t kRadiotapTxNoAck = 0x0008;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: IEEE80211_RADIOTAP_F_TX_NOACK already exists in src/ieee80211_radiotap.h (0x0008) — use it instead of a local constant.

Related observation (fine as a follow-up, not this PR): jaguar1 parses TX_FLAGS into a variable it never uses, and jaguar2/kestrel ignore the field entirely — after this PR, NOACK means something on jaguar3 only. Worth either a docs note or a TxCaps flag so callers can tell.

Comment thread tests/txagg_selftest.cpp
"8822e: forced-rate fallback bits=0x%02x want DISRTSFB|DISDATAFB",
blk[0x0d] & 0x06);
CHECK((blk[0x12] & 0x02) != 0 && (blk[0x12] & 0xfc) == 0,
"8822e: TX_NOACK retry descriptor byte=0x%02x want enable/limit=0",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good coverage of the retry=false leg (and the bit positions all check out against the descriptor defines). Please add the retry=true leg too — one more fill with /*retry=*/true asserting the limit field reads 6 — so both halves of the new policy are pinned.

@josephnef

Copy link
Copy Markdown
Collaborator

Now that #354 is merged (bb03774), the retry half of this PR is superseded, and master conflicts with this branch on the same descriptor lines — a status recap so effort doesn't go into rebasing parts that can't land.

Retry policy — superseded. DEVOURER_TX_RETRY_LIMIT (default 0, all of jaguar1/2/3, per-session) now covers the motivation here: WFB-style no-MAC-retry injection is the out-of-the-box behavior. It also avoids the regression found in the hardware A/B of this branch: mapping radiotap TX_FLAGS NOACK → retry 0 zeroes retries for every devourer demo frame (they all set NOACK unconditionally — RadiotapBuilder kTxFlagsNoAck, txdemo's hand-rolled 0x08), which kills the SetAckResponder hardware-ARQ loop with no opt-out. Measured on-air at the time: master retries_mean=12.00 → this branch 0.00 on unanswered unicast, and tests/ack_txreport_matrix.sh off-phase fails. wfb-ng owns its frames and can carry the flag deliberately; devourer's demos and the scheduled-MAC path can't.

EN_HWSEQ 1→0 + SW_SEQ from header — the remaining open question, still not landable as-is. Unconditional software-sequence means every demo frame airs seq 0 on jaguar3 (txdemo hardcodes 0x0000 with the comment "(EN_HWSEQ overwrites)"), and the A-MPDU/BlockAck reorder results in docs/aggregation.md were measured under hwseq. If wfb-ng needs caller-owned sequence numbers, that's a legitimate feature — but it has to be opt-in (a config knob or keyed off the radiotap actually carrying a nonzero seq), not the new default for all injected frames.

Suggested path: rebase onto master, drop the retry hunks (covered by #354DEVOURER_TX_RETRY_LIMIT=0 is already the default), and either drop the hwseq flip or resubmit it as an explicit opt-in knob with the ARQ/A-MPDU paths re-measured under it. Happy to re-run the on-air A/B against a rebased version.

For the record: the port itself was verified faithful against both svpcom/rtl8812eu and the in-tree reference/rtl88x2eu-5mhz inject branch — the objection was never the fidelity, it's that wfb-ng's frame-ownership assumptions don't hold for the rest of this library's consumers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants