From b900356de04ec43566ca17ceb8c32b11a32a0277 Mon Sep 17 00:00:00 2001 From: Aoto Date: Wed, 30 Sep 2026 13:53:34 +0200 Subject: [PATCH] fix ecn on dual-stack sockets in both directions --- docs/metadata.md | 20 +++++-- docs/sockets.md | 2 + docs/testing.md | 2 +- src/dgram/address.cppm | 9 +++ src/dgram/batch.cppm | 7 ++- src/dgram/metadata.cppm | 25 +++++--- tests/loopback_tests.cpp | 18 ++++++ tests/metadata_tests.cpp | 119 +++++++++++++++++++++++++++++++++++++++ 8 files changed, 184 insertions(+), 18 deletions(-) diff --git a/docs/metadata.md b/docs/metadata.md index 7716ea3..1a85b95 100644 --- a/docs/metadata.md +++ b/docs/metadata.md @@ -77,9 +77,10 @@ Unset features contribute nothing. The control block is built per datagram and `msg_controllen` set to exactly what was written, zero included, so a marking on one datagram cannot leak onto the next one staged in the same slot. -The family for the build step comes from the destination endpoint, so a -dual-stack socket sending to both families emits the right level and type per -datagram. +The family for the build step is the destination's `wire_family()`, not the +socket's. A dual-stack socket sends to a v4-mapped peer down the kernel's IPv4 +path, which ignores `IPV6_TCLASS` and every other v6-level message except +`IPV6_PKTINFO`, so those datagrams get `IP_TOS` and `IP_PKTINFO` instead. ## `pktinfo` @@ -94,7 +95,9 @@ The reason a wildcard-bound socket can answer from the address the peer actually addressed, which matters the moment a host has more than one. On transmit it selects the source address and interface. Leaving both zero lets -the routing table choose, which is the same as attaching nothing. +the routing table choose, which is the same as attaching nothing. A reply can +hand back the `local_info` it received unchanged, including the v4-mapped one a +dual-stack socket reports for an IPv4 peer. Sized for the larger of the two families' payloads, because the receive path cannot know which will arrive: a dual-stack socket reports `IPV6_PKTINFO` even @@ -114,6 +117,10 @@ for v4-mapped traffic. An unmarked datagram reads as `not_ect`, not as absent, provided reporting was enabled. +On a v6 socket, enabling it sets both `IPV6_RECVTCLASS` and `IP_RECVTOS`. IPv4 +peers of a dual-stack socket arrive through the kernel's IPv4 path, which reports +`IP_TOS` and never `IPV6_TCLASS`. + The two families use different widths, and the difference is not documented anywhere obvious: `IP_TOS` arrives as a **single byte** and `IPV6_TCLASS` as a **four-byte int**. `parse` branches on the length the kernel actually wrote @@ -129,7 +136,7 @@ struct my_feature { static constexpr std::size_t space{dgram::detail::space_for(sizeof(payload))}; static bool matches(int level, int type) noexcept; static std::optional parse(const ::cmsghdr*) noexcept; - static std::size_t build(::cmsghdr* dst, const value_type&, dgram::family) noexcept; // optional + static std::size_t build(::cmsghdr* dst, const value_type&, dgram::family wire) noexcept; // optional static dgram::result<> enable(int fd, dgram::family) noexcept; // optional }; ``` @@ -142,6 +149,9 @@ a receive and a transmit batch take their own sets. A feature appearing in both families sets `space` to the larger payload. +`build` gets the family the datagram travels as, which is `inet4` for a +v4-mapped destination on a v6 socket. Emit the IPv4-level message for it. + `parse` returns nothing when the payload is narrower than what it would read. That check is the feature's job, because only it knows what widths the kernel uses for its own control messages. diff --git a/docs/sockets.md b/docs/sockets.md index f3bf800..458c51d 100644 --- a/docs/sockets.md +++ b/docs/sockets.md @@ -12,6 +12,8 @@ layout. Trivially destructible and cheap to copy. | `static result parse(family, const char* text, uint16_t port)` | Parse `192.0.2.1` or `2001:db8::1`. `invalid_argument` if `text` does not parse for `family`. | | `static endpoint any(family, uint16_t port)` | The wildcard address, for binding. | | `bool is_v4() / is_v6() / valid()` | Family predicates. | +| `bool is_v4_mapped()` | A v6 address of the form `::ffff:a.b.c.d`, which the kernel sends and receives as IPv4. | +| `family wire_family()` | The family a datagram to this endpoint travels as: `inet4` for a v4-mapped address. | | `uint16_t port()` | Port in host byte order. | | `socklen_t size()` | Bytes the kernel needs for this family, not `sizeof(sockaddr_storage)`. | | `const sockaddr* raw()` | Pointer for passing to a syscall. | diff --git a/docs/testing.md b/docs/testing.md index 52ba47c..5a3fa51 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -19,7 +19,7 @@ checkout. | `doc_examples.cpp` | Every snippet printed in `README.md` and `docs/`, compiled and run. A snippet in a document is untested code that looks authoritative; this is what stops one rotting. | | `compile_checks.cpp` | Pure `static_assert`. Feature concepts, control-buffer sizing, batch geometry, which resources may be carved from, which metadata is readable, and SipHash against its published vectors, split at every word boundary. Compiling is the test; it is never run. | | `loopback_tests.cpp` | Batch reuse, truncation reporting (including a caller-supplied `MSG_TRUNC`), nonblocking receive, staging limits, a failed flush leaving the batch staged, the error combinators. | -| `metadata_tests.cpp` | Destination address and ECN over loopback, both families, including a feature that was never enabled and an undersized control buffer. | +| `metadata_tests.cpp` | Destination address and ECN over loopback, both families, including a feature that was never enabled and an undersized control buffer. Dual-stack sockets: ECN from and to IPv4 peers, and a reply from the arrival address. | | `malformed_control_tests.cpp` | Control buffers the kernel would never write. Every case here was found by the fuzzer first. | | `offload_tests.cpp` | GRO and GSO over loopback, and the segmentation walk on its own: short tail, exact multiple, stride of zero, stride past the end, empty payload. | | `demux_tests.cpp` | Key equality and hash agreement (including the flow-label and padding cases), keys crafted to collide under one seed spreading under another, the table bucketing by its own seed, the flow table under churn and at capacity, projections, routing a GRO slot whose datagrams belong to different flows, an `unmatched` callback claiming or declining a datagram, arguments forwarded to a sink that takes them and ignored by one that does not, and the per-slot count. | diff --git a/src/dgram/address.cppm b/src/dgram/address.cppm index 6d3b96f..a11da9a 100644 --- a/src/dgram/address.cppm +++ b/src/dgram/address.cppm @@ -81,6 +81,15 @@ public: [[nodiscard]] constexpr bool is_v6() const noexcept { return storage_.ss_family == AF_INET6; } [[nodiscard]] constexpr bool valid() const noexcept { return is_v4() || is_v6(); } + /** @brief `::ffff:a.b.c.d`: a v6 address the kernel sends and receives as IPv4. */ + [[nodiscard]] bool is_v4_mapped() const noexcept { + constexpr std::array prefix{{{}, {}, {}, {}, {}, {}, {}, {}, {}, {}, std::byte{0xFF}, std::byte{0xFF}}}; + return is_v6() && std::ranges::equal(address_bytes().first<12>(), prefix); + } + + /** @brief The family a datagram to this endpoint travels as: v4 for a v4-mapped address. */ + [[nodiscard]] family wire_family() const noexcept { return (is_v4() || is_v4_mapped()) ? family::inet4 : family::inet6; } + [[nodiscard]] std::uint16_t port() const noexcept { return ::ntohs(is_v4() ? reinterpret_cast(storage_).sin_port : reinterpret_cast(storage_).sin6_port); } diff --git a/src/dgram/batch.cppm b/src/dgram/batch.cppm index 9ce49d9..f81fd3a 100644 --- a/src/dgram/batch.cppm +++ b/src/dgram/batch.cppm @@ -386,8 +386,9 @@ public: * earlier flush would have the kernel read whatever that datagram left * behind, which is the transmit mirror of the receive-side rearm. * - * The family for the build step comes from `to`, so a dual-stack socket - * sending to both families emits the right level and type per datagram. + * The build step gets `to.wire_family()`, not the socket's family: the + * kernel sends to a v4-mapped peer down its IPv4 path, which ignores + * `IPV6_TCLASS` and every other v6-level message except `IPV6_PKTINFO`. * * @return `false` if the batch is already full. */ @@ -458,7 +459,7 @@ private: return 0; } else { const std::span block{control_.data() + (i * Features::control_space), Features::control_space}; - return ancillary.build_into(block, to.is_v4() ? family::inet4 : family::inet6); + return ancillary.build_into(block, to.wire_family()); } } diff --git a/src/dgram/metadata.cppm b/src/dgram/metadata.cppm index 25bed62..a1f9683 100644 --- a/src/dgram/metadata.cppm +++ b/src/dgram/metadata.cppm @@ -83,13 +83,16 @@ export struct pktinfo { * * Only the address and interface are set; leaving both zero lets the routing * table choose, which is the same thing as not attaching the header at all. + * + * A v4 build accepts a v4-mapped address, which is what a dual-stack socket + * reports for IPv4 arrivals, so a reply can reuse the `local_info` as is. */ [[nodiscard]] static std::size_t build(::cmsghdr* dst, const value_type& value, const family fam) noexcept { if (fam == family::inet4) { ::in_pktinfo raw{}; raw.ipi_ifindex = static_cast(value.interface); - if (value.address.is_v4()) { - raw.ipi_spec_dst = reinterpret_cast(value.address.raw())->sin_addr; + if (value.address.is_v4() || value.address.is_v4_mapped()) { + std::memcpy(&raw.ipi_spec_dst, value.address.address_bytes().last().data(), sizeof(raw.ipi_spec_dst)); } return detail::write_message(dst, IPPROTO_IP, IP_PKTINFO, raw); } @@ -166,15 +169,19 @@ export struct ecn { return detail::write_message(dst, IPPROTO_IPV6, IPV6_TCLASS, bits); } - /** @brief Ask the kernel to report the traffic class. */ + /** + * @brief Ask the kernel to report the traffic class. + * + * A v6 socket also gets `IP_RECVTOS`: IPv4 traffic on a dual-stack socket + * takes the kernel's IPv4 path, which reports `IP_TOS` and never + * `IPV6_TCLASS`. The option is accepted on a v6-only socket too. + */ [[nodiscard]] static result<> enable(const int fd, const family fam) noexcept { - const int on{1}; - const int level{fam == family::inet4 ? IPPROTO_IP : IPPROTO_IPV6}; - const int name{fam == family::inet4 ? IP_RECVTOS : IPV6_RECVTCLASS}; - if (::setsockopt(fd, level, name, &on, static_cast<::socklen_t>(sizeof(on))) < 0) [[unlikely]] { - return fail<>(); + const auto report_tos = [fd] { return detail::set_option(fd, IPPROTO_IP, IP_RECVTOS, 1); }; + if (fam == family::inet4) { + return report_tos(); } - return {}; + return detail::set_option(fd, IPPROTO_IPV6, IPV6_RECVTCLASS, 1) | then(report_tos); } private: diff --git a/tests/loopback_tests.cpp b/tests/loopback_tests.cpp index 7c4b9b8..d70a1b6 100644 --- a/tests/loopback_tests.cpp +++ b/tests/loopback_tests.cpp @@ -63,6 +63,24 @@ TEST(Endpoint, RoundTripsTextAndPort) { EXPECT_NE(*ep, *v6); } +TEST(Endpoint, V4MappedTravelsAsIPv4) { + const auto mapped{*dgram::endpoint::parse(dgram::family::inet6, "::ffff:192.0.2.7", 443)}; + EXPECT_TRUE(mapped.is_v4_mapped()); + EXPECT_EQ(mapped.wire_family(), dgram::family::inet4); + + const auto v6{*dgram::endpoint::parse(dgram::family::inet6, "2001:db8::1", 443)}; + EXPECT_FALSE(v6.is_v4_mapped()); + EXPECT_EQ(v6.wire_family(), dgram::family::inet6); + + const auto compatible{*dgram::endpoint::parse(dgram::family::inet6, "::192.0.2.7", 443)}; + EXPECT_FALSE(compatible.is_v4_mapped()) << "the deprecated v4-compatible form is a plain v6 address"; + + const auto v4{*dgram::endpoint::parse(dgram::family::inet4, "192.0.2.7", 443)}; + EXPECT_FALSE(v4.is_v4_mapped()); + EXPECT_EQ(v4.wire_family(), dgram::family::inet4); + EXPECT_FALSE(dgram::endpoint{}.is_v4_mapped()); +} + TEST(Endpoint, RejectsMalformedText) { EXPECT_FALSE(dgram::endpoint::parse(dgram::family::inet4, "not-an-address", 1).has_value()); } diff --git a/tests/metadata_tests.cpp b/tests/metadata_tests.cpp index 3bda21e..7c5febf 100644 --- a/tests/metadata_tests.cpp +++ b/tests/metadata_tests.cpp @@ -7,6 +7,7 @@ */ #include +#include #include import std; @@ -131,6 +132,124 @@ TEST(Metadata, UnmarkedDatagramReadsAsNotEct) { EXPECT_EQ(*got, dgram::ecn_codepoint::not_ect); } +/* ============================================================================ + * Dual-stack sockets: IPv4 peers on a v6 socket + * ============================================================================ */ + +/* Explicit, because the default is the net.ipv6.bindv6only sysctl. */ +struct dual_stack { + [[nodiscard]] static dgram::result<> apply(const int fd, dgram::family) noexcept { + const int off{0}; + if (::setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, &off, sizeof(off)) < 0) { + return std::unexpected{dgram::errc{errno}}; + } + return {}; + } +}; + +constexpr std::array marked_codepoints{dgram::ecn_codepoint::ect0, dgram::ecn_codepoint::ect1, dgram::ecn_codepoint::ce}; + +/* IPv4 arrivals take the kernel's IPv4 path, which reports IP_TOS and never + IPV6_TCLASS, so the v6 socket has to ask for both. */ +TEST(DualStack, ReportsEcnForIPv4Arrivals) { + fixture net{dgram::family::inet4}; + dgram::socket receiver{std::move(*dgram::socket::open>(dgram::family::inet6))}; + ASSERT_TRUE(receiver.bind(dgram::endpoint::any(dgram::family::inet6, 0)).has_value()); + const auto target{*dgram::endpoint::parse(dgram::family::inet4, "127.0.0.1", receiver.local_address()->port())}; + + auto rx{rx_batch::carve(net.arena)}; + auto tx{tx_batch::carve(net.arena)}; + ASSERT_TRUE(rx.has_value() && tx.has_value()); + + for (const auto marked : marked_codepoints) { + dgram::control ancillary{}; + ancillary.set(marked); + ASSERT_TRUE(tx->stage_copy(bytes_of("from v4"), target, ancillary)); + ASSERT_TRUE(tx->flush(net.sender).has_value()); + + ASSERT_EQ(*rx->receive(receiver), 1u); + const auto d{*rx->datagrams().begin()}; + ASSERT_TRUE(d.intact()) << "IP_TOS and IPV6_PKTINFO both fit the control buffer"; + EXPECT_TRUE(d.from().is_v4_mapped()); + const auto meta{d.meta()}; + ASSERT_TRUE(meta.get().has_value()) << "codepoint " << static_cast(std::to_underlying(marked)); + EXPECT_EQ(*meta.get(), marked); + ASSERT_TRUE(meta.get().has_value()); + EXPECT_TRUE(meta.get()->address.is_v4_mapped()); + } +} + +/* Sending to a v4-mapped peer goes down the IPv4 path, which ignores IPV6_TCLASS. */ +TEST(DualStack, MarksEcnOnDatagramsToV4MappedPeers) { + fixture net{dgram::family::inet4}; + dgram::socket sender{std::move(*dgram::socket::open(dgram::family::inet6))}; + const auto target{*dgram::endpoint::parse(dgram::family::inet6, "::ffff:127.0.0.1", net.target.port())}; + + auto rx{rx_batch::carve(net.arena)}; + auto tx{tx_batch::carve(net.arena)}; + ASSERT_TRUE(rx.has_value() && tx.has_value()); + + for (const auto marked : marked_codepoints) { + dgram::control ancillary{}; + ancillary.set(marked); + ASSERT_TRUE(tx->stage_copy(bytes_of("to v4-mapped"), target, ancillary)); + const auto sent{tx->flush(sender)}; + ASSERT_TRUE(sent.has_value()) << dgram::describe(sent.error()); + + ASSERT_EQ(*rx->receive(net.receiver), 1u); + const auto& got{(*rx->datagrams().begin()).meta().get()}; + ASSERT_TRUE(got.has_value()); + EXPECT_EQ(*got, marked) << "codepoint " << static_cast(std::to_underlying(marked)); + } +} + +/* The server pattern: answer an IPv4 client from the local address it reached, + reflecting its marking, by handing the arrival's local_info straight back. */ +TEST(DualStack, RepliesToAnIPv4PeerFromTheArrivalAddress) { + fixture client{dgram::family::inet4}; + dgram::socket server{std::move(*dgram::socket::open>(dgram::family::inet6))}; + ASSERT_TRUE(server.bind(dgram::endpoint::any(dgram::family::inet6, 0)).has_value()); + const auto server_port{server.local_address()->port()}; + const auto server_target{*dgram::endpoint::parse(dgram::family::inet4, "127.0.0.1", server_port)}; + + libmem::arena server_arena{rx_batch::footprint() + tx_batch::footprint()}; + auto client_rx{rx_batch::carve(client.arena)}; + auto client_tx{tx_batch::carve(client.arena)}; + auto server_rx{rx_batch::carve(server_arena)}; + auto server_tx{tx_batch::carve(server_arena)}; + ASSERT_TRUE(client_rx.has_value() && client_tx.has_value() && server_rx.has_value() && server_tx.has_value()); + + // The client's receive socket sends too, so the reply comes back to it. + dgram::control request{}; + request.set(dgram::ecn_codepoint::ect1); + ASSERT_TRUE(client_tx->stage_copy(bytes_of("ping"), server_target, request)); + ASSERT_TRUE(client_tx->flush(client.receiver).has_value()); + + ASSERT_EQ(*server_rx->receive(server), 1u); + const auto arrival{*server_rx->datagrams().begin()}; + const auto meta{arrival.meta()}; + ASSERT_TRUE(meta.get().has_value() && meta.get().has_value()); + + dgram::control reply{}; + reply.set(*meta.get()); + reply.set(*meta.get()); + ASSERT_TRUE(server_tx->stage_copy(bytes_of("pong"), arrival.from(), reply)); + const auto sent{server_tx->flush(server)}; + ASSERT_TRUE(sent.has_value()) << dgram::describe(sent.error()); + + ASSERT_EQ(*client_rx->receive(client.receiver), 1u); + const auto back{*client_rx->datagrams().begin()}; + EXPECT_EQ(back.from(), server_target) << "answered from the address the client addressed"; + const auto& ecn{back.meta().get()}; + ASSERT_TRUE(ecn.has_value()); + EXPECT_EQ(*ecn, dgram::ecn_codepoint::ect1) << "the reflected marking survived the v4-mapped send"; +} + +TEST(DualStack, V6OnlySocketStillEnablesEcn) { + const auto sock{dgram::socket::open>(dgram::family::inet6)}; + ASSERT_TRUE(sock.has_value()) << dgram::describe(sock.error()); +} + /* ============================================================================ * The paths that are easy to get wrong * ============================================================================ */