queueing: report a RED early drop as congestion

[adamgeorge309]

RedDropper (Random Early Detection, RED) declares one statistic of its own, packetDropCongestion, which counts drops with reason CONGESTION, but it emitted every drop with OTHER_PACKET_DROP, so the statistic always read 0 while RED dropped packets. RedDropper now reports an early drop as CONGESTION and a drop at its packet capacity as QUEUE_OVERFLOW, as PacketQueue does for a full buffer.

Risk: only the reason attached to the packetDropped signal changes; which packets are dropped, and when, does not. The packetDropCongestion scalars and vectors of every RED queue that makes early drops, including the RED filters inside EthernetQosRedQueue and the Differentiated Services AFxyQueue, become non-zero, and any analysis that splits drops by reason counts those drops as CONGESTION instead of OTHER_PACKET_DROP. No fingerprint changes.

Closes #1327

One commit: queueing: fix: report a RED early drop as congestion.

The problem

RedDropper.ned:51 declares the statistic:

// src/inet/queueing/filter/RedDropper.ned:51
@statistic[packetDropCongestion](title="packet drop: congestion"; source=packetDropReasonIsCongestion(packetDropped); record=count,sum(packetBytes),vector(packetBytes); interpolationmode=none);

When RedDropper::matchesPacket() returns false, PacketFilterBase drops the packet through dropPacket(Packet *), which hard-codes the reason (src/inet/queueing/base/PacketFilterBase.cc:264-267):

void PacketFilterBase::dropPacket(Packet *packet)
{
    dropPacket(packet, OTHER_PACKET_DROP);
}

RedDropper did not override it. That overload was added for this purpose by 27c58038a9 ("to allow specifying the reason in derived classes"), and ProtocolChecker overrides it to report NO_PROTOCOL_FOUND. The statistic was introduced with a CONGESTION reason in 7b79379969, in the old queue package's AlgorithmicDropperBase::dropPacket(); when b63e2327d9 added the queueing package that replaced it, the @statistic line was carried over to the new RedDropper.ned and the reason was not.

The fix

RedDropper::dropPacket(Packet *) passes the reason from the result of the last RED decision (lastResult, stored by handlePacketFiltered(), which PacketFilterBase calls before each drop, at the start of a streamed packet):

• RANDOMLY_ABOVE_LIMIT and ABOVE_MAX_LIMIT, the early drops of a packet that is not marked with Explicit Congestion Notification (ECN): CONGESTION;
• QUEUE_FULL, the queue already holds packetCapacity packets: QUEUE_OVERFLOW, the reason PacketQueue, CompoundPacketQueueBase and PacketBuffer give for a full buffer. This keeps packetDropCongestion to the drops RED decides on, and departs from the old AlgorithmicDropperBase, which reported full-buffer drops as CONGESTION as well.
• any other result, which a drop does not normally follow: OTHER_PACKET_DROP, as before.

Architectural surface

RedDropper overrides one more virtual function, dropPacket(Packet *), declared in PacketFilterBase, and keeps one more protected member, lastResult. The one contract that changes is the reason in the PacketDropDetails that RedDropper attaches to packetDropped: CONGESTION or QUEUE_OVERFLOW instead of OTHER_PACKET_DROP. No Network Description (NED) parameter, statistic declaration, message field or feature descriptor changes.

Verification

Reproduction: client --100Mbps-- router --10Mbps-- server, IPv4, a RedDropperQueue on the router's 10 Mb/s interface (minth = 3, maxth = 4, maxp = 1, wq = 1, packetCapacity = 24), and User Datagram Protocol (UDP) traffic of 1000 B every 0.4 ms (20 Mb/s) from t=8 s to t=9 s. A second run adds a probe statistic to the RED module, @statistic[packetDropUndefined](source=packetDropReasonIsUndefined(packetDropped); record=count), which counts drops with reason OTHER_PACKET_DROP.

