Repository navigation
feat: v2.21.0 — L2 (native Ethernet) transport + UDP Annex J conformance fix - #99
Merged
Merged
Conversation
…nce fix ROADMAP.md milestone 65. Two parts, per two independent public secondary sources (a Wireshark issue tracker discussion of the real Annex J wire text, and the COVESA Open1722 open-source reference implementation's Avtp_Udp_t header, github.com/COVESA/Open1722) — this repo has no verified access to the paywalled IEEE 1722-2016 standard text itself, so neither part is presented as verified against the primary standard: - rcp/udp.hpp Annex J conformance fix: an Annex J UDP payload actually begins with a 4-byte big-endian encapsulation sequence number before the AVTPDU, which udp::Frame/MultiFrame never had — encode_annexj_datagram/ decode_annexj_datagram add that envelope, and Server/Client now transparently prepend/strip it. The standard destination UDP ports are 17220 (Continuous) and 17221 (Discrete/control); RCP traffic is control-plane, so udp::kAnnexJControlPort (17221) is now the default port via a defaulted constructor parameter, with the explicit-port form unchanged for back-compat/testing. - rcp/l2.hpp (new): the native IEEE 1722-over-Ethernet transport udp.hpp's own header comment always described as the alternative to Annex J, now actually built — destination MAC + source MAC (auto-read via SIOCGIFHWADDR) + EtherType 0x22F0 + the AVTPDU directly, no encapsulation sequence number (that field is Annex J/UDP-specific). l2::Server/ l2::Client are structurally parallel to udp::Server/udp::Client (same method names, same Handler signature) with no new Transport interface. Linux-only (AF_PACKET/SOCK_RAW, needs CAP_NET_RAW/root); every other platform gets the same function_not_supported stub udp.hpp's own Windows branch already established. The pure Ethernet-header/Frame/MultiFrame codec is available on every platform unconditionally (unlike udp.hpp's own structure) so tests/test_l2.cpp can exercise it with no privileges and no Linux requirement. Neither transport deprecates or is preferred over the other; both are permanent, equally supported options. Verification: tests/l2_veth_roundtrip.cpp is a new standalone (non-ctest) program that drives a real l2::Server/l2::Client pair over a real Linux veth interface pair, run by the new l2-veth CI job (.github/workflows/ci.yml), which creates the veth pair with sudo and asserts a real Ethernet frame's payload round-trips byte-for-byte. Verified directly in a Linux container during development, including a genuine correctness fix that verification surfaced: AF_PACKET sockets do not support shutdown() on Linux (silently a no-op, unlike the AF_INET sockets rcp/udp.hpp uses), so l2::Server/l2::Client's serve/read threads would otherwise hang forever in close() after a request/response had already round-tripped correctly — fixed via a short SO_RCVTIMEO (detail::set_recv_timeout, 200ms) so those threads periodically recheck their own closed flag. Full local build (C++17/C++20, GCC 11/12, Clang 14, plus macOS) and ctest (53/53 suites, up from 52) pass. kVersion/CMakeLists.txt bump 2.20.0 → 2.21.0. Signed-off-by: Matt <47545907+SoundMatt@users.noreply.github.com>
Frame::sequence_num/MultiFrame::sequence_num are uint16_t, matching rcp/udp.hpp's identical fields, but avtp::TscfHeader/NtscfHeader's own sequence_num is the real 8-bit wire field (avtp.hpp's own header comment: "8 bits, not 16"). udp.hpp's identical assignment happens to not trip MSVC's /W4 today, but l2.hpp's did, failing the Windows build. Added the same explicit static_cast<uint8_t> this codebase already uses elsewhere for intentional narrowing; behavior is unchanged (encode_tscf_header/encode_ntscf_header already only ever wrote the low 8 bits either way). Full local suite still 53/53. Signed-off-by: Matt <47545907+SoundMatt@users.noreply.github.com>
…ER018 false positive) cpfusa's CYBER018 rule flags any printf/fprintf/sprintf/vprintf call whose first argument isn't a string literal -- correct for printf's own signature (format string first), but a false positive against fprintf's real signature (FILE* stream first, format string second): every fprintf(stderr, "literal", ...) call in this file was flagged even though the actual format string was always a literal. Confirmed by reading cpfusa's own regex source (src/cyber/cyber.cpp:154-159, v0.18.0) rather than assuming. No other file in this repo uses fprintf, so there's no established workaround to match -- switched this file's three fprintf(stderr, ...) call sites (plus the CHECK macro) to std::cerr, which has no format-string argument at all and sidesteps the false positive by construction rather than silencing the finding. Verified against cpfusa's actual regex (not just assumed fixed) and confirmed the binary's behavior is unchanged (macOS smoke test: correct message and exit code on the non-Linux early-return path). Full local suite still 53/53. The regex bug itself is being filed as a separate issue against cpp-FuSa (SoundMatt/cpp-FuSa) -- this commit works around it here, not there. Signed-off-by: Matt <47545907+SoundMatt@users.noreply.github.com>
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.
Summary
ROADMAP.md milestone 65. Adds a real Layer-2 (raw Ethernet) transport alongside a corrected UDP transport — both are permanent, first-class, equally supported options; neither deprecates the other.
Provenance caveat (applies to every Annex J detail below): this codebase has no verified access to the paywalled IEEE 1722-2016 standard text. The Annex J details here rest on two independent public secondary sources: a Wireshark issue tracker discussion of the real Annex J wire text, and the COVESA Open1722 open-source reference implementation's
Avtp_Udp_theader (include/avtp/Udp.h, BSD-3-Clause, github.com/COVESA/Open1722). "Annex J conformant" below means conformant with these two sources' reading, not verified against the primary standard.1.
rcp/udp.hppAnnex J conformance fixudp::Frame/MultiFramenever had this. Newencode_annexj_datagram/decode_annexj_datagramare the pure codec for that envelope;Server/Clientnow transparently prepend/strip it around the existing AVTPDU encode/decode path. Both track and expose the value they send/receive (Client::last_sent_encap_seq/last_recv_encap_seq,Server::last_recv_encap_seq(client)) for a future caller's own use (e.g. loss detection) — no such detection is implemented, and the field's exact intended semantics are not claimed as verified.udp::kAnnexJControlPort(17221) is now the default port via a defaulted constructor parameter, while the explicit-port constructor form is unchanged for back-compat/testing.2.
rcp/l2.hpp(new): native IEEE 1722-over-Ethernet transportThe transport
udp.hpp's own header comment always named as the alternative to Annex J but never implemented, now built:SIOCGIFHWADDR) + EtherType0x22F0(2) + the AVTPDU directly — no encapsulation sequence number (that field is Annex J/UDP-specific, not part of native L2 framing).l2::Server/l2::Clientare structurally parallel toudp::Server/udp::Client— same method names, sameHandlersignature, same request/response correlation strategy — with no newTransportinterface introduced (this codebase has none today; matching shapes without formal inheritance matches its existing style).AF_PACKET/SOCK_RAW, needsCAP_NET_RAWor root — documented in the header). Every other platform gets the same "compiles cleanly, every operation returnsstd::errc::function_not_supported" stubudp.hpp's own Windows branch already established as precedent.udp.hpp: the pure Ethernet-header/Frame/MultiFramecodec is available unconditionally on every platform (not gated behind the socket-availability guard), specifically sotests/test_l2.cppcan exercise it everywhere with no privileges and no Linux requirement — documented explicitly in the header comment.3. Tests
tests/test_udp.cpp: new pure tests forencode_annexj_datagram/decode_annexj_datagram(round-trip, rejects short buffer, monotonic increment), a real Client/Server integration test proving the sequence number increments across successive requests and the Server observes it, a default-port test, plus the existing raw-socket multi-datagram test updated to speak the new encapsulated wire format.tests/test_l2.cpp(new): pureEthHeader/Frame/MultiFrame/combined-L2-frame codec tests — no socket, runs on every platform (macOS/Windows exercise the stub instead).tests/l2_veth_roundtrip.cpp(new): standalone (non-ctest) program driving a reall2::Server/l2::Clientpair over a real Linux interface pair. Built bytests/CMakeLists.txtbut deliberately not registered viaadd_test— needsCAP_NET_RAW/root and real interfaces, neither of which the normalctestrun has or should require.l2-vethCI job (.github/workflows/ci.yml): creates a realvethpair withsudo, buildsl2_veth_roundtrip, and runs it againstveth0/veth1, asserting a real frame round-trips byte-for-byte.4. Docs/requirements
ROADMAP.mdmilestone 65 (GA renumbered [cpp-RCP-N2-02] write_whole_map accepts mismatched vector sizes and never resizes endpoint_owner_ #65 → [cpp-RCP-N2-03] E2E CRC coverage omits ACF_GBB message_timestamp, mismatching the transmitted bytes #66 accordingly), and theudp.hppheader comment's own prior "no additional encapsulation header" claim is corrected..fusa-reqs.jsongainsREQ-UDP-013/REQ-UDP-014andREQ-L2-001..008, tagged in the new/changed code via// fusa:req/// fusa:testcomments matching this repo's existing convention.Self-verification (real bytes, not just "tests pass")
RELAY_BUILD_CLI=ON) andctest— 53/53 suites pass (up from 52;test_l2is new), 116 assertions intest_udp, 43 intest_l2(macOS exercises the L2 stub).ubuntu:22.04, GCC 11 and Clang 14, C++17 and C++20): full build clean,ctest53/53 pass;test_l2runs the real (non-stub) pure-codec path (40 assertions, no socket).vethround trip (Linux container with--cap-add=NET_ADMIN --cap-add=NET_RAW): created a realveth0/veth1pair, ranl2_veth_roundtripwith an 8-byte payload — exit 0, payload round-tripped byte-for-byte, confirmed clean shutdown, and repeated 3x with no flakiness. This surfaced and fixed a real bug during development:AF_PACKETsockets don't supportshutdown()on Linux (silent no-op, unlikeudp.hpp'sAF_INETsockets), soclose()was hanging forever after a successful round trip — fixed with a 200msSO_RCVTIMEOpoll loop (detail::set_recv_timeout).CAP_NET_RAW— failed cleanly with a clear diagnostic and exit code 1, no crash/hang, confirming the documentedCAP_NET_RAW/root requirement and the "construct, then checkok()" contract.-WnonnullGCC warning under-DCMAKE_BUILD_TYPE=Releaseon this environment's specific GCC 16.1.0 toolchain was traced back and confirmed (viagit stash) to already exist on unmodifiedmain, unrelated to this change — not something this PR introduced or needed to fix (no-Werrorfor GCC/Clang in this repo; only MSVC's/WXand clang-tidy's own--warnings-as-errorsgate, neither of which this warning affects).Not merging — leaving for review per repo process.