queueing: generalize DynamicClassifier for pull-based per-class structures - #1122
queueing: generalize DynamicClassifier for pull-based per-class structures#1122adamgeorge309 wants to merge 2 commits into
Conversation
Schedulers that get their inputs wired up at runtime -- rather than from the NED topology -- need a way to be told about a newly connected input gate, so that they start considering it. ~DynamicClassifier creates and connects the gate; addInput() is how the scheduler learns about it. This makes it possible for a dynamic classifier to build pull-based per-class structures (queue + scheduler), not just push-based demux/remux chains terminating in a ~PacketMultiplexer.
~DynamicClassifier could only build one shape: create a submodule of the configured type in a preexisting submodule vector, and wire it to a submodule literally named "multiplexer". Three generalizations: - The downstream aggregator is now named by the aggregatorSubmoduleName parameter (still "multiplexer" by default). If it implements ~IDynamicInputScheduler, the newly created input gate is registered with it, so the aggregator can be a pull scheduler instead of a push multiplexer. - With spliceBranchSubmodules, a compound moduleType forming a linear in -> a -> b -> ... -> out chain is flattened: its inner submodules are reparented into the classifier's parent as vector elements named after them (a[k], b[k]), rather than staying behind the compound's boundary. A downstream matched-pair scheduler can then address them directly, which a compound boundary would prevent. - forwardMatchingParams() copies same-named parameters from the enclosing module into the created branch before it is finalized, so a per-class compound picks up the enclosing queue's configuration. Branch module initialization is deferred until the whole chain, including the aggregator connection, is wired -- a module that resolves its downstream peer in initialize() would otherwise see a dangling gate. submoduleName is now optional (it is unused when splicing), and the missing-submodule-vector and missing-aggregator cases fail with a clear error instead of a null dereference.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| outputGates.push_back(classifierOutputGate); | ||
| PassivePacketSinkRef consumer; | ||
| consumer.reference(classifierOutputGate, false); | ||
| consumers.push_back(consumer); | ||
| ActivePacketSinkRef collector; | ||
| collector.reference(classifierOutputGate, false); | ||
| collectors.push_back(collector); |
There was a problem hiding this comment.
🔴 Newly created traffic-class branches are never linked to the classifier, so packets of a new class break delivery
The downstream target of a newly added output is looked up (consumer.reference(...) at src/inet/queueing/classifier/DynamicClassifier.cc:56) before the branch is actually attached to that output, so the classifier ends up with no known destination for that class.
Impact: The first packet of every new traffic class either aborts the simulation with a null-reference error or silently falls back to a different delivery path, breaking normal flow control.
Reference resolution happens on a still-unconnected gate
ModuleRefByGate::reference(gate, false) (src/inet/common/ModuleRefByGate.h:80-89) resolves the peer immediately by walking gate->getNextGate() (src/inet/common/ModuleAccess.h:126-134). At DynamicClassifier.cc:55-60 the new out[index] gate has just been created by setGateSize() and is not connected yet -- the connection is only made later inside createModuleBranch() (src/inet/queueing/classifier/DynamicClassifier.cc:98) or spliceBranch() (src/inet/queueing/classifier/DynamicClassifier.cc:140). With mandatory == false the lookup silently yields nullptr, and the references are never re-resolved.
Consequences in PacketClassifierBase:
canPushSomePacket()/canPushPacket()(src/inet/queueing/base/PacketClassifierBase.cc:86-98) call intoconsumers[i], whosecheckReference()throws"Dereferencing nullptr...".pushPacket()usespushOrSendPacket()(src/inet/queueing/base/PacketProcessorBase.cc:118-126), which falls back tosend()when the consumer is null, bypassing the synchronous push API and back-pressure.
The previous implementation connected the gate first and only then created the reference, so it resolved correctly. The fix is to move the outputGates/consumers/collectors bookkeeping after the branch (and aggregator) wiring is done.
Prompt for agents
In DynamicClassifier::createBranch() (src/inet/queueing/classifier/DynamicClassifier.cc), the PassivePacketSinkRef and ActivePacketSinkRef for the new out[index] gate are resolved via reference(gate, false) immediately after setGateSize(), while the gate is still unconnected. ModuleRefByGate::reference() resolves the peer eagerly by following the connection, so with mandatory==false both refs become nullptr permanently. The gate is only connected later, in createModuleBranch()/spliceBranch(). This makes consumers[index] null, which makes PacketClassifierBase::canPushPacket()/canPushSomePacket() throw and makes pushPacket() fall back to send() instead of the push API. Restructure createBranch() so that outputGates/consumers/collectors are populated only after the branch has been created and classifierOutputGate has been connected to the branch input (the previous implementation connected first, then referenced). Note outputGates must stay index-aligned with consumers/collectors.
Was this helpful? React with 👍 or 👎 to provide feedback.
| cGate *DynamicClassifier::createModuleBranch(int index, cGate *classifierOutputGate, std::vector<cModule *>& modulesToInitialize) | ||
| { | ||
| cModule *parent = getParentModule(); | ||
| parent->setSubmoduleVectorSize(submoduleName, index + 1); |
There was a problem hiding this comment.
🟡 Creating the first per-class branch can delete pre-existing branch modules declared in the network description
The branch container is resized to exactly the new branch position (setSubmoduleVectorSize(submoduleName, index + 1) at src/inet/queueing/classifier/DynamicClassifier.cc:96) instead of only ever growing it, so any pre-existing branches beyond that position are destroyed.
Impact: Statically configured per-class branches can silently disappear at runtime, so traffic that should flow through them is lost or the run aborts.
Removal of the std::max() guard
The previous code deliberately used parentModule->setSubmoduleVectorSize(submoduleName, std::max(origVectorSize, submoduleIndex + 1)) so the vector was never shrunk. The new code passes index + 1 unconditionally. index is the classifier's current out gate count, which is not necessarily >= the NED-declared vector size (e.g. a parent declaring defragmenter[numDefragmenter] whose classifier out gate vector was sized independently). Shrinking an existing submodule vector deletes the elements above the new size. The same unguarded resize is repeated in the splice path at src/inet/queueing/classifier/DynamicClassifier.cc:133.
| parent->setSubmoduleVectorSize(submoduleName, index + 1); | |
| parent->setSubmoduleVectorSize(submoduleName, std::max(parent->getSubmoduleVectorSize(submoduleName), index + 1)); |
Was this helpful? React with 👍 or 👎 to provide feedback.
|
The IDynamicInputScheduler interface has no implementors, how does this work? What's the point of having this interface? Why doesn't the module use the signals emitted when a gate gets connected? |
2 participants
DynamicClassifiercould only build one shape: create a submodule of the configured type in a preexisting submodule vector, and wire it to a submodule literally namedmultiplexer. This generalizes it along three axes so it can also build pull-based per-class structures.aggregatorSubmoduleName(stillmultiplexerby default). If it implements the newIDynamicInputSchedulercontract, the runtime-created input gate is registered with it — so the aggregator can be a pull scheduler instead of a push multiplexer.spliceBranchSubmodules, a compoundmoduleTypeforming a linearin -> a -> b -> ... -> outchain is flattened: its inner submodules are reparented into the classifier's parent as vector elements named after them (a[k],b[k]). A downstream matched-pair scheduler can then address them directly, which a compound boundary would prevent.forwardMatchingParams()copies same-named parameters from the enclosing module into the created branch before it is finalized.Branch module initialization is deferred until the whole chain — including the aggregator connection — is wired, since a module that resolves its downstream peer in
initialize()would otherwise see a dangling gate.submoduleNamebecomes optional (unused when splicing), and the missing-vector / missing-aggregator cases now fail with a clear error instead of a null dereference.Behaviour of existing configurations is unchanged: the defaults reproduce the previous push-multiplexer shape.
This is the enabling change for the per-station airtime-fair 802.11 transmit queue, which is proposed separately on top of this branch.
Test
Builds at every commit. Existing
DynamicClassifierusers are unaffected by construction (defaults unchanged).🤖 Generated with Claude Code