Scalar	Master	This change
router.eth[1].queue.red droppedPackets:count	1324	1324
router.eth[1].queue.red packetDropCongestion:count	0	1324
router.eth[1].queue.red packetDropUndefined:count (probe)	1324	0
server.app[0] packetReceived:count	1172	1172

The queue behind RED peaks at 4 packets, so all 1324 drops are early drops. tests/module/RedDropper_congestion_drop_reason.test runs the same network with 0.5 s of traffic, to keep the test short, and checks that packetDropCongestion:count is not 0; it fails on master and passes with this change.

Release build (make MODE=release in the root and tests/module/lib), each suite run on master 1cdb362a2a with the new test file added, and with this change, before the rebase onto 41eee1d619:

Command	Master	This change
tests/fingerprint: ./fingerprinttest -s -F tyf	1774 run, 5 failures, 63 errors	the same failing and erroring rows
tests/module: inet_run_module_tests -m release --no-build -l ERROR	626 run, 347 pass, 279 fail	348 pass, 278 fail: the same failing tests, except that RedDropper_congestion_drop_reason passes

The 5 fingerprint failures are rows of mipv6-refactoring.csv whose recorded ~tNlb fingerprints are stale on master. Of the 63 errors, 62 are configurations whose optional features are not built, and one is marked as expected. The drop reason is not a fingerprint ingredient, so no passing row moves, including those of examples/inet/redmarker and examples/inet/dctcp, which use RedDropperQueue. Of the 279 module failures on master, 264 are tcp_* tests, one is the new test, and 14 are unrelated to queueing; all but the new test fail with this change as well.

After the rebase onto master 41eee1d619, which contains 741a0baa6b (RedDropper decides in the const matchesPacket() and stores the state in handlePacketFiltered()), with and without this change: the reproduction gives the same numbers as above; the new test fails on master and passes with this change; tests/queueing: inet_run_queueing_tests -m release --no-build -l ERROR gives the same results (75 run, 63 pass, the same 12 fail, RedDropper_1 among them, because it expects EV_DEBUG lines that a release build compiles out); and ./fingerprinttest -s -F tyf -m 'RedDropper|onedomain|dctcp|netperfmeter|redmarker' passes its 24 rows.

Debug build, before the rebase (make MODE=debug in the root and tests/module/lib, exit 0, no compiler warning in RedDropper): cd tests/module && inet_run_module_tests -m debug --no-build -l ERROR -f RedDropper passes the new test.

Statistical baselines (the separate statistics repository at 8fdf7f5, where 076c3e9 and 8fdf7f5 re-recorded the RED configurations after 741a0baa6b): of the configurations whose recorded results contain a RED module, three record drops. Each was run with this change on 41eee1d619. All three are also fingerprint rows that pass unchanged, so the drops themselves do not change:

Configuration	RED drops	packetDropCongestion:count with this change	Baseline packetDropCongestion:count (droppedPackets:count)
tutorials/queueing RedDropper	7	0, so all seven are full-buffer drops	0 (7)
examples/inet/netperfmeter SCTP-with-OLIA (Stream Control Transmission Protocol with the Opportunistic Linked-Increases Algorithm)	108	41	0 (108)
examples/inet/dctcp TcpRenoIncast	27	27	0 (27)

examples/inet/dctcp DcTcpIncast records no RED drops, in the baseline and with this change. The SCTP-with-OLIA and TcpRenoIncast baselines need re-recording for packetDropCongestion. They are not updated here: they belong in a commit to the statistics repository after this merges, naming this one. Not verified by a test: a QUEUE_FULL drop reported as QUEUE_OVERFLOW; in the reproduction the buffer never fills.

Gates: check-pull-request.sh --base origin/master (commits, classification, includes, NED parameters, description), check-source-seals.sh --base origin/master and check-architecture.sh src/inet/queueing pass.

Relationship to other pull requests

#1250 (RED with ECN for IPv6) mentions this as a side observation and does not change it. #1322, #1324, #1326 and #1334 also add an entry under "Notable bug fixes" in WHATSNEW; whichever merges later renumbers.

[levy]

Could you please rebase this to master again?