Tunnel mode reception, AL-FEC support and various bugfixes - #68
Open
jordijoangimenez wants to merge 20 commits into
Open
Tunnel mode reception, AL-FEC support and various bugfixes#68jordijoangimenez wants to merge 20 commits into
jordijoangimenez wants to merge 20 commits into
Conversation
…r alive until Boost.Asio calls the completion lambda.
…ebuild inner IPv4/UDP headers with byte-wise network-order checksum generation so tunneled end-to-end delivery passes reliably in release builds.
…eceive-buffer overflow Squashed re-application of accumulated fixes/features previously developed on a personal fork whose history had diverged from 5G-MAG/rt-libflute's actual current development/main (unrelated histories, confirmed via git merge-base returning nothing, despite matching content up to a shared point -- likely from a prior history rewrite on one side only). Re-applied here as a single clean commit against the real upstream base rather than replaying the original, now-irreconcilable commit sequence: - Add SSM (source-specific multicast) join support to Receiver, so a session can admit packets only from a specified source address rather than any-source multicast. - Fix a use-after-free: an async_receive_from completion already queued on the io_context when a Receiver is destroyed could run after the destructor returns and touch a freed `this` -- boost::asio only guarantees a cancelled operation's handler eventually runs with operation_aborted, not that it runs before the destructor returns. Receiver now tracks its own liveness via a shared atomic flag copied into each completion handler, checked before touching `this`. - Fix Receiver's fixed 2048-byte receive buffer silently truncating larger encoding symbols: recvfrom() on a datagram socket doesn't error on a too-small buffer, it silently truncates, so any FEC-OTI configuration with larger symbols corrupted every symbol beyond 2048 bytes without any visible error until an FDT Content-MD5 check (if present) caught it. Buffer is now 65536 bytes, covering the maximum possible IPv4 UDP payload.
Transmitter::send() reused the same FileDescription/TOI correctly for carousel-style repeated objects, but two bugs meant each resend still grew the file delivery state without bound: - _files.insert() is a no-op if the TOI key is already present, so a resend's updated File object was silently discarded in favour of the stale one already in the map. - FileDeliveryTable::add() unconditionally appends a new <File> entry with no dedup by TOI, so every resend left the previous cycle's entry for the same TOI in place -- the serialised FDT grows by one entry per resend indefinitely (observed growing from ~850 bytes to several hundred KB over a couple of hours of a 10-second carousel), eventually becoming too large for a receiver to reassemble at all. Per RFC 6726 SS3.3/3.4.2, an FDT Instance describes the current state of the file delivery session; a Content-Location may be redescribed under a new TOI to signal a new version, but parameters already described for a given TOI must not change -- so on an actual resend of unchanged content, the sender should replace that TOI's single File entry, not accumulate duplicates of it. This restores that invariant: track whether this send is a resend (TOI already assigned), replace the File map entry instead of no-op'ing, and remove the TOI's existing FDT entry before adding the current one.
The previous fix (same TOI, resent unchanged content) only handled one growth vector. A second, distinct one remained: when set_content()/set_compression() detect the content genuinely changed, they zero the FileDescription's TOI so Transmitter::send() assigns a fresh one -- but nothing ever removed the FDT entry for the TOI being vacated. Confirmed live: a carousel object whose content legitimately changes each cycle (e.g. a randomly-regenerated MIME boundary) grew its FDT to 135-170KB within about a minute, well before the previous fix's growth timescale, eventually failing to parse (XML_ERROR_PARSING_ATTRIBUTE). FileDescription now remembers the TOI it's vacating (_previous_toi, set by a new _reset_toi() helper used everywhere the TOI was zeroed) so Transmitter::send() can remove that stale entry before assigning the replacement TOI, restoring the "one current entry per logical object" invariant regardless of which of the two ways an object's description changes.
- File.cpp: use fmt::format instead of manual std::to_string() concatenation for the two bounds-check exception messages, per the suggestion -- fmt::format rather than std::format since this project targets C++17 (std::format needs C++20); same result, already a dependency via spdlog. - Transmitter.h/.cpp: added FileDescription::previous_toi()/ reset_previous_toi() public accessors instead of Transmitter::send() reaching into FileDescription's private _previous_toi directly via friend access, per the suggestion. - Receiver.cpp: restored IPv6 support the SSM/specific-interface-join fix had inadvertently dropped (the original code let Boost infer v4 vs v6 from the address types passed; the fix hardcoded .to_v4() throughout). Now branches on the multicast address's actual family: IPv6 ASM join uses join_group(address_v6, interface_index) and SSM join uses MCAST_JOIN_SOURCE_GROUP/group_source_req, both interface-by-index (unlike IPv4's interface-by-address), so a new resolve_iface_index() helper resolves the existing iface address string to its owning interface's index via getifaddrs()/if_nametoindex(). IPv4 behaviour (bind/ASM/SSM) is unchanged, verified by rerunning the existing test_end_to_end.cpp unmodified.
…nflict Both branches touched Transmitter::send_next_packet()'s tunnelled-send path: this branch (PR #56) made the encapsulated packet buffer lifetime-safe by owning it in a shared_ptr<vector<char>> captured by the async_send_to completion lambda, replacing a raw new[] with no matching delete[] (a leak on every tunnelled packet). development, in parallel, added _source_address as an explicit override for the local address used to build the inner IPv4/UDP headers, falling back to _tunnel_local_address when unset. Kept both: the shared_ptr-owned buffer from this branch, with the _source_address-or-_tunnel_local_address fallback from development.
…56 branch Integration base for building the Raptor/RaptorQ FEC work on top of both outstanding PRs' assumed-merged state: PR #56's tunnel-buffer lifetime fix (reconciled with development's _source_address fallback) plus PR #60's FDT-growth fixes with the previous_toi()/reset_previous_toi() accessor pattern and full IPv4/IPv6 parity in Receiver's SSM/ASM join and bind logic. No conflicts with the prior merge; both touch disjoint enough regions of Transmitter.cpp/.h that git combined them cleanly.
…ed state This branch is built assuming both outstanding PRs land first: PR #56's tunnel-buffer lifetime fix (with development's _source_address fallback, already reconciled in an earlier commit on this branch) and PR #60's FDT-growth fixes with the previous_toi()/reset_previous_toi() accessor pattern and full IPv4/IPv6 parity in Receiver. No shared git history exists between this fork's line of development and the current upstream development branch (a previously-diagnosed history rewrite upstream, see PR #60's own 184e482), so this reconciliation was done as a content-level 3-way patch apply against this integration base, not a rebase -- 14 files had genuine overlapping hunks, resolved as follows: - flute_types.h, AlcPacket.cpp, EncodingSymbol.cpp, File.cpp, FileDeliveryTable.cpp: kept the Raptor/RaptorQ-aware superset (FecOti's scheme-specific fields, multi-scheme FEC Payload ID / EXT_FTI / FDT attribute parsing) -- these fully subsume the integration base's CompactNoCode-only logic. - Receiver.cpp: kept the integration base's version entirely -- PR #60's IPv6-aware SSM/ASM join and bind logic is strictly more complete than this branch's older IPv4-only copy of the same code, and Raptor/RaptorQ content needs no Receiver-side changes beyond what File/EncodingSymbol already provide. - Transmitter.h/.cpp: combined per-hunk -- kept the accessor pattern (previous_toi()/reset_previous_toi()) and the graceful deactivate(bool finish_file_transmissions) lifecycle from the integration base; kept this branch's content_fec_oti constructor parameter and fec_oti()/fdt() accessors; and rebuilt this branch's dual-send fix (send both a plain copy and a tunnelled copy, needed for N3mb GTP-U tunnelling per TS 23.247 while still supporting direct SSM subscribers) on top of PR #56's cleaner byte-wise checksum/header implementation (create_udp_pkt/create_ip_hdr/calculate_sum operating on uint8_t* with explicit write_uint16_be/write_uint32_be helpers, not the older uint16_t*-punned struct-overlay approach) instead of keeping two divergent checksum implementations. - CMakeLists.txt, tests/CMakeLists.txt, examples/flute-transmitter.cpp, include/File.h: trivial additive conflicts (version bump, new test targets, new #includes, an unrelated exception-slicing/format-string bug fix already on this branch). - tests/test_transmitter.cpp, tests/test_end_to_end.cpp: kept the integration base's superset (adds a UDP-tunnel e2e test and a graceful- deactivation lifecycle test this branch didn't have); this branch's own duplicate of the basic transmit/receive test added nothing Raptor- specific -- that coverage lives in test_raptor_e2e.cpp/test_raptorq_fec.cpp. Verified: full library + all 6 test binaries (25 tests total) build and pass against the reconciled tree.
PR #56 landed this morning as a squash commit, superseding the hand-built integration base this branch was previously reconciled against. Re-merged against the real development tip: - Transmitter.cpp: one conflict, same shape as before -- kept this branch's dual-send feature (plain + tunnelled copy, needed for N3mb GTP-U per TS 23.247) rebuilt on #56's shared_ptr-owned buffer, since dual-send was never part of #56 itself and #56's actual merged content confirms that. - tests/test_end_to_end.cpp: 12 conflicts, all the same underlying change -- the real #56 merge added a std::mutex protecting TunnelBridgeStats from a genuine data race between the tunnel-bridge thread and the test's main thread, which this branch's copy of the test lacked. Took development's thread-safe version throughout. - tests/tmp/e2e_payload.bin deleted, matching development: the current test generates its payload in-code and no longer reads this fixture. Verified: full library + all 6 test binaries (25 tests) build and pass.
- FLUTE version nibble in EXT_FDT: was hardcoded to 1, now 2 per RFC 6726 SS3.1/3.4.1 (LCT header version field, a separate RFC 5651 field, is correctly left at 1). - TSI truncated to 16 bits on transmit: AlcPacket's send-side constructor now takes a uint64_t tsi and picks the narrowest half_word_flag/tsi_flag combination (16/32/48 bits) able to carry it, mirroring TOI's existing wide receive-side decode. Also fixed a latent decode-side bug (uint32_t shifted before promotion to uint64_t) that silently discarded the upper 16 bits of a 48-bit TSI/TOI -- unreachable before this fix since nothing previously transmitted tsi_flag==1. - FDT Instance ID wraparound: FileDeliveryTable now tracks expired instance IDs and, once the 20-bit space is exhausted, reuses the smallest expired ID that isn't the one just superseded (RFC 6726 SS3.4.1), instead of a plain ++ silently bit-masking on the wire. - FDT Complete attribute: read and write support added, wired to Transmitter::close_session(). - Close Session / Close Object LCT flags: AlcPacket gained public accessors and send-side support; Transmitter gained close_session()/close_object(toi) APIs; Receiver surfaces both via callbacks and session_closed(). - EXT_FTI bootstrap: Receiver now bootstraps a File from a content packet's own EXT_FTI when no FDT entry exists yet for that TOI (RFC 6726 SS3.4.1), instead of discarding it. - IPSec authentication: configure_state() now also sets XFRMA_ALG_AUTH (HMAC-SHA256), per RFC 6726 SS7.5's SHALL for authentication alongside encryption; derives a key from the AES key when no separate one is given. Added tests/test_protocol_fixes.cpp covering all of the above. Full existing suite (unit/e2e/raptor/raptorq/fdt_growth) plus the new tests all pass; a live flute-transmitter/flute-receiver run with a 32-bit TSI over real UDP multicast confirms round-trip delivery.
The Receiver-side counterpart to Transmitter's existing udp_tunnel_address() support. Per the issue discussion, the library has no business knowing about any particular encapsulation format -- that is entirely the controlling application's concern: "Stripping the GTP-U header doesn't feel like something that a generic FLUTE library should be asked to do... A better design pattern would be for the controlling application to pass in a 'helper' function that the library invokes to do application-specific mangling of packets before the generic code in the library starts processing the ALC/LCT payload." -- rjb1000 Adds three new optional constructor parameters: - tunnel_address: if given, ALSO bind a plain unicast UDP socket to this local endpoint and accept tunnelled datagrams there, in addition to the normal multicast join. The two paths are independent and both feed the same session state -- deliberately not an either/or choice like Transmitter's tunnel mode, since a Receiver has no way to know in advance which path will actually work in a given deployment. - tunnel_source: if given, only accept tunnel datagrams from this source address -- the tunnel-socket equivalent of the existing source_address parameter's SSM admit-only-this-source semantics. This is the "extra address checking" the library itself does, on top of whatever packet_modifier does; source-address admission is a generic, encapsulation-agnostic concept the library can reasonably own, unlike parsing any particular header format. - packet_modifier: required whenever tunnel_address is set. Given the whole received datagram (as a mutable vector, so a modifier can also decrypt/rewrite in place, not just locate the payload), returns the byte offset at which the ALC/LCT payload begins -- an offset >= the buffer's size means "discard, nothing usable here". No default implementation is provided, since any default would itself bake an encapsulation assumption into the library. This mirrors the de-tunnelling logic that already exists, hand-written, in tests/test_end_to_end.cpp's run_tunnel_bridge() (added alongside Transmitter's own tunnel mode in #56): a std::thread there receives on a plain UDP socket, manually parses a hand-built inner IPv4+UDP header out of the payload, and forwards just the FLUTE bytes onward over loopback to a receiver with no tunnel-awareness at all. This moves that capability inside Receiver proper as a caller-supplied, protocol-agnostic hook, so any deployment's actual encapsulation (Transmitter's own wrapper, real GTP-U, or anything else) is expressed purely by what modifier is passed in -- enabling rt-mbs-client (referenced in the issue as "MBSTF Client") to take advantage of it for reception paths where local multicast delivery isn't available at all. Confirmed live end-to-end: real Service Announcement content broadcast over an actual gNB/UE radio link, captured on the UE's TUN device via a tunnel_address + packet_modifier pairing, correctly parsed into a complete FDT and announcement bundle.
handle_receive_from() and handle_tunnel_receive_from() only called arm_receive()/arm_tunnel_receive() again in the success branch -- a single transient socket error (e.g. an ICMP port-unreachable surfacing as a UDP socket error on a subsequent read, hit live via the raw capture relay's loopback sendto() path) permanently killed reception for the rest of the process's life, with just one log line and no way to recover short of restarting the client. (Attempted as a cross-repo cherry-pick of jordijoangimenez/rt-libflute commit d453c05, 'Fix FDT/TOI reassembly corruption, multicast bind/join, ...' -- turned out this branch already independently has every other fix from that commit (FDT-instance-discard reassembly logic, catch(const char*), the INADDR_ANY bind, the per-interface multicast join, the array-specialised shared_ptr scratch buffers); the re-arm bug above was the only genuinely missing piece, found here by live strace/tcpdump debugging, not by that commit.)
rjb1000
changed the title
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
2 participants
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.
Supersedes #67, which was opened from my personal fork -- same content, pushed directly to this repo instead.
Summary
This branch (issue #66) adds tunnel-mode reception to
LibFlute::Receiver— the capability MBS Broadcast reception inrt-mbs-clientdepends on when running against a software TUN-based radio link, where a multicast datagram written to the TUN device's write() side never reaches a socket joined to its destination group on that interface at all (see the linked rt-mbs-client PR for the full live-traced explanation). It also carries the Raptor/RaptorQ FEC work (previously reconciled from #56/#60) and one new bug fix found tonight while live-testing MBS Broadcast reception end-to-end.Known gap, found while preparing this PR: this branch is missing 4 commits that are on #62/#65 (
ff1d547widen TSI to 48 bits,637ce5cFDT Instance ID wraparound RFC 6726 §3.4.1,0ceaa26LCT Close Session/Object flags,6c7137dEXT_FTI bootstrap without an FDT entry) — flagging this rather than silently merging it in, since I haven't verified those against this branch's current state.What's included
c8afd5f, fixes Add tunnelled operation to Receiver class #66):LibFlute::Receivergains an optional tunnel endpoint + packet-modifier constructor path, so a caller (likert-mbs-client'sTunRawRelay) can forward decapsulated payloads to a local unicast socket the Receiver actually listens on, alongside its normal multicast join.ff483cd).a7a233e,9a0beeb,92d2140,7331294,72dea58) — RFC 5053 Raptor and RFC 6330 RaptorQ FEC schemes (also tracked separately as Implement RFC 5053 Raptor #61/RFC 6330 RaptorQ FEC (not 3GPP-mandated — reference/future work, depends on #61) #64), needed for reliable delivery of larger objects over a lossy broadcast channel with no ARQ.7d98512,a5a956d).184e482).12770ad,2db5a8a).fe9ab9athrough1e1fa90).f4100f9, new tonight):handle_receive_from()/handle_tunnel_receive_from()only re-armed the next async read in the success branch — a single transient socket error (e.g. an ICMP port-unreachable surfacing as a UDP socket error on a later read, hit live via the raw-capture relay's own loopbacksendto()path) permanently killed reception for the rest of the process's life, with one log line and no way to recover short of restarting the client. Found via strace/tcpdump live debugging.