IEEE 802.11: add HT capability and operation signalling - #1144
Closed
mgonzalezlopezudc wants to merge 15 commits into
Closed
IEEE 802.11: add HT capability and operation signalling#1144mgonzalezlopezudc wants to merge 15 commits into
mgonzalezlopezudc wants to merge 15 commits into
Conversation
Mode-set timing previously depended on the entry that happened to sort first by bitrate. This made SIFS, slot time, receive-start delay, and PHY-specific TXOP defaults sensitive to mode ordering instead of an explicit authority. Store and validate a reference mode and operating PHY for every mode set, and make timing and TXOP consumers use those properties directly. Correct the HT and VHT receive-start delay to the standards-defined 24 microseconds at the same boundary so response timeout calculations use the right value. The focused mode-set, response-timeout, and TXOP tests cover reordered modes, invalid references, PHY-family defaults, and ACK/CTS/Block Ack timing.
Management frame decoding and Supported Rates construction did not preserve all of the information needed to describe the BSS legacy operational rates. Derived frame types, basic-rate membership, and rates beyond the primary element could be lost or inferred from mode ordering. Preserve the concrete management frame subtype during deserialization, validate Supported and Extended Supported Rates element lengths and values, and encode the basic-rate bit from an explicit legacy operational set. Derive that set from eligible mode entries and reject mode sets that cannot advertise any representable legacy operational rate. The Supported Rates and mode-set tests cover malformed encodings, primary and extended element splitting, basic membership, legacy timing, and empty-set rejection.
HT capability and operation information needs a typed, shared representation before management signalling and peer negotiation can use it consistently. Inferring dense MCS support or carrying partially modeled elements would make the advertised state diverge from the configured PHY modes. Add typed HT capability and operation values, derive exact MCS, channel-width, mandatory-rate, and short-guard-interval sets from the mode set, and keep the local advertised state in the IEEE 802.11 MIB. Add the corresponding fixed-size management elements, serializers, conversion helpers, and sparse protocol printer output. Focused tests verify capability derivation, sparse MCS sets, width and guard interval handling, byte-level codecs, validation, and diagnostic rendering.
The AP needs transaction identity to follow every locally transmitted fragment until the MAC reports terminal completion. That identity is sender-local control state, however, and must not become observable as received packet metadata at the peer. Introduce a typed management transaction tag, preserve it with the correct packet and region semantics during fragmentation, and remove it at the layered PHY receive boundary. This keeps completion correlation intact locally without leaking implementation metadata across the simulated wireless link. The focused tag and packet-domain tests exercise fragmentation preservation and confirm that the receiver retains protocol metadata but not the local tag.
Association state could be finalized without a reliable terminal result for the corresponding response. Queue eviction, retry exhaustion, RTS failure, or replacement could therefore leak an AID, commit stale HT/channel state, or emit an association notification for a response that was never acknowledged. Reserve AIDs while responses are pending and commit or release the exact transaction snapshot only after the MAC reports success or failure. Propagate synchronous completion through queue-drop callbacks, DCF/HCF retry paths, and the MAC-management bridge. Bind channel and HT Operation state to the pending response, correct marked AID wire encoding, and make authentication and peer replacement use the same ownership rules. Focused unit, module, and queueing tests cover ACK success, DCF/HCF failures, RTS timeout, queue drops, unavailable channels, reassociation snapshots, AID reuse, and exactly-once terminal notification.
Association and reassociation primitives could lose their derived type during dispatch, while detailed STA management lacked one explicit owner for pending targets, request subtypes, timers, and late responses. This made terminal confirmation and teardown behavior depend on incomplete transaction context. Preserve the concrete primitive type and model association and reassociation as correlated transactions. Track the pending AP and subtype, select the matching confirmation, reject stale or mismatched responses, and clear timers and pending state on every success, failure, timeout, restart, and teardown path. The focused primitive-dispatch test covers subtype preservation, correlation, confirmation selection, late responses, timeout, failure, and restart safety.
Stations could not consistently recover HT state from serialized discovery frames or distinguish a genuinely legacy AP from malformed or incompatible HT signalling. Installing peer state from local or incomplete information would make association results and later rate selection unreliable. Advertise typed HT capabilities and HT Operation state in discovery and association exchanges, recover the primary channel from the received wire elements, and classify absent, valid, and invalid HT responses explicitly. Build compatible requests and install negotiated peer state only when the correlated association or reassociation transaction succeeds. Focused unit and module tests cover serialized discovery, legacy fallback, invalid HT classification, ordinary and forced-RTS association, and negotiated peer-state visibility at completion.
Rate selection did not have an authoritative way to restrict individually addressed transmissions to modes supported by both the local mode set and the negotiated peer state. A nominally fast mode could therefore violate the peer's MCS, channel-width, HT Operation, or short-GI constraints. Add a deterministic peer-mode selector that intersects the exact negotiated MCS and bandwidth state with local mode membership and guard-interval support. Wire the MIB into DCF and HCF rate selection, and fall back to the mode set's fastest mandatory legacy operational mode when HT is unavailable or invalid. Focused tests cover sparse MCS sets, 20/40 MHz operation, short GI, local-mode membership, deterministic tie breaking, malformed state, and legacy fallback.
Stop, crash, restart, destruction, disassociation, and deauthentication did not share one cleanup policy. Scan timers, pending transactions, or negotiated peer state could survive a lifecycle transition, and a frame from a pending or unrelated peer could affect the current association. Centralize detailed STA timer, transaction, association, and peer-state cleanup and distinguish current and pending peers using the transmitter identity. Apply matching initialization and lifecycle ownership to simplified STA management, including AP-side peer cleanup, while retaining its reduced state machine. Focused module tests cover initialization, restart, stop, crash, destruction, scan cancellation, same- and different-peer teardown, and discovery interaction.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Disassociation commands for any noncurrent address unconditionally cleared the station's pending association timer. An agent request for an unrelated peer could therefore abort an association or reassociation to a different AP, leaving the eventual target response to be rejected as late. Correlate cancellation with the AP stored in the pending timer context while retaining the existing current-AP teardown and Disassociation frame transmission paths. Extend the focused station-management module test to cover unrelated and matching targets for both association and reassociation, successful continuation, cancellation, transmitted destinations, and late-response rejection.
Update the 37 fingerprint baselines produced by the IEEE 802.11 behavior changes on this branch: 17 example rows, 16 showcase rows, and 4 tutorial rows. Thirty detailed-infrastructure scenarios change because legacy g(mixed) management frames now encode the complete operational rate set correctly. Mandatory rates are marked basic, 24 Mbps is no longer omitted, and optional rates beyond the first eight are carried in Extended Supported Rates. The resulting Beacon, Probe, and Association frame bodies and lengths change the event and packet trajectory. The new association transaction lifecycle can also change later events by exposing AP/STA state only after a successfully acknowledged response. These legacy configurations do not emit the newly modeled HT management elements. Five ad-hoc HCF scenarios change because TXOP selection now classifies g(mixed) through its ERP operating PHY instead of its slowest mandatory DSSS mode. This changes the VI limit from 6.016 ms to 3.008 ms and the VO limit from 3.264 ms to 1.504 ms, affecting aggregation, fragmentation grouping, backoff, and subsequent transmission timing. The wireless TXOP showcase changes because peer-aware HT rate selection falls back to the 24 Mbps legacy operational rate when ad-hoc peers have no negotiated HT state. The 802.11ac Ping1 trajectory changes because the VHT PHY receive-start delay is corrected from 33 us to 24 us, moving response timeout scheduling while leaving its network-layer fingerprint unchanged. All 37 new values exactly match the calculated fingerprints from the full debug fingerprint run. A clean debug build of upstream/master at f07d0e7 reproduced the old checked-in hashes for one representative from each causal group, ruling out stale baselines or build contamination. The pre-existing expected 5 Gbps half-duplex Ethernet ERROR remains unchanged.
Reassociation tunes the station radio to the target access point. When a different-target attempt was refused or timed out, the existing association was intentionally retained, but the radio remained on the rejected target's channel and could no longer exchange traffic with the current AP. Restore assocAP.channel from the shared reassociation failure handler whenever the previous association is still active. Preserve the existing same-target disassociation behavior and avoid retuning after the old association has already been cleared. Extend Ieee80211MgmtStaDiscovery_1 to exercise production reassociation refusal and timeout handling. Assert the target-to-current channel transition, retained association, beacon timer and current peer HT state, target-state cleanup, transaction cleanup, and confirmation result codes. Pin the test seed for reproducibility.
levy
reviewed
Aug 31, 2026
levy
left a comment
levy
left a comment
Contributor
There was a problem hiding this comment.
Let's see how the reviewing process works.
| }; | ||
|
|
||
| template<typename Frame> | ||
| class Ieee80211TypedMgmtFrameSerializer : public Ieee80211MgmtFrameSerializer |
Contributor
There was a problem hiding this comment.
why do we need to add template parameters to serializers? I think we should not, INET did work without template parameterized header serializers in the past. There should be a very good reason to introduce this.
Contributor
Author
There was a problem hiding this comment.
Codex response follows. In summary: eliminate through a cleanup. Tell me what you think about it.
The template is useful here, but only as an adapter around a limitation in FieldsChunkSerializer.
Chunk::deserialize() already knows and passes the requested concrete type (Chunk.cc). However, FieldsChunkSerializer discards that typeInfo and invokes a protected overload without it (FieldsChunkSerializer.cc).
Consequently, the management-frame body cannot tell whether it should produce an Association Request, Beacon, Probe Response, etc. The template binds that missing information into each registered serializer:
Ieee80211TypedMgmtFrameSerializer<Ieee80211BeaconFrame>Its advantages are:
- preserves the requested concrete frame subtype;
- shares all actual serialization/deserialization code;
- requires no mutable type field or constructor argument;
- adds only a tiny compile-time adapter per registered subtype.
It is not providing generic serialization logic—the implementation still dispatches through type_info in Ieee80211MgmtFrameSerializer.cc. It also is not completely type-safe: someone could accidentally register FrameA with Ieee80211TypedMgmtFrameSerializer<FrameB>.
My recommendation:
- For this focused branch, keep the template. The local non-template alternatives require duplicated subclasses, stored runtime type state, or duplicated
FieldsChunkSerializerbookkeeping, all of which are worse. - Long-term, eliminate it through a framework-level cleanup: change
FieldsChunkSerializerso its protected deserialization hook receives thetypeInfoalready present in the publicChunkSerializercontract (ChunkSerializer.h). Then all management types could register the same non-templateIeee80211MgmtFrameSerializer.
So: avoiding the template would produce a cleaner final API, but only if the underlying FieldsChunkSerializer interface is corrected. Avoiding it solely inside this serializer would not be an improvement.
levy
reviewed
Aug 31, 2026
levy
reviewed
Aug 31, 2026
levy
left a comment
levy
left a comment
Contributor
There was a problem hiding this comment.
I think the new packet drop signaling mechanism is redundant with the existing one. See PacketDropReason and it's usage in Simsignals.msg.
It feels like this patch re-invents something that's already there: modules signal dropping a packet along with its reason: queue is full, retry limit reached, etc
if we need a new reason we should simply add it.
|
|
||
| virtual void enqueuePacket(Packet *packet) override; | ||
| virtual Packet *dequeuePacket() override; | ||
| virtual void setPacketDropCallback(IPacketQueue::ICallback *callback) override { packetDropCallback = callback; } |
Contributor
There was a problem hiding this comment.
INET already has a mechanism for signaling packet drops. When a packet is dropped, the module emits a packetDroppedSignal, see dropPacket call in PacketQueue, and also PacketProcessorBase::dropPacket. This new mechanism seems to be redundant for me and re-inventing something that already exists. Why don't we use the existing mechanims? Statistics and other model behavior already depends on packets being dropped, so it should be signalled by the model anyway.
Contributor
Author
There was a problem hiding this comment.
I don't remember if this is the case, but usually this kind of "redundant" new mechanisms are introduced by the AI agent not on its first implementation but later on to solve bugs/flags signaled by Devin. I'm reviewing if we can avoid introducing this new redundant packet drop signaling mechanism
Contributor
Author
There was a problem hiding this comment.
Codex response (LGTM, tell me what you think):
Verdict: accept [Levy’s comment](#1144 (review)). It is correct in substance.
PacketDropReasonalready definesQUEUE_OVERFLOWandRETRY_LIMIT_REACHEDin Simsignals.msg.PacketProcessorBase::dropPacket()emitspacketDroppedSignalbefore deleting the packet, so listeners can synchronously inspect the live packet and transaction tag. The callback’s lifetime rationale therefore does not distinguish it from the signal: PacketProcessorBase.cc.- The branch currently invokes the new callback and then immediately emits the existing signal through
dropPacket(): PacketQueue.cc. This is duplicate notification. - The callback carries less information, supports only one consumer, and adds a pure virtual method to the generic
IPacketQueuecontract. - OMNeT++ signals propagate to listeners on ancestor components.
CompoundPacketQueueBasealready relies on this behavior to observe drops from child queues: CompoundPacketQueueBase.cc.
Recommended resolution:
- Remove
IPacketQueue::ICallback,setPacketDropCallback(),notifyPacketDropped(), and their queue/buffer propagation machinery. - Have DCF/HCF subscribe to
packetDroppedSignalon their pending-queue components and handle tagged management frames inreceiveSignal(). - Keep
IFrameTransmissionCallback: it represents the higher-level terminal management outcome, including successful ACK completion, so it is not equivalent to a drop signal. - Do not add another
PacketDropReason. Queue overflow already describes the cause;DROPPED_BEFORE_TRANSMISSIONis the management transaction’s derived outcome.
The only caveat is an externally configured PacketBuffer outside the queue subtree: its drop signal originates from the buffer. Current 802.11 pending queues do not use such buffers, so the branch’s generic external-buffer callback and test are unnecessary scope for this fix. Supporting that configuration should be a separate signal-routing change.
This review does not require a change in Ieee80211MgmtSta.cc; the affected path is the AP response transaction through DCF/HCF pending queues.
The enum values (OFDM, HR_DSSS, ERP, HT, VHT) identify a PHY specification/generation, not an "operating" role, and the bare Phy name suggested a PHY module rather than a type tag. Renamed the enum, field, and accessor to PhyType/phyType/getPhyType().
levy
reviewed
Aug 31, 2026
levy
left a comment
levy
left a comment
Contributor
There was a problem hiding this comment.
New comments related to packet drop signalling.
Contributor
There was a problem hiding this comment.
I think this file should not exist, because it's whole functionality is covered by sending signals about dropped packets.
| * The packet is never nullptr. | ||
| */ | ||
| virtual void handlePacketRemoved(Packet *packet) = 0; | ||
|
|
Contributor
There was a problem hiding this comment.
This should not be here for the same packet dropped signal reason.
| */ | ||
| class INET_API IPacketQueue : public virtual IPacketCollection, public virtual IPassivePacketSink, public virtual IPassivePacketSource | ||
| { | ||
| public: |
Contributor
There was a problem hiding this comment.
This should not be here for the same packet dropped signal reason.
Complete the built-in n(mixed-2.4Ghz) profile with every MCS 0 through 31 combination for 20 and 40 MHz operation using both long and short guard intervals. HT capabilities advertise the MCS bitmap, supported channel widths, and short-GI support independently, so the selectable mode catalog must contain their full Cartesian product. Add the missing long-GI modes for MCS 8 through 31 at 20 MHz and MCS 0 through 31 at 40 MHz. Also add the reverse missing short-GI modes for MCS 0 through 7 at 20 MHz while keeping only long-GI 20 MHz MCS 0 through 7 mandatory. Extend the HT mode-set and peer-mode-selection tests to verify all 128 exact tuples, mandatory-mode boundaries, both former mismatch directions, capability gating, fallback behavior, and deterministic selection.
Contributor
Author
Remove the temporary IPacketQueue drop callback API and use the packetDroppedSignal already emitted by queue processors. DCF and HCF now subscribe to their pending queues and translate tagged management frame drops into DROPPED_BEFORE_TRANSMISSION, while IFrameTransmissionCallback continues to report the full terminal outcome including acknowledgment and retry-limit exhaustion. Preserve the compound queue's ownership transfer before deleting a dropped child packet, restore PacketBuffer's original removal notification, and update DCF/HCF tests to validate behavior instead of callback internals. Remove the obsolete external PacketBuffer callback test. Verified with a debug build and focused DCF, HCF, retry-limit, timeout, and management transaction tests.
levy
added a commit
that referenced
this pull request
Sep 1, 2026
levy
added a commit
that referenced
this pull request
Sep 1, 2026
The first output of the change-summary tool, applied to a real pull request. It states what the change does to the interface — 100 additions, 5 changed, 1 signature changed, nothing removed — where the diff is 92 files and 9258 lines. It sits beside pr-1144.md, and the two answer different questions: that one judges the commits against the PR-* rules, this one states what the change does to the things other people depend on. The report is generated and carries "do not edit" in its header. The process behind it is opp_repl/plan/pending/change-summary.md.
levy
added a commit
that referenced
this pull request
Sep 1, 2026
The report now states what changed rather than the state before and after: base classes as what joined, const as gained, and the constructor as the three arguments it gained.
levy
added a commit
that referenced
this pull request
Sep 1, 2026
The branch was rebased since the 2026-08-31 audit: head 42d0174 became 14725f1, the merge base moved to e385662, and a WHATSNEW commit was added. A re-audit rewrites the report, as audit/README.md says. The verdict is PASS with 3 findings, and two of the rules are passed in an exemplary way. Commit 1 adds a shared networklayer capability in its own commit, before the 802.11 commit that needs it, with a message that never says "802.11" -- which is PR-SPLIT-UPSTREAM passed on the first reading, and the rule that #1144 broke. The new module test states both its measurement and what the old behavior produced instead. F-1 is the one that blocks the merge. Commit 3 seeds the rate control interval timer at first use rather than at t=0, which is a trajectory change for any scenario whose first transmission is not at t=0. Three checked-in fingerprints run AarfRateControl, and the branch touches no baseline and says nothing about them. Either outcome is fine; the series has to state which. The change summary is generated beside it. It found the contract change by itself: IRateControl::getRate gained a MacAddress argument, and both implementations with it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Validation
make MODE=debug -j$(nproc)Fingerprint checks and updates were intentionally deferred to the last commit of the PR. After making that commit, Devin signaled a new issue, and we took care of it. That new issue had nothing to do with the fingerprint update: it was already present but for some reason Devin did not catch it before.