Repository navigation
networklayer: carry ECN through an IPv6-in-IPv6 tunnel - #1264
adamgeorge309 wants to merge 1 commit into
Conversation
Ipv6Tunnel::handleUpperPacket() cleared the tags of the inner datagram and asked Ipv6 for an outer header with only the tunnel endpoints and the payload protocol, so every tunnel packet left the entry point with an ECN (Explicit Congestion Notification) field of Not-ECT (not ECN-Capable Transport). The tunnel branch of Ipv6::localDeliverFinish() re-processed the inner datagram without reading the outer header. A router inside the tunnel had nothing to mark, and a Congestion Experienced (CE) mark that the outer header did get was lost at the exit. RFC 6040 (Tunnelling of Explicit Congestion Notification) Section 4.1: "In normal mode, an encapsulator compliant with this specification MUST construct the outer encapsulating IP header by copying the two-bit ECN field of the incoming IP header." Section 4.3: "a tunnel ingress MUST at least implement normal mode". Section 4.2, Figure 4, gives the egress. For the bidirectional tunnel between a Mobile Access Gateway and its Local Mobility Anchor, which INET builds as an Ipv6TunnelInterface, RFC 5213 (Proxy Mobile IPv6) Section 5.6.3 requires the full-functionality option of RFC 3168 (The Addition of Explicit Congestion Notification to IP): copy ECN-Capable Transport on entry, propagate CE on exit. RFC 6040 updates RFC 3168; its normal mode and its Figure 4 egress meet both requirements. The entry point passes the inner ECN field to Ipv6 in an EcnReq, the tag Ipv6 already honours for locally sent datagrams, so the IPv6 core needs no tunnel-specific encapsulation code. At the exit, Ipv6::propagateTunnelEcn() applies Figure 4: the inner field takes the more severe of the two markings, and a Not-ECT inner datagram under a CE outer header is dropped, with packetDropped and reason CONGESTION. The DSCP (Differentiated Services Code Point) of the outer header stays zero, the default of RFC 2473 (Generic Packet Tunneling in IPv6) Section 6.4. tests/module/IPv6_tunnel_ecn.test reproduces it. Host A sends two UDP (User Datagram Protocol) flows to host H through a tunnel to router B, and router R inside the tunnel sets CE on each tunnel packet with a test-only netfilter hook, tests/module/lib/TunnelCeMarker. On master no stock queue marks an IPv6 packet (#1249); with #1250 a RED (Random Early Detection) queue with useEcn does, and the hook keeps the test independent of that pull request. Before this commit the outer ECN field at R is 0 (Not-ECT); the ECT(0) (ECN-Capable Transport (0), ECN field value 2) flow reaches H with traffic class 186 (DSCP 46, ECT(0)), and 2 of 2 Not-ECT packets reach H. After it the outer field is 2 (ECT(0)); the ECT(0) flow reaches H with traffic class 187 (DSCP 46, CE), and 0 of 2 Not-ECT packets reach H. No fingerprint moves (./fingerprinttest -s -F tyf, which excludes the graphical fingerprints); the 63 rows that error on master error the same way here. Change: src.networklayer | behavior.change.fix | test whatsnew
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Devin Review found 2 potential issues.
2 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
| if (verdict == INetfilter::IHook::ACCEPT) | ||
| preroutingFinish(packet, fromIE, nullptr, Ipv6Address::UNSPECIFIED_ADDRESS); | ||
| else if (verdict == INetfilter::IHook::DROP) | ||
| if (!propagateTunnelEcn(packet, ipv6Header->getEcn())) { |
There was a problem hiding this comment.
🔴 Tunnel congestion marks hidden from receivers
When tunnel traffic carries CE, propagateTunnelEcn updates the inner header but leaves the outer EcnInd and TosInd tags. Final delivery preserves those tags, so TCP misses CE and applications read the wrong traffic class.
Learn more
The outer IPv6 header is removed by decapsulate, which installs its own traffic-class and ECN indication tags. After the inner header is updated, the same packet is routed and locally delivered again. The second call to decapsulate uses addTagIfAbsent for both tags, so the outer values survive even though the inner header holds CE. TCP receive uses EcnInd to decide whether to report congestion, and UDP socket diagnostics use TosInd.
Example: An inner ECT(0) datagram with DSCP 46 crosses an outer header marked CE and DSCP 0. The inner header becomes traffic class 187, but the receiving TCP sees the outer EcnInd value 3 only if the outer header was CE; if the outer header is ECT(0) and the inner header is CE, TCP sees 2 instead of 3. UDP sees outer traffic class 2 or 3 rather than 187.
Recommended fix: Before routing the inner packet, remove or replace outer-header TosInd, DscpInd, EcnInd and other header-derived indications. Ensure the final delivery rebuilds them from the inner IPv6 header, including the propagated ECN mark; cover an inner CE under outer ECT(0) and a marked outer CE under inner ECT(0).
Was this helpful? React with 👍 or 👎 to provide feedback.
| else if (verdict == INetfilter::IHook::DROP) | ||
| delete packet; |
There was a problem hiding this comment.
🔴 Pre-routing drops free tunnel packets twice
When a pre-routing hook drops a decapsulated packet, datagramPreRoutingHook already deletes it. The DROP branch deletes it again, causing a double free during tunnel traffic.
Learn more
The datagramPreRoutingHook implementation deletes the packet itself before returning DROP. The tunnel exit calls it after removing the outer IPv6 header. The added DROP branch then deletes the same pointer for the second time; this was also present in the replaced branch, so the changed code preserves a tunnel-specific failure.
Example: A netfilter hook at the tunnel exit returns DROP for an encapsulated IPv6 datagram. The hook dispatcher destroys the inner packet, then the tunnel branch destroys it again instead of ending delivery safely.
Recommended fix: Do not delete the packet when datagramPreRoutingHook returns DROP. Keep the ACCEPT path and leave QUEUE and STOLEN owned by their respective consumers.
| else if (verdict == INetfilter::IHook::DROP) | |
| delete packet; | |
| // datagramPreRoutingHook() owns and deletes packets on DROP. | |
Was this helpful? React with 👍 or 👎 to provide feedback.
This branch has not been deployed
1 participant
An
Ipv6TunnelInterfacehid Explicit Congestion Notification (ECN) from the network: every tunnel packet left the entry point as Not-ECT (not ECN-Capable Transport), and a Congestion Experienced (CE) mark on the outer header was lost at the exit point. Both ends now follow RFC 6040 (Tunnelling of Explicit Congestion Notification).Closes #1263
The problem
Ipv6Tunnel::handleUpperPacket()(src/inet/networklayer/ipv6tunneling/Ipv6Tunnel.cc:48-56on master) clears the tags of the inner datagram and asks Ipv6 for an outer header with only the tunnel endpoints and the payload protocol, so the outer ECN field is always Not-ECT. The tunnel branch ofIpv6::localDeliverFinish()(src/inet/networklayer/ipv6/Ipv6.cc:845-857) re-processes the inner datagram without reading the outer header.RFC 6040 Section 4.3 requires a tunnel ingress to implement normal mode, which Section 4.1 defines as copying the inner ECN field into the outer header; Section 4.2, Figure 4, gives the egress. For the bidirectional tunnel between a Mobile Access Gateway and its Local Mobility Anchor, which INET builds as an
Ipv6TunnelInterface, RFC 5213 (Proxy Mobile IPv6) Section 5.6.3 requires the full-functionality option of RFC 3168 (The Addition of Explicit Congestion Notification to IP): copy ECN-Capable Transport on entry, propagate CE on exit.The fix
The entry point hands the inner ECN field to Ipv6 in an
EcnReq, the tag Ipv6 already honours for locally sent datagrams, so the IPv6 core gains no tunnel-specific encapsulation code. At the exit pointIpv6::propagateTunnelEcn()applies Figure 4: the inner field takes the more severe of the two markings, and a Not-ECT inner datagram under a CE outer header is dropped. The Differentiated Services Code Point (DSCP) of the outer header stays zero, the default of RFC 2473 (Generic Packet Tunneling in IPv6) Section 6.4; INET has no option to copy it.The
WHATSNEWentry is under the backward incompatible changes, because the results of simulations that send ECN-capable traffic through a tunnel change.Architectural surface
Ipv6gains one protected virtual method,propagateTunnelEcn(), following the class's convention that each processing step (encapsulate(),decapsulate(),fragmentAndSend()) can be replaced in a subclass; an override could apply a different egress table in place of Figure 4.Ipv6Tunnelnow attaches anEcnReq.Ipv6emitspacketDroppedSignalwith reasonCONGESTIONat a new site, for a Not-ECT datagram under a CE outer header.Ipv6feature of.oppfeatures. The module test adds a test-only netfilter hook,tests/module/lib/TunnelCeMarker.Verification
tests/module/IPv6_tunnel_ecn.test: host A sends two UDP (User Datagram Protocol) flows to host H through a tunnel that ends at router B; router R, inside the tunnel, sets CE on each tunnel packet with the test-only hook. On master no stock queue marks an IPv6 packet, becauseEcnMarkerreads only IPv4 headers (#1249). With #1250 a RED (Random Early Detection) queue withuseEcndoes; the hook keeps this test independent of that pull request.Run on this branch and on its base, master
4eb3bb4fa9, release build (make MODE=release):tests/protocol/ipv6tests/protocol/nd)The failing and erroring rows are identical by name. The 63 fingerprint errors are 62 rows for disabled optional features (
VoipStreamSender,TcpLwip,Z3GateScheduleConfigurator, the OSG (OpenSceneGraph) visualizer networks) and theethernet-nonstandardspeed.inirow, which the fingerprint CSV records as an expected error. The 46 module tests fail on master too: 34tcp_*tests,ConvolutionalCoder12,ConvolutionalCoder34,EtherHost_lifecycle,ExternalProcess_3,Ieee80211BitDomain,Ieee80211SymbolDomain,Ieee8021d-Rstp,Ieee8021d-Stp,IPv6_packet_too_big,MIPv6_tcp_handover,PacketGate_1andUDPSocket_1. The ipv6 protocol failure isRfc8200OverlappingFragments. The 15 Neighbour Discovery protocol failures are conformance tests (Rfc4861*,Rfc4862*,Rfc6980*) that fail on master with the same verdicts; none involves a tunnel.In a debug build (
make MODE=debug) the focused tests pass on this branch, 38 of 38 and 27 of 27;Rfc8200OverlappingFragmentsfails only in the release run:Statistical tests (
inet_run_statistical_tests): not run.The branch is one commit, built in both modes. Gates, run from the repository root:
The 25 architecture candidates (in
src/inet/common,mobilityandenvironment) and the naming and interface violations name no path this branch touches, so they are on master too.Relationship to other pull requests
Ipv6::localDeliverFinish(). Neither depends on the other, but both edit these lines; whichever merges second needs a rebase.EcnMarkerchange, and queueing: mark ECN-capable IPv6 packets in a RED queue instead of dropping them #1250 touches no tunnel code.