ieee80211: add AirtimeFairnessQueue, a per-station airtime-fair transmit queue - #1123
ieee80211: add AirtimeFairnessQueue, a per-station airtime-fair transmit queue#1123adamgeorge309 wants to merge 14 commits into
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if (!fairnessEnabled || gate->getDeficit() >= SIMTIME_ZERO) { | ||
| // Eligible -> serve this station. Airtime-fair keeps the cursor on it so it | ||
| // drains its whole airtime quantum (many small frames) before yielding; the | ||
| // async airtime charge lands before the next pull and eventually closes its | ||
| // gate, at which point it is topped up and rotated. Frame-fair rotates now. | ||
| cursor = fairnessEnabled ? index : (index + 1) % n; |
There was a problem hiding this comment.
🔴 Broadcast and multicast traffic can permanently monopolize the transmit queue and starve all client stations
The station being served keeps its turn indefinitely (cursor = fairnessEnabled ? index : (index + 1) % n; at src/inet/linklayer/ieee80211/mac/queue/AirtimeFairnessScheduler.cc:72) until it is billed for the airtime it used, but group-addressed traffic is never billed, so a steady stream of broadcast/multicast frames blocks every other station forever.
Impact: While broadcast or multicast frames keep arriving, an access point stops sending anything to its individual clients.
Cursor pinning relies on an airtime charge that never arrives for group addresses
The deficit round robin deliberately leaves cursor on the served input so the station can drain its whole quantum; the comment states "the async airtime charge lands before the next pull and eventually closes its gate". Both coordination functions explicitly skip group addresses when reporting airtime (if (!receiver.isMulticast()) in src/inet/linklayer/ieee80211/mac/coordinationfunction/Dcf.cc:251 and src/inet/linklayer/ieee80211/mac/coordinationfunction/Hcf.cc:380), and MacAddress::isMulticast() is also true for the broadcast address.
Ieee80211ReceiverAddressClassifier (src/inet/linklayer/ieee80211/mac/queue/Ieee80211ReceiverAddressClassifier.cc:31) creates a per-receiver branch for the broadcast/multicast address just like for any unicast peer. That branch's gate therefore keeps deficit == 0 forever, is always eligible, and — because the cursor never moves off an eligible, backlogged input — schedulePacket() returns it on every pull for as long as it is backlogged. Unicast branches are never reached.
The same pinning also means any frame that is dequeued but never actually transmitted (e.g. dropped in the recovery procedure) leaves the cursor parked on that station.
Prompt for agents
AirtimeFairnessScheduler::schedulePacket() intentionally keeps the round-robin cursor on the station it just served (fairnessEnabled branch), relying on the asynchronous airtime charge from Dcf/Hcf to eventually drive that station's deficit negative and close its gate. However Dcf::transmissionComplete() and Hcf::transmissionComplete() deliberately do not report airtime for group-addressed (multicast/broadcast) receivers, while Ieee80211ReceiverAddressClassifier still allocates a per-receiver branch for the broadcast/multicast address. That branch's deficit stays at zero forever, so as long as it is backlogged the cursor never leaves it and all unicast stations are starved. Consider one of: charging airtime for group-addressed transmissions as well (attributing them to the group branch), routing all group-addressed frames to a branch that is not subject to the pinning behaviour, or bounding how long the cursor may stay on one gate (e.g. advance the cursor whenever the served gate's deficit did not decrease since it was selected).
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Fixed in 946a8f6198. The cursor now only stays on the served station if its deficit actually moved since the last time that station was served, which covers the unbilled group-address branch and the dequeued-but-never-transmitted case with one rule rather than special-casing group addresses.
Reproduced first: with a saturating multicast flow alongside the three unicast flows, all three stations received 0 packets over 2s while the AP put 7085 multicast frames on air. After the fix, 1508/1508/1506.
| double weight = default(1); // per-station airtime weight; equal weight means equal airtime share | ||
| bool fairnessEnabled = default(true); // when false, degrades to a plain per-station round robin (for OFF/ON contrast) | ||
| string subqueueTypename = default("inet.queueing.queue.PacketQueue"); // NED type of each per-station sub-queue | ||
| dropperClass = default("inet::ieee80211::Ieee80211LongestFlowDropper"); // on overflow, drop the tail frame of the longest per-station backlog |
There was a problem hiding this comment.
🟡 The new queue has no size limit by default, so its documented overload protection never runs
The queue ships with an overflow drop policy but no default limit on how many frames it may hold (dropperClass = default(...) at src/inet/linklayer/ieee80211/mac/queue/AirtimeFairnessQueue.ned:52 without a packetCapacity default), so it grows without bound and the drop-from-longest rule never triggers.
Impact: Replacing the standard pending queue silently removes its 100-frame limit, letting a saturated access point accumulate unbounded backlog and latency.
Inherited default is unlimited, unlike the queue it replaces
CompoundPacketQueueBase.ned:20 defines int packetCapacity = default(-1) (no limit), and AirtimeFairnessQueue.ned does not override it. CompoundPacketQueueBase::isOverloaded() returns false when packetCapacity == -1, so AirtimeFairnessCompoundQueue::pushPacket() never calls the dropper and Ieee80211LongestFlowDropper is dead code in the default configuration.
This contradicts the module documentation ("A shared packetCapacity is enforced across all stations with a drop-from-longest overflow policy ... so a slow station's backlog cannot lock the others out"). The module it is meant to replace, PendingQueue.ned:24, defaults to packetCapacity = 100, so a typename override changes an bounded queue into an unbounded one.
| dropperClass = default("inet::ieee80211::Ieee80211LongestFlowDropper"); // on overflow, drop the tail frame of the longest per-station backlog | |
| packetCapacity = default(100); // shared capacity across all per-station sub-queues | |
| dropperClass = default("inet::ieee80211::Ieee80211LongestFlowDropper"); // on overflow, drop the tail frame of the longest per-station backlog |
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Fixed in bb0854334a — packetCapacity = default(100), matching PendingQueue, which this queue replaces by typename override.
|
This one seems to be mostly ok, except for the addInput method on the IDynamicInputScheduler, should use a signal listener on gate connected. I'm not sure about the frameTransmittedAirtime signal, is subscribing happens on the same node that is emitting the signal? |
|
On the Checked on the |
a1a0db5 to
bb08543
Compare
|
The It is folded into Two more changes, from the Devin findings:
One thing to flag for the baselines: this queue's fingerprint moves from Still open, not addressed here: the branch submodules spliced in by |
|
The splice vector naming defect is fixed in the base PR (#1122, What is still open there, and worth knowing for this queue: an ini assignment naming a spliced submodule by its final path is silently ignored, because parameters are finalized under the temporary name as well -- so |
bb08543 to
92bfcdb
Compare
2ad8345 to
47ba427
Compare
…ssifyPacket Invert the class lookup into an early return, so that the block creating the branch of a first-seen class sits at function level instead of inside the conditional. Whitespace-only except for the inverted condition and the hoisted return -- review with a whitespace-ignoring diff. No change in behavior. This puts the block in position for the next commit to move it out verbatim.
Extract-function move: the block that builds a branch -- grows the submodule vector, creates the module, wires it between the classifier and the multiplexer, and initializes it -- becomes createBranch(), the lines byte-identical (review with --color-moved). The class-to-branch map entry stays at the call site, fed by the return value: the map is classification bookkeeping, and createBranch() is topology only. No change in behavior. classifyPacket() reads as what it is: look the class up, create its branch on first sight.
~DynamicClassifier could only wire a branch into a submodule literally named "multiplexer". The downstream aggregator is now named by the aggregatorSubmoduleName parameter (still "multiplexer" by default), and it may be a pull scheduler instead of a push multiplexer: an aggregator that has to take notice of an input appearing at runtime learns about it from the POST_MODEL_CHANGE notification of the connection being made (cPostPathCreateNotification), so no contract is needed between the classifier and the aggregator beyond wiring the gate. For the pull side the classifier now also takes a collector reference per branch, the way it already took a consumer reference for the push side. The missing-submodule-vector and missing-aggregator cases fail with a clear error naming the module instead of a null dereference.
… wired Branch modules were initialized right after being built, before the branch was connected to the aggregator, and the classifier took its sink references on its new out gate while the far end of the path was still incomplete. Both are traps for a compound branch: a module that resolves its downstream peer in initialize() would see a dangling gate, and ModuleRefByGate::reference() resolves the peer eagerly by walking the connection -- with mandatory=false it silently stores a nullptr that nothing ever re-resolves, leaving a permanently null consumer whose canPushPacket() throws and whose pushPacket() quietly degrades to send(), bypassing back-pressure. Wire first, resolve and initialize after: createModuleBranch() builds the branch module (with its final name and index, so its parameters, display string and result recording are all resolved for the module path it keeps) and defers its initialization; createBranch() connects the chain up to and including the aggregator, then takes the references and runs the deferred initializations. No change in behavior for the existing simple-branch users, where the old order happened to be safe.
… indices The class-to-branch map was keyed on the result of PacketClassifier::classifyPacket(), which maps the classifier function's index through getOutputGateIndex(). With reverseOrder that mapping is relative to the current number of output gates -- which grows with each branch created -- so the same class would be looked up under a different key later, miss, and get a second branch. Key the map on the classifier function's index directly, taken through the new getClassIndex(), which classifies without the branch-creating side effect of classifyPacket().
canPushSomePacket() is inherited as "one of the existing branches can take a packet", which is false for a classifier that has not built any branch yet. An active source in front of such a classifier stops, waits for the notification that would tell it packets can be pushed again, and never gets it, because nothing else creates the first branch. Answer true instead: a packet of a class that has not been seen yet is taken by the branch created for it, and the range of the classifier function is not known here, so there may always be such a class. canPushPacket() is worse than useless in its inherited form here: it classifies the packet, and for this classifier classifying creates the branch of a new class, so a query that is supposed to be a query builds submodules, grows gate vectors, wires connections and initializes the new modules. Look the class up instead, and only delegate to the branch that already exists. (The pull side classifies in canPullPacket() too, and is left alone: there the query is what drives branch creation, and this classifier has no pull user.) The module test covers this -- the producer is connected to the classifier directly, so without the fix it never produces and no branch is built -- along with the rest of the contract: two branches built on demand, an ini file assignment addressing a submodule of a branch taking effect, and the statistics of the branch submodules being recorded under the branch path.
cc81132 to
6916dc3
Compare
47ba427 to
ad7b15b
Compare
…classification classifyPacket() is a query: the capacity checks (canPushPacket(), canPullPacket()) classify the very packet whose delivery classifies it again, and the pull path classifies it on every peek, so classification must be free of side effects. The contract is now spelled out, together with the convention -- already followed by PriorityClassifier, WrrClassifier and the diffserv classifiers -- that -1 means "no existing output gate suits this packet" rather than an out-of-range error. What happens to such a packet is decided on the delivery path only: pushPacket() and startPacketStreaming() ask the new createGateForPacket() hook, whose default refuses the packet just as the range check did, and canPushPacket() asks its side-effect-free query pair, canCreateGateForPacket(). A classifier that extends itself on demand creates its new output gate in the hook, never from a query. The pull-side callers pass -1 through unchanged: canPullPacket() already answers "not for this gate", the pullPacket() family already refuses, and handleCanPullPacketChanged() now skips the notification instead of indexing collectors[-1]. The stateful classifyPacket() implementations that remain (WrrClassifier, TokenBucketClassifier, MultiTokenBucketClassifier) are out of scope here; they are why callClassifyPacket() keeps its const_cast kludge.
classifyPacket() now only answers where a packet of an already-seen class goes, and -1 for a class that has no branch yet; the branch is created in createGateForPacket(), which the base class calls on packet delivery only. The canPushPacket() override disappears: the inherited implementation, with canCreateGateForPacket() answering that a branch can be created for every class, gives the same answers -- an unseen class is always welcome, a seen one asks its branch.
…atistics On overflow, CompoundPacketQueueBase removed the victim packet with its own removePacket(), which emits packetRemoved, and then dropped it with dropPacket(), which emits packetDropped. The queueLength statistic subtracts both signals, so every overflow drop was subtracted twice and the recorded queue length drifted downwards. The victim was also still owned by the submodule it was queued in, so the delete inside dropPacket() warned about deleting an object owned by another module. Remove the victim directly from the underlying collection -- the submodule it leaves emits its own packetRemoved, which the compound's localSignal() statistics rightly ignore -- and take it before the delete. Affects every compound queue configured with a dropperClass.
6916dc3 to
f706e92
Compare
Adds an Ieee80211AirtimeInd-carrying frameTransmittedAirtime signal, emitted by ~Dcf and ~Hcf from transmissionComplete() for every unicast data/mgmt frame, naming the receiver and the on-air duration. The duration is computed from the mode actually selected for the frame and the frame length, so it is exact rather than estimated, and because it is emitted per completed transmission rather than per queued frame, retransmissions are reported individually. Control frames and group-addressed frames are not reported: they are not attributable to a single peer. This is the input an airtime-fair transmit scheduler needs in order to charge a station for what it actually consumed on the medium.
…duler The two halves of a per-station airtime-fair transmit scheduler, modelled on the Linux mac80211 airtime fairness feature. They form a matched pair: ~AirtimeFairnessGate sits on one station's transmit path and owns that station's airtime deficit and its open/closed eligibility. It learns which receiver it serves from the first frame that passes through it, and subscribes to frameTransmittedAirtime on the containing network interface to charge the deficit from the frame's actual on-air time. This mirrors the way a ~PeriodicGate gates one sub-queue in a ~GatingPriorityQueue, except that eligibility is driven by consumed airtime instead of by a schedule. ~AirtimeFairnessScheduler owns the rotation: it visits the gates in round-robin order, serves the first backlogged station whose gate is still open, and grants an ineligible-but-backlogged station one quantum * weight of credit before moving on. In fair mode the cursor stays on the served station so it drains its whole airtime quantum -- which is what distinguishes airtime fairness from frame fairness when one station's frames take much longer on the air. It stays there only while the station's deficit actually moves between visits, though: a station that is never charged -- a group-address branch, group frames being deliberately unreported since they are not attributable to a single peer, or a station whose dequeued frame was dropped instead of transmitted -- would otherwise park the cursor and starve every other station. A station that is not being billed gets frame fairness rather than the whole medium. Two consequences of gating a queue this way needed explicit handling. A closed gate would hide its backlog from the generic pull interface, so the gate reports the true upstream backlog through ~IPacketCollection regardless of its state, and forwards backlog-change notifications even while closed -- a station can become backlogged while out of credit, and the scheduler must still learn about it in order to top it up. For the same reason the scheduler reports a pullable packet whenever any station is backlogged, even when every gate is momentarily shut, since it can always grant credit. Stations come and go, so the gates are wired to the scheduler as the receivers first appear (by the ~DynamicClassifier of the enclosing queue), not from the NED topology. The scheduler picks such an input up by listening for the POST_MODEL_CHANGE notification of its own input gate being connected, so nothing has to call into it and no contract is needed between it and whatever builds the branch; it finds the airtime gate of the new branch at the far end of the connection path arriving at its input, which may cross a branch compound's boundary. It deliberately does not notify its downstream collector at that point: the branch is still empty and its modules are not initialized yet, and the frame whose arrival created the station notifies through the branch a moment later anyway. With fairnessEnabled = false the gates stay open and the pair degrades to a plain per-station round robin, which is the frame-fair baseline to contrast against.
…policy Two small registered functions that an airtime-fair queue needs to divide traffic into per-station branches and to stay fair under overload. Ieee80211ReceiverAddressClassifier assigns a dense class index per receiver MAC address in first-seen order, so a dynamic classifier can open one branch per destination station. Ieee80211LongestFlowDropper is the overflow policy: it drops the tail frame of the station with the most queued frames, rather than the frame that has just arrived. Under a shared capacity this matters -- a slow station drains slowly and would otherwise fill the whole queue and lock the other stations out, defeating the fairness the scheduler provides. It is the drop-from-longest rule used by FQ-CoDel, for the same reason.
…mit queue Assembles the parts into a drop-in replacement for the pendingQueue of an ~Edcaf or ~Dcaf: a ~DynamicClassifier routes each frame to a per-receiver sub-queue, each sub-queue is followed by an ~AirtimeFairnessGate, and an ~AirtimeFairnessScheduler serves the gates so that every backlogged station gets an equal share of on-air time rather than an equal share of frames. This addresses the downlink form of the 802.11 rate anomaly: when an access point saturates a mix of fast and slow clients, a FIFO -- or even a frame-fair round robin -- lets the slow client's long frames drag the fast clients down toward its throughput, because a frame is a frame regardless of how long it occupies the medium. The per-station branches are created on demand as receivers appear, so the queue follows the set of stations an access point actually serves instead of requiring it to be declared up front. A branch is one ~PerStationAirtimeQueue compound (sub-queue -> gate) in the branch submodule vector; the scheduler finds the airtime gate of a branch at the far end of the connection path arriving at its input gate, across the compound boundary. The branch is created with its final name and index, so a single station can also be configured individually, as in branch[2].weight = 2, and its statistics are recorded under pendingQueue.branch[k].queue and pendingQueue.branch[k].gate. packetCapacity defaults to 100, the same limit as the ~PendingQueue this queue replaces: ~CompoundPacketQueueBase defaults it to -1, no limit, which would silently turn a 100-frame queue into an unbounded one where isOverloaded() never fires and the shared-capacity overflow policy is dead code. ~AirtimeFairnessCompoundQueue only publishes the number of per-station branches created so far as a numStations watch, for the queue's display string. The module test drives the airtime disparity with frame sizes rather than rates, which keeps it independent of per-receiver rate configuration: the access point saturates one station with long frames and two with short ones, all at 54 Mbps, and the short-frame stations must recover to well over 0.25x the long-frame station's bytes (frame fairness leaves them at 0.14x). The quantum is set fine enough (100us) for the deficit to bind on every frame, because the coordination function pulls the next frame before the previous frame's airtime is reported, so at the default quantum a whole burst passes between charges and the schedule degrades toward frame fairness.
While the branch was flattened into the queue, the airtime deficit of a station
and the frames waiting for it were written above the gate and the sub-queue
directly. Now that a branch is a compound module, put the same information on
the branch itself: PerStationAirtimeQueue extends Module and states its
displayStringTextFormat, with the ModuleMixin expression resolver reading the
state from the submodules -- {.gate.deficit} for the deficit held by the gate,
{.queue.numPackets} and {.queue.totalLength} for the frames waiting in the
sub-queue.
The submodule references need the leading dot: getModuleByPath() takes a bare
`gate.deficit` as an absolute path in OMNeT++ 6, so it throws instead of
finding the gate submodule, and that would break every refreshDisplay.
f706e92 to
ff3907c
Compare
2f94d75 to
55d4626
Compare
2 participants
Adds
AirtimeFairnessQueue, a per-station airtime deficit round-robin transmit queue for IEEE 802.11, modelled on the Linuxmac80211airtime fairness feature. It drops into thependingQueueslot of anEdcaforDcafvia a typename override.Why. This addresses the downlink form of the 802.11 rate anomaly: when an access point saturates a mix of fast and slow clients, a FIFO — or even a frame-fair round robin — lets the slow client's long frames drag the fast clients down toward its throughput, because a frame is a frame regardless of how long it occupies the medium.
Structure. A
DynamicClassifierroutes each frame to a per-receiver sub-queue, each sub-queue is followed by anAirtimeFairnessGate, and anAirtimeFairnessSchedulerserves the gates. The gate owns one station's airtime deficit and its open/closed eligibility; the scheduler owns the rotation and the top-up trigger. Per-station branches are created on demand as receivers appear, so the queue follows the set of stations an access point actually serves.Airtime accounting.
Dcf/Hcfemit a newframeTransmittedAirtimesignal carrying anIeee80211AirtimeInd(receiver + on-air duration) for every unicast data/mgmt frame. The duration comes from the mode actually selected and the frame length, so it is exact; because it is emitted per completed transmission rather than per queued frame, retransmissions are charged individually.Overload. A shared
packetCapacityis enforced with a drop-from-longest policy (Ieee80211LongestFlowDropper, the FQ-CoDel rule), so a slow station's slowly-draining backlog cannot fill the queue and lock the others out.With
fairnessEnabled = falsethe gates stay open and the queue degrades to a plain per-station round robin — the frame-fair baseline to contrast against.AirtimeFairnessCompoundQueueexists only to correctCompoundPacketQueueBase's shared-capacity overflow path for a sub-queue-based compound: the base class emits bothpacketRemovedandpacketDroppedfor the victim, so it is subtracted twice from the queue-length statistic, and the frame stays owned by its sub-queue so deleting it warns.Based on
topic/gy/queueing-dynamic-classifier— please review/merge that first.Test
Builds at every commit. Instantiated and run in
examples/wireless/hiddennodewith**.mac.dcf.channelAccess.pendingQueue.typename = "AirtimeFairnessQueue"— 38k events over 2 simulated seconds, exercising dynamic branch creation, submodule splicing, and the gate/scheduler pair. Inert unless explicitly configured.🤖 Generated with Claude Code