Fix IEEE 802.11 ADDBA transaction handling - #1129
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| [Config MacQosWithTransactionalBlockAck] | ||
| description = "Exercises successful and timed-out ADDBA transactions" | ||
| extends = MacQosWithoutAggregation | ||
| sim-time-limit = 3s |
There was a problem hiding this comment.
🔴 Existing Block Ack example loses its radio medium setting and can abort with an error
The new example configuration is inserted (at examples/wireless/qos/omnetpp.ini:86) between the previous last configuration and the trailing radio-medium setting lines, so the setting that used to belong to the Block Ack example silently moves to the new example.
Impact: The pre-existing Block Ack example now runs with the strict default and can stop with a runtime error when two transmissions start at the same moment; its recorded results change too.
How the ini section boundary shifts the setting
In an omnetpp.ini file, keys belong to the section they textually follow. Before this PR, the lines
# radio medium
*.radioMedium.sameTransmissionStartTimeCheck = "ignore"
were the last lines of [Config MacQosWithBlockAck]. The new [Config MacQosWithTransactionalBlockAck] block is inserted above them, so those two lines now belong to the new config and MacQosWithBlockAck falls back to the NED default error (src/inet/physicallayer/wireless/common/medium/RadioMedium.ned:44), which raises a runtime error via src/inet/physicallayer/wireless/common/medium/RadioMedium.cc:484-497. Note also that MacQosWithTransactionalBlockAck extends MacQosWithoutAggregation, so the new config would not have inherited the setting either; this reassignment is accidental. The existing fingerprint row for MacQosWithBlockAck in tests/fingerprint/examples.csv:661 is left unchanged.
Prompt for agents
The two trailing lines of examples/wireless/qos/omnetpp.ini ('# radio medium' and '*.radioMedium.sameTransmissionStartTimeCheck = "ignore"') were part of [Config MacQosWithBlockAck] because ini keys belong to the preceding section. The newly added [Config MacQosWithTransactionalBlockAck] section was inserted before them, so MacQosWithBlockAck lost the setting and now uses the RadioMedium default 'error', which can abort the simulation. Restore the setting to MacQosWithBlockAck (e.g. append the new config after those lines, or explicitly duplicate/hoist the radioMedium assignment where it is actually needed).
Was this helpful? React with 👍 or 👎 to provide feedback.
| if (!hasFrameToTransmit(ac)) { | ||
| EV_DETAIL << "Releasing channel because no eligible frame is available.\n"; | ||
| edcaf->releaseChannel(this); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🔴 Traffic queues can stall after the radio is handed back without sending anything
When the radio is granted to a queue that currently has nothing sendable, the radio is handed back immediately (edcaf->releaseChannel(this) at src/inet/linklayer/ieee80211/mac/coordinationfunction/Hcf.cc:229) before the other queues that lost the simultaneous grant are told to back off and retry, so those queues stop contending.
Impact: Packets waiting in other priority queues can sit unsent until unrelated new traffic arrives, causing extra delay or loss.
Skipped internal-collision recovery
When several access categories reach their transmit time simultaneously, only the highest-priority EDCAF gets channelGranted(); the losers are marked as internal collisions (src/inet/linklayer/ieee80211/mac/contention/EdcaCollisionController.cc:33-46) and do not get their callback (src/inet/linklayer/ieee80211/mac/channelaccess/Edcaf.cc:127-133). Their recovery (backoff update, drop on retry limit, and edcaf->requestChannel(this)) is performed exclusively by the winner in Hcf::handleInternalCollision() (src/inet/linklayer/ieee80211/mac/coordinationfunction/Hcf.cc:229-297 region). The new early return at line 230 executes before edca->getInternallyCollidedEdcafs() is consulted, so the collided EDCAFs' contention has already ended and nobody restarts it. The early return is reachable because frame eligibility can change between requesting the channel and being granted it (a pending ADDBA transaction makes all same peer/TID QoS frames ineligible via Hcf::hasFrameToTransmit(AccessCategory)).
Prompt for agents
In Hcf::channelGranted(), the new early return releases the channel when no eligible frame exists, but it bypasses the internal-collision handling that follows (edca->getInternallyCollidedEdcafs() / handleInternalCollision()). EDCAFs that lost the simultaneous grant rely on the winner to run their recovery procedure and re-request the channel, so they stop contending. Handle the internally collided EDCAFs (and emit edcaCollisionDetectedSignal) before releasing the channel and returning.
Was this helpful? React with 👍 or 👎 to provide feedback.
| else { | ||
| // TODO send a new one? | ||
| terminateAgreement(addbaResp->getTransmitterAddress(), addbaResp->getTid()); | ||
| scheduleAddbaResponseTimer(callback); | ||
| return nullptr; | ||
| } |
There was a problem hiding this comment.
🔴 Traffic to a peer that refuses Block Ack is throttled to one frame per timeout period
The Block Ack setup record is deleted (terminateAgreement(...) at src/inet/linklayer/ieee80211/mac/blockack/OriginatorBlockAckAgreementHandler.cc:200, and by the timeout cleanup at lines 60-69) when the peer refuses or never answers, so the very next data frame starts a brand new setup attempt that again blocks all further frames for that peer and priority.
Impact: Data sent to a receiver that does not support Block Ack is repeatedly held for the whole setup timeout, collapsing throughput and overflowing the queue.
Retry loop between eligibility gating and agreement deletion
While a setup is pending, Hcf's frame-eligibility function (src/inet/linklayer/ieee80211/mac/coordinationfunction/Hcf.cc:63-66) makes every QoS data frame to that receiver/TID ineligible, so they stay queued. On rejection (processReceivedAddbaResp) or on timeout (addbaResponseTimeoutExpired) the agreement object is erased entirely. processTransmittedDataFrame (src/inet/linklayer/ieee80211/mac/blockack/OriginatorBlockAckAgreementHandler.cc:176-183) creates a new agreement whenever getAgreement() returns null and isAddbaReqNeeded() is true; the default policy's isAddbaReqNeeded() only checks frame type and length (src/inet/linklayer/ieee80211/mac/blockack/OriginatorBlockAckAgreementPolicy.cc:38-41), so it is true for every QoS data frame. The result is: one frame goes out, a new ADDBA is issued, all following frames are held for addbaFailureTimeout (default 1s), the setup fails again, and the cycle repeats indefinitely. Previously the pending agreement was kept forever, so only one ADDBA was ever sent and data flowed normally with normal ACK. Consider remembering failed peers/TIDs (or a retry limit/backoff) so ADDBA is not re-attempted for every frame, and/or not gating frames when no agreement can be established.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if (agreement && agreement->isPending() && agreement->getDialogToken() == addbaReq->getDialogToken()) { | ||
| if (agreement->getAddbaResponseDeadline() < 0) { | ||
| auto addbaFailureTimeout = blockAckAgreementPolicy->computeAddbaFailureTimeout(); | ||
| if (addbaFailureTimeout <= 0) | ||
| throw cRuntimeError("ADDBA failure timeout must be greater than zero"); | ||
| agreement->setAddbaResponseDeadline(simTime() + addbaFailureTimeout); | ||
| } | ||
| agreement->setIsAddbaRequestSent(true); | ||
| scheduleAddbaResponseTimer(callback); | ||
| } |
There was a problem hiding this comment.
🟡 Frames can be held forever if the Block Ack setup request is never sent on air
The deadline for giving up on a Block Ack setup is only armed when the request frame actually goes on air (agreement->setAddbaResponseDeadline(...) at src/inet/linklayer/ieee80211/mac/blockack/OriginatorBlockAckAgreementHandler.cc:222), so a setup whose request is discarded before transmission never expires.
Impact: All data to that receiver and priority stays stuck in the queue indefinitely, silently dropping the flow.
Path where the request is dropped before transmission
processTransmittedDataFrame creates the agreement and hands the ADDBA Request to processMgmtFrame, which enqueues it. From that moment the eligibility function in Hcf::initialize (src/inet/linklayer/ieee80211/mac/coordinationfunction/Hcf.cc:63-66) holds all QoS data for that receiver/TID because isAddbaResponsePending() is true. The deadline, and hence computeEarliestAddbaResponseDeadline() (which requires getIsAddbaRequestSent()), only becomes active in processTransmittedAddbaReq. If the management frame is dropped before it is ever transmitted — e.g. retry-limit reached during internal collision handling in Hcf::handleInternalCollision (src/inet/linklayer/ieee80211/mac/coordinationfunction/Hcf.cc:270-286) — the agreement stays pending with deadline -1 and is never cleaned up by addbaResponseTimeoutExpired. Arming the deadline at agreement creation (or removing the agreement when its ADDBA Request is dropped) would avoid the permanent stall.
Was this helpful? React with 👍 or 👎 to provide feedback.
| Packet *packet = nullptr; | ||
| for (int i = 0; i < pendingQueue->getNumPackets(); i++) { | ||
| auto candidate = pendingQueue->getPacket(i); | ||
| if (isFrameEligible(candidate)) { | ||
| pendingQueue->removePacket(candidate); | ||
| packet = candidate; | ||
| break; | ||
| } | ||
| } | ||
| ASSERT(packet != nullptr); |
There was a problem hiding this comment.
🟡 Queue statistics for outgoing Wi-Fi frames are lost
Frames are now taken out of the transmit queue with a plain removal call (pendingQueue->removePacket(candidate) at src/inet/linklayer/ieee80211/mac/originator/OriginatorMacDataService.cc:66, and the same in the QoS variant) instead of the normal dequeue path, so the queue no longer reports the packet as served.
Impact: Recorded queueing-time and dequeue statistics for all Wi-Fi transmit queues disappear or change, even in configurations unrelated to Block Ack.
Difference between dequeuePacket() and removePacket()
PacketQueueBase::dequeuePacket() calls pullPacket() which stamps the queueing time tag, inserts the packet event and emits packetPulledSignal (src/inet/queueing/queue/PacketQueue.cc:117-133), which feeds @statistic[queueingTime]. removePacket() only emits packetRemovedSignal (src/inet/queueing/queue/PacketQueue.cc:135-143). The new selection loops in both OriginatorMacDataService::extractFramesToTransmit and OriginatorQosMacDataService::extractFramesToTransmit always use removePacket(), including the common case where the selected packet is the queue head and no eligibility filtering is in effect (the non-QoS DCF service never gets an eligibility function). Using dequeuePacket() when the chosen candidate is the head would preserve the previous statistics behaviour.
Was this helpful? React with 👍 or 👎 to provide feedback.
Track each originator ADDBA setup with an exact local transaction ID, start the response timeout only after the complete request is transmitted, and apply a separate retry backoff after timeout, refusal, or discard. Cancel all queued and in-progress fragments belonging to failed or completed transactions so stale requests cannot block or later re-establish state. Establish recipient Block Ack agreements when a request is accepted, remove response-packet staging, reset reorder buffers on renegotiation, and publish distinct agreement-change observations. Preserve internal-collision recovery and materialize frames only at DCF grants and HCF TXOP continuation boundaries so availability queries remain side-effect free. Add typed queue drop callbacks and arbitrary-packet dequeue propagation through queues, flows, and schedulers. Restore QueueingTimeTag, PEK_QUEUED, and packetPulled accounting exactly once while keeping cancellation on removal/drop semantics. Extend the focused regression with real fragmentation and transaction cancellation, reorder reset, queue accounting and overflow callbacks, HCF continuation, and legacy DCF channel-grant coverage. Validation: release and debug builds; focused Ieee80211AddbaTransaction unit test; MacDcf and MacEdca smoke simulations; architecture and WLAN reviews; 22 related fingerprints with 15 unchanged and 7 explained Block Ack trajectory changes. Fingerprint baselines are intentionally unchanged.
Defer ADDBA initiation until the triggering QoS MPDU is acknowledged and its final fragment completes. This preserves the negotiated starting sequence number and prevents an outstanding retry or remaining fragment from falling below the recipient reorder window. Resume eligible channel access when a dropped setup frame terminates a pending transaction, and cancel tagged ADDBA requests when DELBA tears down pending state. Restore DCF pending-frame materialization and defensively handle internal collisions with no selectable frame. Use the configured EDCAF count for HCF queue traversal, track packet-drop callback registration, and unregister callbacks during teardown. Extend the focused unit test with ACK timing, fragmented MSDU, DELBA cancellation, queue wakeup, and DCF materialization coverage. Validation: release build and focused Ieee80211AddbaTransaction_1 unit test pass; git diff --check passes. The full unit and fingerprint suites were not run.
Keep ADDBA setup requests and same-peer/TID data synchronized with the pending originator agreement. Harden cancellation, retry cleanup, DELBA teardown, reordering ownership, and in-progress frame disposal so stale setup traffic cannot be double-freed or leave protocol state behind. Add an HCF-owned per-access-category eligibility index. Track exact queue departures through typed lifecycle callbacks and rebuild eligibility only when Block Ack state changes, making routine channel-access availability checks independent of pending queue depth. Preserve configurable queue-provider policy during selective extraction and A-MSDU formation. Add predicate-aware Priority, WRR, Label, flow, leaf, and compound extraction; batch overflow notifications to avoid reentrant victim selection; and conservatively skip pre-extraction aggregation through transforming packet flows. Document the incompatible queue contracts and DELBA standards trace. Expand focused coverage for ADDBA lifecycle transitions, direct and RTS failure cleanup, custom and compound queues, scheduler accounting, shared buffers, A-MSDU processing, and the no-scan eligibility invariant. Validated with release/debug builds, focused unit tests in both modes, runtime-clean affected fingerprint simulations, and an independent INET/WLAN architecture review.
Prevent predicate-based extraction from bypassing closed gates or implicit guard bands by validating the exact upstream candidate before dequeueing it. Drain PacketQueue instances completely during bulk removal and detach only the queue's own packets from shared buffers so removal callbacks remain complete and isolated. Transfer buffered Block Ack frames to the recipient data service during reset, emit one packet-drop signal per discarded MPDU, and reclaim cancelled out-of-sequence ADDBA frames immediately. Clear both short and long retry state on terminal frame discard, and make scheduler extractor requirements explicit while preserving PriorityScheduler's nullable collection accounting. Add focused regression coverage for closed and guarded gates, multi-packet and shared-buffer removal, reorder-reset drop accounting, idle ADDBA cancellation, retry-state cleanup, and extractor-only priority inputs. Validated with the focused Ieee80211AddbaTransaction_1 unit test in release and debug modes.
Reset recipient-side Block Ack reordering when a locally transmitted recipient DELBA removes an agreement. Keep the removed agreement alive through HCF notification so teardown observers receive valid state, and remove the unused ADDBA-response-sent flag. Return removed recipient agreements from the transmitted-DELBA handler and cover teardown plus same-peer/TID reestablishment with a reorder-buffer regression. The test proves stale sequence state and buffered fragments do not leak into the replacement session. Report shared PacketBuffer removals to PacketQueue observers exactly once. A dedicated pre-drop detach callback preserves batch removal semantics: ordinary removals report REMOVED, overload victims report DROPPED, and all selected victims are detached before drop callbacks can re-enter queue selection. Add focused coverage for direct shared-buffer removal, overload callback reasons, recipient DELBA cleanup, and fresh reorder state after renegotiation. Validation: - Release and debug builds pass. - Focused ADDBA unit tests pass in release and debug. - The full release unit suite passes 78/90; the 12 failures are unrelated pre-existing clock and TCP cases. - Focused Block Ack fingerprint mismatches are unchanged by an old-vs-new HCF channel-resumption A/B, so fingerprint baselines remain untouched. - Architectural review reports no new violations.
3a40816 to
2a389ec
Compare
Return removed originator agreements from transmitted DELBA processing so HCF can emit balanced agreement lifecycle signals only for agreements that were actually established. Treat successful ADDBA responses as established before applying a local policy veto, then queue an initiator DELBA with END_BA. Handle transmitted and pre-transmission-aborted teardown frames idempotently, preserve retry suppression, and restore frame eligibility without leaving a blocked TID. Make DROPPED and REMOVED unsent ADDBA requests terminal while keeping DEQUEUED as an ownership transfer. Count only actionable EDCA internal collisions and retain release/end-TXOP/all-AC resumption ordering without restarting active contention. Rename addbaFailureTimeout to addbaResponseTimeout, document the API and configuration migration, and add focused debug/release coverage for lifecycle signals, queue removal, local veto teardown, retry backoff, collision filtering, and channel-access resumption. Validation: debug and release builds pass; Ieee80211AddbaTransaction_1 passes in both modes; git diff --check passes. Fingerprint baselines are intentionally unchanged pending explicit acceptance of the attributed EDCAF timing shift.
Keep nullable packet collection and extractor capabilities in WrrScheduler, LabelScheduler, and PriorityScheduler. Validate aggregate and predicate operations lazily and consistently so ordinary passive sources remain valid scheduler inputs. Make A-MSDU selection candidate-aware across flow modules and non-order-preserving schedulers. Remove selected members through exact predicate dequeues, preserve provider accounting, and revalidate aggregation-critical frame fields before building the aggregate. Immediately roll back successful ADDBA agreements rejected by local policy while retaining Normal Ack data service. Track best-effort DELBAs by transaction, reject stale generations, handle fragmented transmit and abort lifecycles, and cancel obsolete queued or in-progress teardown packets before replacement setup. Document the updated scheduler and aggregation contracts, update WHATSNEW, and add focused regression coverage for scheduler capabilities, flow and reverse-priority aggregation, agreement signal ordering, stale teardown disposal, fragmentation, and queue cleanup.
Keep transaction-tagged initiator DELBA frames eligible until a final-fragment acknowledgement or terminal abort, allowing failed management transmissions to follow the normal retry and retry-limit paths. Make teardown abort outcomes explicit in HCF and clean up sibling frames and acknowledgement state exactly once. Preserve same-flow MSDU ordering during A-MSDU selection, make extraction contract validation exception-safe, propagate destructive drops through nested compound queues exactly once, reject unsupported PacketBuffer ownership before mutation, and make fragment tag propagation robust. Document the scheduler aggregate-query compatibility change. Add focused coverage for DELBA retry, acknowledgement and retry-limit cleanup; malformed packet extractors; nested queue callbacks; PacketBuffer ownership; scheduler capabilities; and fragment tag propagation. Validated with release and debug builds, focused Ieee80211AddbaTransaction_1 tests in both modes, git diff --check, and independent architecture and IEEE 802.11 semantic review. Fingerprint baselines are intentionally unchanged.
Propagate MAC duplicate detection from the recipient QoS data service into HCF so retransmitted management frames are not processed as new negotiations. Cache the exact ADDBA response per originator and TID and replay it for a recognized duplicate while preserving the existing agreement, reorder window, buffered MPDUs, inactivity timer, and agreement signals. Fresh MAC identities still perform normal renegotiation, including when their ADDBA parameters are unchanged. Clear replay state during agreement teardown and avoid retaining responses for initial rejected requests without an agreement. Add focused transaction coverage for accepted and rejected requests, duplicate response replay, genuine renegotiation, reorder-buffer preservation, DELBA handling, and cache lifecycle.
Forward REMOVED callbacks from descendant queues through CompoundPacketQueueBase so observers are notified whenever a packet leaves the enclosing logical queue, including leaf-initiated and shared-buffer removals. Track the packet currently removed at the compound boundary with a scoped save-and-restore guard. Suppress only the matching descendant callback during boundary removal and overflow victim detachment, preserving exactly-once delivery and the intended DROPPED reason while allowing nested or reentrant removals of other packets to propagate. Extend the ADDBA transaction unit coverage for direct and nested compounds, bulk and shared-buffer removal, boundary remove/dequeue/pull paths, reentrant removal, and compound capacity drops. Validation: debug build; focused Ieee80211AddbaTransaction_1 unit test; focused PriorityQueue and EthernetQosQueue fingerprints; focused architecture check; independent semantic review.
1 participant
Summary
MacQosWithTransactionalBlockAckexample/fingerprint exercising successful and timed-out transactionsWhy
The previous implementation did not correlate ADDBA responses with an outstanding dialog token, treated policy acceptance as sufficient regardless of status, did not implement response-timeout recovery, and created recipient agreement state before the successful response was transmitted. Meanwhile, queued frames could receive sequence numbers while setup was pending, so the first frame sent under an accepted agreement could diverge from the SSN carried by the request.
This change follows IEEE Std 802.11-2024 clauses 9.6.4.2, 10.25.2, 10.25.6.6.1, 11.5.2.2, and 11.5.2.3. It is based directly on
masterand has no dependency on Compressed Block Ack support.Validation
make MODE=release -j$(nproc)make MODE=debug -j$(nproc)inet_run_unit_tests -m release -f '(Ieee80211AddbaTransaction_1|Ieee80211OnWireBitCompliance_1)\.test'— 2/2 passed./fingerprinttest -d -m '/examples/wireless/qos/.*MacQosWithTransactionalBlockAck' -f 'tplx' -f '~tNl' -f '~tND'— 1/1 passedgit diff --checkThe complete release unit run passed all ADDBA/IEEE 802.11 tests; its 12 unexpected failures were confined to unrelated TCP receive-queue, clock, and oscillator tests. The complete fingerprint run identifies intentional trajectory changes in the pre-existing
MacQosWithBlockAckcase and three wireless Block Ack showcase cases; their expected rows are deliberately not updated in this draft.