queueing: mark ECN-capable IPv6 packets in a RED queue instead of dropping them - #1250
Open
adamgeorge309 wants to merge 1 commit into
Open
adamgeorge309 wants to merge 1 commit into
adamgeorge309 wants to merge 1 commit into
Conversation
EcnMarker::getEcn() and EcnMarker::setEcn() handled only Protocol::ipv4. For an IPv6 packet getEcn() returned IP_ECN_NOT_ECT and setEcn() returned without changing the packet. RedDropper (Random Early Detection, RED) decides between marking and dropping with getEcn() (RedDropper.cc:111). With useEcn = true it therefore dropped an IPv6 packet that is Explicit Congestion Notification (ECN) capable, where it sets Congestion Experienced (CE) on an ECN-capable IPv4 packet. The EcnMarker module left an IPv6 packet unchanged whatever its EcnReq tag said. RFC 3168 section 5: "The IPv4 TOS octet corresponds to the Traffic Class octet in IPv6, and the ECN field is defined identically in both cases." RedDropper.ned documents useEcn as "packets are marked with ECN if applicable", with no IP version restriction. setEcn() updates no checksum for IPv6, because the IPv6 header has none. tests/module/IPv6_red_ecn_marking.test reproduces it: the client sends UdpBasicAppData-0 to -4, five User Datagram Protocol (UDP) packets with the ECN codepoint ECT(0) (ECN-Capable Transport), through a RedDropperQueue with useEcn = true, minth = 0, maxth = 1 and wq = 1. Before this commit Random Early Detection (RED) drops UdpBasicAppData-2 to -4 and the server receives 2 packets. After it the server receives all 5, and UdpBasicAppData-2 to -4 arrive with traffic class 3 (ECN Congestion Experienced, CE). No fingerprint or statistical baseline moves: apart from the new test, the only configurations in examples/, showcases/, tutorials/ and tests/ that set useEcn are examples/inet/redmarker and examples/inet/dctcp, both IPv4 only, and none uses EcnMarker. Change: src.queueing.EcnMarker | behavior.change.fix | test whatsnew
This branch has not been deployed
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
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.
A
RedDropper(Random Early Detection, RED) withuseEcn = truenow sets Congestion Experienced (CE) on an IPv6 packet that is Explicit Congestion Notification (ECN) capable, as it already does on an IPv4 one, instead of dropping it.Closes #1249
The problem
The RED module decides between marking and dropping with the static helper
EcnMarker::getEcn()(src/inet/queueing/filter/RedDropper.cc:111) and marks withEcnMarker::setEcn(). Both helpers (src/inet/queueing/marker/EcnMarker.cc:58and:92on master) handled onlyProtocol::ipv4. For an IPv6 packetgetEcn()returnedIP_ECN_NOT_ECT(Not ECN-Capable Transport) andsetEcn()returned without changing the packet. An ECN-capable IPv6 packet was therefore dropped by RED like a Not-ECT packet, with no warning. TheEcnMarkermodule left an IPv6 packet unchanged whatever itsEcnReqtag said.RFC 3168 section 5: "The IPv4 TOS octet corresponds to the Traffic Class octet in IPv6, and the ECN field is defined identically in both cases."
RedDropper.neddocumentsuseEcnas "packets are marked with ECN if applicable", with no IP version restriction.With the configuration in the issue, run on master
49e1fa0945and on this branch, a client sends User Datagram Protocol (UDP) packets with the ECN codepoint ECT(0) (ECN-Capable Transport, codepoint 2) into a 10 Mb/s bottleneck: 1000 B every 0.4 ms (20 Mb/s) for 1 s. The Random Early Detection (RED) queue hasuseEcn = true,minth = 3,maxth = 4,maxp = 1,wq = 1andpacketCapacity = 24. Counts from the server's packet capture and the scalars:droppedPackets:countUDP does not slow down on a CE mark, so after the fix the queue fills to its capacity of 24 and drops there, as with IPv4; the drop count stays high for both. IPv6 delivers about 2% fewer packets than IPv4 before and after the fix (1150 against 1172), because its header is 20 B longer: 1066 B against 1046 B per frame on the 10 Mb/s link. The IPv4 server capture is byte-identical before and after the fix.
The fix
EcnMarker.ccgains an IPv6 branch insetEcn()(line 76) and ingetEcn()(line 115). The ECN field of IPv6 is the low two bits of the Traffic Class, whichIpv6Headerexposes as itsecnfield.setEcn()updates no checksum for IPv6, because the IPv6 header has none.RedDropperis unchanged.Architectural surface
EcnMarker::getEcn()andEcnMarker::setEcn()now handle IPv6 packets. Their signatures do not change. Their callers areEcnMarker::markPacket()andRedDropper(RedDropper.cc:111,128,132,139).useEcn = trueor anEcnMarkernow writes the Explicit Congestion Notification (ECN) bits of the IPv6 Traffic Class.setEcn()replaces theNetworkProtocolIndtag with one that points to the rewritten header, as it already does for IPv4.EcnMarker.ned: the module description names the IPv6 Traffic Class. No parameter changes.EcnMarker.ccalready includes the IPv4 and Ethernet headers behindINET_WITH_IPv4andINET_WITH_ETHERNET. The IPv6 header include and both IPv6 branches follow the same pattern behindINET_WITH_IPv6.doc/project/enforcement/check-architecture.sh src/inet/queueingpasses.No new
AV-*orNV-*ledger row. No sealed path is touched.Verification
tests/module/IPv6_red_ecn_marking.test(new): the client sends five UDP packets,UdpBasicAppData-0to-4, with the Explicit Congestion Notification (ECN) codepoint ECT(0) through aRedDropperQueuewithuseEcn = true,minth = 0,maxth = 1,wq = 1. On master Random Early Detection (RED) dropsUdpBasicAppData-2to-4and the server receives 2 packets. With the fix the server receives all 5, andUdpBasicAppData-2to-4arrive with traffic class 3 (ECN Congestion Experienced, CE).The branch is based on master
49e1fa0945. Both trees were built withmake -j4 MODE=release(exit status 0), and the suites were run on both and diffed by test name:cd tests/fingerprint && ./fingerprinttest -s -F tyf: 1711 pass, 0 fail and 63 errors, the same rows as on master. One error is the rowethernet-nonstandardspeed.ini -r '$datarate==5Gbps && $duplex==false', whichtests/fingerprint/ethernet.csv:168expects to error ("5e+09 bps Ethernet only supports full-duplex links"); the other 62 are scenarios of disabled optional features (VoIPStream,TcpLwip,Z3GateScheduleConfigurator, the OSG visualizer).cd tests/fingerprint && ./fingerprinttest -s -F tyf -m 'redmarker|dctcp' examples.csv(exit status 0): theuseEcnrowsexamples/inet/redmarkerTcpSender1,examples/inet/dctcpDcTcpIncastandTcpRenoIncastpass.cd tests/module && inet_run_module_tests -m release --no-build -l ERROR: 347 tests, 46 failures, the same 46 by name as on master (346 tests there);IPv6_red_ecn_marking.testpasses.cd tests/protocol/ipv6 && inet_run_protocol_tests -m release: 26 of 27 pass;Rfc8200OverlappingFragments.testfails identically on master.cd tests/queueing && inet_run_queueing_tests -m release: 57 tests, 12 failures, the same 12 by name as on master (Gate_1to_3,MultiTokenBucketClassifier_1,MultiTokenBucketMeter_1,OrdinalBasedDropper_1,OrdinalBasedDuplicator_1,PeriodicGate_1,RedDropper_1,Tagger_1,TokenBucketClassifier_1,TokenBucketMeter_1).RedDropper_1does not reach the change: its packets carry no IP header and it leavesuseEcnfalse.make -j4 MODE=debug(exit status 0), thencd tests/module && inet_run_module_tests -m debug -f IPv6_red_ecn_marking: pass.doc/project/enforcement/:check-commits.sh origin/master..HEAD,check-classification.sh origin/master..HEAD,check-architecture.sh src/inet/queueingandcheck-source-seals.shpass. The unscopedcheck-architecture.sh,check-naming.sh --base origin/masterandcheck-interfaces.shexit 1 with hits that are all outside the files this commit touches.No fingerprint or statistical baseline moves: apart from the new test, the only configurations in
examples/,showcases/,tutorials/andtests/that setuseEcnareexamples/inet/redmarkerandexamples/inet/dctcp, both IPv4 only, and none usesEcnMarker.Not addressed here
EcnMarker::setEcn()still returns silently for a packet that is neither IPv4 nor IPv6.RedDropperdoes not call it for such a packet, becausegetEcn()reports Not-ECT for it.RedDropperdeclares apacketDropCongestionstatistic, but it drops throughPacketFilterBase::dropPacket(packet), which uses the reasonOTHER_PACKET_DROP(src/inet/queueing/base/PacketFilterBase.cc:256), so the statistic stays 0 in the runs above.