queueing: let DynamicClassifier accept a packet before it has any branch

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.)

Classification for the map is taken directly from the classifier function
rather than through PacketClassifier::classifyPacket(), whose reverseOrder
mapping is relative to the current number of output gates -- which grows with
each branch, so the same class would be looked up under a different key later
and get a second branch.

The module test no longer needs the BackPressureBarrier that hid the missing
back pressure answer, so it now covers this as well: without the fix the
producer never produces and no branch is built.

Author: adamgeorge309

src/inet/queueing/classifier/DynamicClassifier.cc

@@ -32,9 +32,19 @@ void DynamicClassifier::initialize(int stage)
     }
 }
 
+int DynamicClassifier::getClassIndex(Packet *packet) const
+{
+    // the class of the packet, with no side effect -- unlike classifyPacket() below, which
+    // creates the branch of a class that is seen for the first time. Note that the class index
+    // is taken as it is, and not mapped through getOutputGateIndex(): that mapping depends on
+    // the number of output gates, which grows with each branch, so the same class would end up
+    // under a different key over time, and get a second branch.
+    return packetClassifierFunction->classifyPacket(packet);
+}
+
 int DynamicClassifier::classifyPacket(Packet *packet)
 {
-    int index = PacketClassifier::classifyPacket(packet);
+    int index = getClassIndex(packet);
     auto it = classIndexToGateItMap.find(index);
     if (it != classIndexToGateItMap.end())
         return it->second;
@@ -43,6 +53,25 @@ int DynamicClassifier::classifyPacket(Packet *packet)
     return branchIndex;
 }
 
+bool DynamicClassifier::canPushSomePacket(const cGate *gate) const
+{
+    // Not the inherited "one of the existing branches can take a packet": a packet of a class
+    // that has not been seen yet is taken by the branch created for it, and there may always be
+    // such a class, the range of the classifier function not being known here. Without this, a
+    // classifier that has no branch yet answers that it cannot accept anything, and an active
+    // source in front of it stops before the first branch is ever created. Whether a particular
+    // packet can be pushed is answered by canPushPacket() below.
+    return true;
+}
+
+bool DynamicClassifier::canPushPacket(Packet *packet, const cGate *gate) const
+{
+    // deliberately not the inherited implementation: that one classifies the packet, which
+    // creates the branch of a new class as a side effect of what is supposed to be a query
+    auto it = classIndexToGateItMap.find(getClassIndex(packet));
+    return it == classIndexToGateItMap.end() || consumers[it->second].canPushPacket(packet);
+}
+
 int DynamicClassifier::createBranch()
 {
     cModule *parent = getParentModule();

src/inet/queueing/classifier/DynamicClassifier.h

@@ -47,6 +47,7 @@ class INET_API DynamicClassifier : public PacketClassifier
 
   protected:
     virtual void initialize(int stage) override;
+    virtual int getClassIndex(Packet *packet) const;
     virtual int classifyPacket(Packet *packet) override;
 
     virtual int createBranch();
@@ -55,6 +56,10 @@ class INET_API DynamicClassifier : public PacketClassifier
     virtual void forwardMatchingParams(cModule *module);
     virtual cGate *createModuleBranch(int index, cGate *classifierOutputGate, std::vector<cModule *>& modulesToInitialize);
     virtual cGate *spliceBranch(int index, cGate *classifierOutputGate, std::vector<cModule *>& modulesToInitialize);
+
+  public:
+    virtual bool canPushSomePacket(const cGate *gate) const override;
+    virtual bool canPushPacket(Packet *packet, const cGate *gate) const override;
 };
 
 } // namespace queueing

tests/queueing/DynamicClassifier_1.test

@@ -6,6 +6,10 @@ creates the branch of a class when the first packet of that class arrives. The b
 (TestBranch) is a linear chain of two packet flow modules (BackPressureBarriers), and it is
 spliced, so those are created directly in the classifier's parent, as first[k] and second[k].
 
+The producer is connected to the classifier directly, so the test also covers that a classifier
+which has no branch yet accepts a packet, rather than stopping the producer before the first
+branch is created.
+
 The test checks that the statistics of the spliced submodules are recorded under their final
 module path, and not under the temporary name the branch is built with. Empty output vectors
 are turned off, so a vector appears in the result file only if data was recorded into it.
@@ -67,20 +71,14 @@ network TestDynamicClassifier
         producer: ActivePacketSource {
             @display("p=100,100");
         }
-        // a classifier without any branch reports that it cannot accept a packet, which would
-        // stop the producer before the first branch is ever created
-        barrier: BackPressureBarrier {
-            @display("p=200,100");
-        }
         demultiplexer: TestDemultiplexer {
-            @display("p=300,100");
+            @display("p=200,100");
         }
         consumer: PassivePacketSink {
-            @display("p=400,100");
+            @display("p=300,100");
         }
     connections:
-        producer.out --> barrier.in;
-        barrier.out --> demultiplexer.in;
+        producer.out --> demultiplexer.in;
         demultiplexer.out --> consumer.in;
 }